{"thread":{"id":"23128","subject":"\"git add -i\" with path gives \"Argument list too long\"","startedAt":"2010-03-21T20:52:56Z","lastAt":"2010-03-22T01:55:47Z","messageCount":5,"participants":["Wincent Colaiuta","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"137482","messageId":"45CD9D1B-6774-46F2-A444-654B1DEF7F69@wincent.com","threadId":"23128","inReplyTo":null,"subject":"\"git add -i\" with path gives \"Argument list too long\"","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2010-03-21T20:52:56Z","receivedAt":"2010-03-21T20:52:56Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Back in January I reported a wart in \"git add -i\" and discussion kind of petered out and I forgot about the bug until today when I ran into it again. Now I have a few more cycles to burn I have the time to sort it out.\n\nJust to revise what was discussed back in January:\n\n- my original report: http://lists-archives.org/git/708569-git-add-i-with-path-gives-argument-list-too-long.html\n\n- quick and easy fix posted by Jeff King: http://lists-archives.org/git/708591-git-add-i-with-path-gives-argument-list-too-long.html\n\n- notes on inconsistencies between \"git add\" and \"git add -i\" by Jeff king: http://lists-archives.org/git/708698-git-add-i-with-path-gives-argument-list-too-long.html\n\nTo summarize and expand a bit upon what Jeff noted in that last post:\n\n  - given _tracked_ file \"foo.c\" and _untracked_ file \"bar.c\"\n  - \"git add '*.c'\" ignores the tracked file and stages the untracked file\n  - \"git add -i '*.c'\" ignores the untracked file and operates on the tracked file\n\nSo, I gather that we can agree that this behavior isn't right. Either we treat the arguments as pathspecs or we don't, but it doesn't seem right to do one thing for tracked files and another for untracked ones, and it seems especially wrong for the differential treatment to apply in one direction with \"git add\" and in the other with \"git add -i\".\n\nSo, as a first step I'd like to make some test cases to serve as a specification for what we want the desired behavior to be. I saw in one of the \"what's cooking messages\" in February that there was some changes cooking in the \"cp/add-u-pathspec\" topic, so I am not entirely clear on if there is consensus about the desired behaviour.\n\nBefore I run ahead and start writing test, do we have consensus?\n\n(Will have to worry about the initial cause of my bug report -- the \"argument list too long\" error -- later on.)\n\nCheers,\nWincent\n"},{"id":"137498","messageId":"20100322003915.GA3212@coredump.intra.peff.net","threadId":"23128","inReplyTo":"45CD9D1B-6774-46F2-A444-654B1DEF7F69@wincent.com","subject":"Re: \"git add -i\" with path gives \"Argument list too long\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-22T00:39:15Z","receivedAt":"2010-03-22T00:39:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 21, 2010 at 09:52:56PM +0100, Wincent Colaiuta wrote:\n\n> Back in January I reported a wart in \"git add -i\" and discussion kind\n> of petered out and I forgot about the bug until today when I ran into\n> it again. Now I have a few more cycles to burn I have the time to sort\n> it out.\n\nThanks for following up.\n\n> To summarize and expand a bit upon what Jeff noted in that last post:\n> \n>   - given _tracked_ file \"foo.c\" and _untracked_ file \"bar.c\"\n>   - \"git add '*.c'\" ignores the tracked file and stages the untracked file\n>   - \"git add -i '*.c'\" ignores the untracked file and operates on the\n>   tracked file\n> [...]\n> So, as a first step I'd like to make some test cases to serve as a\n> specification for what we want the desired behavior to be. I saw in\n\nI'm pretty sure the behavior that we want eventually is for both the\ndiff family (including diff-files), and \"git add\" to learn about the\nglobbing pathspecs that ls-files uses. That will make \"git add\" and \"git\nadd -i\" consistent, and once diff-files understands it, we can drop the\ncall to ls-files in add--interactive, which will solve your \"argument\nlist too long\" problem.\n\nAnd yes, I am being intentionally vague about the behavior beyond saying\n\"do what ls-files does\". Personally I am not too familiar with the\nglobbing pathspecs, so I am not qualified to give an exhaustive list of\ntheir behavior. Junio could probably say more, or you will have to read\nthe code.\n\n-Peff\n"},{"id":"137499","messageId":"7vwrx56tet.fsf@alter.siamese.dyndns.org","threadId":"23128","inReplyTo":"20100322003915.GA3212@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-03-22T01:23:38Z","receivedAt":"2010-03-22T01:23:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> their behavior. Junio could probably say more, or you will have to read\n> the code.\n\nOr read what I already said here a few times ;-) I generally do not want\nto repeat myself.\n\nThere are two semantics of pathspecs:\n\n  (1) exact match, or leading path.  e.g.\n\n  \tgit ls-files Makefile Documentation/\n\n  (2) exact match, leading path, or fnmatch(3).\n\n\tgit ls-files Makefile Documentation/ '*.txt'\n\nThe former is used by the diff family, and the latter by pretty much\neverything else.  In very old days, the former was the only kind we\nsupported, and originally, ls-files didn't even take any pathspecs.\n\nThe 5be4efb ([PATCH] Make \"git-ls-files\" work in subdirectories,\n2005-08-21) taught ls-files to take pathspec that can glob, but diff\nfamily never got updated to match that.\n\nIn order to operate on set of paths, you would need to (a) enumerate your\npaths, and (b) filter that enumeration efficiently with pathspecs.  If you\nare iterating over the index (e.g. \"ls-files\", \"diff-files\", \"grep\"),\nthere is nothing tricky in the enumeration step.  We have a flat array of\nnames in the index and you just walk active_cache[] from 0 to active_nr.\nIf on the other hand you are walking in an inherently hierarchical\nnamespace (e.g. \"ls-tree\", \"diff-tree\", \"grep --no-index\") with non empty\nset of pathspecs, you need to take advantage of the filtering behaviour\nwhile enumerating the paths---otherwise your performance will suck.\n\n\"Leading path\" semantics is easier to understand; if a tree entry you are\nlooking at is \"contrib\", and the pathspecs you have are \"Makefile\" and\n\"Documentation/\", then there is no way for anything underneath it\n(e.g. \"contrib/README\") would survive the filtering process, so you can\nskip the entry without even opening the sub-tree object.  Linus's argument\nwas \"teaching globs to pathspec code would suck in performance\" and he is\nright in general.  Because diff-tree is inherently about walking the two\ntree objects in parallel, it does not extend its pathspec semantics to\nglobbing (i.e. if the user asked for '*.txt', you have to open _all_ the\ntree objects down to the leaf level to see if they contain any file whose\nname ends with .txt), and other family members of diff (namely, diff-files\nand \"diff\" without --cached nor any tree-ish argument) match this\nbehaviour for consistency, even though theoretically \"diff-files\" could\neasily do globbing, as it walks the flat index namespace.\n\nBut I think it is Ok to sacrifice the optimization and descend into any\nand all subtrees/directories to see if a path that might match the pattern\nexists when the user asks for '*.txt', as long as (and this is a _very_\nimportant point) an update to pathspec logic on the diff side does not\nbreak the optimization unnecessarily.  E.g.\n\n\tgit diff v1.0.0 v1.2.0 -- Makefile 'Documentation/*.txt'\n\nshould still skip opening tree object for 'contrib/' (because anything\nunderneath contrib/ would never match either pathspecs given), but can\nand should descend into Documentation.  And it should _not_ skip 'howto'\nsubdirectory in Documentation/ directory, as it could find a match with\n'*.txt' in that subdirectory.\n\nTo prepare for this, later reimplementations of pathspec matching logic\n(the one used by \"git grep\") can compute hints meant to be used by the\npath enumeration step, as I explained earlier, enumeration needs to take\nadvantage of what filtering would do to paths that it will find.\n\nBy the way I threw this \"pathspec unification\" to the list of possible\nGSoC ideas, but I suspect it might be a bit too much work to do this\nproperly for a summer student (and also we might not want to trust this\nimportant part of the system to a summer student).  Other things that\nprobably needs to be thought through (and I haven't) that may be related\nto this codepath is how to handle case-insensitive filesystems.  I think\nwe currently do not match paths that we obtain from the filesystem case\ninsensitively with the given pathspecs (we probably shouldn't go case\ninsensitive when we are walking the index or the tree objects, on the\nother hand).\n"},{"id":"137500","messageId":"20100322014100.GA7717@coredump.intra.peff.net","threadId":"23128","inReplyTo":"7vwrx56tet.fsf@alter.siamese.dyndns.org","subject":"Re: \"git add -i\" with path gives \"Argument list too long\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-22T01:41:00Z","receivedAt":"2010-03-22T01:41:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 21, 2010 at 06:23:38PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > their behavior. Junio could probably say more, or you will have to read\n> > the code.\n> \n> Or read what I already said here a few times ;-) I generally do not want\n> to repeat myself.\n\nWell, yeah. :) What I meant was:\n\n> There are two semantics of pathspecs:\n> \n>   (1) exact match, or leading path.  e.g.\n> \n>   \tgit ls-files Makefile Documentation/\n> \n>   (2) exact match, leading path, or fnmatch(3).\n> \n> \tgit ls-files Makefile Documentation/ '*.txt'\n\nIs type (2) exactly fnmatch? Or are there subtle corner cases that\nshould be tested? For the implementation it shouldn't matter, as\nhopefully the same code paths will be used, and \"just like ls-files\"\nshould be specification enough. But for testing, it might be nice to\nenumerate some of the more subtle cases.\n\n-Peff\n"},{"id":"137501","messageId":"7veijd3ysc.fsf@alter.siamese.dyndns.org","threadId":"23128","inReplyTo":"20100322014100.GA7717@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-03-22T01:55:47Z","receivedAt":"2010-03-22T01:55:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Is type (2) exactly fnmatch?\n\nIt calls fnmatch(3) but without FNM_PATHNAME nor FNM_PERIOD, which makes\nme sometimes wonder if it was a historical mistake and go to \"git log\" to\ncheck, but it is more-or-less deliberate choice, as illustrated by the\n'arch/i386/*.[ch]' example in 5be4efb ([PATCH] Make \"git-ls-files\" work in\nsubdirectories, 2005-08-21).\n"}]}