{"thread":{"id":"22094","subject":"\"git add -i\" with path gives \"Argument list too long\"","startedAt":"2010-01-04T18:43:10Z","lastAt":"2010-01-06T12:19:27Z","messageCount":5,"participants":["Wincent Colaiuta","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"130788","messageId":"36FEB8A0-968D-4B43-AEFB-9B0E227A1F88@wincent.com","threadId":"22094","inReplyTo":null,"subject":"\"git add -i\" with path gives \"Argument list too long\"","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2010-01-04T18:43:10Z","receivedAt":"2010-01-04T18:43:10Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Just ran \"git add -i \" with \"\" pointing to a subdirectory \nwhich happens to have a bunch of files in it (about 7k) and it barfed \nthusly:\n\n Can't exec \"git\": Argument list too long at /usr/local/libexec/git- \ncore/git-add--interactive line 158.\n Died at /usr/local/libexec/git-core/git-add--interactive line 158.\n\nI see that what it's trying to do under the hood is:\n\n git diff-index --cached --numstat --summary HEAD -- <7,000+ paths...>\n\nSure, we could divide the paths into smaller groups, run multiple \ninvocations of \"git diff-index\", and concatenate the results. But it \nwould be nicer if there was some other way that we could get at the \nsame information without having to pass 7,000 paths explicitly on the \ncommand line; is there any which I am overlooking?\n\nThe enormous file list is the result of passing into \"git ls- \nfiles -- \". Would it be worth:\n\n- either, modifying \"git diff-index\" to accept a list of paths over \nstdin so that we could at least pipe the output from \"git ls-files\" \ninto \"git diff-index\"\n\n- or, preferably, teach \"git diff index\" to recurse into directories \nrather than expect a list of paths-of-blobs (possibly with a command \nline switch to activate the behaviour if it were deemed a dangerous \ndefault)\n\nThis is one piece of plumbing that I've never dabbled with, so forgive \nme if my questions are a little dumb.\n\nCheers,\nWincent\n"},{"id":"130810","messageId":"20100105041438.GB12574@coredump.intra.peff.net","threadId":"22094","inReplyTo":"36FEB8A0-968D-4B43-AEFB-9B0E227A1F88@wincent.com","subject":"Re: \"git add -i\" with path gives \"Argument list too long\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-05T04:14:38Z","receivedAt":"2010-01-05T04:14:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[cc'd Junio: there is a question of path limiters versus pathspecs near\nthe bottom].\n\nOn Mon, Jan 04, 2010 at 07:43:10PM +0100, Wincent Colaiuta wrote:\n\n> Just ran \"git add -i \" with \"\" pointing to a subdirectory\n> which happens to have a bunch of files in it (about 7k) and it barfed\n> thusly:\n> \n> Can't exec \"git\": Argument list too long at /usr/local/libexec/git-\n> core/git-add--interactive line 158.\n> Died at /usr/local/libexec/git-core/git-add--interactive line 158.\n> \n> I see that what it's trying to do under the hood is:\n> \n> git diff-index --cached --numstat --summary HEAD -- <7,000+ paths...>\n\nYep, and there is a similar diff-files call after that.\n\n> Sure, we could divide the paths into smaller groups, run multiple\n> invocations of \"git diff-index\", and concatenate the results. But it\n> would be nicer if there was some other way that we could get at the\n> same information without having to pass 7,000 paths explicitly on the\n> command line; is there any which I am overlooking?\n\nNo, I don't think there is way to do it with the current code short of\nbreaking up the output.\n\n> The enormous file list is the result of passing into \"git ls-\n> files -- \". Would it be worth:\n> \n> - either, modifying \"git diff-index\" to accept a list of paths over\n> stdin so that we could at least pipe the output from \"git ls-files\"\n> into \"git diff-index\"\n\nWe could do that. It would also need a patch for diff-files. And\nunfortunately it makes their interface somewhat inconsistent then with\ndiff-tree, which already has a --stdin but uses it for a list of\ntree-ishes.\n\n> - or, preferably, teach \"git diff index\" to recurse into directories\n> rather than expect a list of paths-of-blobs (possibly with a command\n> line switch to activate the behaviour if it were deemed a dangerous\n> default)\n\nDoesn't it already do this? If I say \"git diff index subdir\" it\nwill limit the diff only to things inside subdir/.\n\nIn fact, it is tempting to do this:\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex bfd1003..a8b3e30 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -275,15 +275,6 @@ sub list_modified {\n \tmy ($only) = @_;\n \tmy (%data, @return);\n \tmy ($add, $del, $adddel, $file);\n-\tmy @tracked = ();\n-\n-\tif (@ARGV) {\n-\t\t@tracked = map {\n-\t\t\tchomp $_;\n-\t\t\tunquote_path($_);\n-\t\t} run_cmd_pipe(qw(git ls-files --), @ARGV);\n-\t\treturn if (!@tracked);\n-\t}\n \n \tmy $reference;\n \tif (defined $patch_mode_revision and $patch_mode_revision ne 'HEAD') {\n@@ -295,7 +286,7 @@ sub list_modified {\n \t}\n \tfor (run_cmd_pipe(qw(git diff-index --cached\n \t\t\t --numstat --summary), $reference,\n-\t\t\t '--', @tracked)) {\n+\t\t\t '--', @ARGV)) {\n \t\tif (($add, $del, $file) =\n \t\t /^([-\\d]+)\t([-\\d]+)\t(.*)/) {\n \t\t\tmy ($change, $bin);\n@@ -320,7 +311,7 @@ sub list_modified {\n \t\t}\n \t}\n \n-\tfor (run_cmd_pipe(qw(git diff-files --numstat --summary --), @tracked)) {\n+\tfor (run_cmd_pipe(qw(git diff-files --numstat --summary --), @ARGV)) {\n \t\tif (($add, $del, $file) =\n \t\t /^([-\\d]+)\t([-\\d]+)\t(.*)/) {\n \t\t\t$file = unquote_path($file);\n\nbut note that the pathspecs given to ls-files and the path limiters\ngiven to diff are not quite the same. So \"git add -i '*.c'\" will\ncurrently find \"subdir/foo.c\", but would not with the above patch. Is\nthat what you meant when you said \"recurse into directories\"?\n\nI seem to recall Junio noting in the past the inconsistencies in git\nabout what is a path and what is a pathspec. Is this one of those\ninconsistencies, and would it be a positive thing to fix it?\n\n-Peff\n"},{"id":"130816","messageId":"7vzl4t6ru1.fsf@alter.siamese.dyndns.org","threadId":"22094","inReplyTo":"20100105041438.GB12574@coredump.intra.peff.net","subject":"Re: \"git add -i\" with path gives \"Argument list too long\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-05T05:31:02Z","receivedAt":"2010-01-05T05:31:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I seem to recall Junio noting in the past the inconsistencies in git\n> about what is a path and what is a pathspec. Is this one of those\n> inconsistencies, and would it be a positive thing to fix it?\n\nWe actually never take paths and everything we take is pathspec. One\nlongstanding gripe I had around this area is there are two kinds of\npathspecs. \"diff-tree\" family of pathspecs only match \"leading\ndirectories\" while \"dir.c\" and \"builtin-grep.c\" pathspecs allow both\n\"leading directories\" and \"glob\" pathspecs. Teaching \"diff-tree\" family\nto also grok globs would be a very nice thing to do, and the listing of\npaths with ls-files in the sample patch you are removing is indeed a\nworkaround for this issue.\n"},{"id":"130841","messageId":"AAABEE5B-0419-4344-86E2-7EB550CCFC01@wincent.com","threadId":"22094","inReplyTo":"20100105041438.GB12574@coredump.intra.peff.net","subject":"Re: \"git add -i\" with path gives \"Argument list too long\"","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2010-01-05T12:34:32Z","receivedAt":"2010-01-05T12:34:32Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 05/01/2010, a las 05:14, Jeff King escribió:\n\n>> - or, preferably, teach \"git diff index\" to recurse into directories\n>> rather than expect a list of paths-of-blobs (possibly with a command\n>> line switch to activate the behaviour if it were deemed a dangerous\n>> default)\n>\n> Doesn't it already do this? If I say \"git diff index subdir\" it\n> will limit the diff only to things inside subdir/.\n\n[snip patch]\n\nI tried out the patch and it obviously does avoid the \"Argument list \ntoo long\" problem. At least for my usage patterns the superficial \ndifferences in behavior that you note would not be a problem (I \nusually want to limit things to a subdir, and seldom if ever pass in \nthings like '*.c').\n\n> but note that the pathspecs given to ls-files and the path limiters\n> given to diff are not quite the same. So \"git add -i '*.c'\" will\n> currently find \"subdir/foo.c\", but would not with the above patch. Is\n> that what you meant when you said \"recurse into directories\"?\n\nIn my relative ignorance of the finer details here, I meant that I \nwould want \"diff-index\" to give us the exact same set of blobs as we \nget from \"ls-files\", so as to fix the error without modifying the user \nvisible behavior.\n\nAs I said, I personally wouldn't be impacted by the change in behavior \nthat your patch produces, but maybe others might.\n\nCheers,\nWincent\n"},{"id":"130917","messageId":"20100106121927.GC7221@coredump.intra.peff.net","threadId":"22094","inReplyTo":"AAABEE5B-0419-4344-86E2-7EB550CCFC01@wincent.com","subject":"Re: \"git add -i\" with path gives \"Argument list too long\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-06T12:19:27Z","receivedAt":"2010-01-06T12:19:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 05, 2010 at 01:34:32PM +0100, Wincent Colaiuta wrote:\n\n> >but note that the pathspecs given to ls-files and the path limiters\n> >given to diff are not quite the same. So \"git add -i '*.c'\" will\n> >currently find \"subdir/foo.c\", but would not with the above patch. Is\n> >that what you meant when you said \"recurse into directories\"?\n> \n> In my relative ignorance of the finer details here, I meant that I\n> would want \"diff-index\" to give us the exact same set of blobs as we\n> get from \"ls-files\", so as to fix the error without modifying the\n> user visible behavior.\n> \n> As I said, I personally wouldn't be impacted by the change in\n> behavior that your patch produces, but maybe others might.\n\nI also don't care about the globbing feature, though I suppose some\npeople do. However, I'm not sure add's behavior is all that sensible\nnow. Interactive add respects the globs, but regular add does not.\nWorse, it seems that it notes that the pathspec is a wildcard and does\nnot even flag an error for failing to find any files. For example:\n\n $ git init\n $ echo content >foo.c\n $ mkdir subdir && echo content >subdir/bar.c\n $ git add .\n $ echo change >foo.c && echo change >subdir/bar.c\n $ git diff --name-only\n foo.c\n subdir/bar.c\n $ git add foobar ;# should barf, and does\n fatal: pathspec 'foobar' did not match any files\n $ git add '*.c' ;# does not barf, but does not respect wildcard\n $ git diff --name-only\n foo.c\n subdir/bar.c\n $ yes | git add -p '*.c' ;# respects glob\n $ git diff --name-only ;# empty\n\nSo it's an consistency that should probably be fixed. And of course it\nis tempting to fix it by disabling the globs for the interactive case,\nwhich would not involve writing any new code. ;) But I don't think it is\na good idea to punish people by taking away their feature in the name of\nconsistency.\n\n-Peff\n"}]}