{"thread":{"id":"51451","subject":"[PATCH v2] builtin/merge.c - cleanup of code in for-cycle that tests strategies","startedAt":"2019-07-09T03:16:25Z","lastAt":"2019-07-09T03:16:25Z","messageCount":1,"participants":["Edmundo Carmona Antoranz"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"378717","messageId":"20190709031559.21742-1-eantoranz@gmail.com","threadId":"51451","inReplyTo":null,"subject":"[PATCH v2] builtin/merge.c - cleanup of code in for-cycle that tests strategies","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2019-07-09T03:15:59Z","receivedAt":"2019-07-09T03:16:25Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"The cmd_merge() function has a loop that tries different\nmerge strategies in turn, and stops when a strategy gets a\nclean merge, while keeping the \"best\" conflicted merge so\nfar.\n\nMake the loop easier to follow by moving the code around,\nensuring that there is only one \"break\" in the loop where\nan automerge succeeds.  Also group the actions that are\nperformed after an automerge succeeds together to a single\nlocation, outside and after the loop.\n\nSigned-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n---\n builtin/merge.c | 53 +++++++++++++++++++------------------------------\n 1 file changed, 20 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 6e99aead46..e7aeedc77d 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -892,6 +892,7 @@ static int finish_automerge(struct commit *head,\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct object_id result_commit;\n \n+\twrite_tree_trivial(result_tree);\n \tfree_commit_list(common);\n \tparents = remoteheads;\n \tif (!head_subsumed || fast_forward == FF_NO)\n@@ -1586,8 +1587,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t    save_state(&stash))\n \t\toidclr(&stash);\n \n-\tfor (i = 0; i < use_strategies_nr; i++) {\n-\t\tint ret;\n+\tfor (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {\n+\t\tint ret, cnt;\n \t\tif (i) {\n \t\t\tprintf(_(\"Rewinding the tree to pristine...\\n\"));\n \t\t\trestore_state(&head_commit->object.oid, &stash);\n@@ -1604,40 +1605,26 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tret = try_merge_strategy(use_strategies[i]->name,\n \t\t\t\t\t common, remoteheads,\n \t\t\t\t\t head_commit);\n-\t\tif (!option_commit && !ret) {\n-\t\t\tmerge_was_ok = 1;\n-\t\t\t/*\n-\t\t\t * This is necessary here just to avoid writing\n-\t\t\t * the tree, but later we will *not* exit with\n-\t\t\t * status code 1 because merge_was_ok is set.\n-\t\t\t */\n-\t\t\tret = 1;\n-\t\t}\n-\n-\t\tif (ret) {\n-\t\t\t/*\n-\t\t\t * The backend exits with 1 when conflicts are\n-\t\t\t * left to be resolved, with 2 when it does not\n-\t\t\t * handle the given merge at all.\n-\t\t\t */\n-\t\t\tif (ret == 1) {\n-\t\t\t\tint cnt = evaluate_result();\n-\n-\t\t\t\tif (best_cnt <= 0 || cnt <= best_cnt) {\n-\t\t\t\t\tbest_strategy = use_strategies[i]->name;\n-\t\t\t\t\tbest_cnt = cnt;\n+\t\t/*\n+\t\t * The backend exits with 1 when conflicts are\n+\t\t * left to be resolved, with 2 when it does not\n+\t\t * handle the given merge at all.\n+\t\t */\n+\t\tif (ret < 2) {\n+\t\t\tif (!ret) {\n+\t\t\t\tif (option_commit) {\n+\t\t\t\t\t/* Automerge succeeded. */\n+\t\t\t\t\tautomerge_was_ok = 1;\n+\t\t\t\t\tbreak;\n \t\t\t\t}\n+\t\t\t\tmerge_was_ok = 1;\n+\t\t\t}\n+\t\t\tcnt = evaluate_result();\n+\t\t\tif (best_cnt <= 0 || cnt <= best_cnt) {\n+\t\t\t\tbest_strategy = use_strategies[i]->name;\n+\t\t\t\tbest_cnt = cnt;\n \t\t\t}\n-\t\t\tif (merge_was_ok)\n-\t\t\t\tbreak;\n-\t\t\telse\n-\t\t\t\tcontinue;\n \t\t}\n-\n-\t\t/* Automerge succeeded. */\n-\t\twrite_tree_trivial(&result_tree);\n-\t\tautomerge_was_ok = 1;\n-\t\tbreak;\n \t}\n \n \t/*\n-- \n2.20.1\n\n"}]}