# "git add -i" with path gives "Argument list too long" 5 messages from 2010-01-04 to 2010-01-06. Participants: Wincent Colaiuta, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/22094 ## Wincent Colaiuta, 2010-01-04 18:43 Subject: "git add -i" with path gives "Argument list too long" Message-ID: <36FEB8A0-968D-4B43-AEFB-9B0E227A1F88@wincent.com> URL: https://gitlist.dev/e/36FEB8A0-968D-4B43-AEFB-9B0E227A1F88%40wincent.com ``` Just ran "git add -i " with "" pointing to a subdirectory which happens to have a bunch of files in it (about 7k) and it barfed thusly: Can't exec "git": Argument list too long at /usr/local/libexec/git- core/git-add--interactive line 158. Died at /usr/local/libexec/git-core/git-add--interactive line 158. I see that what it's trying to do under the hood is: git diff-index --cached --numstat --summary HEAD -- <7,000+ paths...> Sure, we could divide the paths into smaller groups, run multiple invocations of "git diff-index", and concatenate the results. But it would be nicer if there was some other way that we could get at the same information without having to pass 7,000 paths explicitly on the command line; is there any which I am overlooking? The enormous file list is the result of passing into "git ls- files -- ". Would it be worth: - either, modifying "git diff-index" to accept a list of paths over stdin so that we could at least pipe the output from "git ls-files" into "git diff-index" - or, preferably, teach "git diff index" to recurse into directories rather than expect a list of paths-of-blobs (possibly with a command line switch to activate the behaviour if it were deemed a dangerous default) This is one piece of plumbing that I've never dabbled with, so forgive me if my questions are a little dumb. Cheers, Wincent ``` ## Jeff King, 2010-01-05 04:14 Subject: Re: "git add -i" with path gives "Argument list too long" Message-ID: <20100105041438.GB12574@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100105041438.GB12574%40coredump.intra.peff.net In-Reply-To: <36FEB8A0-968D-4B43-AEFB-9B0E227A1F88@wincent.com> ``` [cc'd Junio: there is a question of path limiters versus pathspecs near the bottom]. On Mon, Jan 04, 2010 at 07:43:10PM +0100, Wincent Colaiuta wrote: > Just ran "git add -i " with "" pointing to a subdirectory > which happens to have a bunch of files in it (about 7k) and it barfed > thusly: > > Can't exec "git": Argument list too long at /usr/local/libexec/git- > core/git-add--interactive line 158. > Died at /usr/local/libexec/git-core/git-add--interactive line 158. > > I see that what it's trying to do under the hood is: > > git diff-index --cached --numstat --summary HEAD -- <7,000+ paths...> Yep, and there is a similar diff-files call after that. > Sure, we could divide the paths into smaller groups, run multiple > invocations of "git diff-index", and concatenate the results. But it > would be nicer if there was some other way that we could get at the > same information without having to pass 7,000 paths explicitly on the > command line; is there any which I am overlooking? No, I don't think there is way to do it with the current code short of breaking up the output. > The enormous file list is the result of passing into "git ls- > files -- ". Would it be worth: > > - either, modifying "git diff-index" to accept a list of paths over > stdin so that we could at least pipe the output from "git ls-files" > into "git diff-index" We could do that. It would also need a patch for diff-files. And unfortunately it makes their interface somewhat inconsistent then with diff-tree, which already has a --stdin but uses it for a list of tree-ishes. > - or, preferably, teach "git diff index" to recurse into directories > rather than expect a list of paths-of-blobs (possibly with a command > line switch to activate the behaviour if it were deemed a dangerous > default) Doesn't it already do this? If I say "git diff index subdir" it will limit the diff only to things inside subdir/. In fact, it is tempting to do this: diff --git a/git-add--interactive.perl b/git-add--interactive.perl index bfd1003..a8b3e30 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -275,15 +275,6 @@ sub list_modified { my ($only) = @_; my (%data, @return); my ($add, $del, $adddel, $file); - my @tracked = (); - - if (@ARGV) { - @tracked = map { - chomp $_; - unquote_path($_); - } run_cmd_pipe(qw(git ls-files --), @ARGV); - return if (!@tracked); - } my $reference; if (defined $patch_mode_revision and $patch_mode_revision ne 'HEAD') { @@ -295,7 +286,7 @@ sub list_modified { } for (run_cmd_pipe(qw(git diff-index --cached --numstat --summary), $reference, - '--', @tracked)) { + '--', @ARGV)) { if (($add, $del, $file) = /^([-\d]+) ([-\d]+) (.*)/) { my ($change, $bin); @@ -320,7 +311,7 @@ sub list_modified { } } - for (run_cmd_pipe(qw(git diff-files --numstat --summary --), @tracked)) { + for (run_cmd_pipe(qw(git diff-files --numstat --summary --), @ARGV)) { if (($add, $del, $file) = /^([-\d]+) ([-\d]+) (.*)/) { $file = unquote_path($file); but note that the pathspecs given to ls-files and the path limiters given to diff are not quite the same. So "git add -i '*.c'" will currently find "subdir/foo.c", but would not with the above patch. Is that what you meant when you said "recurse into directories"? I seem to recall Junio noting in the past the inconsistencies in git about what is a path and what is a pathspec. Is this one of those inconsistencies, and would it be a positive thing to fix it? -Peff ``` ## Junio C Hamano, 2010-01-05 05:31 Subject: Re: "git add -i" with path gives "Argument list too long" Message-ID: <7vzl4t6ru1.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzl4t6ru1.fsf%40alter.siamese.dyndns.org In-Reply-To: <20100105041438.GB12574@coredump.intra.peff.net> ``` Jeff King writes: > I seem to recall Junio noting in the past the inconsistencies in git > about what is a path and what is a pathspec. Is this one of those > inconsistencies, and would it be a positive thing to fix it? We actually never take paths and everything we take is pathspec. One longstanding gripe I had around this area is there are two kinds of pathspecs. "diff-tree" family of pathspecs only match "leading directories" while "dir.c" and "builtin-grep.c" pathspecs allow both "leading directories" and "glob" pathspecs. Teaching "diff-tree" family to also grok globs would be a very nice thing to do, and the listing of paths with ls-files in the sample patch you are removing is indeed a workaround for this issue. ``` ## Wincent Colaiuta, 2010-01-05 12:34 Subject: Re: "git add -i" with path gives "Argument list too long" Message-ID: URL: https://gitlist.dev/e/AAABEE5B-0419-4344-86E2-7EB550CCFC01%40wincent.com In-Reply-To: <20100105041438.GB12574@coredump.intra.peff.net> ``` El 05/01/2010, a las 05:14, Jeff King escribió: >> - or, preferably, teach "git diff index" to recurse into directories >> rather than expect a list of paths-of-blobs (possibly with a command >> line switch to activate the behaviour if it were deemed a dangerous >> default) > > Doesn't it already do this? If I say "git diff index subdir" it > will limit the diff only to things inside subdir/. [snip patch] I tried out the patch and it obviously does avoid the "Argument list too long" problem. At least for my usage patterns the superficial differences in behavior that you note would not be a problem (I usually want to limit things to a subdir, and seldom if ever pass in things like '*.c'). > but note that the pathspecs given to ls-files and the path limiters > given to diff are not quite the same. So "git add -i '*.c'" will > currently find "subdir/foo.c", but would not with the above patch. Is > that what you meant when you said "recurse into directories"? In my relative ignorance of the finer details here, I meant that I would want "diff-index" to give us the exact same set of blobs as we get from "ls-files", so as to fix the error without modifying the user visible behavior. As I said, I personally wouldn't be impacted by the change in behavior that your patch produces, but maybe others might. Cheers, Wincent ``` ## Jeff King, 2010-01-06 12:19 Subject: Re: "git add -i" with path gives "Argument list too long" Message-ID: <20100106121927.GC7221@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100106121927.GC7221%40coredump.intra.peff.net In-Reply-To: ``` On Tue, Jan 05, 2010 at 01:34:32PM +0100, Wincent Colaiuta wrote: > >but note that the pathspecs given to ls-files and the path limiters > >given to diff are not quite the same. So "git add -i '*.c'" will > >currently find "subdir/foo.c", but would not with the above patch. Is > >that what you meant when you said "recurse into directories"? > > In my relative ignorance of the finer details here, I meant that I > would want "diff-index" to give us the exact same set of blobs as we > get from "ls-files", so as to fix the error without modifying the > user visible behavior. > > As I said, I personally wouldn't be impacted by the change in > behavior that your patch produces, but maybe others might. I also don't care about the globbing feature, though I suppose some people do. However, I'm not sure add's behavior is all that sensible now. Interactive add respects the globs, but regular add does not. Worse, it seems that it notes that the pathspec is a wildcard and does not even flag an error for failing to find any files. For example: $ git init $ echo content >foo.c $ mkdir subdir && echo content >subdir/bar.c $ git add . $ echo change >foo.c && echo change >subdir/bar.c $ git diff --name-only foo.c subdir/bar.c $ git add foobar ;# should barf, and does fatal: pathspec 'foobar' did not match any files $ git add '*.c' ;# does not barf, but does not respect wildcard $ git diff --name-only foo.c subdir/bar.c $ yes | git add -p '*.c' ;# respects glob $ git diff --name-only ;# empty So it's an consistency that should probably be fixed. And of course it is tempting to fix it by disabling the globs for the interactive case, which would not involve writing any new code. ;) But I don't think it is a good idea to punish people by taking away their feature in the name of consistency. -Peff ```