@matheusbsbs / matheus-bagliano / commits / 5b1d30cabf

merge: cleanup confusing logic for handling successful merges

builtin/merge.c has a loop over the specified strategies, where if they all fail with conflicts, it picks the one with the least number of conflicts. In the codepath that finds a successful merge, if an automatic commit was wanted, the code breaks out of the above loop, which makes sense. However, if the user requested there be no automatic commit, the loop would continue. That seems weird; --no-commit should not affect the choice of merge strategy, but the code as written makes one think it does. However, since the loop itself embeds "!merge_was_ok" as a condition on continuing to loop, it actually would also exit early if --no-commit was specified, it just exited from a different location. Restructure the code slightly to make it clear that the loop will immediately exit whenever we find a merge strategy that is successful. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Aug 23, 2022 at 02:42 UTC 5b1d30cabfcdd7cb1dc990f22980af32aa6986a9

1 file changed +9 -7

builtin/merge.c

+9 -7

@@ -1692,7 +1692,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)

1692 if (save_state(&stash))

1693 oidclr(&stash);

1694

1695 - for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {

1695 + for (i = 0; i < use_strategies_nr; i++) {

1696 int ret, cnt;

1697 if (i) {

1698 printf(_("Rewinding the tree to pristine...\n"));

@@ -1717,12 +1717,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)

1717 */

1718 if (ret < 2) {

1719 if (!ret) {

1720 - if (option_commit) {

1721 - /* Automerge succeeded. */

1722 - automerge_was_ok = 1;

1723 - break;

1724 - }

1720 + /*

1721 + * This strategy worked; no point in trying

1722 + * another.

1723 + */

1724 merge_was_ok = 1;

1725 + best_strategy = use_strategies[i]->name;

1726 + break;

1727 }

1728 cnt = (use_strategies_nr > 1) ? evaluate_result() : 0;

1729 if (best_cnt <= 0 || cnt <= best_cnt) {

@@ -1736,7 +1737,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)

1737 * If we have a resulting tree, that means the strategy module

1738 * auto resolved the merge cleanly.

1739 */

1739 - if (automerge_was_ok) {

1740 + if (merge_was_ok && option_commit) {

1741 + automerge_was_ok = 1;

1742 ret = finish_automerge(head_commit, head_subsumed,

1743 common, remoteheads,

1744 &result_tree, wt_strategy);