{"thread":{"id":"56368","subject":"'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","startedAt":"2021-08-24T16:50:50Z","lastAt":"2021-08-28T05:21:39Z","messageCount":9,"participants":["Yuri","Jeff King","Bagas Sanjaya","Elijah Newren","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"433594","messageId":"01514401-78f6-3fdc-aa74-c519fb6ab427@rawbw.com","threadId":"56368","inReplyTo":null,"subject":"'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Yuri","fromEmail":"yuri@rawbw.com","sentAt":"2021-08-24T16:41:49Z","receivedAt":"2021-08-24T16:50:50Z","isPatch":false,"sender":{"key":"yuri@rawbw.com","avatar":null},"body":"In the FreeBSD ports repository I resurrected the directory (that was \nremoved a long time ago) with the command:\n\n > $ git checkout {hash}~1 -- math/polymake\n\n\nI made local changes to this directory and called 'git add math/polymake'.\n\nThen 'git pull' complained:\n\n > $ git pull\n > error: Your local changes to the following files would be overwritten \nby merge:\n >   math/polymake/Makefile math/polymake/distinfo \nmath/polymake/files/patch-Makefile \nmath/polymake/files/patch-support_install.pl math/polymake/pkg-descr \nmath/polymake/pkg-plist\n\n\nNo incoming changes affect math/polymake. Nobody has created this \ndirectory simultaneously with me. There is no intersection with incoming \nchanges.\n\n\nWhy does 'git pull' complain then?\n\n\n\nYuri\n\n\n"},{"id":"433685","messageId":"YSWXSWiDWNU93lhC@coredump.intra.peff.net","threadId":"56368","inReplyTo":"01514401-78f6-3fdc-aa74-c519fb6ab427@rawbw.com","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-25T01:05:13Z","receivedAt":"2021-08-25T01:05:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 24, 2021 at 09:41:49AM -0700, Yuri wrote:\n\n> In the FreeBSD ports repository I resurrected the directory (that was\n> removed a long time ago) with the command:\n> \n> > $ git checkout {hash}~1 -- math/polymake\n> \n> \n> I made local changes to this directory and called 'git add math/polymake'.\n> \n> Then 'git pull' complained:\n> \n> > $ git pull\n> > error: Your local changes to the following files would be overwritten by\n> merge:\n> >   math/polymake/Makefile math/polymake/distinfo\n> math/polymake/files/patch-Makefile\n> math/polymake/files/patch-support_install.pl math/polymake/pkg-descr\n> math/polymake/pkg-plist\n> \n> \n> No incoming changes affect math/polymake. Nobody has created this directory\n> simultaneously with me. There is no intersection with incoming changes.\n> \n> \n> Why does 'git pull' complain then?\n\nIt's hard to say without seeing a full example. The merge machinery is\nsupposed to handle this situation (and indeed, in a simple reproduction\nexample it does). So presumably it thinks the other side may have\ntouched those files for some reason. Perhaps it's confused by rename\ndetection?\n\nIs it possible to give us a more complete example, including:\n\n  - a url for the repository\n  - the commit at HEAD when you ran \"git checkout\"\n  - the {hash} commit from which you rescued the files\n  - the state of the remote branch (i.e., what we attempted to merge\n    with \"git pull\")\n\n?\n\n-Peff\n"},{"id":"433709","messageId":"2fe5e6c2-3887-6d7d-7240-2aa9f0e70873@gmail.com","threadId":"56368","inReplyTo":"01514401-78f6-3fdc-aa74-c519fb6ab427@rawbw.com","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-25T05:43:13Z","receivedAt":"2021-08-25T05:43:21Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 24/08/21 23.41, Yuri wrote:\n> In the FreeBSD ports repository I resurrected the directory (that was \n> removed a long time ago) with the command:\n> \n>  > $ git checkout {hash}~1 -- math/polymake\n> \n> \n> I made local changes to this directory and called 'git add math/polymake'.\n> \n> Then 'git pull' complained:\n> \n>  > $ git pull\n>  > error: Your local changes to the following files would be overwritten \n> by merge:\n>  >   math/polymake/Makefile math/polymake/distinfo \n> math/polymake/files/patch-Makefile \n> math/polymake/files/patch-support_install.pl math/polymake/pkg-descr \n> math/polymake/pkg-plist\n> \n> \n> No incoming changes affect math/polymake. Nobody has created this \n> directory simultaneously with me. There is no intersection with incoming \n> changes.\n\nHave you committed or stashed such changes?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"433710","messageId":"acbc5ec8-fe21-3c8e-8f4f-e3be3126c2ed@rawbw.com","threadId":"56368","inReplyTo":"2fe5e6c2-3887-6d7d-7240-2aa9f0e70873@gmail.com","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Yuri","fromEmail":"yuri@rawbw.com","sentAt":"2021-08-25T05:47:12Z","receivedAt":"2021-08-25T05:47:16Z","isPatch":false,"sender":{"key":"yuri@rawbw.com","avatar":null},"body":"On 8/24/21 10:43 PM, Bagas Sanjaya wrote:\n> Have you committed or stashed such changes? \n\n\nNo, they were uncommitted and unstashed.\n\n\nYuri\n\n\n"},{"id":"433745","messageId":"CABPp-BEMXW3EOdT4jt1g63uPyZ5YuKUPfBE9BL=E66QcT5uXXA@mail.gmail.com","threadId":"56368","inReplyTo":"YSWXSWiDWNU93lhC@coredump.intra.peff.net","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-25T15:42:09Z","receivedAt":"2021-08-25T15:42:24Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Aug 24, 2021 at 6:09 PM Jeff King <peff@peff.net> wrote:\n>\n> On Tue, Aug 24, 2021 at 09:41:49AM -0700, Yuri wrote:\n>\n> > In the FreeBSD ports repository I resurrected the directory (that was\n> > removed a long time ago) with the command:\n> >\n> > > $ git checkout {hash}~1 -- math/polymake\n> >\n> >\n> > I made local changes to this directory and called 'git add math/polymake'.\n> >\n> > Then 'git pull' complained:\n> >\n> > > $ git pull\n> > > error: Your local changes to the following files would be overwritten by\n> > merge:\n> > >   math/polymake/Makefile math/polymake/distinfo\n> > math/polymake/files/patch-Makefile\n> > math/polymake/files/patch-support_install.pl math/polymake/pkg-descr\n> > math/polymake/pkg-plist\n> >\n> >\n> > No incoming changes affect math/polymake. Nobody has created this directory\n> > simultaneously with me. There is no intersection with incoming changes.\n> >\n> >\n> > Why does 'git pull' complain then?\n>\n> It's hard to say without seeing a full example. The merge machinery is\n> supposed to handle this situation (and indeed, in a simple reproduction\n> example it does). So presumably it thinks the other side may have\n> touched those files for some reason. Perhaps it's confused by rename\n> detection?\n>\n> Is it possible to give us a more complete example, including:\n>\n>   - a url for the repository\n>   - the commit at HEAD when you ran \"git checkout\"\n>   - the {hash} commit from which you rescued the files\n>   - the state of the remote branch (i.e., what we attempted to merge\n>     with \"git pull\")\n>\n> ?\n\nThe `git checkout {hash}~1 -- math/polymake` is enough to highlight\nthat Yuri doesn't just have local changes (which the merge machinery\nshould allow if the incoming changes don't touch the same files), but\nlocal *staged* changes.  As per the merge manpage:\n\n\"\"\"\nTo avoid recording unrelated changes in the merge commit, git pull and\ngit merge will also abort if there are any changes registered in the\nindex relative to the HEAD commit.\n\"\"\"\n\nWhile this particular example could theoretically be handled by the\nmerge machinery without requiring the index match HEAD, at least with\nthe new ort backend (it'd be way too complex with the recursive\nbackend, see below), in practice I'm so sick of index != HEAD bugs\nthat I have zero motivation to even consider it and I might even NAK\npatches from others who attempted it.  For why, let me quote from the\ncommit message of 9822175d2b (\"Ensure index matches head before\ninvoking merge machinery, round N\", 2019-08-17), which also references\nanother commit that I'm tempted to also quote:\n\n\"\"\"\nEnsure index matches head before invoking merge machinery, round N\n\nThis is the bug that just won't die; there always seems to be another\nform of it somewhere.  See the commit message of 55f39cf7551b (\"merge:\nfix misleading pre-merge check documentation\", 2018-06-30) for a more\ndetailed explanation), but in short:\n\n<quick summary>\n\nbuiltin/merge.c contains this important requirement for merge\nstrategies:\n\n    ...the index must be in sync with the head commit.  The strategies are\n    responsible to ensure this.\n\nThis condition is important to enforce because there are two likely\nfailure cases when the index isn't in sync with the head commit:\n\n  * we silently throw away changes the user had staged before the merge\n\n  * we accidentally (and silently) include changes in the merge that\n    were not part of either of the branches/trees being merged\n\nDiscarding users' work and mis-merging are both bad outcomes, especially\nwhen done silently, so naturally this rule was stated sternly -- but,\nunfortunately totally ignored in practice unless and until actual bugs\nwere found.  But, fear not: the bugs from this were fixed in commit\n  ee6566e8d70d (\"[PATCH] Rewrite read-tree\", 2005-09-05)\nthrough a rewrite of read-tree (again, commit 55f39cf7551b has a more\ndetailed explanation of how this affected merge).  And it was fixed\nagain in commit\n  160252f81626 (\"git-merge-ours: make sure our index matches HEAD\", 2005-11-03)\n...and it was fixed again in commit\n  3ec62ad9ffba (\"merge-octopus: abort if index does not match HEAD\", 2016-04-09)\n...and again in commit\n  65170c07d466 (\"merge-recursive: avoid incorporating uncommitted\nchanges in a merge\", 2017-12-21)\n...and again in commit\n  eddd1a411d93 (\"merge-recursive: enforce rule that index matches head\nbefore merging\", 2018-06-30)\n\n...with multiple testcases added to the testsuite that could be\nenumerated in even more commits.\n\nThen, finally, in a patch in the same series as the last fix above, the\ndocumentation about this requirement was fixed in commit 55f39cf7551b\n(\"merge: fix misleading pre-merge check documentation\", 2018-06-30), and\nwe all lived happily ever after...\n\n</quick summary>\n\nUnfortunately, \"ever after\" apparently denotes a limited time and it\nexpired today.\n\"\"\"\n\nand then the commit message goes on to explain multiple other bugs\nfound and fixed due to not strictly enforcing that the index needs to\nmatch HEAD before starting a merge.\n"},{"id":"433762","messageId":"YSaUcDQ+MIUdw6Ji@coredump.intra.peff.net","threadId":"56368","inReplyTo":"CABPp-BEMXW3EOdT4jt1g63uPyZ5YuKUPfBE9BL=E66QcT5uXXA@mail.gmail.com","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-25T19:05:20Z","receivedAt":"2021-08-25T19:05:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 25, 2021 at 08:42:09AM -0700, Elijah Newren wrote:\n\n> > Is it possible to give us a more complete example, including:\n> >\n> >   - a url for the repository\n> >   - the commit at HEAD when you ran \"git checkout\"\n> >   - the {hash} commit from which you rescued the files\n> >   - the state of the remote branch (i.e., what we attempted to merge\n> >     with \"git pull\")\n> >\n> > ?\n> \n> The `git checkout {hash}~1 -- math/polymake` is enough to highlight\n> that Yuri doesn't just have local changes (which the merge machinery\n> should allow if the incoming changes don't touch the same files), but\n> local *staged* changes.  As per the merge manpage:\n> \n> \"\"\"\n> To avoid recording unrelated changes in the merge commit, git pull and\n> git merge will also abort if there are any changes registered in the\n> index relative to the HEAD commit.\n> \"\"\"\n\nDoh, you're right. I did a simple reproduction test, but in my example,\nthe merge was a fast-forward, which we do allow. Picking a branch more\ncarefully using:\n\n  git for-each-ref --no-contains HEAD --no-merged HEAD refs/remotes/origin\n\nshows the error. Sorry for the confusion.\n\n-Peff\n"},{"id":"433768","messageId":"xmqqzgt5gr3g.fsf@gitster.g","threadId":"56368","inReplyTo":"CABPp-BEMXW3EOdT4jt1g63uPyZ5YuKUPfBE9BL=E66QcT5uXXA@mail.gmail.com","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-25T21:19:15Z","receivedAt":"2021-08-25T21:19:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> ... allow if the incoming changes don't touch the same files), but\n> local *staged* changes.  As per the merge manpage:\n>\n> \"\"\"\n> To avoid recording unrelated changes in the merge commit, git pull and\n> git merge will also abort if there are any changes registered in the\n> index relative to the HEAD commit.\n> \"\"\"\n>\n> While this particular example could theoretically be handled by the\n> merge machinery without requiring the index match HEAD,...\n\nWhile I do not mind seeing a patch that loosens the condition ONLY\nwhen the merge will cleanly auto-resolve without end-user\ninteraction, when any paths conflict and require editing by the\nend-user, it is pretty much essential to require that the index\nmatches HEAD to keep \"git merge\" usable. \n\nThis is because the final step to conclude such an \"automated\nprocedure cannot cleanly resolve, so the end user helps resolving\nwith the editor and mark the resolved paths with 'git add' or 'git\nrm'\" session will become very error prone if we did not have the\nrequirement.  Not just the user MUST remember not to use \"commit -a\"\nor \"git add\" a path that was already dirty in the working tree\nbefore the merge started (which is the consequence of the current\nrequirement, which allows local changes to the unrelated working\ntree files), they must MUST remember to somehow EXCLUDE the changes\nalready registered for unrelted paths from the concluded merge.\n"},{"id":"433895","messageId":"YSg6dshmB5JA7KZO@coredump.intra.peff.net","threadId":"56368","inReplyTo":"xmqqzgt5gr3g.fsf@gitster.g","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-27T01:05:58Z","receivedAt":"2021-08-27T01:06:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 25, 2021 at 02:19:15PM -0700, Junio C Hamano wrote:\n\n> Elijah Newren <newren@gmail.com> writes:\n> \n> > ... allow if the incoming changes don't touch the same files), but\n> > local *staged* changes.  As per the merge manpage:\n> >\n> > \"\"\"\n> > To avoid recording unrelated changes in the merge commit, git pull and\n> > git merge will also abort if there are any changes registered in the\n> > index relative to the HEAD commit.\n> > \"\"\"\n> >\n> > While this particular example could theoretically be handled by the\n> > merge machinery without requiring the index match HEAD,...\n> \n> While I do not mind seeing a patch that loosens the condition ONLY\n> when the merge will cleanly auto-resolve without end-user\n> interaction, when any paths conflict and require editing by the\n> end-user, it is pretty much essential to require that the index\n> matches HEAD to keep \"git merge\" usable. \n> \n> This is because the final step to conclude such an \"automated\n> procedure cannot cleanly resolve, so the end user helps resolving\n> with the editor and mark the resolved paths with 'git add' or 'git\n> rm'\" session will become very error prone if we did not have the\n> requirement.  Not just the user MUST remember not to use \"commit -a\"\n> or \"git add\" a path that was already dirty in the working tree\n> before the merge started (which is the consequence of the current\n> requirement, which allows local changes to the unrelated working\n> tree files), they must MUST remember to somehow EXCLUDE the changes\n> already registered for unrelted paths from the concluded merge.\n\nGood point.\n\nIn theory we could reject the merge only after finding that there were\nconflicts. That lets the happy path continue even with unrelated changes\n(just like a fast-forward does). I suspect we'd need to change the\ninterface to the merge-backends, though (to say \"do the merge, and bail\non conflicts, but _don't_ write out conflict markers or touch any other\nstate in that case\").\n\n(I am just thinking out loud, though. My personal opinion is that if you\nhave a bunch of staged changes and want to do any non-trivial merging,\nyou should considering committing or stashing those changes).\n\n-Peff\n"},{"id":"434006","messageId":"CABPp-BEt=wBMZHP1P+sy1+GPhEgynu-RoUs2ZC7ZuWspfCsTVQ@mail.gmail.com","threadId":"56368","inReplyTo":"YSg6dshmB5JA7KZO@coredump.intra.peff.net","subject":"Re: 'git pull' complains that a locally resurrected directory would be overwritten by merge when no pulled changes are affecting that directory","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-08-28T05:21:25Z","receivedAt":"2021-08-28T05:21:39Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Aug 26, 2021 at 6:05 PM Jeff King <peff@peff.net> wrote:\n>\n> On Wed, Aug 25, 2021 at 02:19:15PM -0700, Junio C Hamano wrote:\n>\n> > Elijah Newren <newren@gmail.com> writes:\n> >\n> > > ... allow if the incoming changes don't touch the same files), but\n> > > local *staged* changes.  As per the merge manpage:\n> > >\n> > > \"\"\"\n> > > To avoid recording unrelated changes in the merge commit, git pull and\n> > > git merge will also abort if there are any changes registered in the\n> > > index relative to the HEAD commit.\n> > > \"\"\"\n> > >\n> > > While this particular example could theoretically be handled by the\n> > > merge machinery without requiring the index match HEAD,...\n> >\n> > While I do not mind seeing a patch that loosens the condition ONLY\n> > when the merge will cleanly auto-resolve without end-user\n> > interaction, when any paths conflict and require editing by the\n> > end-user, it is pretty much essential to require that the index\n> > matches HEAD to keep \"git merge\" usable.\n> >\n> > This is because the final step to conclude such an \"automated\n> > procedure cannot cleanly resolve, so the end user helps resolving\n> > with the editor and mark the resolved paths with 'git add' or 'git\n> > rm'\" session will become very error prone if we did not have the\n> > requirement.  Not just the user MUST remember not to use \"commit -a\"\n> > or \"git add\" a path that was already dirty in the working tree\n> > before the merge started (which is the consequence of the current\n> > requirement, which allows local changes to the unrelated working\n> > tree files), they must MUST remember to somehow EXCLUDE the changes\n> > already registered for unrelted paths from the concluded merge.\n>\n> Good point.\n>\n> In theory we could reject the merge only after finding that there were\n> conflicts. That lets the happy path continue even with unrelated changes\n> (just like a fast-forward does). I suspect we'd need to change the\n> interface to the merge-backends, though (to say \"do the merge, and bail\n> on conflicts, but _don't_ write out conflict markers or touch any other\n> state in that case\").\n\nWe actually wouldn't need to change the interface.  One of the reasons\nthere were so many annoying bugs with index != HEAD, was that\nbuiltin/merge.c stated that the merge backends were responsible for\nenforcing that condition.  So every single merge backend had to be\nindividually fixed -- ours, octopus, resolve, recursive.  And then\nsome of those, like recursive, had multiple affected code paths, and\neach one had to be fixed, and I didn't catch them all the first time.\n\nThat doesn't mean I'm in favor of the change, just pointing out that\none backend could decide to do this on its own even if the other\nbackends didn't support it.\n\n> (I am just thinking out loud, though. My personal opinion is that if you\n> have a bunch of staged changes and want to do any non-trivial merging,\n> you should considering committing or stashing those changes).\n\nAgreed.\n\nThough perhaps I should have pointed out to Yuri that he wouldn't have\nrun into these problems if he would have used\n    git restore --source={hash}~1 -- math/polymake\ninstead of\n    git checkout {hash}~1 -- math/polymake\n\n(since the former defaults to updating just the working directory,\nwhile the later updates both the index and the working directory.)\n"}]}