All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Denton Liu <liu.denton@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	Philippe Blain <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH 3/4] merge: apply autostash if fast-forward fails
Date: Fri, 23 Jul 2021 12:13:15 -0700	[thread overview]
Message-ID: <xmqqo8asn8s4.fsf@gitster.g> (raw)
In-Reply-To: <86becf764243f129c980f073127ec3f08fd4477c.1627042470.git.gitgitgadget@gmail.com> (Philippe Blain via GitGitGadget's message of "Fri, 23 Jul 2021 12:14:29 +0000")

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> Since 'git merge' learned '--autostash' in a03b55530a (merge: teach
> --autostash option, 2020-04-07), 'cmd_merge', in the fast-forward case,
> calls 'create_autostash' before calling 'checkout_fast_forward' if
> '--autostash' is given.
>
> However, if 'checkout_fast_forward' fails, the autostash is not applied
> to the working tree, nor saved in the stash list, since the code simply
> calls 'goto done'.
>
> Be more helpful to the user by applying the autostash in that case.
>
> An easy way to test a failing fast-forward is when we are merging a
> branch that has a tracked file that conflicts with an untracked file in
> the working tree.

I think this one makes sense, as the case that is tested I know
fast-forward (aka two-tree switching) would be atomic. If your
working tree is already broken (e.g. triggers I/O errors in the
middle, some directories having wrong permissions to make them
unwritable by you, etc.), you would also see an error from
fast-forward and you probably cannot tell these two cases apart,
and trying to unstash the local changes may make things even worse,
but I suspect that there isn't much you can do about it.

Thanks.

>
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>  builtin/merge.c  |  1 +
>  t/t7600-merge.sh | 11 +++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 74797b6c7a6..788a6b0cd55 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1560,6 +1560,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  					  &head_commit->object.oid,
>  					  &commit->object.oid,
>  					  overwrite_ignore)) {
> +			apply_autostash(git_path_merge_autostash(the_repository));
>  			ret = 1;
>  			goto done;
>  		}
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 1cbc9715a81..216113d3537 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -122,6 +122,8 @@ test_expect_success 'setup' '
>  	c0=$(git rev-parse HEAD) &&
>  	cp file.1 file &&
>  	git add file &&
> +	cp file.1 other &&
> +	git add other &&
>  	test_tick &&
>  	git commit -m "commit 1" &&
>  	git tag c1 &&
> @@ -711,6 +713,15 @@ test_expect_success 'fast-forward merge with --autostash' '
>  	test_cmp result.1-5 file
>  '
>  
> +test_expect_success 'failed fast-forward merge with --autostash' '
> +	git reset --hard c0 &&
> +	git merge-file file file.orig file.5 &&
> +	cp file.5 other &&
> +	test_must_fail git merge --autostash c1 2>err &&
> +	test_i18ngrep "Applied autostash." err &&
> +	test_cmp file.5 file
> +'
> +
>  test_expect_success 'octopus merge with --autostash' '
>  	git reset --hard c1 &&
>  	git merge-file file file.orig file.3 &&

  parent reply	other threads:[~2021-07-23 19:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23 12:14 [PATCH 0/4] [RFC] merge --autostash: apply autostash in more cases Philippe Blain via GitGitGadget
2021-07-23 12:14 ` [PATCH 1/4] merge: add missing word "strategy" to a message Philippe Blain via GitGitGadget
2021-07-23 15:57   ` Felipe Contreras
2021-07-23 12:14 ` [PATCH 2/4] Documentation: define 'MERGE_AUTOSTASH' Philippe Blain via GitGitGadget
2021-07-23 16:00   ` Felipe Contreras
2021-07-23 12:14 ` [PATCH 3/4] merge: apply autostash if fast-forward fails Philippe Blain via GitGitGadget
2021-07-23 16:02   ` Felipe Contreras
2021-07-23 19:13   ` Junio C Hamano [this message]
2021-07-23 12:14 ` [PATCH 4/4] merge: apply autostash if merge strategy fails Philippe Blain via GitGitGadget
2021-07-23 19:22   ` Junio C Hamano
2021-07-23 16:10 ` [PATCH 0/4] [RFC] merge --autostash: apply autostash in more cases Felipe Contreras

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

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

  git send-email \
    --in-reply-to=xmqqo8asn8s4.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=liu.denton@gmail.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.