merge-tree: add new --mergeability-only option
cover merge-tree: add new --mergeability-only option
This adds a new flag, --mergeability-only, 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 the first 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 --mergeability-only 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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1920/newren/mergeability-only-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1920
Signature · 1 line
gitgitgadget
PATCH 1/2 merge-ort: add a new mergeability_only option
From: Elijah Newren <[email protected]>
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
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.
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.)
--- merge-ort.c | 38 +++++++++++++++++++++++++++++++------- merge-ort.h | 1 + 2 files changed, 32 insertions(+), 7 deletions(-)
merge-ort.c +31 -7
@@ -2127,6 +2127,7 @@ static int handle_content_merge(struct merge_options *opt,const struct version_info *b, const char *pathnames[3], const int extra_marker_size, + const int record_object, struct version_info *result) {/*
@@ -2214,7 +2215,7 @@ static int handle_content_merge(struct merge_options *opt, ret =-1 ; } -if (!ret && +if (!ret && record_object &&write_object_file (result_buf.ptr, result_buf.size,OBJ_BLOB , &result->oid)) { path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
@@ -2897,6 +2898,7 @@ static int process_renames(struct merge_options *opt,struct version_info merged;struct conflict_info *base, *side1, *side2;unsigned was_binary_blob =0 ; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[1 ] = newpath;
@@ -2947,6 +2949,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean_merge <0 )return -1 ;
@@ -3061,6 +3064,7 @@ static int process_renames(struct merge_options *opt,struct conflict_info *base, *side1, *side2;int clean; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[other_source_index] = oldpath;
@@ -3080,6 +3084,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean <0 )return -1 ;
@@ -3931,9 +3936,12 @@ static int write_completed_directory(struct merge_options *opt,* Write out the tree to the git object directory, and also * record the mode and oid in dir_info->result. */ +int record_tree = (!opt->mergeability_only || + opt->priv->call_depth); dir_info->is_null =0 ; dir_info->result.mode =S_IFDIR ; -if (write_tree (&dir_info->result.oid, &info->versions, offset, +if (record_tree && +write_tree (&dir_info->result.oid, &info->versions, offset, opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; }
@@ -4231,10 +4239,13 @@ static int process_entry(struct merge_options *opt,struct version_info *o = &ci->stages[0 ];struct version_info *a = &ci->stages[1 ];struct version_info *b = &ci->stages[2 ]; +int record_object = (!opt->mergeability_only || + opt->priv->call_depth); clean_merge =handle_content_merge (opt, path, o, a, b, ci->pathnames, opt->priv->call_depth *2 , + record_object, &merged_file);if (clean_merge <0 )return -1 ;
@@ -4395,6 +4406,8 @@ static int process_entries(struct merge_options *opt,STRING_LIST_INIT_NODUP , NULL, 0 };int ret =0 ; +const int record_tree = (!opt->mergeability_only || + opt->priv->call_depth);trace2_region_enter ("merge" ,"process_entries setup" , opt->repo);if (strmap_empty (&opt->priv->paths)) {
@@ -4454,6 +4467,12 @@ static int process_entries(struct merge_options *opt, ret =-1 ; goto cleanup; }; +if (!ci->merged.clean && opt->mergeability_only && + !opt->priv->call_depth) { + ret =0 ; + goto cleanup; + } + } }trace2_region_leave ("merge" ,"processing" , opt->repo);
@@ -4468,7 +4487,8 @@ static int process_entries(struct merge_options *opt,fflush (stdout);BUG ("dir_metadata accounting completely off; shouldn't happen" ); } -if (write_tree (result_oid, &dir_metadata.versions,0 , +if (record_tree && +write_tree (result_oid, &dir_metadata.versions,0 , opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; cleanup:
@@ -4715,6 +4735,8 @@ void merge_display_update_messages(struct merge_options *opt,if (opt->record_conflict_msgs_as_headers)BUG ("Either display conflict messages or record them as headers, not both" ); +if (opt->mergeability_only) +BUG ("Displaying conflict messages incompatible with mergeability-only checks" );trace2_region_enter ("merge" ,"display messages" , opt->repo);
@@ -5171,10 +5193,12 @@ redo: result->path_messages = &opt->priv->conflicts;if (result->clean >=0 ) { - result->tree =parse_tree_indirect (&working_tree_oid); -if (!result->tree) -die (_ ("unable to read tree (%s)" ), -oid_to_hex (&working_tree_oid)); +if (!opt->mergeability_only) { + result->tree =parse_tree_indirect (&working_tree_oid); +if (!result->tree) +die (_ ("unable to read tree (%s)" ), +oid_to_hex (&working_tree_oid)); + }/* existence of conflicted entries implies unclean */ result->clean &=strmap_empty (&opt->priv->conflicted); }
merge-ort.h +1 -0
@@ -83,6 +83,7 @@ struct merge_options {/* miscellaneous control options */ const char *subtree_shift; unsigned renormalize : 1; + unsigned mergeability_only : 1; /* exit early, write fewer objects */ unsigned record_conflict_msgs_as_headers : 1;const char *msg_header_prefix;
Signature · 1 line
gitgitgadget
PATCH 2/2 merge-tree: add a new --mergeability-only flag
In reply to Elijah Newren via GitGitGadgetFrom: Elijah Newren <[email protected]>
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 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
--- Documentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 22 ++++++++++++++++++ t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)
Documentation/git-merge-tree.adoc +6 -0
@@ -65,6 +65,12 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. +--mergeability-only:: + Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early on the first conflict it finds, and allows it to avoid + writing most objects created by merges. + --allow-unrelated-histories:: merge-tree will by default error out if the two branches specified share no common history. This flag can be given to override that
builtin/merge-tree.c +22 -0
@@ -490,6 +490,9 @@ static int real_merge(struct merge_tree_options *o,if (result.clean <0 )die (_ ("failure to merge" )); +if (o->merge_options.mergeability_only) + goto cleanup; +if (show_messages ==-1 ) show_messages = !result.clean;
@@ -522,6 +525,8 @@ static int real_merge(struct merge_tree_options *o, }if (o->use_stdin)putchar (line_termination); + +cleanup:merge_finalize (&opt, &result);clear_merge_options (&opt);return !result.clean;/* result.clean < 0 handled above */
@@ -538,6 +543,7 @@ int cmd_merge_tree(int argc,int original_argc;const char *merge_base =NULL ;int ret; +int mergeability_only =0 ;const char *const merge_tree_usage[] = {N_ ("git merge-tree [--write-tree] [<options>] <branch1> <branch2>" ),
@@ -552,6 +558,10 @@ 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, +N_ ("suppress all output; only exit status wanted" ), +PARSE_OPT_NONEG ),OPT_SET_INT ('z' ,NULL , &line_termination,N_ ("separate paths with the NUL character" ),'\0' ), OPT_BOOL_F(0, "name-only",
@@ -583,6 +593,18 @@ 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 ) + o.show_messages =0 ; + o.merge_options.mergeability_only = mergeability_only; +die_for_incompatible_opt2 (mergeability_only,"--mergeability-only" , + o.show_messages,"--messages" ); +die_for_incompatible_opt2 (mergeability_only,"--mergeability-only" , + o.name_only,"--name-only" ); +die_for_incompatible_opt2 (mergeability_only,"--mergeability-only" , + o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (mergeability_only,"--mergeability-only" , + !line_termination,"-z" ); +if (xopts.nr && o.mode ==MODE_TRIVIAL )die (_ ("--trivial-merge is incompatible with all other options" ));for (size_t x =0 ; x < xopts.nr; x++)
t/t4301-merge-tree-write-tree.sh +38 -0
@@ -54,6 +54,25 @@ test_expect_success setup ' git commit -m first-commit ' +test_expect_success '--mergeability-only on clean merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge is successful (exit code of 0) + git merge-tree --write-tree --mergeability-only side1 side3 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Clean merge' ' TREE_OID=$(git merge-tree --write-tree side1 side3) && q_to_tab <<-EOF >expect &&
@@ -72,6 +91,25 @@ test_expect_success 'Failed merge without rename detection' ' grep "CONFLICT (modify/delete): numbers deleted" out ' +test_expect_success '--mergeability-only on conflicted merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge has conflict + test_expect_code 1 git merge-tree --write-tree --mergeability-only side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Content merge and a few conflicts' ' git checkout side1^0 && test_must_fail git merge side2 &&
Signature · 1 line
gitgitgadget
Quoted 5 lines · Elijah Newren via GitGitGadget
This adds a new flag, --mergeability-only, 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.
Sounds useful, but wouldn't that usually called --dry-run?
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren via GitGitGadget
This adds a new flag, --mergeability-only, 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.
Sounds useful, but wouldn't that usually called --dry-run?
I thought about that, but I was worried that folks would expect "--dry-run" to not make any changes. This mode does not prevent writing objects to the object store, it merely avoids it in the "outer layer" of the merge. More precisely, objects will still be written to the object store for the merging of merge bases, and also be written to the object store in the case of rename/rename conflicts if the contents of the files involved in the conflicting renames were also modified by both sides.
Quoted 4 lines · Elijah Newren
I thought about that, but I was worried that folks would expect "--dry-run" to not make any changes. This mode does not prevent writing objects to the object store, it merely avoids it in the "outer layer" of the merge.
I think we have already precedence to call something that creates new objects in the object database, as long as the resulting objects are not made reachable ("git fetch --dry-run" probably falls into that category). The idea is that it does not make a change that is "observable" by end-users (and what "gc" sees is not part of what the users would be observaing).
We have "--check" (in "git apply"), which is an exact counterpart in the patch based workflow to this thing. It reads
Instead of applying the patch, see if the patch is applicable to the current working tree and/or the index file and detects errors. Turns off "apply".
I feel that apply --check should have been apply --dry-run, so I would not recommend calling it --check for merge-tree, though.
Thanks.
Quoted 8 lines · Junio C Hamano
Quoted 4 lines · Elijah Newren
I thought about that, but I was worried that folks would expect "--dry-run" to not make any changes. This mode does not prevent writing objects to the object store, it merely avoids it in the "outer layer" of the merge.
Quoted 6 lines
I think we have already precedence to call something that creates new objects in the object database, as long as the resulting objects are not made reachable ("git fetch --dry-run" probably falls into that category). The idea is that it does not make a change that is "observable" by end-users (and what "gc" sees is not part of what the users would be observaing).
Oh, I was unaware of git fetch --dry-run for some reason. And its documentation even states "without making any changes" despite the fact that it downloads more objects to the object store, so it indeed sounds like a good precedent.
I'll switch the flag name to --dry-run. (I have a suspicion, however, that the primary users of this new merge-tree flag will care about whether objects are created, so I still want the documentation to call it out, unlike git fetch's --dry-run option.)
Quoted 9 lines
We have "--check" (in "git apply"), which is an exact counterpart in the patch based workflow to this thing. It reads
Instead of applying the patch, see if the patch is applicable to the current working tree and/or the index file and detects errors. Turns off "apply".
I feel that
apply --checkshould have beenapply --dry-run, so I would not recommend calling it--checkformerge-tree, though.
Makes sense; thanks for the pointers.
v2 cover merge-tree: add new --dry-run option
In reply to Elijah Newren via GitGitGadgetChanges 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.
2: 0b32c80d286 ! 2: 1d18ab7feb8 merge-tree: add a new --mergeability-only flag
@@ Metadata
Author: Elijah Newren <[email protected]>
## 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 <[email protected]>
@@ 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 &&Signature · 1 line
gitgitgadget
PATCH v2 1/2 merge-ort: add a new mergeability_only option
From: Elijah Newren <[email protected]>
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[-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 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":
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
Quoted 1 line
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.
--- merge-ort.c | 38 +++++++++++++++++++++++++++++++------- merge-ort.h | 1 + 2 files changed, 32 insertions(+), 7 deletions(-)
merge-ort.c +31 -7
@@ -2127,6 +2127,7 @@ static int handle_content_merge(struct merge_options *opt,const struct version_info *b, const char *pathnames[3], const int extra_marker_size, + const int record_object, struct version_info *result) {/*
@@ -2214,7 +2215,7 @@ static int handle_content_merge(struct merge_options *opt, ret =-1 ; } -if (!ret && +if (!ret && record_object &&write_object_file (result_buf.ptr, result_buf.size,OBJ_BLOB , &result->oid)) { path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
@@ -2897,6 +2898,7 @@ static int process_renames(struct merge_options *opt,struct version_info merged;struct conflict_info *base, *side1, *side2;unsigned was_binary_blob =0 ; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[1 ] = newpath;
@@ -2947,6 +2949,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean_merge <0 )return -1 ;
@@ -3061,6 +3064,7 @@ static int process_renames(struct merge_options *opt,struct conflict_info *base, *side1, *side2;int clean; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[other_source_index] = oldpath;
@@ -3080,6 +3084,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean <0 )return -1 ;
@@ -3931,9 +3936,12 @@ static int write_completed_directory(struct merge_options *opt,* Write out the tree to the git object directory, and also * record the mode and oid in dir_info->result. */ +int record_tree = (!opt->mergeability_only || + opt->priv->call_depth); dir_info->is_null =0 ; dir_info->result.mode =S_IFDIR ; -if (write_tree (&dir_info->result.oid, &info->versions, offset, +if (record_tree && +write_tree (&dir_info->result.oid, &info->versions, offset, opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; }
@@ -4231,10 +4239,13 @@ static int process_entry(struct merge_options *opt,struct version_info *o = &ci->stages[0 ];struct version_info *a = &ci->stages[1 ];struct version_info *b = &ci->stages[2 ]; +int record_object = (!opt->mergeability_only || + opt->priv->call_depth); clean_merge =handle_content_merge (opt, path, o, a, b, ci->pathnames, opt->priv->call_depth *2 , + record_object, &merged_file);if (clean_merge <0 )return -1 ;
@@ -4395,6 +4406,8 @@ static int process_entries(struct merge_options *opt,STRING_LIST_INIT_NODUP , NULL, 0 };int ret =0 ; +const int record_tree = (!opt->mergeability_only || + opt->priv->call_depth);trace2_region_enter ("merge" ,"process_entries setup" , opt->repo);if (strmap_empty (&opt->priv->paths)) {
@@ -4454,6 +4467,12 @@ static int process_entries(struct merge_options *opt, ret =-1 ; goto cleanup; }; +if (!ci->merged.clean && opt->mergeability_only && + !opt->priv->call_depth) { + ret =0 ; + goto cleanup; + } + } }trace2_region_leave ("merge" ,"processing" , opt->repo);
@@ -4468,7 +4487,8 @@ static int process_entries(struct merge_options *opt,fflush (stdout);BUG ("dir_metadata accounting completely off; shouldn't happen" ); } -if (write_tree (result_oid, &dir_metadata.versions,0 , +if (record_tree && +write_tree (result_oid, &dir_metadata.versions,0 , opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; cleanup:
@@ -4715,6 +4735,8 @@ void merge_display_update_messages(struct merge_options *opt,if (opt->record_conflict_msgs_as_headers)BUG ("Either display conflict messages or record them as headers, not both" ); +if (opt->mergeability_only) +BUG ("Displaying conflict messages incompatible with mergeability-only checks" );trace2_region_enter ("merge" ,"display messages" , opt->repo);
@@ -5171,10 +5193,12 @@ redo: result->path_messages = &opt->priv->conflicts;if (result->clean >=0 ) { - result->tree =parse_tree_indirect (&working_tree_oid); -if (!result->tree) -die (_ ("unable to read tree (%s)" ), -oid_to_hex (&working_tree_oid)); +if (!opt->mergeability_only) { + result->tree =parse_tree_indirect (&working_tree_oid); +if (!result->tree) +die (_ ("unable to read tree (%s)" ), +oid_to_hex (&working_tree_oid)); + }/* existence of conflicted entries implies unclean */ result->clean &=strmap_empty (&opt->priv->conflicted); }
merge-ort.h +1 -0
@@ -83,6 +83,7 @@ struct merge_options {/* miscellaneous control options */ const char *subtree_shift; unsigned renormalize : 1; + unsigned mergeability_only : 1; /* exit early, write fewer objects */ unsigned record_conflict_msgs_as_headers : 1;const char *msg_header_prefix;
Signature · 1 line
gitgitgadget
PATCH v2 2/2 merge-tree: add a new --dry-run flag
In reply to Elijah Newren via GitGitGadgetFrom: Elijah Newren <[email protected]>
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 --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 early when a conflict is detected
* avoid writing (most) merged blobs/trees to the object store
--- Documentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 22 ++++++++++++++++++ t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)
Documentation/git-merge-tree.adoc +6 -0
@@ -65,6 +65,12 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. +--mergeability-only:: + Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early on the first conflict it finds, and allows it to avoid + writing most objects created by merges. + --allow-unrelated-histories:: merge-tree will by default error out if the two branches specified share no common history. This flag can be given to override that
builtin/merge-tree.c +22 -0
@@ -490,6 +490,9 @@ static int real_merge(struct merge_tree_options *o,if (result.clean <0 )die (_ ("failure to merge" )); +if (o->merge_options.mergeability_only) + goto cleanup; +if (show_messages ==-1 ) show_messages = !result.clean;
@@ -522,6 +525,8 @@ static int real_merge(struct merge_tree_options *o, }if (o->use_stdin)putchar (line_termination); + +cleanup:merge_finalize (&opt, &result);clear_merge_options (&opt);return !result.clean;/* result.clean < 0 handled above */
@@ -538,6 +543,7 @@ int cmd_merge_tree(int argc,int original_argc;const char *merge_base =NULL ;int ret; +int dry_run =0 ;const char *const merge_tree_usage[] = {N_ ("git merge-tree [--write-tree] [<options>] <branch1> <branch2>" ),
@@ -552,6 +558,10 @@ 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 ,"dry-run" , + &dry_run, +N_ ("suppress all output; only exit status wanted" ), +PARSE_OPT_NONEG ),OPT_SET_INT ('z' ,NULL , &line_termination,N_ ("separate paths with the NUL character" ),'\0' ), OPT_BOOL_F(0, "name-only",
@@ -583,6 +593,18 @@ int cmd_merge_tree(int argc, argc =parse_options (argc, argv, prefix, mt_options, merge_tree_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +if (dry_run && o.show_messages ==-1 ) + o.show_messages =0 ; + o.merge_options.mergeability_only = dry_run; +die_for_incompatible_opt2 (dry_run,"--mergeability-only" , + o.show_messages,"--messages" ); +die_for_incompatible_opt2 (dry_run,"--mergeability-only" , + o.name_only,"--name-only" ); +die_for_incompatible_opt2 (dry_run,"--mergeability-only" , + o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (dry_run,"--mergeability-only" , + !line_termination,"-z" ); +if (xopts.nr && o.mode ==MODE_TRIVIAL )die (_ ("--trivial-merge is incompatible with all other options" ));for (size_t x =0 ; x < xopts.nr; x++)
t/t4301-merge-tree-write-tree.sh +38 -0
@@ -54,6 +54,25 @@ test_expect_success setup ' git commit -m first-commit ' +test_expect_success '--dry-run on clean merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge is successful (exit code of 0) + git merge-tree --write-tree --dry-run side1 side3 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Clean merge' ' TREE_OID=$(git merge-tree --write-tree side1 side3) && q_to_tab <<-EOF >expect &&
@@ -72,6 +91,25 @@ test_expect_success 'Failed merge without rename detection' ' grep "CONFLICT (modify/delete): numbers deleted" out ' +test_expect_success '--dry-run on conflicted merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge has conflict + test_expect_code 1 git merge-tree --write-tree --dry-run side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Content merge and a few conflicts' ' git checkout side1^0 && test_must_fail git merge side2 &&
Signature · 1 line
gitgitgadget
Re: [PATCH v2 2/2] merge-tree: add a new --dry-run flag
Documentation/git-merge-tree.adoc OPTIONS · line 72
Quoted 6 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
+--mergeability-only::Documentation/git-merge-tree.adoc +4
-0+ Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early on the first conflict it finds, and allows it to avoid + writing most objects created by merges.
The previous name is being used here.
Signature · 1 line
Kristoffer Haugsbakk
Re: [PATCH v2 2/2] merge-tree: add a new --dry-run flag
In reply to Elijah Newren via GitGitGadgetQuoted 7 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
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 --dry-run flag which will make use of the new
mergeability_onlyflag added to merge-ort in the previous commit. This option allows the merge machinery to, in the
The first three lines are almost identical to [1/2] here, modulo the internal name still being mergeability-only while the external name is now dry-run, which is perfectly fine as long as that is done consistently.
Documentation/git-merge-tree.adoc OPTIONS · line 68
Quoted 9 lines
Documentation/git-merge-tree.adoc +1
-0@@ -65,6 +65,12 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. +--mergeability-only::
But is this internal or external name?
Documentation/git-merge-tree.adoc OPTIONS · line 72
Quoted 4 lines
Documentation/git-merge-tree.adoc +4
-0+ Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early on the first conflict it finds, and allows it to avoid + writing most objects created by merges.
OK.
Re: [PATCH v2 2/2] merge-tree: add a new --dry-run flag
In reply to Kristoffer HaugsbakkQuoted 10 lines · Kristoffer Haugsbakk
Documentation/git-merge-tree.adoc OPTIONS · line 72
Quoted 6 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
+--mergeability-only::Documentation/git-merge-tree.adoc +4
-0+ Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early on the first conflict it finds, and allows it to avoid + writing most objects created by merges.The previous name is being used here.
Oops; thanks for catching that. I'll fix it up.
Re: [PATCH v2 2/2] merge-tree: add a new --dry-run flag
In reply to Junio C HamanoQuoted 24 lines · Junio C Hamano
Quoted 7 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
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 --dry-run flag which will make use of the new
mergeability_onlyflag added to merge-ort in the previous commit. This option allows the merge machinery to, in theThe first three lines are almost identical to [1/2] here, modulo the internal name still being mergeability-only while the external name is now dry-run, which is perfectly fine as long as that is done consistently.
Quoted 8 lines
Documentation/git-merge-tree.adoc +0
-0@@ -65,6 +65,12 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise.
Quoted 3 lines
Documentation/git-merge-tree.adoc OPTIONS · line 68
Quoted 1 line
Documentation/git-merge-tree.adoc +1
-0+--mergeability-only::But is this internal or external name?
external; Kristoffer caught this oversight too. I'll send a re-roll with this fixed. (I apparently also missed it in the die-if-incompatible-options too, not sure how I missed all of these, but I'll ensure they're all fixed up).
v3 cover merge-tree: add new --dry-run option
In reply to Elijah Newren via GitGitGadgetChanges since v2:
* Converted locations missed in v1 in changing --mergeability-only -> --dry-run
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-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1920/newren/mergeability-only-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1920
Range-diff vs v2:
1: 4757c4810d3 = 1: 4757c4810d3 merge-ort: add a new mergeability_only option
2: 1d18ab7feb8 ! 2: f11824317a8 merge-tree: add a new --dry-run flag
@@ Documentation/git-merge-tree.adoc: OPTIONS
default is to include these messages if there are merge
conflicts, and to omit them otherwise.
-+--mergeability-only::
++--dry-run::
+ Disable all output from the program. Useful when you are only
+ interested in the exit status. Allows merge-tree to exit
-+ early on the first conflict it finds, and allows it to avoid
-+ writing most objects created by merges.
++ early when it finds a conflict, and allows it to avoid writing
++ most objects created by merges.
+
--allow-unrelated-histories::
merge-tree will by default error out if the two branches specified
@@ builtin/merge-tree.c: int cmd_merge_tree(int argc,
+ if (dry_run && o.show_messages == -1)
+ o.show_messages = 0;
+ o.merge_options.mergeability_only = dry_run;
-+ die_for_incompatible_opt2(dry_run, "--mergeability-only",
++ die_for_incompatible_opt2(dry_run, "--dry-run",
+ o.show_messages, "--messages");
-+ die_for_incompatible_opt2(dry_run, "--mergeability-only",
++ die_for_incompatible_opt2(dry_run, "--dry-run",
+ o.name_only, "--name-only");
-+ die_for_incompatible_opt2(dry_run, "--mergeability-only",
++ die_for_incompatible_opt2(dry_run, "--dry-run",
+ o.use_stdin, "--stdin");
-+ die_for_incompatible_opt2(dry_run, "--mergeability-only",
++ die_for_incompatible_opt2(dry_run, "--dry-run",
+ !line_termination, "-z");
+
if (xopts.nr && o.mode == MODE_TRIVIAL)Signature · 1 line
gitgitgadget
PATCH v3 1/2 merge-ort: add a new mergeability_only option
From: Elijah Newren <[email protected]>
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[-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 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":
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
Quoted 1 line
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.
--- merge-ort.c | 38 +++++++++++++++++++++++++++++++------- merge-ort.h | 1 + 2 files changed, 32 insertions(+), 7 deletions(-)
merge-ort.c +31 -7
@@ -2127,6 +2127,7 @@ static int handle_content_merge(struct merge_options *opt,const struct version_info *b, const char *pathnames[3], const int extra_marker_size, + const int record_object, struct version_info *result) {/*
@@ -2214,7 +2215,7 @@ static int handle_content_merge(struct merge_options *opt, ret =-1 ; } -if (!ret && +if (!ret && record_object &&write_object_file (result_buf.ptr, result_buf.size,OBJ_BLOB , &result->oid)) { path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
@@ -2897,6 +2898,7 @@ static int process_renames(struct merge_options *opt,struct version_info merged;struct conflict_info *base, *side1, *side2;unsigned was_binary_blob =0 ; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[1 ] = newpath;
@@ -2947,6 +2949,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean_merge <0 )return -1 ;
@@ -3061,6 +3064,7 @@ static int process_renames(struct merge_options *opt,struct conflict_info *base, *side1, *side2;int clean; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[other_source_index] = oldpath;
@@ -3080,6 +3084,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean <0 )return -1 ;
@@ -3931,9 +3936,12 @@ static int write_completed_directory(struct merge_options *opt,* Write out the tree to the git object directory, and also * record the mode and oid in dir_info->result. */ +int record_tree = (!opt->mergeability_only || + opt->priv->call_depth); dir_info->is_null =0 ; dir_info->result.mode =S_IFDIR ; -if (write_tree (&dir_info->result.oid, &info->versions, offset, +if (record_tree && +write_tree (&dir_info->result.oid, &info->versions, offset, opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; }
@@ -4231,10 +4239,13 @@ static int process_entry(struct merge_options *opt,struct version_info *o = &ci->stages[0 ];struct version_info *a = &ci->stages[1 ];struct version_info *b = &ci->stages[2 ]; +int record_object = (!opt->mergeability_only || + opt->priv->call_depth); clean_merge =handle_content_merge (opt, path, o, a, b, ci->pathnames, opt->priv->call_depth *2 , + record_object, &merged_file);if (clean_merge <0 )return -1 ;
@@ -4395,6 +4406,8 @@ static int process_entries(struct merge_options *opt,STRING_LIST_INIT_NODUP , NULL, 0 };int ret =0 ; +const int record_tree = (!opt->mergeability_only || + opt->priv->call_depth);trace2_region_enter ("merge" ,"process_entries setup" , opt->repo);if (strmap_empty (&opt->priv->paths)) {
@@ -4454,6 +4467,12 @@ static int process_entries(struct merge_options *opt, ret =-1 ; goto cleanup; }; +if (!ci->merged.clean && opt->mergeability_only && + !opt->priv->call_depth) { + ret =0 ; + goto cleanup; + } + } }trace2_region_leave ("merge" ,"processing" , opt->repo);
@@ -4468,7 +4487,8 @@ static int process_entries(struct merge_options *opt,fflush (stdout);BUG ("dir_metadata accounting completely off; shouldn't happen" ); } -if (write_tree (result_oid, &dir_metadata.versions,0 , +if (record_tree && +write_tree (result_oid, &dir_metadata.versions,0 , opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; cleanup:
@@ -4715,6 +4735,8 @@ void merge_display_update_messages(struct merge_options *opt,if (opt->record_conflict_msgs_as_headers)BUG ("Either display conflict messages or record them as headers, not both" ); +if (opt->mergeability_only) +BUG ("Displaying conflict messages incompatible with mergeability-only checks" );trace2_region_enter ("merge" ,"display messages" , opt->repo);
@@ -5171,10 +5193,12 @@ redo: result->path_messages = &opt->priv->conflicts;if (result->clean >=0 ) { - result->tree =parse_tree_indirect (&working_tree_oid); -if (!result->tree) -die (_ ("unable to read tree (%s)" ), -oid_to_hex (&working_tree_oid)); +if (!opt->mergeability_only) { + result->tree =parse_tree_indirect (&working_tree_oid); +if (!result->tree) +die (_ ("unable to read tree (%s)" ), +oid_to_hex (&working_tree_oid)); + }/* existence of conflicted entries implies unclean */ result->clean &=strmap_empty (&opt->priv->conflicted); }
merge-ort.h +1 -0
@@ -83,6 +83,7 @@ struct merge_options {/* miscellaneous control options */ const char *subtree_shift; unsigned renormalize : 1; + unsigned mergeability_only : 1; /* exit early, write fewer objects */ unsigned record_conflict_msgs_as_headers : 1;const char *msg_header_prefix;
Signature · 1 line
gitgitgadget
PATCH v3 2/2 merge-tree: add a new --dry-run flag
In reply to Elijah Newren via GitGitGadgetFrom: Elijah Newren <[email protected]>
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 --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 early when a conflict is detected
* avoid writing (most) merged blobs/trees to the object store
--- Documentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 22 ++++++++++++++++++ t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)
Documentation/git-merge-tree.adoc +6 -0
@@ -65,6 +65,12 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. +--dry-run:: + Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early when it finds a conflict, and allows it to avoid writing + most objects created by merges. + --allow-unrelated-histories:: merge-tree will by default error out if the two branches specified share no common history. This flag can be given to override that
builtin/merge-tree.c +22 -0
@@ -490,6 +490,9 @@ static int real_merge(struct merge_tree_options *o,if (result.clean <0 )die (_ ("failure to merge" )); +if (o->merge_options.mergeability_only) + goto cleanup; +if (show_messages ==-1 ) show_messages = !result.clean;
@@ -522,6 +525,8 @@ static int real_merge(struct merge_tree_options *o, }if (o->use_stdin)putchar (line_termination); + +cleanup:merge_finalize (&opt, &result);clear_merge_options (&opt);return !result.clean;/* result.clean < 0 handled above */
@@ -538,6 +543,7 @@ int cmd_merge_tree(int argc,int original_argc;const char *merge_base =NULL ;int ret; +int dry_run =0 ;const char *const merge_tree_usage[] = {N_ ("git merge-tree [--write-tree] [<options>] <branch1> <branch2>" ),
@@ -552,6 +558,10 @@ 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 ,"dry-run" , + &dry_run, +N_ ("suppress all output; only exit status wanted" ), +PARSE_OPT_NONEG ),OPT_SET_INT ('z' ,NULL , &line_termination,N_ ("separate paths with the NUL character" ),'\0' ), OPT_BOOL_F(0, "name-only",
@@ -583,6 +593,18 @@ int cmd_merge_tree(int argc, argc =parse_options (argc, argv, prefix, mt_options, merge_tree_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +if (dry_run && o.show_messages ==-1 ) + o.show_messages =0 ; + o.merge_options.mergeability_only = dry_run; +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.show_messages,"--messages" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.name_only,"--name-only" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + !line_termination,"-z" ); +if (xopts.nr && o.mode ==MODE_TRIVIAL )die (_ ("--trivial-merge is incompatible with all other options" ));for (size_t x =0 ; x < xopts.nr; x++)
t/t4301-merge-tree-write-tree.sh +38 -0
@@ -54,6 +54,25 @@ test_expect_success setup ' git commit -m first-commit ' +test_expect_success '--dry-run on clean merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge is successful (exit code of 0) + git merge-tree --write-tree --dry-run side1 side3 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Clean merge' ' TREE_OID=$(git merge-tree --write-tree side1 side3) && q_to_tab <<-EOF >expect &&
@@ -72,6 +91,25 @@ test_expect_success 'Failed merge without rename detection' ' grep "CONFLICT (modify/delete): numbers deleted" out ' +test_expect_success '--dry-run on conflicted merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge has conflict + test_expect_code 1 git merge-tree --write-tree --dry-run side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Content merge and a few conflicts' ' git checkout side1^0 && test_must_fail git merge side2 &&
Signature · 1 line
gitgitgadget
Re: [PATCH v2 2/2] merge-tree: add a new --dry-run flag
In reply to Elijah NewrenQuoted 4 lines · Elijah Newren
external; Kristoffer caught this oversight too. I'll send a re-roll with this fixed. (I apparently also missed it in the die-if-incompatible-options too, not sure how I missed all of these, but I'll ensure they're all fixed up).
Thanks. Will hold.
Re: [PATCH v3 0/2] merge-tree: add new --dry-run option
In reply to Elijah Newren via GitGitGadgetQuoted 25 lines · Elijah Newren via GitGitGadget
Changes since v2:
* Converted locations missed in v1 in changing --mergeability-only -> --dry-run
Changes since v1:
* Renamed --mergeability-only flag to --dry-run, as per suggestion from Junio
* added some commit message clarificationsThis 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).
Quoted 3 lines
Elijah Newren (2):
merge-ort: add a newmergeability_onlyoption
merge-tree: add a new --dry-run flag
All I can say is that this looks good considering the comments on v2.
Interdiff:
``` diff --git a/Documentation/git-merge-tree.adoc b/Documentation/git-merge-tree.adoc index 7dcc1780619..74716b91019 100644 --- a/Documentation/git-merge-tree.adoc +++ b/Documentation/git-merge-tree.adoc @@ -65,11 +65,11 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. ---mergeability-only:: +--dry-run:: Disable all output from the program. Useful when you are only interested in the exit status. Allows merge-tree to exit - early on the first conflict it finds, and allows it to avoid - writing most objects created by merges. + early when it finds a conflict, and allows it to avoid writing + most objects created by merges. --allow-unrelated-histories:: merge-tree will by default error out if the two branches specified diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index 579e81d5184..273ec171e98 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -596,13 +596,13 @@ int cmd_merge_tree(int argc, if (dry_run && o.show_messages == -1) o.show_messages = 0; o.merge_options.mergeability_only = dry_run; - die_for_incompatible_opt2(dry_run, "--mergeability-only", + die_for_incompatible_opt2(dry_run, "--dry-run", o.show_messages, "--messages"); - die_for_incompatible_opt2(dry_run, "--mergeability-only", + die_for_incompatible_opt2(dry_run, "--dry-run", o.name_only, "--name-only"); - die_for_incompatible_opt2(dry_run, "--mergeability-only", + die_for_incompatible_opt2(dry_run, "--dry-run", o.use_stdin, "--stdin"); - die_for_incompatible_opt2(dry_run, "--mergeability-only", + die_for_incompatible_opt2(dry_run, "--dry-run", !line_termination, "-z"); if (xopts.nr && o.mode == MODE_TRIVIAL) ```
Re: [PATCH v3 2/2] merge-tree: add a new --dry-run flag
In reply to Elijah Newren via GitGitGadgetDocumentation/git-merge-tree.adoc OPTIONS · line 73
Quoted 6 lines · Elijah Newren via GitGitGadget
+--dry-run::
Documentation/git-merge-tree.adoc +5
-0+ Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early when it finds a conflict, and allows it to avoid writing + most objects created by merges. +
OK.
Quoted 5 lines
builtin/merge-tree.c +1
-0@@ -583,6 +593,18 @@ int cmd_merge_tree(int argc, argc =parse_options (argc, argv, prefix, mt_options, merge_tree_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +if (dry_run && o.show_messages ==-1 )
builtin/merge-tree.c int cmd_merge_tree(int argc, · line 606
Quoted 10 lines
builtin/merge-tree.c +10
-0+ o.show_messages =0 ; + o.merge_options.mergeability_only = dry_run; +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.show_messages,"--messages" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.name_only,"--name-only" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + !line_termination,"-z" );
Ah, I missed this one. Would have been caught in the previous round if we had test coverage for incompatible options.
This round looks quite sensible to me. Queued. Thanks.
Re: [PATCH v3 2/2] merge-tree: add a new --dry-run flag
In reply to Elijah Newren via GitGitGadgetHi Elijah
Documentation/git-merge-tree.adoc OPTIONS · line 72
Quoted 7 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
+--dry-run::
Documentation/git-merge-tree.adoc +4
-0+ Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early when it finds a conflict, and allows it to avoid writing + most objects created by merges.
I don't want to bike-shed but to me this feels more like "git diff --quiet" than "git push --dry-run" or "git send-email --dry-run" which still print diagnostic messages.
Best Wishes
Phillip
Quoted 120 lines
--allow-unrelated-histories::
merge-tree will by default error out if the two branches specified share no common history. This flag can be given to override thatbuiltin/merge-tree.c +22
-0@@ -490,6 +490,9 @@ static int real_merge(struct merge_tree_options *o,if (result.clean <0 )die (_ ("failure to merge" )); +if (o->merge_options.mergeability_only) + goto cleanup; +if (show_messages ==-1 ) show_messages = !result.clean;@@ -522,6 +525,8 @@ static int real_merge(struct merge_tree_options *o, }if (o->use_stdin)putchar (line_termination); + +cleanup:merge_finalize (&opt, &result);clear_merge_options (&opt);return !result.clean;/* result.clean < 0 handled above */ @@ -538,6 +543,7 @@ int cmd_merge_tree(int argc,int original_argc;const char *merge_base =NULL ;int ret; +int dry_run =0 ;const char *const merge_tree_usage[] = {N_ ("git merge-tree [--write-tree] [<options>] <branch1> <branch2>" ),@@ -552,6 +558,10 @@ 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 ,"dry-run" , + &dry_run, +N_ ("suppress all output; only exit status wanted" ), +PARSE_OPT_NONEG ),OPT_SET_INT ('z' ,NULL , &line_termination,N_ ("separate paths with the NUL character" ),'\0' ), OPT_BOOL_F(0, "name-only",@@ -583,6 +593,18 @@ int cmd_merge_tree(int argc, argc =parse_options (argc, argv, prefix, mt_options, merge_tree_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +if (dry_run && o.show_messages ==-1 ) + o.show_messages =0 ; + o.merge_options.mergeability_only = dry_run; +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.show_messages,"--messages" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.name_only,"--name-only" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (dry_run,"--dry-run" , + !line_termination,"-z" ); +if (xopts.nr && o.mode ==MODE_TRIVIAL )die (_ ("--trivial-merge is incompatible with all other options" ));for (size_t x =0 ; x < xopts.nr; x++)t/t4301-merge-tree-write-tree.sh +38
-0@@ -54,6 +54,25 @@ test_expect_success setup ' git commit -m first-commit ' +test_expect_success '--dry-run on clean merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge is successful (exit code of 0) + git merge-tree --write-tree --dry-run side1 side3 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Clean merge' ' TREE_OID=$(git merge-tree --write-tree side1 side3) && q_to_tab <<-EOF >expect &&@@ -72,6 +91,25 @@ test_expect_success 'Failed merge without rename detection' ' grep "CONFLICT (modify/delete): numbers deleted" out ' +test_expect_success '--dry-run on conflicted merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge has conflict + test_expect_code 1 git merge-tree --write-tree --dry-run side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Content merge and a few conflicts' ' git checkout side1^0 && test_must_fail git merge side2 &&
Re: [PATCH v3 2/2] merge-tree: add a new --dry-run flag
Quoted 12 lines · Phillip Wood
Hi Elijah
Documentation/git-merge-tree.adoc OPTIONS · line 72
Quoted 7 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
+--dry-run::
Documentation/git-merge-tree.adoc +4
-0+ Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early when it finds a conflict, and allows it to avoid writing + most objects created by merges.
Quoted 7 lines
I don't want to bike-shed but to me this feels more like "git diff --quiet" than "git push --dry-run" or "git send-email --dry-run" which still print diagnostic messages.
Best Wishes
Phillip
--quiet seems fine to me. I'll make the change and then expect someone to comment on it with an even better name... ;-)
v4 cover merge-tree: add new --quiet option
In reply to Elijah Newren via GitGitGadgetChanges since v3:
* Renamed --dry-run -> --quiet . Any further naming suggestions?
Changes since v2:
* Converted locations missed in v1 in changing --mergeability-only -> --dry-run
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 --quiet flag
Documentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 18 +++++++++++++++ merge-ort.c | 38 +++++++++++++++++++++++++------ merge-ort.h | 1 + t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 7 deletions(-)
base-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1920%2Fnewren%2Fmergeability-only-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1920/newren/mergeability-only-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1920
Range-diff vs v3:
1: 4757c4810d3 = 1: 4757c4810d3 merge-ort: add a new mergeability_only option
2: f11824317a8 ! 2: 7c40d3c9216 merge-tree: add a new --dry-run flag
@@ Metadata
Author: Elijah Newren <[email protected]>
## Commit message ##
- merge-tree: add a new --dry-run flag
+ merge-tree: add a new --quiet 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 --dry-run flag which
+ conflicted. For such cases, add a new --quiet 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:
@@ Documentation/git-merge-tree.adoc: OPTIONS
default is to include these messages if there are merge
conflicts, and to omit them otherwise.
-+--dry-run::
++--quiet::
+ Disable all output from the program. Useful when you are only
+ interested in the exit status. Allows merge-tree to exit
+ early when it finds a conflict, and allows it to avoid writing
@@ builtin/merge-tree.c: int cmd_merge_tree(int argc,
int original_argc;
const char *merge_base = NULL;
int ret;
-+ int dry_run = 0;
++ int quiet = 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, "dry-run",
-+ &dry_run,
++ OPT_BOOL_F(0, "quiet",
++ &quiet,
+ 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 (dry_run && o.show_messages == -1)
++ if (quiet && o.show_messages == -1)
+ o.show_messages = 0;
-+ o.merge_options.mergeability_only = dry_run;
-+ die_for_incompatible_opt2(dry_run, "--dry-run",
-+ o.show_messages, "--messages");
-+ die_for_incompatible_opt2(dry_run, "--dry-run",
-+ o.name_only, "--name-only");
-+ die_for_incompatible_opt2(dry_run, "--dry-run",
-+ o.use_stdin, "--stdin");
-+ die_for_incompatible_opt2(dry_run, "--dry-run",
-+ !line_termination, "-z");
++ o.merge_options.mergeability_only = quiet;
++ die_for_incompatible_opt2(quiet, "--quiet", o.show_messages, "--messages");
++ die_for_incompatible_opt2(quiet, "--quiet", o.name_only, "--name-only");
++ die_for_incompatible_opt2(quiet, "--quiet", o.use_stdin, "--stdin");
++ die_for_incompatible_opt2(quiet, "--quiet", !line_termination, "-z");
+
if (xopts.nr && o.mode == MODE_TRIVIAL)
die(_("--trivial-merge is incompatible with all other options"));
@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
git commit -m first-commit
'
-+test_expect_success '--dry-run on clean merge' '
++test_expect_success '--quiet 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 --dry-run side1 side3 >output &&
++ git merge-tree --write-tree --quiet 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 '--dry-run on conflicted merge' '
++test_expect_success '--quiet 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 --dry-run side1 side2 >output &&
++ test_expect_code 1 git merge-tree --write-tree --quiet side1 side2 >output &&
+
+ # Ensure there is no output
+ test_must_be_empty output &&Signature · 1 line
gitgitgadget
PATCH v4 1/2 merge-ort: add a new mergeability_only option
From: Elijah Newren <[email protected]>
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[-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 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":
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
Quoted 1 line
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.
--- merge-ort.c | 38 +++++++++++++++++++++++++++++++------- merge-ort.h | 1 + 2 files changed, 32 insertions(+), 7 deletions(-)
merge-ort.c +31 -7
@@ -2127,6 +2127,7 @@ static int handle_content_merge(struct merge_options *opt,const struct version_info *b, const char *pathnames[3], const int extra_marker_size, + const int record_object, struct version_info *result) {/*
@@ -2214,7 +2215,7 @@ static int handle_content_merge(struct merge_options *opt, ret =-1 ; } -if (!ret && +if (!ret && record_object &&write_object_file (result_buf.ptr, result_buf.size,OBJ_BLOB , &result->oid)) { path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
@@ -2897,6 +2898,7 @@ static int process_renames(struct merge_options *opt,struct version_info merged;struct conflict_info *base, *side1, *side2;unsigned was_binary_blob =0 ; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[1 ] = newpath;
@@ -2947,6 +2949,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean_merge <0 )return -1 ;
@@ -3061,6 +3064,7 @@ static int process_renames(struct merge_options *opt,struct conflict_info *base, *side1, *side2;int clean; +const int record_object = true; pathnames[0 ] = oldpath; pathnames[other_source_index] = oldpath;
@@ -3080,6 +3084,7 @@ static int process_renames(struct merge_options *opt, &side2->stages[2 ], pathnames,1 +2 * opt->priv->call_depth, + record_object, &merged);if (clean <0 )return -1 ;
@@ -3931,9 +3936,12 @@ static int write_completed_directory(struct merge_options *opt,* Write out the tree to the git object directory, and also * record the mode and oid in dir_info->result. */ +int record_tree = (!opt->mergeability_only || + opt->priv->call_depth); dir_info->is_null =0 ; dir_info->result.mode =S_IFDIR ; -if (write_tree (&dir_info->result.oid, &info->versions, offset, +if (record_tree && +write_tree (&dir_info->result.oid, &info->versions, offset, opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; }
@@ -4231,10 +4239,13 @@ static int process_entry(struct merge_options *opt,struct version_info *o = &ci->stages[0 ];struct version_info *a = &ci->stages[1 ];struct version_info *b = &ci->stages[2 ]; +int record_object = (!opt->mergeability_only || + opt->priv->call_depth); clean_merge =handle_content_merge (opt, path, o, a, b, ci->pathnames, opt->priv->call_depth *2 , + record_object, &merged_file);if (clean_merge <0 )return -1 ;
@@ -4395,6 +4406,8 @@ static int process_entries(struct merge_options *opt,STRING_LIST_INIT_NODUP , NULL, 0 };int ret =0 ; +const int record_tree = (!opt->mergeability_only || + opt->priv->call_depth);trace2_region_enter ("merge" ,"process_entries setup" , opt->repo);if (strmap_empty (&opt->priv->paths)) {
@@ -4454,6 +4467,12 @@ static int process_entries(struct merge_options *opt, ret =-1 ; goto cleanup; }; +if (!ci->merged.clean && opt->mergeability_only && + !opt->priv->call_depth) { + ret =0 ; + goto cleanup; + } + } }trace2_region_leave ("merge" ,"processing" , opt->repo);
@@ -4468,7 +4487,8 @@ static int process_entries(struct merge_options *opt,fflush (stdout);BUG ("dir_metadata accounting completely off; shouldn't happen" ); } -if (write_tree (result_oid, &dir_metadata.versions,0 , +if (record_tree && +write_tree (result_oid, &dir_metadata.versions,0 , opt->repo->hash_algo->rawsz) <0 ) ret =-1 ; cleanup:
@@ -4715,6 +4735,8 @@ void merge_display_update_messages(struct merge_options *opt,if (opt->record_conflict_msgs_as_headers)BUG ("Either display conflict messages or record them as headers, not both" ); +if (opt->mergeability_only) +BUG ("Displaying conflict messages incompatible with mergeability-only checks" );trace2_region_enter ("merge" ,"display messages" , opt->repo);
@@ -5171,10 +5193,12 @@ redo: result->path_messages = &opt->priv->conflicts;if (result->clean >=0 ) { - result->tree =parse_tree_indirect (&working_tree_oid); -if (!result->tree) -die (_ ("unable to read tree (%s)" ), -oid_to_hex (&working_tree_oid)); +if (!opt->mergeability_only) { + result->tree =parse_tree_indirect (&working_tree_oid); +if (!result->tree) +die (_ ("unable to read tree (%s)" ), +oid_to_hex (&working_tree_oid)); + }/* existence of conflicted entries implies unclean */ result->clean &=strmap_empty (&opt->priv->conflicted); }
merge-ort.h +1 -0
@@ -83,6 +83,7 @@ struct merge_options {/* miscellaneous control options */ const char *subtree_shift; unsigned renormalize : 1; + unsigned mergeability_only : 1; /* exit early, write fewer objects */ unsigned record_conflict_msgs_as_headers : 1;const char *msg_header_prefix;
Signature · 1 line
gitgitgadget
PATCH v4 2/2 merge-tree: add a new --quiet flag
In reply to Elijah Newren via GitGitGadgetFrom: Elijah Newren <[email protected]>
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 --quiet 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 early when a conflict is detected
* avoid writing (most) merged blobs/trees to the object store
--- Documentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 18 +++++++++++++++ t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+)
Documentation/git-merge-tree.adoc +6 -0
@@ -65,6 +65,12 @@ OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. +--quiet:: + Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early when it finds a conflict, and allows it to avoid writing + most objects created by merges. + --allow-unrelated-histories:: merge-tree will by default error out if the two branches specified share no common history. This flag can be given to override that
builtin/merge-tree.c +18 -0
@@ -490,6 +490,9 @@ static int real_merge(struct merge_tree_options *o,if (result.clean <0 )die (_ ("failure to merge" )); +if (o->merge_options.mergeability_only) + goto cleanup; +if (show_messages ==-1 ) show_messages = !result.clean;
@@ -522,6 +525,8 @@ static int real_merge(struct merge_tree_options *o, }if (o->use_stdin)putchar (line_termination); + +cleanup:merge_finalize (&opt, &result);clear_merge_options (&opt);return !result.clean;/* result.clean < 0 handled above */
@@ -538,6 +543,7 @@ int cmd_merge_tree(int argc,int original_argc;const char *merge_base =NULL ;int ret; +int quiet =0 ;const char *const merge_tree_usage[] = {N_ ("git merge-tree [--write-tree] [<options>] <branch1> <branch2>" ),
@@ -552,6 +558,10 @@ 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 ,"quiet" , + &quiet, +N_ ("suppress all output; only exit status wanted" ), +PARSE_OPT_NONEG ),OPT_SET_INT ('z' ,NULL , &line_termination,N_ ("separate paths with the NUL character" ),'\0' ), OPT_BOOL_F(0, "name-only",
@@ -583,6 +593,14 @@ int cmd_merge_tree(int argc, argc =parse_options (argc, argv, prefix, mt_options, merge_tree_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +if (quiet && o.show_messages ==-1 ) + o.show_messages =0 ; + o.merge_options.mergeability_only = quiet; +die_for_incompatible_opt2 (quiet,"--quiet" , o.show_messages,"--messages" ); +die_for_incompatible_opt2 (quiet,"--quiet" , o.name_only,"--name-only" ); +die_for_incompatible_opt2 (quiet,"--quiet" , o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (quiet,"--quiet" , !line_termination,"-z" ); +if (xopts.nr && o.mode ==MODE_TRIVIAL )die (_ ("--trivial-merge is incompatible with all other options" ));for (size_t x =0 ; x < xopts.nr; x++)
t/t4301-merge-tree-write-tree.sh +38 -0
@@ -54,6 +54,25 @@ test_expect_success setup ' git commit -m first-commit ' +test_expect_success '--quiet on clean merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge is successful (exit code of 0) + git merge-tree --write-tree --quiet side1 side3 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Clean merge' ' TREE_OID=$(git merge-tree --write-tree side1 side3) && q_to_tab <<-EOF >expect &&
@@ -72,6 +91,25 @@ test_expect_success 'Failed merge without rename detection' ' grep "CONFLICT (modify/delete): numbers deleted" out ' +test_expect_success '--quiet on conflicted merge' ' + # Get rid of loose objects to start with + git gc && + echo "0 objects, 0 kilobytes" >expect && + git count-objects >actual && + test_cmp expect actual && + + # Ensure merge has conflict + test_expect_code 1 git merge-tree --write-tree --quiet side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output && + + # Ensure no loose objects written (all new objects written would have + # been in "outer layer" of the merge) + git count-objects >actual && + test_cmp expect actual +' + test_expect_success 'Content merge and a few conflicts' ' git checkout side1^0 && test_must_fail git merge side2 &&
Signature · 1 line
gitgitgadget
Re: [PATCH v4 2/2] merge-tree: add a new --quiet flag
builtin/merge-tree.c int cmd_merge_tree(int argc, · line 602
Quoted 8 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
builtin/merge-tree.c +7
-0+if (quiet && o.show_messages ==-1 ) + o.show_messages =0 ; + o.merge_options.mergeability_only = quiet; +die_for_incompatible_opt2 (quiet,"--quiet" , o.show_messages,"--messages" ); +die_for_incompatible_opt2 (quiet,"--quiet" , o.name_only,"--name-only" ); +die_for_incompatible_opt2 (quiet,"--quiet" , o.use_stdin,"--stdin" ); +die_for_incompatible_opt2 (quiet,"--quiet" , !line_termination,"-z" );
I’ve been using git-merge-tree(1) for some scripting but only today tried out --stdin for printing refs that conflict.
```
# Pipe in pairs
merge_pairs=$(mktemp)
tee $merge_pairs \
| git merge-tree --stdin --no-messages \
| tr '\0' '\n' \
| grep --extended-regexp '^(1|0)$' \
| paste -d' ' - $merge_pairs \
| grep '^0' \
| cut -d' ' -f2-
```(Previously I called the command in a loop)
I could imagine a --format option to just keep one of the arguments, which means the tee(1) (for cross-referencing the ref) and all the other things are gone:
``` git merge-tree --format='%(if)%(conflicted)%(then)oid2%(end)' --stdin ```
(But imagined options aside)
--stdin is presumably for efficiency and --quiet/--dry-run definitely is. But --quiet can only be used in the mode where you can only do a single merge, not in the --stdin batch mode.
--quiet/--dry-run with informational output (c.f. the above die-for-incompatible) would “break” the documented output format since conflicts haven’t been computed all the way and there are no OIDs for successful merges. But the user is opting into a new mode here, never seen before. Can’t they opt into a new informational mode where --stdin --quiet can co-exist? Then you can have dry-run batch mode.
Signature · 1 line
Kristoffer Haugsbakk
Re: [PATCH v4 2/2] merge-tree: add a new --quiet flag
Quoted 2 lines · Kristoffer Haugsbakk
Can’t they opt into a new informational mode where
--stdin --quietcan co-exist?
Yes, I now immediately see the contradiction in the literal text: “informational mode” and --quiet. But I shall not weigh in on the naming matter.
Signature · 1 line
Kristoffer Haugsbakk
Re: [PATCH v4 0/2] merge-tree: add new --quiet option
In reply to Elijah Newren via GitGitGadgetHi Elijah
Quoted 3 lines · Elijah Newren via GitGitGadget
Changes since v3:
* Renamed --dry-run -> --quiet . Any further naming suggestions?
Thanks for re-rolling this version looks good to me.
Thanks
Phillip
Quoted 150 lines
Changes since v2:
* Converted locations missed in v1 in changing --mergeability-only ->
--dry-runChanges since v1:
* Renamed --mergeability-only flag to --dry-run, as per suggestion from Junio
* added some commit message clarificationsThis 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 newmergeability_onlyoption
merge-tree: add a new --quiet flagDocumentation/git-merge-tree.adoc | 6 +++++ builtin/merge-tree.c | 18 +++++++++++++++ merge-ort.c | 38 +++++++++++++++++++++++++------ merge-ort.h | 1 + t/t4301-merge-tree-write-tree.sh | 38 +++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 7 deletions(-)base-commit:
6c0bd1fc70efaf053abe4e57c976afdc72d15377
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1920%2Fnewren%2Fmergeability-only-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1920/newren/mergeability-only-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1920Range-diff vs v3:
1: 4757c4810d3 = 1: 4757c4810d3 merge-ort: add a new mergeability_only option 2: f11824317a8 ! 2: 7c40d3c9216 merge-tree: add a new --dry-run flag @@ Metadata Author: Elijah Newren <[email protected]> ## Commit message ## - merge-tree: add a new --dry-run flag + merge-tree: add a new --quiet 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 --dry-run flag which + conflicted. For such cases, add a new --quiet 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: @@ Documentation/git-merge-tree.adoc: OPTIONS default is to include these messages if there are merge conflicts, and to omit them otherwise. -+--dry-run:: ++--quiet:: + Disable all output from the program. Useful when you are only + interested in the exit status. Allows merge-tree to exit + early when it finds a conflict, and allows it to avoid writing @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, int original_argc; const char *merge_base = NULL; int ret; -+ int dry_run = 0; ++ int quiet = 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, "dry-run", -+ &dry_run, ++ OPT_BOOL_F(0, "quiet", ++ &quiet, + 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 (dry_run && o.show_messages == -1) ++ if (quiet && o.show_messages == -1) + o.show_messages = 0; -+ o.merge_options.mergeability_only = dry_run; -+ die_for_incompatible_opt2(dry_run, "--dry-run", -+ o.show_messages, "--messages"); -+ die_for_incompatible_opt2(dry_run, "--dry-run", -+ o.name_only, "--name-only"); -+ die_for_incompatible_opt2(dry_run, "--dry-run", -+ o.use_stdin, "--stdin"); -+ die_for_incompatible_opt2(dry_run, "--dry-run", -+ !line_termination, "-z"); ++ o.merge_options.mergeability_only = quiet; ++ die_for_incompatible_opt2(quiet, "--quiet", o.show_messages, "--messages"); ++ die_for_incompatible_opt2(quiet, "--quiet", o.name_only, "--name-only"); ++ die_for_incompatible_opt2(quiet, "--quiet", o.use_stdin, "--stdin"); ++ die_for_incompatible_opt2(quiet, "--quiet", !line_termination, "-z"); + if (xopts.nr && o.mode == MODE_TRIVIAL) die(_("--trivial-merge is incompatible with all other options")); @@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup ' git commit -m first-commit ' -+test_expect_success '--dry-run on clean merge' ' ++test_expect_success '--quiet 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 --dry-run side1 side3 >output && ++ git merge-tree --write-tree --quiet 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 '--dry-run on conflicted merge' ' ++test_expect_success '--quiet 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 --dry-run side1 side2 >output && ++ test_expect_code 1 git merge-tree --write-tree --quiet side1 side2 >output && + + # Ensure there is no output + test_must_be_empty output &&
Re: [PATCH v4 0/2] merge-tree: add new --quiet option
Quoted 11 lines · Phillip Wood
Hi Elijah
Quoted 2 lines · Elijah Newren via GitGitGadget
Changes since v3:
* Renamed --dry-run -> --quiet . Any further naming suggestions?Thanks for re-rolling this version looks good to me.
Thanks
Phillip
Thanks, all. Let's mark it for 'next' and merge it down.