Re: [PATCH v3 1/9] show, log: provide a --remerge-diff capability
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-01-19 15:53:37
On Thu, Dec 30 2021, Elijah Newren via GitGitGadget wrote:
From: Elijah Newren <redacted>
quoted hunk ↗ jump to hunk
+static int do_remerge_diff(struct rev_info *opt, + struct commit_list *parents, + struct object_id *oid, + struct commit *commit) +{ + struct merge_options o; + struct commit_list *bases; + struct merge_result res = {0}; + struct pretty_print_context ctx = {0}; + struct commit *parent1 = parents->item; + struct commit *parent2 = parents->next->item; + struct strbuf parent1_desc = STRBUF_INIT; + struct strbuf parent2_desc = STRBUF_INIT; + + /* Setup merge options */ + init_merge_options(&o, the_repository); + o.show_rename_progress = 0; + + ctx.abbrev = DEFAULT_ABBREV; + format_commit_message(parent1, "%h (%s)", &parent1_desc, &ctx); + format_commit_message(parent2, "%h (%s)", &parent2_desc, &ctx); + o.branch1 = parent1_desc.buf; + o.branch2 = parent2_desc.buf; + + /* Parse the relevant commits and get the merge bases */ + parse_commit_or_die(parent1); + parse_commit_or_die(parent2); + bases = get_merge_bases(parent1, parent2);
There's existing leaks all over the place here unrelated to this new code, so this is no big deal, but I noticed that get_merge_bases() here leaks. Shouldn't it call free_commit_list() like e.g. diff_get_merge_base() which invokes get_merge_bases() does on the return value?
quoted hunk ↗ jump to hunk
+test_description='remerge-diff handling' + +. ./test-lib.sh + +test_expect_success 'setup basic merges' ' + test_write_lines 1 2 3 4 5 6 7 8 9 >numbers && + git add numbers && + git commit -m base && + + git branch feature_a && + git branch feature_b && + git branch feature_c && + + git branch ab_resolution && + git branch bc_resolution && + + git checkout feature_a && + test_write_lines 1 2 three 4 5 6 7 eight 9 >numbers && + git commit -a -m change_a && + + git checkout feature_b && + test_write_lines 1 2 tres 4 5 6 7 8 9 >numbers && + git commit -a -m change_b && + + git checkout feature_c && + test_write_lines 1 2 3 4 5 6 7 8 9 10 >numbers && + git commit -a -m change_c && + + git checkout bc_resolution && + git merge --ff-only feature_b && + # no conflict + git merge feature_c && + + git checkout ab_resolution && + git merge --ff-only feature_a && + # conflicts! + test_must_fail git merge feature_b && + # Resolve conflict...and make another change elsewhere + test_write_lines 1 2 drei 4 5 6 7 acht 9 >numbers && + git add numbers &&
Just a matter of taste, but FWIW some of the custom test_write_lines/commit here could nowadays use test_commit with --printf: 47c88d16ba6 (test-lib functions: add --printf option to test_commit, 2021-05-10) I don't think it's worth the churn to change it here, just an FYI.