On Mon, May 1, 2023 at 5:48 PM Junio C Hamano [off-list ref] wrote:

Tao Klerks [off-list ref] writes:

quoted

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.