On Mon, May 1, 2023 at 5:48 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Tao Klerks <tao@xxxxxxxxxx> writes: > > > At first I hoped that "git switch" might have fixed this - and in a > > limited sense it has, in that instead of *silently discarding* the > > merge state/metadata, it refuses with "fatal: cannot switch branch > > while merging". > > I think this is in generally seen as a vast improvement over the > "silently discarding" to have the safety valve. > Heh, that's fair - my apologies for the "ungrateful" tone. > > I think this is mostly that nobody spent enough brain cycles to > ponder on things like: (I'm not sure I *have* enough brain cycles, or background knowledge, to be sure of these things, but I have some tentative answers) > > - Is it always safe to switch from a merge in progress to a newly > created branch that point at the same commit as HEAD? Why? Is > the story the same if the switched-to branch is an existing one > that happens to point at the same commit as HEAD? To the best of my knowledge, the only role that the HEAD *ref* plays in any of merge processing (before or after resolution), if there even is one, is its use in the commit message. My proposal is that keeping the original prepared merge message, but warning about its use, makes more sense / is more straightforward than anything else we might choose to do here, like re-generating the message. With respect to the safety of index operations - when conflicts have been resolved already the index is in a "normal" state, same as a non-merge checkout with changes, so there is no reason to believe there would be any "unsafeness" to worry about? > > - Maybe the answer to the above question is not "it is not quite > safe to switch during a conflicted merge, but by adjusting X and > Y while switching to a new branch, we could make it safe". The sense in which it is potentially *not* safe, is that some part of checkout may mess with the index in some way that does not expect to encounter conflicts - given that conflicts explicitly prevent not only a "git switch", but also a "git checkout", at this time (due to a check for unmerged paths in builtin/checkout.c#merge_working_tree()). As such, the "simple answer" is to address the case where conflicts are already resolved, and defer the case of open conflicts to someone who is better able to check or enable the safety of a switch in this state. I believe the urgency/criticality of opening up a "switch while you still have unmerged paths" usecase is much lower than the simpler-to-prove "switch before committing an already-resolved index" usecase. > > - Even if the answer to the question above is "yes it is always > safe", if it is not safe for a similar situation that ends up in > unmerged index (i.e. the index with any path at non-0 stage), the > way we determine if we are in "a merge in progress" situation > must be reliable. Is it? I'm not sure what "a path at 0 stage" means, but I do get the *impression* that the conflict checks that are in place in builtin/checkout.c#merge_working_tree() look reasonable...? That said, I do not intend to modify or circumvent these checks, because I don't think the user flow / use case requires it. > > - Conversely, perhaps it is also safe to switch to a different > branch that points at the same commit as HEAD from the conflicted > state after some (but not necessarily all) of the following > operations: "am -3", "cherry-pick" (single commit), "revert" > (single commit), etc. Can we come up with a reliable way to > determine if we are in such a situation? I suspect it is, but I also suspect this is a far less common case, and as such one I'm less inclined to worry about. In merge-based workflows, merges can be *large*, and the case where you have a merge ready, conflicts resolved, and you realize you'd like to test it on another branch, is more common and impactful, than when you have some (far more typically limited in scope) stash or cherry-pick...? (Or maybe I'm just making excuses for the patch I want to submit :) ) My proposal at least is to carve out an exception, for now, only for merges, and only with a clean (not-conflicted) index. > > - These "other mergy operations" that can leave unmerged index may > share the same "is not safe out of the box, but can be made safe > by adjusting some stuff". > > After that is done, we could update the codepath in "git switch" > that says "ah, your index is unmerged, so I will not let you switch" > to instead say "your index is unmerged; let's first see if you are > switching to the same commit as HEAD and then the way your index is > unmerged was caused by an operation that we decided is safe to > switch out of. It seems to be one of the situations that could be > made safe, so let's do adjustment X and Y and let you switch". This is what I am proposing, but no adjustment needed if the scope is a merge only and the index is already unconflicted. > > And the reason why "git switch" punts is because nobody bothered to, > not because there fundamentally is some reason why it shouldn't work. Yep, that makes sense.
