Re: [PATCH 2/2] merge: avoid searching for strategies with fewer than 0 conflicts

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 22, 2022, 16:19 UTC
Message-ID: <xmqqr118dzek.fsf@gitster.g>
In-Reply-To: <CABPp-BHy-Xs0S1-9kL7X4mi6JNfd420-vpVsswXNuXsc5C7AtA@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 14 quoted lines

> No, the original code would not have taken 'ort'.  You have overlooked
> the part of the code immediately above these two lines:
>
>     if (ret < 2) {
>         if (!ret) {
>             if (option_commit) {
>                 /* Automerge succeeded. */
>                 automerge_was_ok = 1;
>                 break;
>             }
>         merge_was_ok = 1;
>         }
>
> In particular, the "break" is key.

I noticed that much but then would "git merge --no-commit -s A -s B" have the issue?

> ...  In contrast, if the user did specify
> --no-commit and the previous strategy succeeded, then we will continue
> the loop.  That seems rather inconsistent, since --no-commit should
> not affect the choice of strategy.

Yeah, exactly.

Show 7 quoted lines

> However, I missed two things in my reading.  You are correct that I
> missed the "<=" as opposed to "<" when I wrote my commit message,
> though that turns out to not matter much due to the second thing.  The
> second thing I missed was part of the code at the beginning of the
> loop:
>
>     for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {

Ahhhh, that explains it. We leave as soon as we find merge_was_ok so this patch is not necessary. There is nothing to fix. The original was fine as-is.

Thanks.