RE: [RFC PATCH 01/35] merge: improve fatal fast-forward message
On July 6, 2021 5:12 PM, Felipe Contreras wrote:
Show 52 quoted lines
>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`.Well, you know that and I know that, but having to explain this to every new git user who will operationally use git merge --ff-only within hours or days of their first clone is a different matter.
Show 8 quoted lines
>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.
Thanks. I understand the purpose here.
-Randall
Randall S. Becker <rsbecker@nexbridge.com>