{"thread":{"id":"20818","subject":"[BUG] 'add -u' doesn't work from untracked subdir","startedAt":"2009-09-02T08:03:05Z","lastAt":"2009-09-10T20:32:09Z","messageCount":35,"participants":["SZEDER Gábor","Jeff King","Clemens Buchacher","Junio C Hamano","Jakub Narebski","Matthieu Moy","Nanako Shiraishi"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"122297","messageId":"20090902080305.GA11549@neumann","threadId":"20818","inReplyTo":null,"subject":"[BUG] 'add -u' doesn't work from untracked subdir","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-09-02T08:03:05Z","receivedAt":"2009-09-02T08:03:05Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nHi,\n\n\nAs the subject says, 'git add -u' does not work from an untracked\nsubdir, because it doesn't add modified files to the index.  The\nfollowing script reproduces the issue:\n\nmkdir repo\ncd repo\ngit init\necho 1 >foo\ngit add foo\ngit commit -m first\necho 2 >foo\nmkdir untracked_subdir\ncd untracked_subdir\ngit add -u\ngit diff\n\nIt worked in the initial 'git add -u' implementation (dfdac5d, git-add\n-u: match the index with working tree, 2007-04-20), but 2ed2c222\n(git-add -u paths... now works from subdirectory, 2007-08-16) broke it\nlater, and is broken ever since.\n\n\nRegards,\nGábor\n"},{"id":"122298","messageId":"20090902081917.GA5447@coredump.intra.peff.net","threadId":"20818","inReplyTo":"20090902080305.GA11549@neumann","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-02T08:19:17Z","receivedAt":"2009-09-02T08:19:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 02, 2009 at 10:03:05AM +0200, SZEDER Gábor wrote:\n\n> As the subject says, 'git add -u' does not work from an untracked\n> subdir, because it doesn't add modified files to the index.  The\n> following script reproduces the issue:\n> \n> mkdir repo\n> cd repo\n> git init\n> echo 1 >foo\n> git add foo\n> git commit -m first\n> echo 2 >foo\n> mkdir untracked_subdir\n> cd untracked_subdir\n> git add -u\n> git diff\n> \n> It worked in the initial 'git add -u' implementation (dfdac5d, git-add\n> -u: match the index with working tree, 2007-04-20), but 2ed2c222\n> (git-add -u paths... now works from subdirectory, 2007-08-16) broke it\n> later, and is broken ever since.\n\nIt is not just untracked subdirs. Try:\n\n  mkdir repo && cd repo && git init\n  echo 1 >foo\n  mkdir subdir\n  echo 1 >subdir/bar\n  git add . && git commit -m first\n  echo 2 >foo\n  echo 2 >subdir/bar\n  cd subdir\n  git add -u\n  git diff ;# still shows foo/1 in index\n  git diff --cached ;# shows subdir/bar was updated\n\nWhile I have sometimes found the behavior a bit annoying[1], I always\nassumed that was the intended behavior.\n\nAnd indeed, in modern builtin-add.c, we find this:\n\n        if ((addremove || take_worktree_changes) && !argc) {\n                static const char *here[2] = { \".\", NULL };\n                argc = 1;\n                argv = here;\n        }\n\nwhich seems pretty explicit.\n\n-Peff\n\n[1] I would prefer \"git add -u .\" to add only the current directory, and\n\"git add -u\" to touch everything. But then, I am one of the people who\nturn off status.relativepaths, so I think I may be in the minority in\nalways wanting to think of the project as a whole.\n"},{"id":"122389","messageId":"20090904070216.GA3996@darc.dnsalias.org","threadId":"20818","inReplyTo":"20090902081917.GA5447@coredump.intra.peff.net","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-04T07:02:16Z","receivedAt":"2009-09-04T07:02:16Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:\n\n> [1] I would prefer \"git add -u .\" to add only the current directory, and\n> \"git add -u\" to touch everything.\n\nFWIW, I feel the same way. And there is no easy way to do that now. (cd `git\nrev-parse --show-cdup`; git add -u) ?\n\n> But then, I am one of the people who\n> turn off status.relativepaths, so I think I may be in the minority in\n> always wanting to think of the project as a whole.\n\nThat mindset is one of git's greatest strengths IMO.\n\nClemens\n"},{"id":"122397","messageId":"20090904083242.GT6144@neumann","threadId":"20818","inReplyTo":"20090902081917.GA5447@coredump.intra.peff.net","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-09-04T08:32:42Z","receivedAt":"2009-09-04T08:32:42Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\n[Oops, I 've just noticed that my reply to Jeff didn't made it to the\ngit list, because I hit 'reply' instead of 'reply to all'...]\n\n\n\nHi Jeff,\n\n\nthanks for your quick reply.\n\nOn Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:\n> On Wed, Sep 02, 2009 at 10:03:05AM +0200, SZEDER Gábor wrote:\n>\n> > As the subject says, 'git add -u' does not work from an untracked\n> > subdir, because it doesn't add modified files to the index.  The\n> > following script reproduces the issue:\n> >\n> > mkdir repo\n> > cd repo\n> > git init\n> > echo 1 >foo\n> > git add foo\n> > git commit -m first\n> > echo 2 >foo\n> > mkdir untracked_subdir\n> > cd untracked_subdir\n> > git add -u\n> > git diff\n> >\n> > It worked in the initial 'git add -u' implementation (dfdac5d,\n> > git-add\n> > -u: match the index with working tree, 2007-04-20), but 2ed2c222\n> > (git-add -u paths... now works from subdirectory, 2007-08-16)\n> > broke it\n> > later, and is broken ever since.\n>\n> It is not just untracked subdirs. Try:\n>\n>   mkdir repo && cd repo && git init\n>   echo 1 >foo\n>   mkdir subdir\n>   echo 1 >subdir/bar\n>   git add . && git commit -m first\n>   echo 2 >foo\n>   echo 2 >subdir/bar\n>   cd subdir\n>   git add -u\n>   git diff ;# still shows foo/1 in index\n>   git diff --cached ;# shows subdir/bar was updated\n>\n> While I have sometimes found the behavior a bit annoying[1], I\n> always\n> assumed that was the intended behavior.\n>\n> And indeed, in modern builtin-add.c, we find this:\n>\n>         if ((addremove || take_worktree_changes) && !argc) {\n>                 static const char *here[2] = { \".\", NULL };\n>                 argc = 1;\n>                 argv = here;\n>         }\n>\n> which seems pretty explicit.\n\nSince then I looked at the man page (I should have done that right\naway ;), and it says under the description of -u that \"If no paths are\nspecified, all tracked files in the current directory and its\nsubdirectories are updated.\"  So this is indeed the intended\nbehaviour, but I was just not aware of it.  Oh well, sorry for the\nnoise.\n\n> [1] I would prefer \"git add -u .\" to add only the current directory,\n> and\n> \"git add -u\" to touch everything. But then, I am one of the people\n> who\n> turn off status.relativepaths, so I think I may be in the minority\n> in\n> always wanting to think of the project as a whole.\n\nI don't really know which would I prefer.\n\nI was updating some Javadoc documentation in Eclipse, and checking the\ngenerated docs in terminal, deep down in an untracked subdir, and\nperformed some 'add -u ; commit --amend' from there (and was rather\nsurprised after the fifth amend to see all the changes still in the\nworktree).  Doing perform the desired add -u from there I should have\nrun 'git add -u ../../../../../..', what doesn't seem very convenient.\nBut since this was the first time I've done that since 2007-08-16, I\nguess it's not a very common use case.\n\n\nGábor\n"},{"id":"122482","messageId":"20090905061804.GB29863@coredump.intra.peff.net","threadId":"20818","inReplyTo":"20090904070216.GA3996@darc.dnsalias.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-05T06:18:04Z","receivedAt":"2009-09-05T06:18:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 04, 2009 at 09:02:16AM +0200, Clemens Buchacher wrote:\n\n> On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:\n> \n> > [1] I would prefer \"git add -u .\" to add only the current directory, and\n> > \"git add -u\" to touch everything.\n> \n> FWIW, I feel the same way. And there is no easy way to do that now. (cd `git\n> rev-parse --show-cdup`; git add -u) ?\n\nI suspect it is too late to change it due to compatibility issues. OTOH,\nI think the intent of v1.7.0 is to allow a few small breakages like\nthese. You could always write an RFC patch and generate some discussion;\nI'm not 100% sure that there are enough people that agree with us to\nchange the default.\n\nI guess we could add a \"git add --absolute\" option, though it is\nslightly annoying, because I generally do not realize that I needed to\nuse such an option until several minutes after running \"git add\".\n\nI would be fine with a \"be absolute, not relative\" config option such as\nwhat we have for status (in fact, a global \"be absolute, not relative\"\noption to cover all commands might be handy). The only obstacle is that\nI think \"git add\" is often used as plumbing in scripts (arguably, they\nshould be using update-index, but \"git add .\" is just so convenient).\n\n-Peff\n"},{"id":"122488","messageId":"7v8wgt98ms.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090905061804.GB29863@coredump.intra.peff.net","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-05T07:02:35Z","receivedAt":"2009-09-05T07:02:35Z","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> I suspect it is too late to change it due to compatibility issues. OTOH,\n> I think the intent of v1.7.0 is to allow a few small breakages like\n> these. You could always write an RFC patch and generate some discussion;\n> I'm not 100% sure that there are enough people that agree with us to\n> change the default.\n\nThe intent of 1.7.0 is to fix usability glitches and warts that everybody\nhas long agreed are problematic.  People have *just started* discussing\nabout this---it is not remotely close to \"everybody has long agreed are\nproblematic\" criteria.  It is too late for 1.7.0.\n\nI agree that there are parts of git that is very whole tree oriented, and\nthe later \"usability\" part that are cwd centric.  \"add -u\" and \"grep\" are\nexamples of the latter.\n\nI personally find \"add -u\" that defaults to the current directory more\nnatural than always going to the root; same preference for \"grep\".\nBesides, \"add -u subdir\" must add subdir relative to the cwd, without\ngoing to the root.  Why should \"add -u\" sans argument behave drastically\ndifferently?\n\nSpeaking of cwd-ness, I sometimes find ls-tree irritating, but I think\nthis is in \"if we had known better we would have designed it differently,\nbut because we didn't, because many scripts already depend on the current\nbehaviour, and because we have an --full-name escape hatch, we are not\nlikely to change it, ever\" category.\n\nIf \"git add -u ../..\" (I mean \"the grand parent directory\", not \"an\nunnamed subdirectory\") did not work, it would be unexcusable and we would\nwant to devise an migration path, but otherwise I do not think it is such\na big deal.  I would say the commands that are used to incrementally build\ntowards the next commit should be friendly to the practice of limiting the\nscope of the work by chdir, i.e. they should be cwd centric.  On the other\nhand, the commands that are used to review the next commit as a whole,\ne.g. diff and patch, should be whole-tree oriented.\n\nOh, \"git grep -e foo ../..\", however, does not seem to work.  That might be\nsomething people may want to tackle.\n"},{"id":"122489","messageId":"20090905072017.GA5152@coredump.intra.peff.net","threadId":"20818","inReplyTo":"7v8wgt98ms.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-05T07:20:17Z","receivedAt":"2009-09-05T07:20:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:\n\n> The intent of 1.7.0 is to fix usability glitches and warts that everybody\n> has long agreed are problematic.  People have *just started* discussing\n> about this---it is not remotely close to \"everybody has long agreed are\n> problematic\" criteria.  It is too late for 1.7.0.\n\nWhat about a config option that could change the behavior? True, the\ntime-frame for discussion is much shorter, but we are not proposing to\nmake a change that would affect users who do not agree to it. And I\nthink the point of giving a long time-frame for discussion is to let\npeople decide whether a change that users do not agree to may affect\nthem in a bad way.\n\nThe real danger here is that users of the config option may be breaking\nan interface that is used by scripts. But I feel that 1.7.0 is probably\nthe best time in the forseeable future to do that, as script-writers\nalready must be wary of the version change.\n\n> I personally find \"add -u\" that defaults to the current directory more\n> natural than always going to the root; same preference for \"grep\".\n> Besides, \"add -u subdir\" must add subdir relative to the cwd, without\n> going to the root.  Why should \"add -u\" sans argument behave drastically\n> differently?\n\nI agree that there is a certain consistency to the current behavior. But\nI also find it terribly annoying, because I _always_ want it to do the\nother thing, and it silently accepts the command without even telling\nme, leaving me to find out ten minutes later that what I thought was\nadded was not (\"git add\", by contrast, yells at you in the same\nsituation).\n\nI also happen to prefer the other behavior because it is easy to switch\nthe two options: \"git add -u\" versus \"git add -u .\", whereas with\ncurrent behavior I am stuck calculating (and typing) the correct number\nof \"../\" markers.\n\nBut I respect the fact that even if we had infinite time for discussion,\nthere would be people who prefer it the opposite way to me. So how about\nthat config option?\n\n> Speaking of cwd-ness, I sometimes find ls-tree irritating, but I think\n> this is in \"if we had known better we would have designed it differently,\n> but because we didn't, because many scripts already depend on the current\n> behaviour, and because we have an --full-name escape hatch, we are not\n> likely to change it, ever\" category.\n\nI assume you mean \"ls-files\".  I have every once in a while been annoyed\nby that, but given how infrequently I run ls-files, it is not a big\ndeal. :)\n\n> If \"git add -u ../..\" (I mean \"the grand parent directory\", not \"an\n> unnamed subdirectory\") did not work, it would be unexcusable and we would\n> want to devise an migration path, but otherwise I do not think it is such\n> a big deal.  I would say the commands that are used to incrementally build\n\nAs I mentioned above, not only is that annoying to use, but the real\nproblem is that I _expect_ the other behavior and it silently does the\nopposite of what I want. You can argue that my brain is defective (for\nnot remembering, I mean -- we _know_ it's defective in other ways), but\ncertainly a config option would be useful to me.\n\n> Oh, \"git grep -e foo ../..\", however, does not seem to work.  That might be\n> something people may want to tackle.\n\nThanks for mentioning \"git grep\"; I had forgotten that I have been\nbitten by expecting full-tree behavior from that in the past, too. A\nconfig option should cover that, too. ;)\n\n-Peff\n"},{"id":"122490","messageId":"7vljkt7szf.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"7v8wgt98ms.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-05T07:25:56Z","receivedAt":"2009-09-05T07:25:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> I suspect it is too late to change it due to compatibility issues. OTOH,\n>> I think the intent of v1.7.0 is to allow a few small breakages like\n>> these. You could always write an RFC patch and generate some discussion;\n>> I'm not 100% sure that there are enough people that agree with us to\n>> change the default.\n>\n> The intent of 1.7.0 is to fix usability glitches and warts that everybody\n> has long agreed are problematic.  People have *just started* discussing\n> about this---it is not remotely close to \"everybody has long agreed are\n> problematic\" criteria.  It is too late for 1.7.0.\n\nI just wanted to see if I am being fair.  Here are the topics that we may\nhave in 1.7.0.\n\n * jc/1.7.0-push-safety\n\n   Prevents gremlin updates from sideways to a repository with a work\n   tree, that confused countless new people.  I've resisted this change\n   for a long time on the backward compatiblity ground, but it is very\n   fair to say that it was long agreed that the benefit from the change\n   far outweigh the donesides of having to say \"I do want the old\n   behaviour\" by setting an configuration variable or two.\n\n * jc/1.7.0-send-email-no-thread-default\n\n   Defaults multi-message send-email to thread shallowly.  This change was\n   requested by kernel folks for a long time ago, and discussion on-list\n   resulted in a declaration that unless nobody objects 1.6.2 release\n   notes will say the default will change in 1.6.3.  We did not hear any\n   objection, but the switchover did not happen ;-).\n\n * jc/1.7.0-status\n\n   Everybody hated that \"status $args\" being \"commit --dry-run $args\"\n   since 1.4.0 days.  We will give \"commit --dry-run $args\" in 1.6.5.\n\n * jc/1.7.0-diff-whitespace-only-status\n\n   We said \"diff -w\" only affects the appearance but not the exit code, so\n   \"diff -w --exit-code\" never returned success if there were only\n   whitespace changes.  It was noticed to be illogical since day one\n   of the introduction of --exit-code, but we simply did not bother to fix\n   the implementation of -b and -w, since the combination of these two\n   options were thought to be unlikely, and we were simply lazy ;-)\n\nI think the first three are clearly 1.7.0 candidates, judging from the\nbenefit/downside perspective and also from the escape-hatch perspective,\nThe last one is probably less so, but on the other hand it is of far\nlessor impact that we could even roll it out as a bugfix on 'maint'.\n"},{"id":"122491","messageId":"7v3a717rgl.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090905072017.GA5152@coredump.intra.peff.net","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-05T07:58:50Z","receivedAt":"2009-09-05T07:58:50Z","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> I assume you mean \"ls-files\".  I have every once in a while been annoyed\n> by that, but given how infrequently I run ls-files, it is not a big\n> deal. :)\n\nI did mean ls-tree, but I misspelled the name of the escape hatch.\n\nTry this:\n\n    $ cd Documentation\n    $ git ls-tree HEAD\n\nIf I were designing this as a proper plumbing command from scratch, I\nwouldn't have given it a cwd behaviour.  ls-files is somewhat more\nunderstandable, as it has other cruft relating the work tree, but ls-tree\nis worse:\n\n    $ cd Documentation\n    $ git ls-tree origin/html\n\nWhoa???  Yes, it tried to do what \"git ls-tree origin/html:Documentation\"\nwould have done if it were unaware of cwd.  It's just crazy.\n\n>> If \"git add -u ../..\" (I mean \"the grand parent directory\", not \"an\n>> unnamed subdirectory\") did not work, it would be unexcusable and we would\n>> want to devise an migration path, but otherwise I do not think it is such\n>> a big deal.  I would say the commands that are used to incrementally build\n>\n> As I mentioned above, not only is that annoying to use, but the real\n> problem is that I _expect_ the other behavior and it silently does the\n> opposite of what I want. You can argue that my brain is defective (for\n> not remembering, I mean -- we _know_ it's defective in other ways), but\n> certainly a config option would be useful to me.\n\nAt this moment (as my brain is not quite functioning), I can only say we\nagreed to disagree what feels more natural here.\n"},{"id":"122492","messageId":"20090905080249.GA8801@coredump.intra.peff.net","threadId":"20818","inReplyTo":"7v3a717rgl.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-05T08:02:49Z","receivedAt":"2009-09-05T08:02:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I assume you mean \"ls-files\".  I have every once in a while been annoyed\n> > by that, but given how infrequently I run ls-files, it is not a big\n> > deal. :)\n> \n> I did mean ls-tree, but I misspelled the name of the escape hatch.\n\nOh, I never noticed that behavior before. For \"ls-files\", I think it is\nat least a little sane, but it makes no sense whatsoever for ls-tree.\n\n> At this moment (as my brain is not quite functioning), I can only say we\n> agreed to disagree what feels more natural here.\n\nI agree that we agreed to disagree. But there is still one open\nquestion: would you take a patch for a \"full-tree\" config variable that\nwould impact add (and probably grep) for 1.7.0? You can sleep on it if\nyou want. ;)\n\n-Peff\n"},{"id":"122495","messageId":"20090905081915.GA11392@coredump.intra.peff.net","threadId":"20818","inReplyTo":"20090905072017.GA5152@coredump.intra.peff.net","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-05T08:19:16Z","receivedAt":"2009-09-05T08:19:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 05, 2009 at 03:20:17AM -0400, Jeff King wrote:\n\n> As I mentioned above, not only is that annoying to use, but the real\n> problem is that I _expect_ the other behavior and it silently does the\n> opposite of what I want. You can argue that my brain is defective (for\n> not remembering, I mean -- we _know_ it's defective in other ways), but\n> certainly a config option would be useful to me.\n\nBah. Even after this long thread, I _still_ forgot. I just now typed\n\"git add -u\" from t/ and got annoyed that my changes in the root weren't\nadded.\n\n-Peff\n"},{"id":"122497","messageId":"7vy6ot4x61.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090905080249.GA8801@coredump.intra.peff.net","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-05T08:23:50Z","receivedAt":"2009-09-05T08:23:50Z","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> I agree that we agreed to disagree. But there is still one open\n> question: would you take a patch for a \"full-tree\" config variable that\n> would impact add (and probably grep) for 1.7.0?\n\nUnless there is a simple and sane way for script writers (and by \"script\",\nI do not mean \"Porcelain that is supposed to be written using only the\nplumbing\", but things like scripts you would give to \"git bisect run\",\nwhich would freely use Porcelains like \"git reset\" etc.) to defeat the\nconfiguration without much pain, I am fairly negative on adding any\nconfiguration variable of that nature.\n\nWe could probably declare \"In 1.X.0 everything will be relative to the\nroot and you have to give an explicit '.' if you mean the cwd\".\n\nThree questions:\n\n #1 What are the commands that will be affected, other than \"add -u\" and\n    \"grep\"?  Are there others?\n\n #2 Do all the commands in the answer to #1 currently behave exactly the\n    same when run without any path parameter and when run with a single\n    '.'?\n\n #3 Do all the commands that are already relative to the root currently\n    limit their action to the cwd when run with a single '.'?\n\nIf the number of commands in the answer to #1 is not too excessive, it is\na plus, but even if it is more than just several, we will be getting\nconsistency and sanity if #2 and #3 hold.  However, if there are even a\nsingle violator in #2 and #3, we would need to fix them first before we\ncan proceed.  And the transition clock starts ticking after everything is\nfixed (if such a fix is indeed needed).  As usual, I'd prefer to keep the\nclock running for at least 6 months, preferrably longer, and during that\ntime, we may need the usual \"You invoked me without any paths, but this\ncommand will start behaving differently in 1.X.0, you have been warned.\"\n\nA command line option to explicitly ask full-tree can be added anytime\nwithout waiting for 1.7.0.  I do not think it will be ready for 1.6.5 but\nwe can always have 1.6.6 if needed.\n"},{"id":"122499","messageId":"20090905084641.GA24865@darc.dnsalias.org","threadId":"20818","inReplyTo":"7v8wgt98ms.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-05T08:46:41Z","receivedAt":"2009-09-05T08:46:41Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:\n\n> I personally find \"add -u\" that defaults to the current directory more\n> natural than always going to the root; same preference for \"grep\".\n> Besides, \"add -u subdir\" must add subdir relative to the cwd, without\n> going to the root.  Why should \"add -u\" sans argument behave drastically\n> differently?\n\nSorry for stating the obvious here, but the following commands affect the\nentire repository, even though they limit themselves to the current\ndirectory, if passed a '.'.\n\n\tgit commit\n\tgit log\n\tgit diff\n\tgit checkout\n\tgit reset\n\nDue to the frequent use of these commands, I believe many users (myself\nincluded) expect \"git add\" and \"git grep\" to do the same. AFAICT the\nfollowing commands are the only non-plumbing ones that behave differently:\n\n\tgit add -u\n\tgit add -A\n\tgit grep\n\nSo I argue that _that_ is the real inconsistency.\n\n> If \"git add -u ../..\" (I mean \"the grand parent directory\", not \"an\n> unnamed subdirectory\") did not work, it would be unexcusable and we would\n> want to devise an migration path, but otherwise I do not think it is such\n> a big deal.\n\n> I would say the commands that are used to incrementally build\n> towards the next commit should be friendly to the practice of limiting the\n> scope of the work by chdir.\n\n\"git add -u .\" is friendly enough. Just like \"git commit .\" versus \"git\ncommit -a\", which is exactly the same concept and should therefore have the\nsame behavior.\n\nYou are assuming that people are in a subdirectory because they want to\nlimit the scope. But I am usually in a subdirectory for totally\nversioning-unrelated reasons. Like running tests in git.git:t/ . I\nmistakenly use \"git add -u\" in there all the time, because I think I don't\nhave to worry about which directory I'm in. Except in this instance I do.\n\nIn any case, I think it is better to have consistent behavior than to try\nand read users' minds with defaults.\n\nClemens\n"},{"id":"122512","messageId":"20090905123117.GA3099@darc.dnsalias.org","threadId":"20818","inReplyTo":"7v3a717rgl.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] grep: accept relative paths outside current working directory","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-05T12:31:17Z","receivedAt":"2009-09-05T12:31:17Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:\n> >> If \"git add -u ../..\" (I mean \"the grand parent directory\", not \"an\n> >> unnamed subdirectory\") did not work \n\nI just noticed that this doesn't work with grep. In git.git:\n\n$ cd t\n$ git grep addremove -- ../\nfatal: git grep: cannot generate relative filenames containing '..'\n\nSo here's a fix for that. And a configurable solution for add and grep's\nscope as a follow-up. I did not look at any other commands yet.\n\nClemens\n\n--o<--\nPreviously, \"git grep\" would bark at relative paths pointing outside\nthe current working directory (or subdirectories thereof). We already\nhave quote_path_relative(), which can handle such cases just fine. So\nuse that instead.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n builtin-grep.c |   44 ++++++++++++++------------------------------\n grep.h         |    1 +\n 2 files changed, 15 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex ad0e0a5..f6af3d4 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -13,6 +13,7 @@\n #include \"parse-options.h\"\n #include \"userdiff.h\"\n #include \"grep.h\"\n+#include \"quote.h\"\n \n #ifndef NO_EXTERNAL_GREP\n #ifdef __unix__\n@@ -152,40 +153,27 @@ static int pathspec_matches(const char **paths, const char *name, int max_depth)\n \treturn 0;\n }\n \n-static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1, const char *name, int tree_name_len)\n+static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1, const char *path, int tree_name_len)\n {\n \tunsigned long size;\n \tchar *data;\n \tenum object_type type;\n-\tchar *to_free = NULL;\n \tint hit;\n+\tstruct strbuf pathbuf = STRBUF_INIT;\n \n \tdata = read_sha1_file(sha1, &type, &size);\n \tif (!data) {\n-\t\terror(\"'%s': unable to read %s\", name, sha1_to_hex(sha1));\n+\t\terror(\"'%s': unable to read %s\", path, sha1_to_hex(sha1));\n \t\treturn 0;\n \t}\n \tif (opt->relative && opt->prefix_length) {\n-\t\tstatic char name_buf[PATH_MAX];\n-\t\tchar *cp;\n-\t\tint name_len = strlen(name) - opt->prefix_length + 1;\n-\n-\t\tif (!tree_name_len)\n-\t\t\tname += opt->prefix_length;\n-\t\telse {\n-\t\t\tif (ARRAY_SIZE(name_buf) <= name_len)\n-\t\t\t\tcp = to_free = xmalloc(name_len);\n-\t\t\telse\n-\t\t\t\tcp = name_buf;\n-\t\t\tmemcpy(cp, name, tree_name_len);\n-\t\t\tstrcpy(cp + tree_name_len,\n-\t\t\t       name + tree_name_len + opt->prefix_length);\n-\t\t\tname = cp;\n-\t\t}\n+\t\tquote_path_relative(path + tree_name_len, -1, &pathbuf, opt->prefix);\n+\t\tstrbuf_insert(&pathbuf, 0, path, tree_name_len);\n+\t\tpath = pathbuf.buf;\n \t}\n-\thit = grep_buffer(opt, name, data, size);\n+\thit = grep_buffer(opt, path, data, size);\n+\tstrbuf_release(&pathbuf);\n \tfree(data);\n-\tfree(to_free);\n \treturn hit;\n }\n \n@@ -195,6 +183,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tint i;\n \tchar *data;\n \tsize_t sz;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n \tif (lstat(filename, &st) < 0) {\n \terr_ret:\n@@ -219,8 +208,9 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \t}\n \tclose(i);\n \tif (opt->relative && opt->prefix_length)\n-\t\tfilename += opt->prefix_length;\n+\t\tfilename = quote_path_relative(filename, -1, &buf, opt->prefix);\n \ti = grep_buffer(opt, filename, data, sz);\n+\tstrbuf_release(&buf);\n \tfree(data);\n \treturn i;\n }\n@@ -798,6 +788,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t};\n \n \tmemset(&opt, 0, sizeof(opt));\n+\topt.prefix = prefix;\n \topt.prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;\n \topt.relative = 1;\n \topt.pathname = 1;\n@@ -868,15 +859,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tverify_filename(prefix, argv[j]);\n \t}\n \n-\tif (i < argc) {\n+\tif (i < argc)\n \t\tpaths = get_pathspec(prefix, argv + i);\n-\t\tif (opt.prefix_length && opt.relative) {\n-\t\t\t/* Make sure we do not get outside of paths */\n-\t\t\tfor (i = 0; paths[i]; i++)\n-\t\t\t\tif (strncmp(prefix, paths[i], opt.prefix_length))\n-\t\t\t\t\tdie(\"git grep: cannot generate relative filenames containing '..'\");\n-\t\t}\n-\t}\n \telse if (prefix) {\n \t\tpaths = xcalloc(2, sizeof(const char *));\n \t\tpaths[0] = prefix;\ndiff --git a/grep.h b/grep.h\nindex 28e6b2a..f6eecc6 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -59,6 +59,7 @@ struct grep_opt {\n \tstruct grep_pat *pattern_list;\n \tstruct grep_pat **pattern_tail;\n \tstruct grep_expr *pattern_expression;\n+\tconst char *prefix;\n \tint prefix_length;\n \tregex_t regexp;\n \tint linenum;\n-- \n1.6.4.2.264.g79b4f\n"},{"id":"122513","messageId":"20090905123304.GB3099@darc.dnsalias.org","threadId":"20818","inReplyTo":"20090905123117.GA3099@darc.dnsalias.org","subject":"[PATCH 2/2] add 'scope' config option","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-05T12:33:04Z","receivedAt":"2009-09-05T12:33:04Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Documentation/config.txt says it all.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n Documentation/config.txt |    6 ++++++\n builtin-add.c            |    2 +-\n builtin-grep.c           |    2 +-\n cache.h                  |    1 +\n config.c                 |    8 ++++++++\n environment.c            |    1 +\n 6 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5256c7f..f587cf1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -439,6 +439,12 @@ On some file system/operating system combinations, this is unreliable.\n Set this config setting to 'rename' there; However, This will remove the\n check that makes sure that existing object files will not get overwritten.\n \n+core.scope::\n+\tBy default, the commands 'git add -u', 'git add -A' and 'git grep'\n+\tare limited to files below the current working directory\n+\t(scope='workdir'). Set this variable to scope='global' to make these\n+\tcommands act on the whole tree instead.\n+\n add.ignore-errors::\n \tTells 'git-add' to continue adding files when some files cannot be\n \tadded due to indexing errors. Equivalent to the '--ignore-errors'\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 006fd08..33ea3e4 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -285,7 +285,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (addremove && take_worktree_changes)\n \t\tdie(\"-A and -u are mutually incompatible\");\n-\tif ((addremove || take_worktree_changes) && !argc) {\n+\tif (!scope_global && (addremove || take_worktree_changes) && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n \t\targc = 1;\n \t\targv = here;\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex f6af3d4..447b195 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -861,7 +861,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (i < argc)\n \t\tpaths = get_pathspec(prefix, argv + i);\n-\telse if (prefix) {\n+\telse if (!scope_global && prefix) {\n \t\tpaths = xcalloc(2, sizeof(const char *));\n \t\tpaths[0] = prefix;\n \t\tpaths[1] = NULL;\ndiff --git a/cache.h b/cache.h\nindex 5fad24c..85c5fee 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -523,6 +523,7 @@ extern int auto_crlf;\n extern int read_replace_refs;\n extern int fsync_object_files;\n extern int core_preload_index;\n+extern int scope_global;\n \n enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\ndiff --git a/config.c b/config.c\nindex e87edea..8dec019 100644\n--- a/config.c\n+++ b/config.c\n@@ -503,6 +503,14 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.scope\")) {\n+\t\tif (!strcasecmp(value, \"global\"))\n+\t\t\tscope_global = 1;\n+\t\tif (!strcasecmp(value, \"worktree\"))\n+\t\t\tscope_global = 0;\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex 5de6837..4d1c6e1 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -50,6 +50,7 @@ enum push_default_type push_default = PUSH_DEFAULT_MATCHING;\n #endif\n enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;\n int grafts_replace_parents = 1;\n+int scope_global = 0;\n \n /* Parallel index stat data preload? */\n int core_preload_index = 0;\n-- \n1.6.4.2.264.g79b4f\n"},{"id":"122516","messageId":"20090905131033.GA3742@localhost","threadId":"20818","inReplyTo":"20090905123304.GB3099@darc.dnsalias.org","subject":"[PATCH 2/2 v2] add 'scope' config option","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-05T13:10:33Z","receivedAt":"2009-09-05T13:10:33Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Sep 05, 2009 at 02:33:04PM +0200, Clemens Buchacher wrote:\n> -\tif ((addremove || take_worktree_changes) && !argc) {\n> +\tif (!scope_global && (addremove || take_worktree_changes) && !argc) {\n>  \t\tstatic const char *here[2] = { \".\", NULL };\n>  \t\targc = 1;\n>  \t\targv = here;\n\nOk, scrape that. Seems validate_pathspec does that implicitly already. So\nthis code was redundant. The attached patch should now work (I tested it\nthis time...).\n\nIt also seems that \"git add -A\" doesn't work without a pathspec any more?\nWas that intentional?\n\nClemens\n\n--o<--\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n Documentation/config.txt |    6 ++++++\n builtin-add.c            |   12 +++++-------\n builtin-grep.c           |    2 +-\n cache.h                  |    1 +\n config.c                 |    8 ++++++++\n environment.c            |    1 +\n 6 files changed, 22 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5256c7f..f587cf1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -439,6 +439,12 @@ On some file system/operating system combinations, this is unreliable.\n Set this config setting to 'rename' there; However, This will remove the\n check that makes sure that existing object files will not get overwritten.\n \n+core.scope::\n+\tBy default, the commands 'git add -u', 'git add -A' and 'git grep'\n+\tare limited to files below the current working directory\n+\t(scope='workdir'). Set this variable to scope='global' to make these\n+\tcommands act on the whole tree instead.\n+\n add.ignore-errors::\n \tTells 'git-add' to continue adding files when some files cannot be\n \tadded due to indexing errors. Equivalent to the '--ignore-errors'\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 006fd08..737b155 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -285,14 +285,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (addremove && take_worktree_changes)\n \t\tdie(\"-A and -u are mutually incompatible\");\n-\tif ((addremove || take_worktree_changes) && !argc) {\n-\t\tstatic const char *here[2] = { \".\", NULL };\n-\t\targc = 1;\n-\t\targv = here;\n-\t}\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n-\trequire_pathspec = !take_worktree_changes;\n+\trequire_pathspec = !addremove && !take_worktree_changes;\n \n \tnewfd = hold_locked_index(&lock_file, 1);\n \n@@ -308,7 +303,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tfprintf(stderr, \"Maybe you wanted to say 'git add .'?\\n\");\n \t\treturn 0;\n \t}\n-\tpathspec = validate_pathspec(argc, argv, prefix);\n+\tif (scope_global && !argc)\n+\t\tpathspec = NULL;\n+\telse\n+\t\tpathspec = validate_pathspec(argc, argv, prefix);\n \n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex f6af3d4..447b195 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -861,7 +861,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (i < argc)\n \t\tpaths = get_pathspec(prefix, argv + i);\n-\telse if (prefix) {\n+\telse if (!scope_global && prefix) {\n \t\tpaths = xcalloc(2, sizeof(const char *));\n \t\tpaths[0] = prefix;\n \t\tpaths[1] = NULL;\ndiff --git a/cache.h b/cache.h\nindex 5fad24c..85c5fee 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -523,6 +523,7 @@ extern int auto_crlf;\n extern int read_replace_refs;\n extern int fsync_object_files;\n extern int core_preload_index;\n+extern int scope_global;\n \n enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\ndiff --git a/config.c b/config.c\nindex e87edea..8dec019 100644\n--- a/config.c\n+++ b/config.c\n@@ -503,6 +503,14 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.scope\")) {\n+\t\tif (!strcasecmp(value, \"global\"))\n+\t\t\tscope_global = 1;\n+\t\tif (!strcasecmp(value, \"worktree\"))\n+\t\t\tscope_global = 0;\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex 5de6837..4d1c6e1 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -50,6 +50,7 @@ enum push_default_type push_default = PUSH_DEFAULT_MATCHING;\n #endif\n enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;\n int grafts_replace_parents = 1;\n+int scope_global = 0;\n \n /* Parallel index stat data preload? */\n int core_preload_index = 0;\n-- \n1.6.4.2.266.gbaa17\n"},{"id":"122524","messageId":"7veiql1etz.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090905084641.GA24865@darc.dnsalias.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-05T17:28:08Z","receivedAt":"2009-09-05T17:28:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> \"git add -u .\" is friendly enough. Just like \"git commit .\" versus \"git\n> commit -a\", which is exactly the same concept and should therefore have the\n> same behavior.\n>\n> You are assuming that people are in a subdirectory because they want to\n> limit the scope. But I am usually in a subdirectory for totally\n> versioning-unrelated reasons.\n\nLimit the scope of what you see in \"ls\" (no argument) output, shorten the\npaths you must type to non-git commands.  They are the kind of \"limit the\nscope\" I meant, and they are totally versioning-unrelated.  In other\nwords, cwd-centric default helps the users (especially the new ones) by\nmaking git behave consistently with other commands.\n\nSo if anything, I personally think it would be much less surprising if all\ngit commands worked relative to the cwd (not whole tree root) when run\nwithout path argument, at least from the newbie's point of view [*1*].\n\nBut notice that the above is qualified with \"personally\".  An alternative\nwould be to declare that in 1.8.0, all commands run without path argument\nwill work on the whole tree and you have to give an explicit '.' when you\nwant to limit their effect to the cwd.\n\nThis may be slightly less intuitive to newbies than the \"relative to cwd\",\nbut nevertheless that is the course I would suggest us taking, because of\nthe following observations:\n\n (1) if the commands work on the whole tree when run without paths, it is\n     easy to limit to the cwd with \"git frotz .\" when you want to.\n\n (2) if the commands work on the cwd when run without paths, you have to\n     always be aware how deep you are, and say \"git frotz ../../..\" when\n     you want to extend their effects to the whole tree.\n\nThe latter is much more irritating.\n\nPlease also see:\n\n    Message-ID: <7vy6ot4x61.fsf@alter.siamese.dyndns.org> ($gmane/127795)\n\nthink about the three questions there, and help us move the discussion\nforward.\n\nThe first part of the message has some comments related to your patch, by\nthe way.\n\n[Footnote]\n\n*1* Except for the ones that cannot make any sense to limit their\noperation to a subdirectory you happen to be in (e.g. it would be insane\nif \"git am\" run to accept somebody's patch ignored paths outside your\ncwd).\n"},{"id":"122526","messageId":"m3tyzhjmvc.fsf@localhost.localdomain","threadId":"20818","inReplyTo":"7veiql1etz.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-09-05T17:58:56Z","receivedAt":"2009-09-05T17:58:56Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > \"git add -u .\" is friendly enough. Just like \"git commit .\" versus \"git\n> > commit -a\", which is exactly the same concept and should therefore have the\n> > same behavior.\n> >\n> > You are assuming that people are in a subdirectory because they want to\n> > limit the scope. But I am usually in a subdirectory for totally\n> > versioning-unrelated reasons.\n> \n> Limit the scope of what you see in \"ls\" (no argument) output, shorten the\n> paths you must type to non-git commands.  They are the kind of \"limit the\n> scope\" I meant, and they are totally versioning-unrelated.  In other\n> words, cwd-centric default helps the users (especially the new ones) by\n> making git behave consistently with other commands.\n\nWell, there is still complication that some commands are considered\nwhole-tree in absence of pathspec, like \"git commit\".\n\n> \n> So if anything, I personally think it would be much less surprising if all\n> git commands worked relative to the cwd (not whole tree root) when run\n> without path argument, at least from the newbie's point of view [*1*].\n\nI think it would be very suprising if \"git commit\" in subdirectory was\nlimited to changes affecting given subdirectory...\n\n> \n> But notice that the above is qualified with \"personally\".  An alternative\n> would be to declare that in 1.8.0, all commands run without path argument\n> will work on the whole tree and you have to give an explicit '.' when you\n> want to limit their effect to the cwd.\n> \n> This may be slightly less intuitive to newbies than the \"relative to cwd\",\n> but nevertheless that is the course I would suggest us taking, because of\n> the following observations:\n> \n>  (1) if the commands work on the whole tree when run without paths, it is\n>      easy to limit to the cwd with \"git frotz .\" when you want to.\n> \n>  (2) if the commands work on the cwd when run without paths, you have to\n>      always be aware how deep you are, and say \"git frotz ../../..\" when\n>      you want to extend their effects to the whole tree.\n> \n> The latter is much more irritating.\n\nWell, we can always invent some magic notation meaning either \"up to\ntop directory\", e.g. make\n\n  $ git frotz ...\n\nmore or less equivalent to\n\n  $ git frotz \"$(git rev-parse --show-cdup)\"\n\n(The other solution of having \"git frotz /\" to refer to top directory\nis slightly worse, because there are git commands that work without\ngit repository, and \"/\" is legitimate parameter, like e.g. for \n\"git diff --no-index\").\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"122528","messageId":"20090905184508.GA20124@localhost","threadId":"20818","inReplyTo":"7veiql1etz.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-05T18:45:08Z","receivedAt":"2009-09-05T18:45:08Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Sep 05, 2009 at 10:28:08AM -0700, Junio C Hamano wrote:\n> But notice that the above is qualified with \"personally\".  An alternative\n> would be to declare that in 1.8.0, all commands run without path argument\n> will work on the whole tree and you have to give an explicit '.' when you\n> want to limit their effect to the cwd.\n\nThat's fine by me.\n\n[...]\n>\n> Please also see:\n> \n>     Message-ID: <7vy6ot4x61.fsf@alter.siamese.dyndns.org> ($gmane/127795)\n> \n> think about the three questions there, and help us move the discussion\n> forward.\n\nThe patch is my way of saying that I'm not just whining, but that I'm\nwilling to put some effort into getting this done. I am aware of the issues\nyou raised in the above message and I was hoping that my patch would help us\nas a starting point to move the discussion forward.\n\n> The first part of the message has some comments related to your patch, by\n> the way.\n\nI can only guess that you mean the \"sane way for script writers to defeat\nthe configuration without much pain\" part. But I'm not sure how that's a\nproblem. If you want the script to continue to work as before you either\nconfigure \"workdir scope\", or you add a '.' to the affected commands.\n\nClemens\n"},{"id":"122530","messageId":"7vk50dysif.fsf_-_@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090905184508.GA20124@localhost","subject":"Re: 'add -u' without path is relative to cwd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-05T21:46:16Z","receivedAt":"2009-09-05T21:46:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> I can only guess that you mean the \"sane way for script writers to defeat\n> the configuration without much pain\" part. But I'm not sure how that's a\n> problem. If you want the script to continue to work as before you either\n> configure \"workdir scope\", or you add a '.' to the affected commands.\n\nOne who writes the script and lends it to you may be using different\n`scope` from what the recipient uses, so that is not an escape hatch at\nall.\n\nOne crudest form of workaround may be for your code to notice an\nenvironment variable to override that `scope` configuration setting, so\nthat you can advise the script writers to set it in their script.  But\nthat is so ugly I'd rather not go there if we do not absolutely have to.\n\nThat is why in general we should be very careful and avoid any magic that\nmakes the same command behave completely differently depending on how a\nrepository is configured.\n"},{"id":"122553","messageId":"vpqfxb0s177.fsf@bauges.imag.fr","threadId":"20818","inReplyTo":"20090905084641.GA24865@darc.dnsalias.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-09-06T12:32:44Z","receivedAt":"2009-09-06T12:32:44Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:\n>\n>> I personally find \"add -u\" that defaults to the current directory more\n>> natural than always going to the root; same preference for \"grep\".\n>> Besides, \"add -u subdir\" must add subdir relative to the cwd, without\n>> going to the root.  Why should \"add -u\" sans argument behave drastically\n>> differently?\n>\n> Sorry for stating the obvious here, but the following commands affect the\n> entire repository, even though they limit themselves to the current\n> directory, if passed a '.'.\n>\n> \tgit commit\n> \tgit log\n> \tgit diff\n> \tgit checkout\n> \tgit reset\n\nYou have to add \"git add -e\", \"git add -i\" and \"git add -p\" here.\n\nI completely agree that \"git add -u\" should have been a full-tree\noriented command, just like other \"git add\" variants and other Git\ncommands, from the beginning.\n\nNow, I'm unconfortable with both a behavior change and a config\noption. Someone used to the cwd-limited behavior typing \"git add -u\"\non a machine configured to git the full-tree behavior could be really\nannoyed (not even mentionning scripts).\n\nI think it has already been proposed to introduce \"git add -a\" doing\nwhat \"git add -u\" do, but for the full tree. The \"-a\" option here\nbeing analogous to the one of \"git commit\": roughly, \"git add -a; git\ncommit\" would be equivalent to \"git commit -a\". This would allow a\nlong deprecation period for \"git add -u\". I find the proposal\nsensible, but IIRC it has already been rejected.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"122562","messageId":"20090906181621.GA23412@localhost","threadId":"20818","inReplyTo":"vpqfxb0s177.fsf@bauges.imag.fr","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-06T18:16:21Z","receivedAt":"2009-09-06T18:16:21Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Sep 06, 2009 at 02:32:44PM +0200, Matthieu Moy wrote:\n> I think it has already been proposed to introduce \"git add -a\" doing\n> what \"git add -u\" do, but for the full tree.\n\nI like that, actually. AFAICT it's completely analogous to \"git commit -a\".\nWe also need something for \"git add -A\" though.\n\nDo you feel the same way about changing the behavior of \"git grep\"? I don't\nreally want to change the command's name.\n\nClemens\n"},{"id":"122564","messageId":"20090906182855.GB23412@localhost","threadId":"20818","inReplyTo":"7vy6ot4x61.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-06T18:28:55Z","receivedAt":"2009-09-06T18:28:55Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Sep 05, 2009 at 01:23:50AM -0700, Junio C Hamano wrote:\n\n>  #1 What are the commands that will be affected, other than \"add -u\" and\n>     \"grep\"?  Are there others?\n\nThere are \"ls-tree\", \"ls-files\" and \"clean\". I can't find anything else.\n\n>  #2 Do all the commands in the answer to #1 currently behave exactly the\n>     same when run without any path parameter and when run with a single\n>     '.'?\n\nYes. But \"ls-tree\" and \"ls-files\" do not accept superdirectories in their\npath specs.\n\n>  #3 Do all the commands that are already relative to the root currently\n>     limit their action to the cwd when run with a single '.'?\n\nI can't think of one that doesn't.\n\nClemens\n"},{"id":"122575","messageId":"7v3a6zu1dk.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090905123117.GA3099@darc.dnsalias.org","subject":"Re: [PATCH 1/2] grep: accept relative paths outside current working directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-06T22:58:15Z","receivedAt":"2009-09-06T22:58:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:\n>> >> If \"git add -u ../..\" (I mean \"the grand parent directory\", not \"an\n>> >> unnamed subdirectory\") did not work \n>\n> In git.git:\n>\n> $ cd t\n> $ git grep addremove -- ../\n> fatal: git grep: cannot generate relative filenames containing '..'\n>\n> So here's a fix for that. And a configurable solution for add and grep's\n> scope as a follow-up. I did not look at any other commands yet.\n\nThanks.  This was oa breakage I pointed out in an earlier message in this\ndiscussion, and it is worth fixing.\n\nYour patch is queued in 'pu', but it seems to break the exit status in a\nstrange way with my limited test.\n\nHere is a non-broken behaviour without the \"look in uplevel\":\n\n: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete .\n: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete .; echo $?\n1\n: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete . >/dev/null; echo $?\n1\n\nNow we go down, and grep from an uplevel:\n\n: git.git/cb/maint-1.6.3-grep-relative-up; cd t\n: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete ..\n: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete .. ; echo $?\n1\n: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete .. >/dev/null; echo $?\n0\n\nThe command should not give different exit status depending on the\ndestination of standard output stream.\n"},{"id":"122576","messageId":"20090907090713.6117@nanako3.lavabit.com","threadId":"20818","inReplyTo":"20090905084641.GA24865@darc.dnsalias.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-09-07T00:07:13Z","receivedAt":"2009-09-07T00:07:13Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Clemens Buchacher <drizzd@aon.at>\n\n> Sorry for stating the obvious here, but the following commands affect the\n> entire repository, even though they limit themselves to the current\n> directory, if passed a '.'.\n>\n> \tgit commit\n> \tgit log\n> \tgit diff\n> \tgit checkout\n> \tgit reset\n>\n> Due to the frequent use of these commands, I believe many users (myself\n> included) expect \"git add\" and \"git grep\" to do the same. AFAICT the\n> following commands are the only non-plumbing ones that behave differently:\n>\n> \tgit add -u\n> \tgit add -A\n> \tgit grep\n>\n> So I argue that _that_ is the real inconsistency.\n\nThe default behavior for 'git-grep' has already been discussed in length, and I don't think it is likely to change. See \n\n  http://thread.gmane.org/gmane.comp.version-control.git/111519/focus=111717\n\nThe original design for the other two in your list was to be a whole tree operation. This commit broke it. \n\n  2ed2c22 \"git-add -u paths... now works from subdirectory\".\n\n'git-add -u' in a subdirectory without any other argument used to work on the entire working tree before that commit, but it didn't prefix the current directory in front of the paths... arguments. \n\nThat commit 2ed2c22 fixed 'git-add -u paths...' by prepending the prefix to the arguments, but it broke 'git-add -u' to always limit the updates to the current directory. \n\nI think it is a good idea to fix this as an old regression in the maint branch. You don't have to introduce \"git add -a\". In fact the -a option was explicitly rejected when \"git add -A\" option was added with this commit. \n\n  3ba1f11 \"git-add --all: add all files\"\n\nbecause \"git commit -a\" will never include new files and it will be inconsistent if \"git add -a\" did so.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"122581","messageId":"7v1vmj9wcq.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090907090713.6117@nanako3.lavabit.com","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-07T05:07:01Z","receivedAt":"2009-09-07T05:07:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n[jc: sometimes but not always your messages have looooooooooooooong lines.\nplease line-wrap for readability. I learned to type W Q so often I do not\ncomplain but other people would find them irritating.]\n\n> The default behavior for 'git-grep' has already been discussed in\n> length, and I don't think it is likely to change. See\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/111519/focus=111717\n\nInteresting.\n\nThe first message in that thread lists things we have scheduled for 1.7.0\nbut it has another item.  It is an off-topic for the thread, but we might\nwant to resurrect the \"core.quotepath defaults to false\" proposal, before\nit gets too late for 1.7.0.\n\nAs to \"grep\", I am open to the proposal to make git commands consistent by\nletting them operate on everything when no path argument is given, if grep\nis really the single odd-man out remaining after changing the \"add -u\" and\n\"add -A\" to work on the whole tree from subdirectories when given no paths.\n\n> The original design for the other two in your list was to be a whole\n> tree operation. This commit broke it.\n>\n>   2ed2c22 \"git-add -u paths... now works from subdirectory\".\n> ...\n> I think it is a good idea to fix this as an old regression in the maint\n> branch. You don't have to introduce \"git add -a\". In fact the -a option\n> was explicitly rejected when \"git add -A\" option was added with this\n> commit.\n\nGeez, you are good at digging things up.\n\nIt is very tempting to follow the suggestion above, but I suspect that,\neven though it may be a regression from historical point of view, some\npeople who are used to the current behaviour may look at the corrected\none as a regression.  We've had the change by 2ed2c22 for a long time.\n\nI dunno.\n"},{"id":"122583","messageId":"vpqpra3p92g.fsf@bauges.imag.fr","threadId":"20818","inReplyTo":"20090906181621.GA23412@localhost","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-09-07T06:23:19Z","receivedAt":"2009-09-07T06:23:19Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Sun, Sep 06, 2009 at 02:32:44PM +0200, Matthieu Moy wrote:\n>> I think it has already been proposed to introduce \"git add -a\" doing\n>> what \"git add -u\" do, but for the full tree.\n>\n> I like that, actually. AFAICT it's completely analogous to \"git commit -a\".\n> We also need something for \"git add -A\" though.\n>\n> Do you feel the same way about changing the behavior of \"git grep\"? I don't\n> really want to change the command's name.\n\nI don't have particular feeling about \"git grep\", probably because I\ndon't use it much. One argument in favour of keeping the current\nbehavior is the consistancy with plain \"grep\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"122588","messageId":"20090907073322.GA6021@neumann","threadId":"20818","inReplyTo":"vpqpra3p92g.fsf@bauges.imag.fr","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-09-07T07:33:22Z","receivedAt":"2009-09-07T07:33:22Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Sep 07, 2009 at 08:23:19AM +0200, Matthieu Moy wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > On Sun, Sep 06, 2009 at 02:32:44PM +0200, Matthieu Moy wrote:\n> >> I think it has already been proposed to introduce \"git add -a\" doing\n> >> what \"git add -u\" do, but for the full tree.\n> >\n> > I like that, actually. AFAICT it's completely analogous to \"git commit -a\".\n> > We also need something for \"git add -A\" though.\n> >\n> > Do you feel the same way about changing the behavior of \"git grep\"? I don't\n> > really want to change the command's name.\n> \n> I don't have particular feeling about \"git grep\", probably because I\n> don't use it much. One argument in favour of keeping the current\n> behavior is the consistancy with plain \"grep\".\n\nI'm not sure how important should be the consistency with plain grep\nin this case.  After all, plain grep does not work without pathspec at\nall (ok, it searches stdin, but it's completely different thing),\nwhile git grep does.  And plain grep is not recursive by default,\nwhile git grep is.  And plain 'grep -r foo .' searches all files in\nthe current directory and below, while 'git grep foo .' does not\nsearch in untracked files.\n\n\nRegards,\nGábor\n"},{"id":"122590","messageId":"20090907075058.GA10797@localhost","threadId":"20818","inReplyTo":"20090907090713.6117@nanako3.lavabit.com","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-07T07:50:58Z","receivedAt":"2009-09-07T07:50:58Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Sep 07, 2009 at 09:07:13AM +0900, Nanako Shiraishi wrote:\n\n> The default behavior for 'git-grep' has already been discussed in length,\n> and I don't think it is likely to change. See \n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/111519/focus=111717\n\nActually, most responded with the request for a command or config option,\nand did not refuse the idea outright. One was not even aware that this is\nhow \"git grep\" behaves. And neither was I until a few days ago.\n\nAnd that is kind of dangerous with this command. You expect it to behave\nanalogously to other git commands, but it doesn't. And because grep simply\ndoes not search superdirectories, you may think that there are in fact no\nmatches so you don't even notice that behavior!\n\n> I think it is a good idea to fix this as an old regression in the maint\n> branch. You don't have to introduce \"git add -a\". In fact the -a option\n> was explicitly rejected when \"git add -A\" option was added with this\n> commit. \n> \n>   3ba1f11 \"git-add --all: add all files\"\n> \n> because \"git commit -a\" will never include new files and it will be inconsistent if \"git add -a\" did so.\n\nI certainly don't mind fixing \"git add -u\". But I was not suggesting \"git\nadd -a\" instead of \"git add -A\". The idea was to introduce it instead of\n\"git add -u\" (which can be deprecated later), so that the following are\nexactly the same.\n\n\t\"git add -a; git commit\"\n\t\"git commit -a\"\n\nThat way, scripts are not silently broken.\n\nOTOH, \"git add --all\" is already inconsistent with \"git commit --all\". And\nwe would still need a new command for 'global' \"add -A\". *sigh*\n\nClemens\n"},{"id":"122593","messageId":"vpq4orfgovn.fsf@bauges.imag.fr","threadId":"20818","inReplyTo":"20090907073322.GA6021@neumann","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-09-07T08:06:36Z","receivedAt":"2009-09-07T08:06:36Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n>> I don't have particular feeling about \"git grep\", probably because I\n>> don't use it much. One argument in favour of keeping the current\n>> behavior is the consistancy with plain \"grep\".\n>\n> I'm not sure how important should be the consistency with plain grep\n> in this case.  After all, plain grep does not work without pathspec at\n> all\n\nYes, right, forget what I wrote, except the \"I don't have particular\nfeeling about 'git grep'\" part ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"122603","messageId":"20090907084801.GB10797@localhost","threadId":"20818","inReplyTo":"7v3a6zu1dk.fsf@alter.siamese.dyndns.org","subject":"[PATCH] grep: fix exit status if external_grep() returns error","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-07T08:48:01Z","receivedAt":"2009-09-07T08:48:01Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Sep 06, 2009 at 03:58:15PM -0700, Junio C Hamano wrote:\n\n> Your patch is queued in 'pu', but it seems to break the exit status in a\n> strange way with my limited test.\n[...]\n> The command should not give different exit status depending on the\n> destination of standard output stream.\n\nI could also reproduce this in master using\n\n\t:~/git/t$ git grep --no-color not-going-to-match .; echo $?\n\nThe problem is a small bug in grep_cache(), which is triggered here because\nexternal_grep() cannot deal with relative paths (not sure why that's the\ncase though). Fix below.\n\nClemens\n--o<--\n\nIf external_grep() is called and returns an error, grep_cache() mistakenly\nreported a hit, even if there were none. The bug can be triggered by\ncalling \"git grep --no-color\" from a subdirectory.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n builtin-grep.c |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex ad0e0a5..b577738 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -500,9 +500,9 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n \t * be a lot more optimized\n \t */\n \tif (!cached && external_grep_allowed) {\n-\t\thit = external_grep(opt, paths, cached);\n-\t\tif (hit >= 0)\n-\t\t\treturn hit;\n+\t\tint ret = external_grep(opt, paths, cached);\n+\t\tif (ret >= 0)\n+\t\t\treturn ret;\n \t}\n #endif\n \n-- \n1.6.4.2.266.gbaa17\n"},{"id":"122636","messageId":"7vskeyvd0c.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090907084801.GB10797@localhost","subject":"Re: [PATCH] grep: fix exit status if external_grep() returns error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-07T18:13:55Z","receivedAt":"2009-09-07T18:13:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index ad0e0a5..b577738 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -500,9 +500,9 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n>  \t * be a lot more optimized\n>  \t */\n>  \tif (!cached && external_grep_allowed) {\n> -\t\thit = external_grep(opt, paths, cached);\n> -\t\tif (hit >= 0)\n> -\t\t\treturn hit;\n> +\t\tint ret = external_grep(opt, paths, cached);\n> +\t\tif (ret >= 0)\n> +\t\t\treturn ret;\n\nWell caught, and this deserves to go to maint.\n\nAn alternative would be to reset hit to zero if we decide to use the\ninternal one after this conditional.\n\nThanks.\n"},{"id":"122786","messageId":"20090910084653.6117@nanako3.lavabit.com","threadId":"20818","inReplyTo":"7vy6ot4x61.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-09-09T23:46:53Z","receivedAt":"2009-09-09T23:46:53Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com>\n\n> We could probably declare \"In 1.X.0 everything will be relative to the\n> root and you have to give an explicit '.' if you mean the cwd\".\n>\n> Three questions:\n>\n>  #1 What are the commands that will be affected, other than \"add -u\" and\n>     \"grep\"?  Are there others?\n>\n>  #2 Do all the commands in the answer to #1 currently behave exactly the\n>     same when run without any path parameter and when run with a single\n>     '.'?\n\n'git-archive' behaves relative to your current directory.\n\n  http://thread.gmane.org/gmane.comp.version-control.git/41300/focus=44125\n\nYou can limit it to the current directory with a dot.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"122833","messageId":"7vmy527f0b.fsf@alter.siamese.dyndns.org","threadId":"20818","inReplyTo":"20090910084653.6117@nanako3.lavabit.com","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-10T19:53:40Z","receivedAt":"2009-09-10T19:53:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> Quoting Junio C Hamano <gitster@pobox.com>\n>\n>> We could probably declare \"In 1.X.0 everything will be relative to the\n>> root and you have to give an explicit '.' if you mean the cwd\".\n>>\n>> Three questions:\n>>\n>>  #1 What are the commands that will be affected, other than \"add -u\" and\n>>     \"grep\"?  Are there others?\n>>\n>>  #2 Do all the commands in the answer to #1 currently behave exactly the\n>>     same when run without any path parameter and when run with a single\n>>     '.'?\n>\n> 'git-archive' behaves relative to your current directory.\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/41300/focus=44125\n>\n> You can limit it to the current directory with a dot.\n\nThanks.\n\nIf you want to make a tarball of the Documentation directory from your\nwork tree, you do this:\n\n    $ cd Documentation\n    $ tar cf - . >/tmp/docs.tar\n\nIf you want to do the same but from your committed content, you do this:\n\n    $ cd Documentation\n    $ git archive HEAD >/tmp/docs.tar\n\nand you do not have to say:\n\n    $ cd Documentation\n    $ git archive HEAD . >/tmp/docs.tar\n\nSo in that sense it does make sense to archive the current directory.  It\nmatches what the users expect from their archivers.\n\nThe traditional archivers may not default to \".\" but we do.  That is about\ngiving a sensible default [*1*].  Perhaps defaulting to the cwd behaviour\nfor one command may seem a sensible thing when looking at that particular\ncommand alone; archive and grep fall into that category.\n\nBut as this \"add -u\" discussion showed us [*2*], people may expect\ndifferent \"sensible default\", and as a suite of commands as the whole, it\nbecomes messy.  People have to remember which ones obey cwd, and to some\npeople the choice is not intuitive.\n\nTo avoid confusion, I am beginning to agree with people who said in the\nthread that it is a good idea to consistently default to the root of the\ncontents.  When we use \"everything\" as the default due to lack of command\nline pathspec, we would use \"everything from root\" no matter where you\nare, regardless of what command we are talking about.  That would make the\nrule easier to remember [*3*].\n\nThis changes the way how \"git (add -u|grep|clean|archive)\" without\npathspec and \"git (add -u|grep|clean|archive) .\" with an explicit dot\nwork.  The former (adds all changed files in, finds hits in, removes\nuntracked paths in, creates a tarball for) the whole tree, while the\nlatter limits the operation explicitly to the current directory.\n\nIf this were going to happen as a list concensus, I am very tempted to\nsuggest that we at least _consider_ applying the same rule even to\nls-files and ls-tree.  That would impact scripts, so we need to be extra\ncareful, though.\n\nAlso this takes us to a tangent.\n\nIf we try to give a sensible default to make it easier for the user,\nperhaps we should also default to HEAD when the user did not specify which\ntree-ish to archive from.  This is a topic in a separate thread.\n\n[Footnote]\n\n*1* Actually we don't allow \"git archive HEAD ..\", which I think is a\nbug.  Also we do not have --full-tree workaround, which makes it slightly\ncumbersome to use.\n\n*2* And the thread you quoted shows us that the argument applies equally\nto \"git archive\" as well; you see me complaining that it is unintuitive\nfor me to care about \"archive\", and the counterargument was that ls-tree\ndoes so.  I however think it is more important for archive to behave in a\nway that is easier for the users to understand, than mimick the historical\nmistake in a plumbing command.\n\n*3* Command line pathspec of course should honor cwd as before.  When you\nsay \"git distim Makefile\" inside t/ directory, we distim t/Makefile, not\nthe toplevel Makefile.  This discussion is only about the case where the\nuser didn't give us any pathspec.\n"},{"id":"122842","messageId":"20090910203209.GA27391@localhost","threadId":"20818","inReplyTo":"7vmy527f0b.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] 'add -u' doesn't work from untracked subdir","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-09-10T20:32:09Z","receivedAt":"2009-09-10T20:32:09Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Thu, Sep 10, 2009 at 12:53:40PM -0700, Junio C Hamano wrote:\n\n> If this were going to happen as a list concensus, I am very tempted to\n> suggest that we at least _consider_ applying the same rule even to\n> ls-files and ls-tree.  That would impact scripts, so we need to be extra\n> careful, though.\n\nI originally thought those commands should be consistent with plain \"ls\",\nsimply because of their name. However, ls-files is already inconsistent\nbecause \"ls-files <subdir>\" lists files relative to the current directory,\nas opposed to \"ls <subdir>\", which does so relative to <subdir>. And ls-tree\nis even more different from \"ls\".\n\nSo I don't think users are tempted to associate those commands with the\nbehavior they are used from \"ls\". From that perspective it would therefore\nbe ok to traverse the entire tree by default. To me that seems perfectly\nnatural, especially for ls-tree.\n\nIn case of ls-files, I don't know. Its current behavior certainly did not\nbother me so far. But the same arguments as for \"add -u\" apply if you think\nof doing something like \"git ls-files -u | cut -f2 | xargs git add\", for\nexample.\n\nI can't really speak for the impact on scripts. But that is certainly an\nissue, more so than with \"add -u\" or \"grep\", which are more typically used\ninteractively.\n\n> If we try to give a sensible default to make it easier for the user,\n> perhaps we should also default to HEAD when the user did not specify which\n> tree-ish to archive from.  This is a topic in a separate thread.\n\nI don't see why not.\n\n> *3* Command line pathspec of course should honor cwd as before.\n\nNo argument there.\n\nClemens\n"}]}