git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: ZheNing Hu <adlternative@gmail.com>,
	Christian Couder <christian.couder@gmail.com>,
	Git List <git@vger.kernel.org>,
	vascomalmeida@sapo.pt
Subject: Re: Question about pre-merge and git merge octopus strategy
Date: Thu, 12 May 2022 08:39:34 -0700	[thread overview]
Message-ID: <xmqqk0aqhia1.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BHmNBMypVDrE=gPMXoHT9uH-u4HJG1dAuY0tjSGrK0yPg@mail.gmail.com> (Elijah Newren's message of "Thu, 12 May 2022 08:04:41 -0700")

Elijah Newren <newren@gmail.com> writes:

>     Merge with strategy octopus failed.
>
> Also, if we check `git status`:
>
>     $ git status
>     On branch main
>     Unmerged paths:
>       (use "git restore --staged <file>..." to unstage)
>       (use "git add <file>..." to mark resolution)
>     both modified:   base
>
>     no changes added to commit (use "git add" and/or "git commit -a")
>
> And in git-merge-octopus.sh we see:
>
>     case "$OCTOPUS_FAILURE" in
>     1)
>     # We allow only last one to have a hand-resolvable
>     # conflicts.  Last round failed and we still had
>     # a head to merge.
>     gettextln "Automated merge did not work."
>     gettextln "Should not be doing an octopus."
>     exit 2
>     esac
>
> and in builtin/merge.c, we see:
>
>     /*
>      * The backend exits with 1 when conflicts are
>      * left to be resolved, with 2 when it does not
>      * handle the given merge at all.
>      */
>
> Which means git-merge-octopus.sh is claiming it can't handle this type
> of merge, and some other merge strategy should be tried, and
> implicitly that it didn't leave any conflicts to be resolved because
> it can't handle this merge.

Correct.  Near the beginning of the loop you found the above
comment, there is this code:

	if (use_strategies_nr == 1 ||
	    /*
	     * Stash away the local changes so that we can try more than one.
	     */
	    save_state(&stash))
		oidclr(&stash);

	for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
		int ret, cnt;
		if (i) {
			printf(_("Rewinding the tree to pristine...\n"));
			restore_state(&head_commit->object.oid, &stash);
		}

but that save-then-restore triggers ONLY when there are multiple
strategies to try.  Unfortunately, octopus has no friends to fall
back on, so we do not do the save-restore dance on the calling side.

> But it clearly decides to leave the
> modifications it made to the index and working tree around, which just
> seems wrong to me.

If merge-recursive or merge-resolve is asked to merge a single
commit to the current branch without any other strategies to use as
a fallback, they leave the working tree and index into a state where
the end-user can conclude the conflict resolution and commit the
result.  In spirit, we are in the same situation, aren't we?

The user, if they want to proceed against octopus's opinion, would
resolve the current conflict, read-tree -m the next one, ..., to
conclude and commit the result. 

So I am not sure if it is a good idea to unconditionally "reset --merge"
in this situation.

> +# If octopus is inapplicable, make sure we undo any changes we made first
> +cannot_octopus() {
> +       git reset --merge
> +       exit 2
> +}

  reply	other threads:[~2022-05-12 15:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-06  8:14 ZheNing Hu
2022-05-06 17:23 ` Christian Couder
2022-05-07  4:09   ` Elijah Newren
2022-05-07 18:37     ` Junio C Hamano
2022-05-08 14:44     ` ZheNing Hu
2022-05-10  7:07       ` Elijah Newren
2022-05-11 11:21         ` ZheNing Hu
2022-05-12 15:04           ` Elijah Newren
2022-05-12 15:39             ` Junio C Hamano [this message]
2022-05-13  5:15               ` Elijah Newren
2022-05-13 12:56                 ` Junio C Hamano
2022-05-19 13:15                 ` ZheNing Hu
2022-05-19 14:46                   ` Elijah Newren
2022-05-08 14:13   ` ZheNing Hu
2022-05-08 15:01 ` Carlo Marcelo Arenas Belón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk0aqhia1.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=adlternative@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=vascomalmeida@sapo.pt \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).