Changes since v1: * Renamed --mergeability-only flag to --dry-run, as per suggestion from Junio * added some commit message clarifications This adds a new flag, --dry-run, to git merge-tree, which suppresses all output and leaves only the exit status (reflecting successful merge or conflict). This is useful for Git Forges in cases where they are only interested in whether two branches can be merged, without needing the actual merge result or conflict details. The advantage of the flag is two fold: * The merge machinery can exit once it detects a conflict, instead of continuing to compute merge result information * The merge machinery can avoid writing merged blobs and trees to the object store when in the outer layer of the merging process (more details in the first commit message). Elijah Newren (2): merge-ort: add a new mergeability_only option merge-tree: add a new --dry-run flag Documentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 22 ++++++++++++++++++ merge-ort.c | 38 +++++++++++++++++++++++++------ merge-ort.h | 1 + t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 7 deletions(-) base-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1920%2Fnewren%2Fmergeability-only-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1920/newren/mergeability-only-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1920 Range-diff vs v1: 1: 09292804cff ! 1: 4757c4810d3 merge-ort: add a new mergeability_only option @@ Commit message Git Forges may be interested in whether two branches can be merged while not being interested in what the resulting merge tree is nor which files conflicted. For such cases, add a new mergeability_only option. This - option allows the merge machinery to, in the outer layer of the merge: - * exit upon first conflict - * avoid writing merged blobs/trees to the object store + option allows the merge machinery to, in the "outer layer" of the merge: + * exit upon first[-ish] conflict + * avoid (not prevent) writing merged blobs/trees to the object store + + I have a number of qualifiers there, so let me explain each: + + "outer layer": Note that since the recursive merge of merge bases (corresponding to - call_depth > 0) can conflict without the outer final merge (corresponding - to call_depth == 0) conflicting, we can't short-circuit nor avoid - writing merges blobs/trees to the object store during those inner - merges. + call_depth > 0) can conflict without the outer final merge + (corresponding to call_depth == 0) conflicting, we can't short-circuit + nor avoid writing merged blobs/trees to the object store during those + inner merges. + + "first-ish conflict": + + The current patch only exits early from process_entries() on the first + conflict it detects, but conflicts could have been detected in a + previous function call, namely detect_and_process_renames(). However: + * conflicts detected by detect_and_process_renames() are quite rare + conflict types + * the detection would still come after regular rename detection + (which is the expensive part of detect_and_process_renames()), so + it is not saving us much in computation time given that + process_entries() directly follows detect_and_process_renames() + * [this overlaps with the next bullet point] process_entries() is the + place where virtually all object writing occurs (object writing is + sometimes more of a concern for Forges than computation time), so + exiting early here isn't saving us much in object writes either + * the code changes needed to handle an earlier exit are slightly + more invasive in detect_and_process_renames() than for + process_entries(). + Given the rareness of the even earlier conflicts, the limited savings + we'd get from exiting even earlier, and in an attempt to keep this + patch simpler, we don't guarantee that we actually exit on the first + conflict detected. We can always revisit this decision later if we + decide that a further micro-optimization to exit slightly earlier in + rare cases is worthwhile. + + "avoid (not prevent) writing objects": - There is a further potential micro-optimization here. rename/rename - conflicts have the potential for nested conflicts even without recursive - merges; because of that, handle_content_merge() can be called multiple - times and is done via different paths. Currently, we only exit early in - process_entries(), which is where the final handle_content_merge() is - invoked. Since rename/rename conflicts have an additional earlier - handle_content_merge() call that can be invoked from - detect_and_process_renames() (via process_renames()), we could - potentially exit earlier at that call point. However, rename/rename - conflicts are exceptionally rare, and feeding the extra logic through - didn't seem worth it. (And, if we don't exit early at that point, then - any resulting blobs need to be written to the store so that subsequent - handle_content_merge() calls trying to use the blob don't throw - exceptions.) + The detect_and_process_renames() call can also write objects to the + object store, when rename/rename conflicts involve one (or more) files + that have also been modified on both sides. Because of this alternate + call path leading to handle_content_merges(), our "early exit" does not + prevent writing objects entirely, even within the "outer layer" + (i.e. even within call_depth == 0). I figure that's fine though, since + we're already writing objects for the inner merges (i.e. for call_depth + > 0), which are likely going to represent vastly more objects than files + involved in rename/rename+modify/modify cases in the outer merge, on + average. Signed-off-by: Elijah Newren <newren@xxxxxxxxx> 2: 0b32c80d286 ! 2: 1d18ab7feb8 merge-tree: add a new --mergeability-only flag @@ Metadata Author: Elijah Newren <newren@xxxxxxxxx> ## Commit message ## - merge-tree: add a new --mergeability-only flag + merge-tree: add a new --dry-run flag Git Forges may be interested in whether two branches can be merged while not being interested in what the resulting merge tree is nor which files - conflicted. For such cases, add a new --mergeability-only flag which + conflicted. For such cases, add a new --dry-run flag which will make use of the new mergeability_only flag added to merge-ort in the previous commit. This option allows the merge machinery to, in the outer layer of the merge: - * exit upon first conflict - * avoid writing merged blobs/trees to the object store + * exit early when a conflict is detected + * avoid writing (most) merged blobs/trees to the object store Signed-off-by: Elijah Newren <newren@xxxxxxxxx> @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, int original_argc; const char *merge_base = NULL; int ret; -+ int mergeability_only = 0; ++ int dry_run = 0; const char * const merge_tree_usage[] = { N_("git merge-tree [--write-tree] [<options>] <branch1> <branch2>"), @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, N_("do a trivial merge only"), MODE_TRIVIAL), OPT_BOOL(0, "messages", &o.show_messages, N_("also show informational/conflict messages")), -+ OPT_BOOL_F(0, "mergeability-only", -+ &mergeability_only, ++ OPT_BOOL_F(0, "dry-run", ++ &dry_run, + N_("suppress all output; only exit status wanted"), + PARSE_OPT_NONEG), OPT_SET_INT('z', NULL, &line_termination, @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, argc = parse_options(argc, argv, prefix, mt_options, merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION); -+ if (mergeability_only && o.show_messages == -1) ++ if (dry_run && o.show_messages == -1) + o.show_messages = 0; -+ o.merge_options.mergeability_only = mergeability_only; -+ die_for_incompatible_opt2(mergeability_only, "--mergeability-only", ++ o.merge_options.mergeability_only = dry_run; ++ die_for_incompatible_opt2(dry_run, "--mergeability-only", + o.show_messages, "--messages"); -+ die_for_incompatible_opt2(mergeability_only, "--mergeability-only", ++ die_for_incompatible_opt2(dry_run, "--mergeability-only", + o.name_only, "--name-only"); -+ die_for_incompatible_opt2(mergeability_only, "--mergeability-only", ++ die_for_incompatible_opt2(dry_run, "--mergeability-only", + o.use_stdin, "--stdin"); -+ die_for_incompatible_opt2(mergeability_only, "--mergeability-only", ++ die_for_incompatible_opt2(dry_run, "--mergeability-only", + !line_termination, "-z"); + if (xopts.nr && o.mode == MODE_TRIVIAL) @@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup ' git commit -m first-commit ' -+test_expect_success '--mergeability-only on clean merge' ' ++test_expect_success '--dry-run on clean merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && @@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup ' + test_cmp expect actual && + + # Ensure merge is successful (exit code of 0) -+ git merge-tree --write-tree --mergeability-only side1 side3 >output && ++ git merge-tree --write-tree --dry-run side1 side3 >output && + + # Ensure there is no output + test_must_be_empty output && @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Failed merge without rena grep "CONFLICT (modify/delete): numbers deleted" out ' -+test_expect_success '--mergeability-only on conflicted merge' ' ++test_expect_success '--dry-run on conflicted merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Failed merge without rena + test_cmp expect actual && + + # Ensure merge has conflict -+ test_expect_code 1 git merge-tree --write-tree --mergeability-only side1 side2 >output && ++ test_expect_code 1 git merge-tree --write-tree --dry-run side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output && -- gitgitgadget
