From: Junio C Hamano Date: Thu, 21 Jul 2022 16:16:00 GMT Subject: Re: [PATCH v3 5/7] merge: make restore_state() restore staged state too Message-ID: In-Reply-To: <91c495c770e3f7f91ae655a503bdd1cd99935e40.1658391391.git.gitgitgadget@gmail.com> "Elijah Newren via GitGitGadget" writes: > 4) Merge strategies can make changes to the index and working tree, > and have no expectation to clean up after themselves, *even* if > they bail out and say they are not an appropriate handler for > the merge in question. (The `octopus` merge strategy does this, > for example.) I personally consider this is a bug in `octopus` (and I can do so without offending anybody, as `octopus` was what I did), but because the point of having pluggable merge strategies is to allow end users and third parties write their own. So save-and-restore dance would be a prudent approach to the issue than forbidding this "buggy" behaviour. > Unfortunately, if users had staged changes before calling `git merge`, > builtin/merge.c could do the following: > >    * stash the changes, in order to clean up after the strategies >    * try all the merge strategies in turn, each of which report they > cannot function due to the index not matching HEAD >    * restore the changes via "git stash apply" > test_seq 0 10 >a && > git add a && > + git rev-parse :a >expect && > ... > + # Changes to "a" should remain staged > + git rev-parse :a >actual && > + test_cmp expect actual Makes sense. Thanks.