Git development
 help / color / mirror / Atom feed
* [PATCH 3/3] add shebang line to git-mergetool--lib.sh
From: Jeff King @ 2010-01-29 10:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, David Aguilar, git
In-Reply-To: <20100129102518.GA5875@coredump.intra.peff.net>

Even though this script is expected to be sourced instead of
executed on its own, the #!/bin/sh line provides simple
documentation about what format the file is in.

In particular, the lack of such a line was confusing the
valgrind support of our test scripts, which assumed that any
executable without a #!-line should be intercepted and run
through valgrind. So during valgrind-enabled tests, any
script sourcing this file actually sourced the valgrind
interception script instead.

Signed-off-by: Jeff King <peff@peff.net>
---
The valgrind script could perhaps be a bit smarter instead, but checking
#!-lines is nice and simple, and this change makes other programs like
"file" happier, too.

This problem has been around since 21d0ba7 (difftool/mergetool: refactor
commands to use git-mergetool--lib, 2009-04-08), released in v1.6.3. But
since it is only about our internal tests, and even then only about
running them with valgrind enabled, I don't know if it is worth a fix on
'maint'.

 git-mergetool--lib.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 5b62785..51dd0d6 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,3 +1,4 @@
+#!/bin/sh
 # git-mergetool--lib is a library for common merge tool functions
 diff_mode() {
 	test "$TOOL_MODE" = diff
-- 
1.7.0.rc0.41.g538720

^ permalink raw reply related

* Re: build warnings
From: Jeff King @ 2010-01-29 11:02 UTC (permalink / raw)
  To: Michael Wookey; +Cc: Git Mailing List, Johannes Sixt
In-Reply-To: <d2e97e801001290103r5b9cfc2aq8daec90c8c88f2ff@mail.gmail.com>

On Fri, Jan 29, 2010 at 08:03:37PM +1100, Michael Wookey wrote:

> With current master (dace5dd1), the following build warnings appear on
> Ubuntu 9.10 (x86):
> 
>   run-command.c: In function ‘notify_parent’:
>   run-command.c:70: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result
>   run-command.c: In function ‘die_child’:
>   run-command.c:80: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result
>   run-command.c:81: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result
>   run-command.c:82: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result

There is no point in looking at the return value of any of those calls.
The first one is about notifying the parent process of a child's failure
to exec while it is dying (the surrounding function is even an atexit
handler!). If we can't do that, there is really no alternative behavior.
The latter three are printing fatal error messages. If we fail at that,
there is not much to do (unless we should print an error...).

>   ~$ gcc --version
>   gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1

I have heard that Ubuntu recently switched on unused result warnings by
default, and I have seen complaints that it is generating a lot of
uninteresting warnings like these.

Does anybody know if this behavior is here to stay? Can it be worked
around with -Wno-warn-unused-result or something? There are few enough
callsites here that I am not entirely opposed to annotating them with
"(void)write" (does that actually work?), but I worry that this is a
slippery slope. There are a lot of other calls whose return values are
also uninteresting (just looking in the vicinity of this code, I see an
fflush and a close, neither of whose failure would be interesting). I'm
not excited at the prospect of annotating all of them.

-Peff

^ permalink raw reply

* Re: [PATCH 3/3] reject @{-1} not at beginning of object name
From: Jeff King @ 2010-01-29 11:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Johannes Schindelin
In-Reply-To: <7vsk9qknf6.fsf@alter.siamese.dyndns.org>

On Thu, Jan 28, 2010 at 12:02:53PM -0800, Junio C Hamano wrote:

> We might want to use @{-some string that has non digit} for other purposes
> and it may be a safer change to tweak the "do we only have digits" check
> in the post-context to detect and reject only @{-<all digits>}.

I considered that, but I didn't think it was really worth it. If we
later want to make @{-foobar} meaningful, we can loosen the safety check
then.

> But what I am puzzled by the code structure of get_sha1_basic(), which
> looks like this:

You are not the only one who is puzzled. :)

But yes, your analysis of what is there now looks right to me.

> And the place that parses @{-1} and @{u} are different, even though both
> dwim_log() called by the third one and dwim_ref() called by the fourth one
> call substitute_branch_name() and they are perfectly capable of resolving
> @{-1} and @{u} (and even nested stuff like @{-1}@{u}@{u} with your patch).

Ooh, gross. I didn't try @{u}@{u} in my tests, but it should work.

>     Side note.  I am wondering if dwim_log()'s current implementation is
>     even correct in the first place.  When you have two "ambiguous" refs,
>     it appears to me that you will get a warning from dwim_ref(), but if
>     only one of them has a reflog associated with it, dwim_log() won't
>     complain.  Why isn't the function be (1) dwim_ref() to find the ref
>     from abbreviated refname given in str; and then (2) check if the log
>     exists for that ref?

I guess the original rationale was that you might have reflog'd one, so
by asking for "foo@{yesterday}" you are disambiguating as "the one with
a reflog". But that seems kind of useless to me since:

  1. It is somewhat error-prone, as it assumes that from the user's
     perspective, the fact that one ref has a log and the other does not
     is somehow a meaningful disambiguation. Which implies that users
     carefully figure out which refs have reflogs and which do not, and
     I don't think that is true.

  2. For quite a while, we have had logallrefupdates on by default (and
     I don't remember the exact semantics before that, but wasn't it
     enough to simply create a "logs" directory, which meant that you
     either logged everything or nothing?). So I don't even know how you
     would get into a situation where one ref has a log and the other
     does not.

In other words, I totally agree with your statement, and we could
probably just drop the dwim_log code.

> It might be cleaner if the logic went like this instead:

Your logic makes sense to me. I think we could also simply do a
left-to-right parse, eating refs, @{-N}, and @{u} as we go and
converting them into a "real ref". If we get to something else, we stop.
If we have a @{} left, it's a reflog.  Otherwise, it's bogus (I think ^
suffixes and such have already been stripped off at this point).

Interpret_branch_name already does most of the "eating..." part above
(and it needs to remain separate from get_sha1_basic, as things like
"checkout" need to use it directly).

But I didn't really look too hard at it, as:

> But that is a kind of code churn that may not be worth doing.  I dunno.

Yeah, the code was sufficiently nasty and sufficiently core that I
didn't really want to risk breaking it for the sake of cleanup
(especially not during the -rc cycle).

-Peff

^ permalink raw reply

* Re: build warnings
From: Erik Faye-Lund @ 2010-01-29 11:23 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael Wookey, Git Mailing List, Johannes Sixt
In-Reply-To: <20100129110201.GB6165@coredump.intra.peff.net>

On Fri, Jan 29, 2010 at 12:02 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 29, 2010 at 08:03:37PM +1100, Michael Wookey wrote:
>
>> With current master (dace5dd1), the following build warnings appear on
>> Ubuntu 9.10 (x86):
>>
>>   run-command.c: In function ‘notify_parent’:
>>   run-command.c:70: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>>   run-command.c: In function ‘die_child’:
>>   run-command.c:80: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>>   run-command.c:81: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>>   run-command.c:82: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>
> There is no point in looking at the return value of any of those calls.
> The first one is about notifying the parent process of a child's failure
> to exec while it is dying (the surrounding function is even an atexit
> handler!). If we can't do that, there is really no alternative behavior.
> The latter three are printing fatal error messages. If we fail at that,
> there is not much to do (unless we should print an error...).
>
>>   ~$ gcc --version
>>   gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1
>
> I have heard that Ubuntu recently switched on unused result warnings by
> default, and I have seen complaints that it is generating a lot of
> uninteresting warnings like these.
>
> Does anybody know if this behavior is here to stay? Can it be worked
> around with -Wno-warn-unused-result or something? There are few enough
> callsites here that I am not entirely opposed to annotating them with
> "(void)write" (does that actually work?), but I worry that this is a
> slippery slope. There are a lot of other calls whose return values are
> also uninteresting (just looking in the vicinity of this code, I see an
> fflush and a close, neither of whose failure would be interesting). I'm
> not excited at the prospect of annotating all of them.
>

In my experience, quieting warn-unused-result globally isn't ideal;
this warning has helped me track down some serious issues many times
in the past. IIRC, gcc requires a specific attribute on a function
prototype in order to generate warnings when the return-value isn't
used. I guess the issue we're seeing here is that the glibc that
Ubuntu ships has recently added this attribute for some CRT-functions.

Personally I think that quieting them ("(void)func(...)" does work
AFAIK) can make sense as long as there's only a few call-sites.
However, if it's so many that it'll generate substantial noise in the
git-sources or Ubuntu-users will get annoyed beyond sense,
"-Wno-warn-unused-result" might be the only choice. But perhaps this
is something Ubuntu-users would put in their config.mak?

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Please pull gitk.git master branch
From: Paul Mackerras @ 2010-01-29 11:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio,

Please do a pull from

git://git.kernel.org/pub/scm/gitk/gitk.git

to get a gitk update - mostly translation updates and bug fixes.

Thanks,
Paul.

^ permalink raw reply

* Re: threaded-grep cause msys build failure
From: Johannes Sixt @ 2010-01-29 12:03 UTC (permalink / raw)
  To: Zoltán Füzesi; +Cc: Git Mailing List
In-Reply-To: <9ab80d151001290227u386616c5o6c825ff10d37f9fd@mail.gmail.com>

Zoltán Füzesi schrieb:
> Building git in msys environment fails:
> ...
>     LINK git.exe
> builtin-grep.o: In function `wait_all':
> D:\devel\msysgit\git/builtin-grep.c:260: undefined reference to
> `pthread_cond_broadcast'

Use this. I'll try to find a better solution over the weekend.

diff --git a/Makefile b/Makefile
index c591d70..bbee373 100644
--- a/Makefile
+++ b/Makefile
@@ -183,6 +183,9 @@ all::
 # Define THREADED_DELTA_SEARCH if you have pthreads and wish to exploit
 # parallel delta searching when packing objects.
 #
+# Define THREADED_GREP if you have pthreads and wish to exploit a
+# parallelized grep.
+#
 # Define INTERNAL_QSORT to use Git's implementation of qsort(), which
 # is a simplified version of the merge sort used in glibc. This is
 # recommended if Git triggers O(n^2) behavior in your platform's qsort().
@@ -723,11 +726,13 @@ ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),UnixWare)
 	CC = cc
@@ -782,6 +787,7 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_MEMMEM = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
@@ -795,6 +801,7 @@ ifeq ($(uname_S),SunOS)
 	NO_MKSTEMPS = YesPlease
 	NO_REGEX = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	ifeq ($(uname_R),5.7)
 		NEEDS_RESOLV = YesPlease
 		NO_IPV6 = YesPlease
@@ -851,6 +858,7 @@ ifeq ($(uname_S),FreeBSD)
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
 		PTHREAD_LIBS = -pthread
 		NO_UINTMAX_T = YesPlease
@@ -865,6 +873,7 @@ ifeq ($(uname_S),OpenBSD)
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),NetBSD)
 	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
@@ -873,6 +882,7 @@ ifeq ($(uname_S),NetBSD)
 	BASIC_CFLAGS += -I/usr/pkg/include
 	BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	NO_MKSTEMPS = YesPlease
 endif
@@ -889,6 +899,7 @@ ifeq ($(uname_S),AIX)
 	BASIC_CFLAGS += -D_LARGE_FILES
 	ifneq ($(shell expr "$(uname_V)" : '[1234]'),1)
 		THREADED_DELTA_SEARCH = YesPlease
+		THREADED_GREP = YesPlease
 	else
 		NO_PTHREADS = YesPlease
 	endif
@@ -916,6 +927,7 @@ ifeq ($(uname_S),IRIX)
 	SHELL_PATH = /usr/gnu/bin/bash
 	NEEDS_LIBGEN = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
 	NO_SETENV=YesPlease
@@ -935,6 +947,7 @@ ifeq ($(uname_S),IRIX64)
 	SHELL_PATH=/usr/gnu/bin/bash
 	NEEDS_LIBGEN = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),HP-UX)
 	NO_IPV6=YesPlease
@@ -1334,6 +1347,7 @@ endif

 ifdef NO_PTHREADS
 	THREADED_DELTA_SEARCH =
+	THREADED_GREP =
 	BASIC_CFLAGS += -DNO_PTHREADS
 else
 	EXTLIBS += $(PTHREAD_LIBS)
@@ -1341,6 +1355,11 @@ endif

 ifdef THREADED_DELTA_SEARCH
 	BASIC_CFLAGS += -DTHREADED_DELTA_SEARCH
+endif
+ifdef THREADED_GREP
+	BASIC_CFLAGS += -DTHREADED_GREP
+endif
+ifneq (,$(THREADED_GREP)$(THREADED_DELTA_SEARCH))
 	LIB_OBJS += thread-utils.o
 endif
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
diff --git a/builtin-grep.c b/builtin-grep.c
index 9bd467c..911d0da 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -16,7 +16,7 @@
 #include "quote.h"
 #include "dir.h"

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 #include "thread-utils.h"
 #include <pthread.h>
 #endif
@@ -28,7 +28,7 @@ static char const * const grep_usage[] = {

 static int use_threads = 1;

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 #define THREADS 8
 static pthread_t threads[THREADS];

@@ -274,7 +274,7 @@ static int wait_all(void)

 	return hit;
 }
-#else /* !NO_PTHREADS */
+#else /* THREADED_GREP */
 #define read_sha1_lock()
 #define read_sha1_unlock()

@@ -439,7 +439,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,

 	name = strbuf_detach(&pathbuf, NULL);

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 	if (use_threads) {
 		grep_sha1_async(opt, name, sha1);
 		return 0;
@@ -501,7 +501,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 		strbuf_addstr(&buf, filename);
 	name = strbuf_detach(&buf, NULL);

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 	if (use_threads) {
 		grep_file_async(opt, name, filename);
 		return 0;
@@ -902,7 +902,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
 		die("cannot mix --fixed-strings and regexp");

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 	if (online_cpus() == 1 || !grep_threads_ok(&opt))
 		use_threads = 0;

-- 
1.7.0.rc0.1096.g3300c.dirty

^ permalink raw reply related

* Custom git completion
From: David Rhodes Clymer @ 2010-01-29 12:57 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Unless I read it incorrectly, the completion script included with
git-core does not make it easy for users to write completion scripts
for custom git commands. I can extend git itself by creating a command
"git-foo", and placing it in my path. The command can then be used
like so: "git foo". However, if I want to add command completion for
that command without modifying (I may not have permission) or
duplicating the system git completion, I can't write a completion
script which matches works on "git foo", only "git-foo", which is not
how I would ever call the script.

Anyway, so I made a simple modification which looks for completion
code for custom commands, and calls that as appropriate. If the
attached patch (or something like it) were applied to the git
completion script, it would be awfully handy.

-davidc

ps. I'm not subscribed to the list, so please copy me on any replies, thanks!

[-- Attachment #2: git_completion.patch --]
[-- Type: text/x-patch, Size: 380 bytes --]

--- /etc/bash_completion.d/git.orig	2010-01-12 14:50:16.000000000 -0500
+++ /etc/bash_completion.d/git	2010-01-12 15:28:27.000000000 -0500
@@ -1403,7 +1403,10 @@
 	svn)         _git_svn ;;
 	tag)         _git_tag ;;
 	whatchanged) _git_log ;;
-	*)           COMPREPLY=() ;;
+	*)
+    COMPREPLY=()
+    $(complete -p |awk '/ '"git-${command}"'$/{print $(NF-1)}')
+  ;;
 	esac
 }
 

^ permalink raw reply

* Implement --password option for git svn perl script
From: Laszlo Papp @ 2010-01-29 14:17 UTC (permalink / raw)
  To: git; +Cc: normalperson, lznuaa

Hello,

It would be nice to use --pasword option on windows, like --username
option with git svn facility. I can't handle SVN repository with git
svn on windows if I'd like to authenticate myself with other user than
the default domain name of my windows.

If I use --username option, Frank Li said me it's not enough in
TortoiseGIT to provide a popup facility to type the password related
to the set --username option.

What's your opinion/suggestion/recommendation ?

Best Regards,
Laszlo Papp

^ permalink raw reply

* [PATCH v2] request-pull: avoid mentioning that the start point is a single commit
From: Miklos Vajna @ 2010-01-29 14:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfx5pjrgm.fsf@alter.siamese.dyndns.org>

Previously we ran shortlog on the start commit which always printed
"(1)" after the start commit, which gives no information, but makes the
output less easy to read. Avoid doing so.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Thu, Jan 28, 2010 at 11:33:13PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I suspect that the last one would be the easiest for the requestee to
> judge the freshness of the branch.
>
> Why isn't the "The following changes..." line not part of the --format
> thing, by the way?

OK, I changed it accordingly, though I did not the change the output of
the "commit <hash>" line. So the output diff is:

         The following changes since commit 103209c6782586d92b04ee1fc71c0fd6f6385f5f:
        -  Junio C Hamano (1):
        -        Merge branch 'jc/maint-reflog-bad-timestamp'
        +
        +  Merge branch 'jc/maint-reflog-bad-timestamp' (2010-01-27 14:57:37 -0800)

         are available in the git repository at:

 git-request-pull.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 630cedd..8fd15f6 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -65,11 +65,11 @@ if [ -z "$branch" ]; then
 	status=1
 fi
 
-echo "The following changes since commit $baserev:"
-git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/  \1/'
+git show -s --format='The following changes since commit %H:
 
-echo "are available in the git repository at:"
-echo
+  %s (%ci)
+
+are available in the git repository at:' $baserev
 echo "  $url $branch"
 echo
 
-- 
1.6.6.1

^ permalink raw reply related

* Fwd: Delivery Status Notification (Failure)
From: Laszlo Papp @ 2010-01-29 14:18 UTC (permalink / raw)
  To: git; +Cc: normalperson, lznuaa@gmail.com - Chinma/Shanghai/31
In-Reply-To: <001636ed7681994278047e4e4a6f@google.com>

Hello,

It would be nice to use --pasword option on windows, like --username
option with git svn facility. I can't handle SVN repository with git
svn on windows if I'd like to authenticate myself with other user than
the default domain name of my windows.

If I use --username option, Frank Li said me it's not enough in
TortoiseGIT to provide a popup facility to type the password related
to the set --username option.

What's your opinion/suggestion/recommendation ?

Best Regards,
Laszlo Papp

^ permalink raw reply

* [PATCH] Do not install shell libraries executable
From: Jonathan Nieder @ 2010-01-29 14:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin, David Aguilar, git
In-Reply-To: <20100129103723.GC6025@coredump.intra.peff.net>

These scripts are expected to be sourced instead of executed on
their own.  Avoid some confusion by not marking them executable.

The executable bit was confusing the valgrind support of our test
scripts, which assumed that any executable without a #!-line
should be intercepted and run through valgrind.  So during
valgrind-enabled tests, any script sourcing this file actually
sourced the valgrind interception script instead.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:

> This problem has been around since 21d0ba7 (difftool/mergetool: refactor
> commands to use git-mergetool--lib, 2009-04-08), released in v1.6.3. But
> since it is only about our internal tests, and even then only about
> running them with valgrind enabled, I don't know if it is worth a fix on
> 'maint'.

It was also confusing dpkg-shlibdeps, so I recently came up with
this fix.  Both fixes seem like good changes to me, and both
could be applied.  Your fix has the virtue of being shorter,
hence safer.

 Makefile        |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)
 mode change 100755 => 100644 git-sh-setup.sh

diff --git a/Makefile b/Makefile
index fd7f51e..70dcc40 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,7 @@ LIB_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_SH =
+SCRIPT_LIB_SH =
 TEST_PROGRAMS =
 
 SCRIPT_SH += git-am.sh
@@ -353,7 +354,6 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
-SCRIPT_SH += git-mergetool--lib.sh
 SCRIPT_SH += git-notes.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
@@ -362,11 +362,13 @@ SCRIPT_SH += git-rebase--interactive.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-repack.sh
 SCRIPT_SH += git-request-pull.sh
-SCRIPT_SH += git-sh-setup.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
+SCRIPT_LIB_SH += git-mergetool--lib.sh
+SCRIPT_LIB_SH += git-sh-setup.sh
+
 SCRIPT_PERL += git-add--interactive.perl
 SCRIPT_PERL += git-difftool.perl
 SCRIPT_PERL += git-archimport.perl
@@ -1428,7 +1430,7 @@ export TAR INSTALL DESTDIR SHELL_PATH
 
 SHELL = $(SHELL_PATH)
 
-all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
+all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
 endif
@@ -1488,6 +1490,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	chmod +x $@+ && \
 	mv $@+ $@
 
+$(patsubst %.sh,%,$(SCRIPT_LIB_SH)) : % : %.sh
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
+	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    -e $(BROKEN_PATH_FIX) \
+	    $@.sh >$@+ && \
+	mv $@+ $@
+
 ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
@@ -1792,6 +1803,7 @@ install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_PERL
@@ -1901,7 +1913,7 @@ distclean: clean
 clean:
 	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
-	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
+	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
 	$(RM) -r autom4te.cache
@@ -1930,7 +1942,7 @@ endif
 ### Check documentation
 #
 check-docs::
-	@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \
+	@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk; \
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
@@ -1975,7 +1987,7 @@ check-docs::
 		documented,gittutorial-2 | \
 		sentinel,not,matching,is,ok ) continue ;; \
 		esac; \
-		case " $(ALL_PROGRAMS) $(BUILT_INS) git gitk " in \
+		case " $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk " in \
 		*" $$cmd "*)	;; \
 		*) echo "removed but $$how: $$cmd" ;; \
 		esac; \
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
old mode 100755
new mode 100644
-- 
1.7.0.rc0

^ permalink raw reply related

* Re: Delivery Status Notification (Failure)
From: Frank Li @ 2010-01-29 15:04 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: git, normalperson
In-Reply-To: <a362e8011001290618g542be5f5y2777a925ba9bd936@mail.gmail.com>

>
> If I use --username option, Frank Li said me it's not enough in
> TortoiseGIT to provide a popup facility to type the password related
> to the set --username option.
>

I prefer git-svn can provide environment to launch a external
application to input password like open ssh.

^ permalink raw reply

* Re: Custom git completion
From: Shawn O. Pearce @ 2010-01-29 15:11 UTC (permalink / raw)
  To: David Rhodes Clymer; +Cc: git
In-Reply-To: <9b69cfcf1001290457s6b7fad6cs5a915f16a11f5782@mail.gmail.com>

David Rhodes Clymer <david@zettazebra.com> wrote:
> Unless I read it incorrectly, the completion script included with
> git-core does not make it easy for users to write completion scripts
> for custom git commands. I can extend git itself by creating a command
> "git-foo", and placing it in my path.

git config --global alias.foo /home/me/bin/my-git-foo

git foo will now complete correctly.  No need to modify the
completion code.

-- 
Shawn.

^ permalink raw reply

* v1.7.0-rc0 shows lots of "unable to find <sha1>" on git-stash
From: Jonathan del Strother @ 2010-01-29 15:12 UTC (permalink / raw)
  To: Git Mailing List

Heya,

git-stash in v1.7.0-rc0 gives me several hundred lines of :

error: unable to find dd4a345a770f5c82a668932b7289f42b1ae38103
error: unable to find dd4a345a770f5c82a668932b7289f42b1ae38103
error: unable to find dd4a345a770f5c82a668932b7289f42b1ae38103
....
error: unable to find 0dc67edc22017de22161dde93baa615767b9c6e0
error: unable to find 0dc67edc22017de22161dde93baa615767b9c6e0
error: unable to find 0dc67edc22017de22161dde93baa615767b9c6e0
error: unable to find 0dc67edc22017de22161dde93baa615767b9c6e0
....
error: unable to find 980f084e71097df3d4086aaab574972321df80f2
error: unable to find 980f084e71097df3d4086aaab574972321df80f2
error: unable to find 980f084e71097df3d4086aaab574972321df80f2
error: unable to find 980f084e71097df3d4086aaab574972321df80f2


Other than that, it appears to work fine.  'git show <sha1>' works too
- they're all a bunch of file contents, with no particular pattern.

This only happens in my main (private) repo.  I can't seem to come up
with a reproducible example for anyone else...

The previous build I was using -
5b15950ac414a8a2d4f5eb480712abcc9fe176d2 from Jan 19th - didn't show
this problem.  Want me to try and bisect further?


-Jonathan

^ permalink raw reply

* Re: [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter
From: Michal Sojka @ 2010-01-29 15:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, j.sixt, Johannes.Schindelin
In-Reply-To: <7vaavxlwoz.fsf@alter.siamese.dyndns.org>

On Thursday 28 of January 2010 22:57:16 Junio C Hamano wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
> > +test_expect_success 'setup submodule' '
> > +	rm -rf * .*
> 
> Yikes.  Please don't do this.
> 
> [...]
> 
> but it would be preferable to be even more explicit "rm -fr ?* .git".

Fixed. I did it like you suggested.

> Also make sure you don't break the chain of "&&" unnecessarily.
> 

Fixed.

I've also split each my test into two separate tests. In the first one I 
perform the rewrite and in the second I test that the result is what was 
expected. I did it because the other tests in this file are structured the 
same way.

Cheers,
Michal

^ permalink raw reply

* [PATCHv5 1/2] filter-branch: Fix to allow replacing submodules with another content
From: Michal Sojka @ 2010-01-29 15:27 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, gitster, Michal Sojka
In-Reply-To: <201001291620.35269.sojkam1@fel.cvut.cz>

When git filter-branch is used to replace a submodule with another
content, it always fails on the first commit. Consider a repository with
submod directory containing a submodule. If I want to remove the
submodule and replace it with a file, the following command fails.

git filter-branch --tree-filter 'rm -rf submod &&
				 git rm -q submod &&
				 mkdir submod &&
				 touch submod/file'

The error message is:
error: submod: is a directory - add files inside instead

The reason is that git diff-index, which generates the first part of the
list of files updated by the tree filter, emits also the removed
submodule even if it was replaced by a real directory.

Adding --ignored-submodules solves the problem for me and
tests in t7003-filter-branch.sh pass correctly.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 git-filter-branch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 195b5ef..7c4ad7d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -331,7 +331,7 @@ while read commit parents; do
 			die "tree filter failed: $filter_tree"
 
 		(
-			git diff-index -r --name-only $commit &&
+			git diff-index -r --name-only --ignore-submodules $commit &&
 			git ls-files --others
 		) > "$tempdir"/tree-state || exit
 		git update-index --add --replace --remove --stdin \
-- 
1.6.6

^ permalink raw reply related

* [PATCHv5 2/2] filter-branch: Add tests for submodules in tree-filter
From: Michal Sojka @ 2010-01-29 15:27 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, gitster, Michal Sojka
In-Reply-To: <201001291620.35269.sojkam1@fel.cvut.cz>

Add tests to make sure:
1) a submodule can be removed and its content replaced with regular
   files ('rewrite submodule with another content'). This test passes
   only with the previous patch applied.

2) it is possible to replace submodule revision by direct index
   manipulation ('replace submodule revision'). Although it would be
   better to run such a filter in --index-filter, this test shows that
   this functionality is not broken by the previous patch. This
   succeeds both with and without the previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..a7f0791 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,49 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf ?* .git &&
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod" &&
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl" &&
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule" &&
+	git branch original HEAD
+'
+
+orig_head=`git rev-parse HEAD`
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || :" HEAD
+'
+test_expect_success 'test that submodule was rewritten' '
+	test -f submod/file &&
+	test $orig_head != `git rev-parse HEAD`
+'
+
+test_expect_success 'replace submodule revision' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "if git ls-files --error-unmatch -- submod > /dev/null 2>&1
+	     then git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submod
+	     fi" HEAD
+'
+test_expect_success 'test that revision was replaced' '
+	test "`git ls-files --stage submod`" = "160000 0123456789012345678901234567890123456789 0	submod" &&
+	test $orig_head != `git rev-parse HEAD`
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related

* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Shawn O. Pearce @ 2010-01-29 15:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nicolas Pitre
In-Reply-To: <7vockdjx6w.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > +static void stream_blob(
> > +	uintmax_t len,
> > +	unsigned char *sha1out,
> > +	uintmax_t mark)
> 
> A funny way to indent and line wrap...

Consistent with something else in the same file that has a similar
task... store_object().  I literally just copied the lines from
there and pasted here.
 
> > +	/* Determine if we should auto-checkpoint. */
> > +	if ((pack_size + 60 + len) > max_packsize
> > +		|| (pack_size + 60 + len) < pack_size)
> > +		cycle_packfile();
> 
> What's "60" in this math?

IIRC 60 is 20 bytes for the SHA-1 footer, and another 40 padding
for the object header, and the deflate() wrapping.

Again, the 60 was stolen from the existing store_object(), which
already has the same assumption.  Only there I think we have len as
the fully compressed version, so the deflate() wrapping is already
being accounted for.
 
> If the data is not compressible, we could even grow and the end result
> might be more than (pack_size + len), busting max_packsize.

Yea, that's a good point.  I probably should run the length through
deflateBound() and use that here in the test.  But I didn't think it
would make a huge difference.  If the file isn't compressible what
is the real increment over the uncompressed size?  There's a header
and footer, and instructions in the stream to indicate literal data,
but its not like its doubling in size.

> As we are
> streaming out, we cannot say "oops, let me try again after truncating and
> closing the current file and then opening a new file", and instead may
> have to copy the data from the current one to a new one, and truncate the
> current one.  Is this something worth worrying about?

Hmm.  I'm not that worried about it, but then there's the case of
a blob that is larger than the max pack size.  We can't store that,
period.  Should we try to exceed max pack size for that one object?
Or should we refuse?

The major reason for this test is to ensure an object offset
starts before the 4 GiB boundary so idx v1 can hold the offset.
A much less important reason is to try to support older 32 bit
filesystems which can't do more than 2 GiB per file.

-- 
Shawn.

^ permalink raw reply

* [PATCH] git-gui: fix shortcut for menu "Commit/Revert Changes"
From: Heiko Voigt @ 2010-01-29 15:57 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Junio C Hamano

The shortcut was not properly recognized previously.

Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
 git-gui.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index debcc5f..7d54511 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3550,6 +3550,8 @@ bind .   <$M1B-Key-s> do_signoff
 bind .   <$M1B-Key-S> do_signoff
 bind .   <$M1B-Key-t> do_add_selection
 bind .   <$M1B-Key-T> do_add_selection
+bind .   <$M1B-Key-j> do_revert_selection
+bind .   <$M1B-Key-J> do_revert_selection
 bind .   <$M1B-Key-i> do_add_all
 bind .   <$M1B-Key-I> do_add_all
 bind .   <$M1B-Key-minus> {show_less_context;break}
-- 
1.7.0.rc0.27.ge8d3a

^ permalink raw reply related

* [PATCH v2] fast-import: Stream very large blobs directly to pack
From: Shawn O. Pearce @ 2010-01-29 16:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nicolas Pitre
In-Reply-To: <20100129152254.GC21821@spearce.org>

If a blob is larger than the configured big-file-threshold, instead
of reading it into a single buffer obtained from malloc, stream it
onto the end of the current pack file.  Streaming the larger objects
into the pack avoids the 4+ GiB memory footprint that occurs when
fast-import is processing 2+ GiB blobs.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Fixed a missing skip_optional_lf() at the end of the large blob
 command case.

 Documentation/git-fast-import.txt |    6 ++
 fast-import.c                     |  172 +++++++++++++++++++++++++++++++++----
 t/t5705-clone-2gb.sh              |    2 +-
 t/t9300-fast-import.sh            |   46 ++++++++++
 4 files changed, 208 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index e6d364f..16d3596 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -50,6 +50,12 @@ OPTIONS
 	importers may wish to lower this, such as to ensure the
 	resulting packfiles fit on CDs.
 
+--big-file-threshold=<n>::
+	Maximum size of a blob that fast-import will attempt to
+	create a delta for, expressed in MiB.  The default is 512.
+	Some importers may wish to lower this on systems with
+	constrained memory.
+
 --depth=<n>::
 	Maximum delta depth, for blob and tree deltification.
 	Default is 10.
diff --git a/fast-import.c b/fast-import.c
index 60d0aa2..8055f73 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -280,6 +280,7 @@ struct recent_command
 /* Configured limits on output */
 static unsigned long max_depth = 10;
 static off_t max_packsize = (1LL << 32) - 1;
+static uintmax_t big_file_threshold = 512 * 1024 * 1024;
 static int force_update;
 static int pack_compression_level = Z_DEFAULT_COMPRESSION;
 static int pack_compression_seen;
@@ -1003,7 +1004,7 @@ static void cycle_packfile(void)
 
 static size_t encode_header(
 	enum object_type type,
-	size_t size,
+	uintmax_t size,
 	unsigned char *hdr)
 {
 	int n = 1;
@@ -1159,6 +1160,118 @@ static int store_object(
 	return 0;
 }
 
+static void truncate_pack(off_t to)
+{
+	if (ftruncate(pack_data->pack_fd, to)
+	 || lseek(pack_data->pack_fd, to, SEEK_SET) != to)
+		die_errno("cannot truncate pack to skip duplicate");
+	pack_size = to;
+}
+
+static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
+{
+	size_t in_sz = 64 * 1024, out_sz = 64 * 1024;
+	unsigned char *in_buf = xmalloc(in_sz);
+	unsigned char *out_buf = xmalloc(out_sz);
+	struct object_entry *e;
+	unsigned char sha1[20];
+	unsigned long hdrlen;
+	off_t offset;
+	git_SHA_CTX c;
+	z_stream s;
+	int status = Z_OK;
+
+	/* Determine if we should auto-checkpoint. */
+	if ((pack_size + 60 + len) > max_packsize
+		|| (pack_size + 60 + len) < pack_size)
+		cycle_packfile();
+
+	offset = pack_size;
+
+	hdrlen = snprintf((char *)out_buf, out_sz, "blob %" PRIuMAX, len) + 1;
+	if (out_sz <= hdrlen)
+		die("impossibly large object header");
+
+	git_SHA1_Init(&c);
+	git_SHA1_Update(&c, out_buf, hdrlen);
+
+	memset(&s, 0, sizeof(s));
+	deflateInit(&s, pack_compression_level);
+
+	hdrlen = encode_header(OBJ_BLOB, len, out_buf);
+	if (out_sz <= hdrlen)
+		die("impossibly large object header");
+
+	s.next_out = out_buf + hdrlen;
+	s.avail_out = out_sz - hdrlen;
+
+	while (status != Z_STREAM_END) {
+		if (0 < len && !s.avail_in) {
+			size_t cnt = in_sz < len ? in_sz : (size_t)len;
+			size_t n = fread(in_buf, 1, cnt, stdin);
+			if (!n && feof(stdin))
+				die("EOF in data (%" PRIuMAX " bytes remaining)", len);
+
+			git_SHA1_Update(&c, in_buf, n);
+			s.next_in = in_buf;
+			s.avail_in = n;
+			len -= n;
+		}
+
+		status = deflate(&s, len ? 0 : Z_FINISH);
+
+		if (!s.avail_out || status == Z_STREAM_END) {
+			size_t n = s.next_out - out_buf;
+			write_or_die(pack_data->pack_fd, out_buf, n);
+			pack_size += n;
+			s.next_out = out_buf;
+			s.avail_out = out_sz;
+		}
+
+		switch (status) {
+		case Z_OK:
+		case Z_BUF_ERROR:
+		case Z_STREAM_END:
+			continue;
+		default:
+			die("unexpected deflate failure: %d", status);
+		}
+	}
+	deflateEnd(&s);
+	git_SHA1_Final(sha1, &c);
+
+	if (sha1out)
+		hashcpy(sha1out, sha1);
+
+	e = insert_object(sha1);
+
+	if (mark)
+		insert_mark(mark, e);
+
+	if (e->offset) {
+		duplicate_count_by_type[OBJ_BLOB]++;
+		truncate_pack(offset);
+
+	} else if (find_sha1_pack(sha1, packed_git)) {
+		e->type = OBJ_BLOB;
+		e->pack_id = MAX_PACK_ID;
+		e->offset = 1; /* just not zero! */
+		duplicate_count_by_type[OBJ_BLOB]++;
+		truncate_pack(offset);
+
+	} else {
+		e->depth = 0;
+		e->type = OBJ_BLOB;
+		e->pack_id = pack_id;
+		e->offset = offset;
+		object_count++;
+		object_count_by_type[OBJ_BLOB]++;
+	}
+
+	free(in_buf);
+	free(out_buf);
+}
+
 /* All calls must be guarded by find_object() or find_mark() to
  * ensure the 'struct object_entry' passed was written by this
  * process instance.  We unpack the entry by the offset, avoiding
@@ -1704,7 +1817,7 @@ static void parse_mark(void)
 		next_mark = 0;
 }
 
-static void parse_data(struct strbuf *sb)
+static int parse_data(struct strbuf *sb, uintmax_t limit, uintmax_t *len_res)
 {
 	strbuf_reset(sb);
 
@@ -1728,9 +1841,15 @@ static void parse_data(struct strbuf *sb)
 		free(term);
 	}
 	else {
-		size_t n = 0, length;
+		uintmax_t len = strtoumax(command_buf.buf + 5, NULL, 10);
+		size_t n = 0, length = (size_t)len;
 
-		length = strtoul(command_buf.buf + 5, NULL, 10);
+		if (limit && limit < len) {
+			*len_res = len;
+			return 0;
+		}
+		if (length < len)
+			die("data is too large to use in this context");
 
 		while (n < length) {
 			size_t s = strbuf_fread(sb, length - n, stdin);
@@ -1742,6 +1861,7 @@ static void parse_data(struct strbuf *sb)
 	}
 
 	skip_optional_lf();
+	return 1;
 }
 
 static int validate_raw_date(const char *src, char *result, int maxlen)
@@ -1806,14 +1926,32 @@ static char *parse_ident(const char *buf)
 	return ident;
 }
 
-static void parse_new_blob(void)
+static void parse_and_store_blob(
+	struct last_object *last,
+	unsigned char *sha1out,
+	uintmax_t mark)
 {
 	static struct strbuf buf = STRBUF_INIT;
+	uintmax_t len;
 
+	if (parse_data(&buf, big_file_threshold, &len))
+		store_object(OBJ_BLOB, &buf, last, sha1out, mark);
+	else {
+		if (last) {
+			strbuf_release(&last->data);
+			last->offset = 0;
+			last->depth = 0;
+		}
+		stream_blob(len, sha1out, mark);
+		skip_optional_lf();
+	}
+}
+
+static void parse_new_blob(void)
+{
 	read_next_command();
 	parse_mark();
-	parse_data(&buf);
-	store_object(OBJ_BLOB, &buf, &last_blob, NULL, next_mark);
+	parse_and_store_blob(&last_blob, NULL, next_mark);
 }
 
 static void unload_one_branch(void)
@@ -1924,15 +2062,12 @@ static void file_change_m(struct branch *b)
 		 * another repository.
 		 */
 	} else if (inline_data) {
-		static struct strbuf buf = STRBUF_INIT;
-
 		if (p != uq.buf) {
 			strbuf_addstr(&uq, p);
 			p = uq.buf;
 		}
 		read_next_command();
-		parse_data(&buf);
-		store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
+		parse_and_store_blob(&last_blob, sha1, 0);
 	} else if (oe) {
 		if (oe->type != OBJ_BLOB)
 			die("Not a blob (actually a %s): %s",
@@ -2058,15 +2193,12 @@ static void note_change_n(struct branch *b)
 		die("Invalid ref name or SHA1 expression: %s", p);
 
 	if (inline_data) {
-		static struct strbuf buf = STRBUF_INIT;
-
 		if (p != uq.buf) {
 			strbuf_addstr(&uq, p);
 			p = uq.buf;
 		}
 		read_next_command();
-		parse_data(&buf);
-		store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
+		parse_and_store_blob(&last_blob, sha1, 0);
 	} else if (oe) {
 		if (oe->type != OBJ_BLOB)
 			die("Not a blob (actually a %s): %s",
@@ -2232,7 +2364,7 @@ static void parse_new_commit(void)
 	}
 	if (!committer)
 		die("Expected committer but didn't get one");
-	parse_data(&msg);
+	parse_data(&msg, 0, NULL);
 	read_next_command();
 	parse_from(b);
 	merge_list = parse_merge(&merge_count);
@@ -2353,7 +2485,7 @@ static void parse_new_tag(void)
 		tagger = NULL;
 
 	/* tag payload/message */
-	parse_data(&msg);
+	parse_data(&msg, 0, NULL);
 
 	/* build the tag object */
 	strbuf_reset(&new_data);
@@ -2473,6 +2605,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		pack_compression_seen = 1;
 		return 0;
 	}
+	if (!strcmp(k, "core.bigfilethreshold")) {
+		long n = git_config_int(k, v);
+		big_file_threshold = 0 < n ? n : 0;
+	}
 	return git_default_config(k, v, cb);
 }
 
@@ -2518,6 +2654,8 @@ int main(int argc, const char **argv)
 		}
 		else if (!prefixcmp(a, "--max-pack-size="))
 			max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
+		else if (!prefixcmp(a, "--big-file-threshold="))
+			big_file_threshold = strtoumax(a + 21, NULL, 0) * 1024 * 1024;
 		else if (!prefixcmp(a, "--depth=")) {
 			max_depth = strtoul(a + 8, NULL, 0);
 			if (max_depth > MAX_DEPTH)
diff --git a/t/t5705-clone-2gb.sh b/t/t5705-clone-2gb.sh
index 9f52154..adfaae8 100755
--- a/t/t5705-clone-2gb.sh
+++ b/t/t5705-clone-2gb.sh
@@ -31,7 +31,7 @@ test_expect_success 'setup' '
 	 echo "data 5" &&
 	 echo ">2gb" &&
 	 cat commit) |
-	git fast-import &&
+	git fast-import --big-file-threshold=2 &&
 	test ! -f exit-status
 
 '
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..513db86 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1254,4 +1254,50 @@ test_expect_success \
 	'Q: verify note for third commit' \
 	'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
 
+##
+## R: very large blobs
+##
+blobsize=$((2*1024*1024 + 53))
+test-genrandom bar $blobsize >expect
+cat >input <<INPUT_END
+commit refs/heads/big-file
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+R - big file
+COMMIT
+
+M 644 inline big1
+data $blobsize
+INPUT_END
+cat expect >>input
+cat >>input <<INPUT_END
+M 644 inline big2
+data $blobsize
+INPUT_END
+cat expect >>input
+echo >>input
+
+test_expect_success \
+	'R: blob bigger than threshold' \
+	'test_create_repo R &&
+	 git --git-dir=R/.git fast-import --big-file-threshold=1 <input'
+test_expect_success \
+	'R: verify created pack' \
+	': >verify &&
+	 for p in R/.git/objects/pack/*.pack;
+	 do
+	   git verify-pack -v $p >>verify || exit;
+	 done'
+test_expect_success \
+	'R: verify written objects' \
+	'git --git-dir=R/.git cat-file blob big-file:big1 >actual &&
+	 test_cmp expect actual &&
+	 a=$(git --git-dir=R/.git rev-parse big-file:big1) &&
+	 b=$(git --git-dir=R/.git rev-parse big-file:big2) &&
+	 test $a = $b'
+test_expect_success \
+	'R: blob appears only once' \
+	'n=$(grep $a verify | wc -l) &&
+	 test 1 = $n'
+
 test_done
-- 
1.7.0.rc0.170.g7207c

-- 
Shawn.

^ permalink raw reply related

* Re: Custom git completion
From: Junio C Hamano @ 2010-01-29 17:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: David Rhodes Clymer, git
In-Reply-To: <20100129151127.GA21821@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> David Rhodes Clymer <david@zettazebra.com> wrote:
>> Unless I read it incorrectly, the completion script included with
>> git-core does not make it easy for users to write completion scripts
>> for custom git commands. I can extend git itself by creating a command
>> "git-foo", and placing it in my path.
>
> git config --global alias.foo /home/me/bin/my-git-foo
>
> git foo will now complete correctly.  No need to modify the
> completion code.

Yes.  Aliases and custom subcommands are found from 'git help" output just
fine (you need to install new subcommand in exec-path).

But.

How does the completion code learn what options and arguments such aliases
and subcommands (e.g. "git foo") take without being told?

An alias that uses another git subcommand (i.e. the ones that do not start
with a bang "!") seems to be handled correctly, but one of my aliases is
this:

    [alias]
	lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"

and the completion code doesn't (and it is unfair to expect it to) notice
that "git log" is run under the hood.  I cannot say "git lgm sp/<TAB>" and
choose from the list of topics from you.

^ permalink raw reply

* Re: Custom git completion
From: Shawn O. Pearce @ 2010-01-29 17:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Rhodes Clymer, git
In-Reply-To: <7v4om4kdt3.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > David Rhodes Clymer <david@zettazebra.com> wrote:
> >> Unless I read it incorrectly, the completion script included with
> >> git-core does not make it easy for users to write completion scripts
> >> for custom git commands. I can extend git itself by creating a command
> >> "git-foo", and placing it in my path.
> >
> > git config --global alias.foo /home/me/bin/my-git-foo
> >
> > git foo will now complete correctly.  No need to modify the
> > completion code.
> 
> Yes.  Aliases and custom subcommands are found from 'git help" output just
> fine (you need to install new subcommand in exec-path).
> 
> But.
> 
> How does the completion code learn what options and arguments such aliases
> and subcommands (e.g. "git foo") take without being told?

Sure.  But the patch offered by the original poster also suffered
from this problem, it didn't know how to complete arguments for
the subcommand.

 
> An alias that uses another git subcommand (i.e. the ones that do not start
> with a bang "!") seems to be handled correctly, but one of my aliases is
> this:
> 
>     [alias]
> 	lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"

Doing this is difficult, because its hard to parse that string and
do completion on it.  On the other hand, we could do something like:

  [completion]
  	lgm = log

and have `git lgm` complete using the same rules as `git log`.
Its somewhat ugly though...
 
-- 
Shawn.

^ permalink raw reply

* Re: Custom git completion
From: Junio C Hamano @ 2010-01-29 18:02 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: David Rhodes Clymer, git
In-Reply-To: <20100129175950.GE21821@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> An alias that uses another git subcommand (i.e. the ones that do not start
>> with a bang "!") seems to be handled correctly, but one of my aliases is
>> this:
>> 
>>     [alias]
>> 	lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"
>
> Doing this is difficult, because its hard to parse that string and
> do completion on it.

That is why I said it is unfair to expect completion code to do that.

>   [completion]
>   	lgm = log
>
> and have `git lgm` complete using the same rules as `git log`.

I actually like that.  It matches _my_ expectation as a user to be able to
say "this subcommand has args that look like those given to 'log'".

^ permalink raw reply

* Re: [PATCH v2] fast-import: Stream very large blobs directly to pack
From: Jakub Narebski @ 2010-01-29 18:29 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git, Nicolas Pitre
In-Reply-To: <20100129163838.GD21821@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

>  Documentation/git-fast-import.txt |    6 ++
>  fast-import.c                     |  172 +++++++++++++++++++++++++++++++++----
>  t/t5705-clone-2gb.sh              |    2 +-
>  t/t9300-fast-import.sh            |   46 ++++++++++
>  4 files changed, 208 insertions(+), 18 deletions(-)

> +--big-file-threshold=<n>::
> +	Maximum size of a blob that fast-import will attempt to
> +	create a delta for, expressed in MiB.  The default is 512.
> +	Some importers may wish to lower this on systems with
> +	constrained memory.
> +

Shouldn't there be config variable corresponding to this command line
option, so that it can be set once for [constrained] system?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH v2] fast-import: Stream very large blobs directly to pack
From: Shawn O. Pearce @ 2010-01-29 18:30 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, git, Nicolas Pitre
In-Reply-To: <m3aavwbwaz.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> >  Documentation/git-fast-import.txt |    6 ++
> >  fast-import.c                     |  172 +++++++++++++++++++++++++++++++++----
> >  t/t5705-clone-2gb.sh              |    2 +-
> >  t/t9300-fast-import.sh            |   46 ++++++++++
> >  4 files changed, 208 insertions(+), 18 deletions(-)
> 
> > +--big-file-threshold=<n>::
> > +	Maximum size of a blob that fast-import will attempt to
> > +	create a delta for, expressed in MiB.  The default is 512.
> > +	Some importers may wish to lower this on systems with
> > +	constrained memory.
> > +
> 
> Shouldn't there be config variable corresponding to this command line
> option, so that it can be set once for [constrained] system?

Implemented as core.bigFileThreshold in this patch... but I didn't
document it...

-- 
Shawn.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox