{"thread":{"id":"34952","subject":"[BUG?] git checkout $commit -- somedir doesn't drop files","startedAt":"2013-09-17T19:06:59Z","lastAt":"2013-09-20T22:54:44Z","messageCount":15,"participants":["Uwe Kleine-König","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"227774","messageId":"20130917190659.GA15588@pengutronix.de","threadId":"34952","inReplyTo":null,"subject":"[BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2013-09-17T19:06:59Z","receivedAt":"2013-09-17T19:06:59Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nafter these commands:\n\n\t$ git init\n\t$ mkdir subdir\n\t$ echo a > subdir/a\n\t$ git add subdir/a\n\t$ git commit -m a\n\t$ echo more a >> subdir/a\n\t$ echo b > subdir/b\n\t$ git add subdir/*\n\t$ git commit -m b\n\t$ git checkout HEAD^ -- subdir\n\nI'd expect that subdir/b is removed from the index as this file didn't\nexist in HEAD^ but git-status only reports:\n\n\t# On branch master\n\t# Changes to be committed:\n\t#   (use \"git reset HEAD <file>...\" to unstage)\n\t#\n\t#\tmodified:   subdir/a\n\t#\n\nIs this intended?\n\n(I'm using git 1.8.4.rc3 as shipped by Debian (package version\n1:1.8.4~rc3-1).)\n\nBest regards\nUwe\n\n-- \nPengutronix e.K.                           | Uwe Kleine-König            |\nIndustrial Linux Solutions                 | http://www.pengutronix.de/  |\n"},{"id":"227781","messageId":"xmqqeh8nxltc.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130917190659.GA15588@pengutronix.de","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-17T19:58:07Z","receivedAt":"2013-09-17T19:58:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:\n\n> after these commands:\n>\n> \t$ git init\n> \t$ mkdir subdir\n> \t$ echo a > subdir/a\n> \t$ git add subdir/a\n> \t$ git commit -m a\n> \t$ echo more a >> subdir/a\n> \t$ echo b > subdir/b\n> \t$ git add subdir/*\n> \t$ git commit -m b\n> \t$ git checkout HEAD^ -- subdir\n>\n> I'd expect that subdir/b is removed from the index as this file didn't\n> exist in HEAD^ but git-status only reports:\n>\n> \t# On branch master\n> \t# Changes to be committed:\n> \t#   (use \"git reset HEAD <file>...\" to unstage)\n> \t#\n> \t#\tmodified:   subdir/a\n> \t#\n>\n> Is this intended?\n>\n> (I'm using git 1.8.4.rc3 as shipped by Debian (package version\n> 1:1.8.4~rc3-1).)\n\nThe intended behaviour of \"git checkout tree-ish -- pathspec\" has\nalways been \"grab paths that match pathspec from tree-ish, add them\nto the index and check them out to the working tree\".  If you have\nsubdir/b in the index and the working tree, it will be kept when the\npaths that match \"subdir\" pathspec is grabbed out of the tree-ish\nHEAD^ (namely, subdir/a) is added to the index and to the working\ntree.\n\nI could argue that the above intended behaviour is suboptimal and it\nshould have been \"the resulting paths in the index and the work tree\nthat match the given pathspec must be identical to that of the\ntree-ish\".  In the resulting index or working tree, paths that match\n\"subdir\" pathspec in the above result is subdir/a and subdir/b, and\nthat is different from what exists in the given tree-ish (which has\nonly subdir/a and not subdir/b), and under that modified definition,\nwhat the current one does is not correct.\n\nI vaguely recall arguing for the updated behaviour described in the\nabove paragraph, and I even might have started working on it, but I\ndo not think we changed this behaviour recently, unfortunately.\n"},{"id":"227783","messageId":"20130917201003.GA16860@sigill.intra.peff.net","threadId":"34952","inReplyTo":"20130917190659.GA15588@pengutronix.de","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-17T20:10:04Z","receivedAt":"2013-09-17T20:10:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2013 at 09:06:59PM +0200, Uwe Kleine-König wrote:\n\n> \t$ git checkout HEAD^ -- subdir\n> \n> I'd expect that subdir/b is removed from the index as this file didn't\n> exist in HEAD^ but git-status only reports:\n\nI'm not sure if this is intentional or not. The definition of \"git\ncheckout $tree $path\" given in commit 0a1283b says:\n\n    Checking paths out of a tree is (currently) defined to do:\n\n     - Grab the paths from the named tree that match the given pathspec,\n       and add them to the index;\n\n     - Check out the contents from the index for paths that match the\n       pathspec to the working tree; and while at it\n\n     - If the given pathspec did not match anything, suspect a typo from the\n       command line and error out without updating the index nor the working\n       tree.\n\nSo we are applying the pathspec to the named tree, and pulling anything\nthat matches into the index. Which by definition cannot involve a\ndeletion, because there is no comparison happening (so we either have\nit, or we do not). Whereas what you are expecting is to compare the tree\nand the index, limited by the pathspec, and pull any changes from the\ntree into the index.\n\nI tend to agree that the latter is more like how other git commands\nbehave (e.g., if you tried to use \"read-tree\" to emulate what\ngit-checkout was doing, I think you would end up with a two-way merge).\nBut there may be implications I haven't thought of.\n\nNote also that \"git checkout -p\" does present \"subdir/b\" as a deletion.\nIt works by doing a pathspec-limited diff between the two endpoints,\nwhich will notice deletions. So there is some inconsistency there with\nthe baseline form.\n\n-Peff\n"},{"id":"227785","messageId":"20130917201259.GB16860@sigill.intra.peff.net","threadId":"34952","inReplyTo":"xmqqeh8nxltc.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-17T20:13:00Z","receivedAt":"2013-09-17T20:13:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2013 at 12:58:07PM -0700, Junio C Hamano wrote:\n\n> I could argue that the above intended behaviour is suboptimal and it\n> should have been \"the resulting paths in the index and the work tree\n> that match the given pathspec must be identical to that of the\n> tree-ish\".  In the resulting index or working tree, paths that match\n> \"subdir\" pathspec in the above result is subdir/a and subdir/b, and\n> that is different from what exists in the given tree-ish (which has\n> only subdir/a and not subdir/b), and under that modified definition,\n> what the current one does is not correct.\n\nOur emails just crossed, but I basically ended up saying a similar\nthing.  Could we simply replace the \"update_some\" in builtin/checkout.c\nwith a two-way merge via unpack-trees?\n\n> I vaguely recall arguing for the updated behaviour described in the\n> above paragraph, and I even might have started working on it, but I\n> do not think we changed this behaviour recently, unfortunately.\n\nYes, I did some digging and I think it has always been this way, even\nbefore git-checkout was a builtin.\n\n-Peff\n"},{"id":"227791","messageId":"xmqq61tzxkgz.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130917201259.GB16860@sigill.intra.peff.net","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-17T20:27:08Z","receivedAt":"2013-09-17T20:27:08Z","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> On Tue, Sep 17, 2013 at 12:58:07PM -0700, Junio C Hamano wrote:\n>\n>> I could argue that the above intended behaviour is suboptimal and it\n>> should have been \"the resulting paths in the index and the work tree\n>> that match the given pathspec must be identical to that of the\n>> tree-ish\".  In the resulting index or working tree, paths that match\n>> \"subdir\" pathspec in the above result is subdir/a and subdir/b, and\n>> that is different from what exists in the given tree-ish (which has\n>> only subdir/a and not subdir/b), and under that modified definition,\n>> what the current one does is not correct.\n>\n> Our emails just crossed, but I basically ended up saying a similar\n> thing.  Could we simply replace the \"update_some\" in builtin/checkout.c\n> with a two-way merge via unpack-trees?\n\nWould it work to resolve a conflicted index by checking out from a\nknown tree?\n\n>> I vaguely recall arguing for the updated behaviour described in the\n>> above paragraph, and I even might have started working on it, but I\n>> do not think we changed this behaviour recently, unfortunately.\n>\n> Yes, I did some digging and I think it has always been this way, even\n> before git-checkout was a builtin.\n>\n> -Peff\n"},{"id":"227792","messageId":"20130917202917.GA20020@sigill.intra.peff.net","threadId":"34952","inReplyTo":"xmqq61tzxkgz.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-17T20:29:17Z","receivedAt":"2013-09-17T20:29:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2013 at 01:27:08PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Tue, Sep 17, 2013 at 12:58:07PM -0700, Junio C Hamano wrote:\n> >\n> >> I could argue that the above intended behaviour is suboptimal and it\n> >> should have been \"the resulting paths in the index and the work tree\n> >> that match the given pathspec must be identical to that of the\n> >> tree-ish\".  In the resulting index or working tree, paths that match\n> >> \"subdir\" pathspec in the above result is subdir/a and subdir/b, and\n> >> that is different from what exists in the given tree-ish (which has\n> >> only subdir/a and not subdir/b), and under that modified definition,\n> >> what the current one does is not correct.\n> >\n> > Our emails just crossed, but I basically ended up saying a similar\n> > thing.  Could we simply replace the \"update_some\" in builtin/checkout.c\n> > with a two-way merge via unpack-trees?\n> \n> Would it work to resolve a conflicted index by checking out from a\n> known tree?\n\nHrm. Probably not. It is almost a one-way merge going to the named tree\n(but limited by the pathspec), except that I think the current\ngit-checkout code may provide some safety checks related to where we are\ncoming from (e.g., do we unconditionally overwrite entries that are not\nuptodate?).\n\n-Peff\n"},{"id":"227795","messageId":"xmqq1u4nxjv2.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130917202917.GA20020@sigill.intra.peff.net","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-17T20:40:17Z","receivedAt":"2013-09-17T20:40:17Z","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> On Tue, Sep 17, 2013 at 01:27:08PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > On Tue, Sep 17, 2013 at 12:58:07PM -0700, Junio C Hamano wrote:\n>> >\n>> >> I could argue that the above intended behaviour is suboptimal and it\n>> >> should have been \"the resulting paths in the index and the work tree\n>> >> that match the given pathspec must be identical to that of the\n>> >> tree-ish\".  In the resulting index or working tree, paths that match\n>> >> \"subdir\" pathspec in the above result is subdir/a and subdir/b, and\n>> >> that is different from what exists in the given tree-ish (which has\n>> >> only subdir/a and not subdir/b), and under that modified definition,\n>> >> what the current one does is not correct.\n>> >\n>> > Our emails just crossed, but I basically ended up saying a similar\n>> > thing.  Could we simply replace the \"update_some\" in builtin/checkout.c\n>> > with a two-way merge via unpack-trees?\n>> \n>> Would it work to resolve a conflicted index by checking out from a\n>> known tree?\n>\n> Hrm. Probably not. It is almost a one-way merge going to the named tree\n> (but limited by the pathspec), except that I think the current\n> git-checkout code may provide some safety checks related to where we are\n> coming from (e.g., do we unconditionally overwrite entries that are not\n> uptodate?).\n\nI think we do unconditionally overwrite and that has been very much\non purpose.\n\n\"git checkout tree-ish -- file.txt\" has always been about replacing\nwhatever cruft is in paths in the worktree that match pathspec, just\nlike \"cat content-created-elsewhere >file.txt\" is.  \"Oops, you have\na local change that do not match index\" is the last thing we want to\nsay, because getting rid of that local change is the primary reason\nwhy \"checkout tree-ish -- file.txt\" exists.\n\nTaking the state of a subdirectory as a whole as \"content\", the\nchange we are discussing will make it more like \"rm -fr dir && tar\nxf some-content dir\" to replace the directory wholesale, which I\npersonally think is a good thing in the longer term.\n"},{"id":"227802","messageId":"20130917212106.GB20178@sigill.intra.peff.net","threadId":"34952","inReplyTo":"xmqq1u4nxjv2.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-17T21:21:06Z","receivedAt":"2013-09-17T21:21:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2013 at 01:40:17PM -0700, Junio C Hamano wrote:\n\n> > Hrm. Probably not. It is almost a one-way merge going to the named tree\n> > (but limited by the pathspec), except that I think the current\n> > git-checkout code may provide some safety checks related to where we are\n> > coming from (e.g., do we unconditionally overwrite entries that are not\n> > uptodate?).\n> \n> I think we do unconditionally overwrite and that has been very much\n> on purpose.\n\nI thought so, too, but I was thrown off by the code in checkout_paths()\nthat warns/aborts if there are unmerged entries. But it looks like we\nwill have thrown out those entries already during the read_tree_some\ncall, which adds the new entries using OK_TO_REPLACE.\n\n> \"git checkout tree-ish -- file.txt\" has always been about replacing\n> whatever cruft is in paths in the worktree that match pathspec, just\n> like \"cat content-created-elsewhere >file.txt\" is.  \"Oops, you have\n> a local change that do not match index\" is the last thing we want to\n> say, because getting rid of that local change is the primary reason\n> why \"checkout tree-ish -- file.txt\" exists.\n> \n> Taking the state of a subdirectory as a whole as \"content\", the\n> change we are discussing will make it more like \"rm -fr dir && tar\n> xf some-content dir\" to replace the directory wholesale, which I\n> personally think is a good thing in the longer term.\n\nYeah, that makes sense. What about untracked files?\n\nRight now we overwrite them if the tree-ish has an entry at the same\npath; that is a bit more dangerous than the rest of git, but does match\nthe \"ignore local modifications\" rule. I assume if we handled deletions,\nthough, that we would simply leave them be.\n\nSo given that, is it fair to say that a one-way \"go here\" merge, limited\nby pathspec, is the closest equivalent?\n\n-Peff\n"},{"id":"227806","messageId":"xmqqk3ifw1km.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130917212106.GB20178@sigill.intra.peff.net","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-17T22:00:41Z","receivedAt":"2013-09-17T22:00:41Z","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> On Tue, Sep 17, 2013 at 01:40:17PM -0700, Junio C Hamano wrote:\n>\n>> Taking the state of a subdirectory as a whole as \"content\", the\n>> change we are discussing will make it more like \"rm -fr dir && tar\n>> xf some-content dir\" to replace the directory wholesale, which I\n>> personally think is a good thing in the longer term.\n>\n> Yeah, that makes sense. What about untracked files?\n\nObviously we cannot literally do \"rm -fr dir && tar x\", but I agree\nthat if tree-ish has a path that is not tracked in the current\nindex, the path should be overwritten and made identical to what is\nin the tree-ish.\n\n> Right now we overwrite them if the tree-ish has an entry at the same\n> path; that is a bit more dangerous than the rest of git, but does match\n> the \"ignore local modifications\" rule. I assume if we handled deletions,\n> though, that we would simply leave them be.\n>\n> So given that, is it fair to say that a one-way \"go here\" merge, limited\n> by pathspec, is the closest equivalent?\n\nSorry, but it is unclear to me what you mean by one-way \"go here\"\nmerge.  Do you mean oneway_merge() in unpack-trees.c?\n"},{"id":"227807","messageId":"20130917220345.GA22914@sigill.intra.peff.net","threadId":"34952","inReplyTo":"xmqqk3ifw1km.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-17T22:03:45Z","receivedAt":"2013-09-17T22:03:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2013 at 03:00:41PM -0700, Junio C Hamano wrote:\n\n> > So given that, is it fair to say that a one-way \"go here\" merge, limited\n> > by pathspec, is the closest equivalent?\n> \n> Sorry, but it is unclear to me what you mean by one-way \"go here\"\n> merge.  Do you mean oneway_merge() in unpack-trees.c?\n\nYes, that is what I meant.\n\nIn my mind, oneway_merge is \"go here unconditionally\", twoway_merge is\n\"go from A to B, respecting changes already made from A\", and\nthreeway_merge is a \"real\" merge. But I am not sure if that is accurate\nor just my confused mental model of unpack-trees. :)\n\n-Peff\n"},{"id":"227808","messageId":"xmqqd2o7w0xc.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130917220345.GA22914@sigill.intra.peff.net","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-17T22:14:39Z","receivedAt":"2013-09-17T22:14:39Z","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> On Tue, Sep 17, 2013 at 03:00:41PM -0700, Junio C Hamano wrote:\n>\n>> > So given that, is it fair to say that a one-way \"go here\" merge, limited\n>> > by pathspec, is the closest equivalent?\n>> \n>> Sorry, but it is unclear to me what you mean by one-way \"go here\"\n>> merge.  Do you mean oneway_merge() in unpack-trees.c?\n>\n> Yes, that is what I meant.\n\nYeah, then I agree that \"git checkout HEAD^ -- subdir\" should be a\none-way \"go HEAD^\" merge limited only to the paths that match\nsubdir/.\n\nIf implemented in a straight-forward way, I suspect that we may end\nup not removing subdir/b in Uwe's sample transcript. I am not sure\nif that is a good thing or not, though.  If the index originally\ntracked and then \"going to\" tree does not, I think removing it would\nmatch \"ignore local modifications\" rule, as subdir/a that is tracked\nin the index and also in \"going to\" tree does get overwritten to\nmatch the state recorded in the tree.\n"},{"id":"227873","messageId":"20130919074616.GA29773@sigill.intra.peff.net","threadId":"34952","inReplyTo":"xmqqd2o7w0xc.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-19T07:46:16Z","receivedAt":"2013-09-19T07:46:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2013 at 03:14:39PM -0700, Junio C Hamano wrote:\n\n> Yeah, then I agree that \"git checkout HEAD^ -- subdir\" should be a\n> one-way \"go HEAD^\" merge limited only to the paths that match\n> subdir/.\n> \n> If implemented in a straight-forward way, I suspect that we may end\n> up not removing subdir/b in Uwe's sample transcript. I am not sure\n> if that is a good thing or not, though.  If the index originally\n> tracked and then \"going to\" tree does not, I think removing it would\n> match \"ignore local modifications\" rule, as subdir/a that is tracked\n> in the index and also in \"going to\" tree does get overwritten to\n> match the state recorded in the tree.\n\nI had assumed the goal was to subdir/b (by the reasoning above, and the\n\"rm -rf && tar\" analogy you gave earlier).\n\nI tried a very hacky attempt at shoving unpack-trees into the right spot\nin checkout_paths. But its pathspec handling from unpack_trees is not\nquite what we want.\n\nIn Uwe's case, it did delete subdir/b and overwrite subdir/a, which I'd\nexpect. But if there was an additional file outside of subdir, it got\ndeleted, too.  The problem is that I was giving the regular index to\nunpack_trees as the destination; so it wrote out only the bits of the\nindex under the pathspec subdir/, and omitted the rest entirely. I had\nhoped instead it would leave those parts untouched from the source.\n\nAn alternative would be to write the new entries to a temporary index\nin memory. And then you can throw away the entries in the current index\nthat match the pathspec, and add in the entries from the temporary\nindex. I was just hoping that unpack-trees would do all of that for me.\n:)\n\nAt this point, I'm giving up for now. I was hoping a naive application\nof unpack-trees would just work (and reduce the code size to boot), but\nit is obviously a bit more complicated than that. I still think it's a\ngood idea for checkout to behave as Uwe described, but I don't think\nit's that high a priority, and I have other stuff I'd prefer to work on\nat the moment.\n\n-Peff\n"},{"id":"227892","messageId":"xmqqbo3ou1ue.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130919074616.GA29773@sigill.intra.peff.net","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-19T18:02:17Z","receivedAt":"2013-09-19T18:02:17Z","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> An alternative would be to write the new entries to a temporary index\n> in memory. And then you can throw away the entries in the current index\n> that match the pathspec, and add in the entries from the temporary\n> index. I was just hoping that unpack-trees would do all of that for me.\n> :)\n\nIsn't a \"go there\" one-way merge with pathspecs very similar to what\nhappens in \"reset -- pathspec\" except for the working tree part?  I\nsuspect that it may be sufficient to mimic the read_from_tree() and\nadapt update_index_from_diff() callback in builtin/reset.c to also\nupdate the working tree (and we can do so unconditionally without\nchecking if we have any local modification in this case, which\nsimplifies things a lot), but I may be missing something.\n"},{"id":"227904","messageId":"20130919221323.GA18085@sigill.intra.peff.net","threadId":"34952","inReplyTo":"xmqqbo3ou1ue.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-19T22:13:23Z","receivedAt":"2013-09-19T22:13:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 19, 2013 at 11:02:17AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > An alternative would be to write the new entries to a temporary index\n> > in memory. And then you can throw away the entries in the current index\n> > that match the pathspec, and add in the entries from the temporary\n> > index. I was just hoping that unpack-trees would do all of that for me.\n> > :)\n> \n> Isn't a \"go there\" one-way merge with pathspecs very similar to what\n> happens in \"reset -- pathspec\" except for the working tree part?\n\nI thought so at first, but now I'm not so sure...\n\n> suspect that it may be sufficient to mimic the read_from_tree() and\n> adapt update_index_from_diff() callback in builtin/reset.c to also\n> update the working tree (and we can do so unconditionally without\n> checking if we have any local modification in this case, which\n> simplifies things a lot), but I may be missing something.\n\nIt almost works to simply update the index as \"reset\" does via\ndiff_cache, marking each updated entry with CE_UPDATE, and then let the\nrest of checkout_paths proceed, which handles updating the working tree\nbased on the new index.\n\nBut I found two problems:\n\n  1. The diff never feeds us entries that are unchanged, so we never\n     mark them for update. But that interferes with later code to check\n     whether our pathspecs matched anything (so we complain on \"git\n     reset --hard; git checkout HEAD foo\" would barf on the checkout,\n     since we do not need to touch foo).\n\n     But I think that points to a larger problem, which is that we do\n     not want to just look at the entries that are different between the\n     tree and the index. We also need to care about the entries that are\n     different in the working tree and index, because those need written\n     out, too.\n\n  2. The code in checkout_paths cannot handle the deletion for us,\n     because it doesn't even know about the path any more (we removed it\n     during the index diff). I think we could get around this by leaving\n     an entry with the CE_WT_REMOVE flag set. But it looks like there is\n     a bit more magic to removing a path than just remove_or_warn().\n     unpack-trees has unlink_entry, which queues up leading directories\n     for removal.\n\nI think (2) is a matter of refactoring (but again, if we could convince\nunpack-trees to do this for us, that might be the nicest way to reuse\nthe code). But (1) points to a larger problem in thinking about this as\na \"diff\"; it is really about re-loading bits of the index, and then\nchecking it out into the working tree.\n\n-Peff\n"},{"id":"227955","messageId":"xmqq8uyrp0i3.fsf@gitster.dls.corp.google.com","threadId":"34952","inReplyTo":"20130919221323.GA18085@sigill.intra.peff.net","subject":"Re: [BUG?] git checkout $commit -- somedir doesn't drop files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-20T22:54:44Z","receivedAt":"2013-09-20T22:54:44Z","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>      But I think that points to a larger problem, which is that we do\n>      not want to just look at the entries that are different between the\n>      tree and the index.\n\nTrue.  The unpack-trees API knows how to walk the index and trees in\nparallel, and I tend to agree that it may be a more suitable vehicle\nfor this purpose.\n"}]}