RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message

[Felipe Contreras <felipe.contreras@gmail.com>]

Randall S. Becker wrote:
> On July 6, 2021 4:40 PM, Felipe Contreras wrote:
> >Subject: Re: [RFC PATCH 01/35] merge: improve fatal fast-forward message
> >
> >Ævar Arnfjörð Bjarmason wrote:
> >>
> >> On Mon, Jul 05 2021, Felipe Contreras wrote:
> >>
> >> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> > ---
> >> >  builtin/merge.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/builtin/merge.c b/builtin/merge.c index
> >> > a8a843b1f5..05e631229d 100644
> >> > --- a/builtin/merge.c
> >> > +++ b/builtin/merge.c
> >> > @@ -1620,7 +1620,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> >> >  	}
> >> >
> >> >  	if (fast_forward == FF_ONLY)
> >> > -		die(_("Not possible to fast-forward, aborting."));
> >> > +		die(_("unable to fast-forward"));
> >>
> >> I read the existing message a bit more like "this makes no sense
> >> anymore" (correct) and the latter more like "we encountered an error".
> >
> >I mean, this is the documentation of --ff-only:
> >
> >  With `--ff-only`, resolve the merge as a fast-forward when possible.
> >  When not possible, refuse to merge and exit with a non-zero status.
> >
> >So if you do `git merge --ff-only` you are telling git: "I want you to exit with an error when the fast-forward is not possible".
> >
> >If you do:
> >
> >  % git merge --ff-only
> >  fatal: Not possible to fast-forward, aborting.
> >
> >That "aborting" part is redundant; we know `git merge` should abort if the fast-forward is not possible, we explicitely told git to do that.
> 
> `git merge` is a special operation where errors (conflicts, for one)
> may leave the repository in a merge pending state where you
> subsequently may have to use `git merge --abort` to reset the
> situation or `git add` to continue. The `aborting` output makes it
> clear that you do not have to do the `--abort` and *cannot* do the
> `add` because there was an implicit `--abort` done resulting from the
> failure.

But this is not a `git merge`, this is a `git merge --ff-only`; they are
different operations. There *never* is a need for `--abort` with
`git merge --ff-only`.

Anyway, the error message is meant for `git fast-forward` which
definitely doesn't need any `--abort`.

Initially I created a new variable to have a different error message for
`git merge --ff-only` and `git fast-forward`, precisely to avoid
changing the current error message of `git merge --ff-only` and thus
avoid any inertial comments like this one. But then I thought there was
no need to complicate the series when both can be improved at once.
Apparently that's not the case.

I guess I'll add it back.

Cheers.

-- 
Felipe Contreras