{"thread":{"id":"28531","subject":"[RFC/PATCH] git checkout $tree path","startedAt":"2011-09-29T22:46:31Z","lastAt":"2011-10-05T02:07:04Z","messageCount":8,"participants":["Junio C Hamano","John Szakmeister","Jeff King","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"176542","messageId":"7vk48rq854.fsf@alter.siamese.dyndns.org","threadId":"28531","inReplyTo":null,"subject":"[RFC/PATCH] git checkout $tree path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-29T22:46:31Z","receivedAt":"2011-09-29T22:46:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Suppose you have two branches, \"master\" with dir/file1 and file2, and\n\"next\" with dir/file3 and file4. You are currently on \"next\" branch.\n\n $ rm dir/file3\n $ git status -suno\n D dir/file3\n\nNow, what should \"git checkout master dir\" do?\n\nChecking paths out of a tree is (currently) defined to:\n\n - Grab the paths from the named tree that match the given pathspec,\n and add them to the index; and then\n\n - Check out the contents from the index for paths that match the\n pathspec to the working tree;\n\n - While at it, if the given pathspec did not match anything, suspect\n a typo from the command line and error out without updating the index\n nor the working tree.\n\nAccording to that definition, because \"master\" has dir/file1, and the\nindex is unchanged since \"next\", we would add dir/file1 to the index, and\nthen check dir/file1 and dir/file3 out of the index. Hence, we end up\nresurrecting dir/file3 out of the index, even though \"master\" does not\nhave that path.\n\nThis is somewhat surprising.\n\nIt may make sense to tweak the semantics a little bit. We can grab the\npaths out of the named tree (\"master\" in this case), update the index, and\nupdate the working tree with only with these paths we grabbed from the\nnamed tree. By doing so, we will keep the local modification to dir/file3\n(in this case, the modification is to \"delete\", but the above observation\nhold equally true if dir/file3 were modified).\n\nAn alternative semantics could be to first remove paths that match the\ngiven pathspec from the index, then update the index with paths taken from\nthe named tree, and update the working tree. \"git checkout master dir\"\nwould then mean \"replace anything currently in dir with whatever is in dir\nin master\". It is more dangerous, and it can easily emulated by doing:\n\n $ git rm -rf dir\n $ git checkout master dir\n\nso I did not go that far with this patch.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is a behaviour change, but it may qualify as a bugfix. I dunno.\n\n builtin/checkout.c | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5e356a6..75dbe76 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -71,7 +71,7 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,\n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len - baselen);\n-\tce->ce_flags = create_ce_flags(len, 0);\n+\tce->ce_flags = create_ce_flags(len, 0) | CE_UPDATE;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n \treturn 0;\n@@ -228,6 +228,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \n \tfor (pos = 0; pos < active_nr; pos++) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n+\t\tif (source_tree && !(ce->ce_flags & CE_UPDATE))\n+\t\t\tcontinue;\n \t\tmatch_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);\n \t}\n \n@@ -266,6 +268,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \tstate.refresh_cache = 1;\n \tfor (pos = 0; pos < active_nr; pos++) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n+\t\tif (source_tree && !(ce->ce_flags & CE_UPDATE))\n+\t\t\tcontinue;\n \t\tif (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {\n \t\t\tif (!ce_stage(ce)) {\n \t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n"},{"id":"176546","messageId":"CAEBDL5VD1nkVK1b_zN+jFAnSrK5X3va0nMdUwypghBcFbz2s0Q@mail.gmail.com","threadId":"28531","inReplyTo":"7vk48rq854.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2011-09-30T01:02:58Z","receivedAt":"2011-09-30T01:02:58Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"On Thu, Sep 29, 2011 at 6:46 PM, Junio C Hamano wrote:\n[snip]\n> According to that definition, because \"master\" has dir/file1, and the\n> index is unchanged since \"next\", we would add dir/file1 to the index, and\n> then check dir/file1 and dir/file3 out of the index. Hence, we end up\n> resurrecting dir/file3 out of the index, even though \"master\" does not\n> have that path.\n>\n> This is somewhat surprising.\n\nThat is surprising! It explains something I saw just yesterday which\nclosely mirrors your recipe.\n\n> It may make sense to tweak the semantics a little bit. We can grab the\n> paths out of the named tree (\"master\" in this case), update the index, and\n> update the working tree with only with these paths we grabbed from the\n> named tree. By doing so, we will keep the local modification to dir/file3\n> (in this case, the modification is to \"delete\", but the above observation\n> hold equally true if dir/file3 were modified).\n\nThat seems sane.\n\n> An alternative semantics could be to first remove paths that match the\n> given pathspec from the index, then update the index with paths taken from\n> the named tree, and update the working tree. \"git checkout master dir\"\n> would then mean \"replace anything currently in dir with whatever is in dir\n> in master\". It is more dangerous, and it can easily emulated by doing:\n\nThis is equally sane, but is probably closer to my expectation.\n\n[snip]\n>  * This is a behaviour change, but it may qualify as a bugfix. I dunno.\n\nPersonally, I lean towards it being a bugfix.\n\n-John\n"},{"id":"176731","messageId":"20111003102647.GD16078@sigill.intra.peff.net","threadId":"28531","inReplyTo":"7vk48rq854.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-03T10:26:47Z","receivedAt":"2011-10-03T10:26:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 29, 2011 at 03:46:31PM -0700, Junio C Hamano wrote:\n\n> According to that definition, because \"master\" has dir/file1, and the\n> index is unchanged since \"next\", we would add dir/file1 to the index, and\n> then check dir/file1 and dir/file3 out of the index. Hence, we end up\n> resurrecting dir/file3 out of the index, even though \"master\" does not\n> have that path.\n> \n> This is somewhat surprising.\n\nAgreed, it is surprising.\n\n> It may make sense to tweak the semantics a little bit. We can grab the\n> paths out of the named tree (\"master\" in this case), update the index, and\n> update the working tree with only with these paths we grabbed from the\n> named tree. By doing so, we will keep the local modification to dir/file3\n> (in this case, the modification is to \"delete\", but the above observation\n> hold equally true if dir/file3 were modified).\n\nHmm. I can see that being what the user expects in some cases. For\nexample, when \"master\" has nothing to do with dir/file3 in the first\nplace. But I can also see this:\n\n> An alternative semantics could be to first remove paths that match the\n> given pathspec from the index, then update the index with paths taken from\n> the named tree, and update the working tree. \"git checkout master dir\"\n> would then mean \"replace anything currently in dir with whatever is in dir\n> in master\". It is more dangerous, and it can easily emulated by doing:\n\nbeing what the user expects. As in, \"master deleted this file; shouldn't\ncheckout pull the deletion to my new branch when I ask it to?\".\n\nBut we can't distinguish those two cases without actually having a merge\nbase. And this isn't a merge; it's not about picking changes from\nmaster, it's about saying \"make dir look like it does in master\". So\nin that sense, the most straightforward thing is your second\nalternative: afterwards, we should have only the files in \"dir\" that\nmaster has.\n\nA related question is what does this do:\n\n git reset master -- dir\n\nMy mental model is that it makes the index for \"dir\" look just like\nmaster:dir. And that seems pretty accurate; it deletes dir/file3 (which\ndoes not exist in \"master\") from the index.\n\nMy mental model of \"git checkout master -- dir\" is similar. It should\nmake the index for \"dir\" look like master:dir, and then check that out.\nIOW, I think of it as:\n\n git reset master -- dir &&\n git checkout -- dir\n\nMaybe that is not accurate (well, clearly it does not match the current\nbehavior), but I think it is at least easy to explain and relatively\nsane. So it is something to shoot for, and makes \"git checkout\"\nconsistent with \"git reset\".\n\n> * This is a behaviour change, but it may qualify as a bugfix. I dunno.\n\nI think it is a bug. I can see both of the alternatives you outlined\nabove making some sense, but checking out content that has _nothing_ to\ndo with master is just confusing. Either make it look like master, or\nleave it alone.\n\n-Peff\n"},{"id":"176750","messageId":"7vmxdigirk.fsf@alter.siamese.dyndns.org","threadId":"28531","inReplyTo":"20111003102647.GD16078@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T16:08:31Z","receivedAt":"2011-10-03T16:08:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But we can't distinguish those two cases without actually having a merge\n> base. And this isn't a merge; it's not about picking changes from\n> master, it's about saying \"make dir look like it does in master\". So\n> in that sense, the most straightforward thing is your second\n> alternative: afterwards, we should have only the files in \"dir\" that\n> master has.\n\nWe can think of it both ways, but the \"make it look like the other one\"\nunfortunately is too big a departure from the traditional semantics. At\nleast I wanted \"checkout master -- $path\" to mean \"I want to copy $path\nout of master and _overlay_ that on top of what I have now\", similar to\nthe way how \"tar xf master.tar $path\" and \"cp -r ../master/$path $path\"\nwould be used, so that the command can help the user advance what is in\nprogress and already underway in $path in the current working tree.\n\nReplacing could be easily done with \"git rm -r [--cached] $path\" followed\nby \"git checkout $tree $path\" under the original semantics, but overlaying\nis not very easily done if \"git checkout $tree $path\" had your \"make $path\nlook like it does in $tree\" semantics.\n\nThe change brought in by the RFC/PATCH does change the behaviour, and I am\nfairly comfortable now to say that it is a bugfix (\"copy and overlay\" a la\n\"tar xf\" never clobbers/removes files not in the source, but the current\ncode does).\n"},{"id":"176816","messageId":"20111004074212.GB7308@sigill.intra.peff.net","threadId":"28531","inReplyTo":"7vmxdigirk.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T07:42:12Z","receivedAt":"2011-10-04T07:42:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 03, 2011 at 09:08:31AM -0700, Junio C Hamano wrote:\n\n> We can think of it both ways, but the \"make it look like the other one\"\n> unfortunately is too big a departure from the traditional semantics. At\n> least I wanted \"checkout master -- $path\" to mean \"I want to copy $path\n> out of master and _overlay_ that on top of what I have now\", similar to\n> the way how \"tar xf master.tar $path\" and \"cp -r ../master/$path $path\"\n> would be used, so that the command can help the user advance what is in\n> progress and already underway in $path in the current working tree.\n\nYeah, I think the \"overlay\" semantics at least makes some sense. The\nstrongest argument for \"replace\" semantics is that \"git reset \"\nuses them, and it would be nice if they were consistent. But I agree\nthat the overlay is closer to what happens now.\n\n> Replacing could be easily done with \"git rm -r [--cached] $path\" followed\n> by \"git checkout $tree $path\" under the original semantics, but overlaying\n> is not very easily done if \"git checkout $tree $path\" had your \"make $path\n> look like it does in $tree\" semantics.\n\nTrue.\n\n> The change brought in by the RFC/PATCH does change the behaviour, and I am\n> fairly comfortable now to say that it is a bugfix (\"copy and overlay\" a la\n> \"tar xf\" never clobbers/removes files not in the source, but the current\n> code does).\n\nThat sounds good to me, and I think properly doing a pure overlay is way\nbetter than the prior behavior.\n\nThis is sufficiently tricky and subtle that it is probably worth\nfuture-proofing with some tests (e.g., the example you gave in the\ncommit message).\n\n-Peff\n"},{"id":"176850","messageId":"CAG+J_Dy7gnDEK-9KAJT4M_wumS_QpzLNMVzAiOHCyEbFBU65=w@mail.gmail.com","threadId":"28531","inReplyTo":"20111003102647.GD16078@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-10-04T15:05:11Z","receivedAt":"2011-10-04T15:05:11Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Oct 3, 2011 at 6:26 AM, Jeff King wrote:\n> On Thu, Sep 29, 2011 at 03:46:31PM -0700, Junio C Hamano wrote:\n>> An alternative semantics could be to first remove paths that match the\n>> given pathspec from the index, then update the index with paths taken from\n>> the named tree, and update the working tree. \"git checkout master dir\"\n>> would then mean \"replace anything currently in dir with whatever is in dir\n>> in master\". It is more dangerous, and it can easily emulated by doing:\n>\n> being what the user expects. As in, \"master deleted this file; shouldn't\n> checkout pull the deletion to my new branch when I ask it to?\".\n>\n> But we can't distinguish those two cases without actually having a merge\n> base. And this isn't a merge; it's not about picking changes from\n> master, it's about saying \"make dir look like it does in master\". So\n> in that sense, the most straightforward thing is your second\n> alternative: afterwards, we should have only the files in \"dir\" that\n> master has.\n\nI think I'd expect the first behavior with:\n\n$ git checkout master -- dir/*\n\nAnd the second with:\n\n$ git checkout master -- dir\n\n$0.02.\n\nj.\n"},{"id":"176851","messageId":"7vk48kdbri.fsf@alter.siamese.dyndns.org","threadId":"28531","inReplyTo":"20111004074212.GB7308@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-04T15:20:17Z","receivedAt":"2011-10-04T15:20:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This is sufficiently tricky and subtle that it is probably worth\n> future-proofing with some tests (e.g., the example you gave in the\n> commit message).\n\nYes, see t2022 in 'pu'.\n"},{"id":"176893","messageId":"7v1uusb393.fsf@alter.siamese.dyndns.org","threadId":"28531","inReplyTo":"CAG+J_Dy7gnDEK-9KAJT4M_wumS_QpzLNMVzAiOHCyEbFBU65=w@mail.gmail.com","subject":"Re: [RFC/PATCH] git checkout $tree path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-05T02:07:04Z","receivedAt":"2011-10-05T02:07:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> I think I'd expect the first behavior with:\n>\n> $ git checkout master -- dir/*\n\nIf we did not have to worry about \"dir/.gitignore\", this proposal does\nhave certain attractiveness.\n"}]}