Skip to content

Commit c6d5ca1

Browse files
newrengitster
authored andcommitted
merge-ort: add a new mergeability_only option
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 > 0), which are likely going to represent vastly more objects than files involved in rename/rename+modify/modify cases in the outer merge, on average. Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 7a1d2bd commit c6d5ca1

File tree

2 files changed

+32
-7
lines changed

2 files changed

+32
-7
lines changed

merge-ort.c

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2127,6 +2127,7 @@ static int handle_content_merge(struct merge_options *opt,
21272127
const struct version_info *b,
21282128
const char *pathnames[3],
21292129
const int extra_marker_size,
2130+
const int record_object,
21302131
struct version_info *result)
21312132
{
21322133
/*
@@ -2214,7 +2215,7 @@ static int handle_content_merge(struct merge_options *opt,
22142215
ret = -1;
22152216
}
22162217

2217-
if (!ret &&
2218+
if (!ret && record_object &&
22182219
write_object_file(result_buf.ptr, result_buf.size,
22192220
OBJ_BLOB, &result->oid)) {
22202221
path_msg(opt, ERROR_OBJECT_WRITE_FAILED, 0,
@@ -2897,6 +2898,7 @@ static int process_renames(struct merge_options *opt,
28972898
struct version_info merged;
28982899
struct conflict_info *base, *side1, *side2;
28992900
unsigned was_binary_blob = 0;
2901+
const int record_object = true;
29002902

29012903
pathnames[0] = oldpath;
29022904
pathnames[1] = newpath;
@@ -2947,6 +2949,7 @@ static int process_renames(struct merge_options *opt,
29472949
&side2->stages[2],
29482950
pathnames,
29492951
1 + 2 * opt->priv->call_depth,
2952+
record_object,
29502953
&merged);
29512954
if (clean_merge < 0)
29522955
return -1;
@@ -3061,6 +3064,7 @@ static int process_renames(struct merge_options *opt,
30613064

30623065
struct conflict_info *base, *side1, *side2;
30633066
int clean;
3067+
const int record_object = true;
30643068

30653069
pathnames[0] = oldpath;
30663070
pathnames[other_source_index] = oldpath;
@@ -3080,6 +3084,7 @@ static int process_renames(struct merge_options *opt,
30803084
&side2->stages[2],
30813085
pathnames,
30823086
1 + 2 * opt->priv->call_depth,
3087+
record_object,
30833088
&merged);
30843089
if (clean < 0)
30853090
return -1;
@@ -3931,9 +3936,12 @@ static int write_completed_directory(struct merge_options *opt,
39313936
* Write out the tree to the git object directory, and also
39323937
* record the mode and oid in dir_info->result.
39333938
*/
3939+
int record_tree = (!opt->mergeability_only ||
3940+
opt->priv->call_depth);
39343941
dir_info->is_null = 0;
39353942
dir_info->result.mode = S_IFDIR;
3936-
if (write_tree(&dir_info->result.oid, &info->versions, offset,
3943+
if (record_tree &&
3944+
write_tree(&dir_info->result.oid, &info->versions, offset,
39373945
opt->repo->hash_algo->rawsz) < 0)
39383946
ret = -1;
39393947
}
@@ -4231,10 +4239,13 @@ static int process_entry(struct merge_options *opt,
42314239
struct version_info *o = &ci->stages[0];
42324240
struct version_info *a = &ci->stages[1];
42334241
struct version_info *b = &ci->stages[2];
4242+
int record_object = (!opt->mergeability_only ||
4243+
opt->priv->call_depth);
42344244

42354245
clean_merge = handle_content_merge(opt, path, o, a, b,
42364246
ci->pathnames,
42374247
opt->priv->call_depth * 2,
4248+
record_object,
42384249
&merged_file);
42394250
if (clean_merge < 0)
42404251
return -1;
@@ -4395,6 +4406,8 @@ static int process_entries(struct merge_options *opt,
43954406
STRING_LIST_INIT_NODUP,
43964407
NULL, 0 };
43974408
int ret = 0;
4409+
const int record_tree = (!opt->mergeability_only ||
4410+
opt->priv->call_depth);
43984411

43994412
trace2_region_enter("merge", "process_entries setup", opt->repo);
44004413
if (strmap_empty(&opt->priv->paths)) {
@@ -4454,6 +4467,12 @@ static int process_entries(struct merge_options *opt,
44544467
ret = -1;
44554468
goto cleanup;
44564469
};
4470+
if (!ci->merged.clean && opt->mergeability_only &&
4471+
!opt->priv->call_depth) {
4472+
ret = 0;
4473+
goto cleanup;
4474+
}
4475+
44574476
}
44584477
}
44594478
trace2_region_leave("merge", "processing", opt->repo);
@@ -4468,7 +4487,8 @@ static int process_entries(struct merge_options *opt,
44684487
fflush(stdout);
44694488
BUG("dir_metadata accounting completely off; shouldn't happen");
44704489
}
4471-
if (write_tree(result_oid, &dir_metadata.versions, 0,
4490+
if (record_tree &&
4491+
write_tree(result_oid, &dir_metadata.versions, 0,
44724492
opt->repo->hash_algo->rawsz) < 0)
44734493
ret = -1;
44744494
cleanup:
@@ -4715,6 +4735,8 @@ void merge_display_update_messages(struct merge_options *opt,
47154735

47164736
if (opt->record_conflict_msgs_as_headers)
47174737
BUG("Either display conflict messages or record them as headers, not both");
4738+
if (opt->mergeability_only)
4739+
BUG("Displaying conflict messages incompatible with mergeability-only checks");
47184740

47194741
trace2_region_enter("merge", "display messages", opt->repo);
47204742

@@ -5171,10 +5193,12 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
51715193
result->path_messages = &opt->priv->conflicts;
51725194

51735195
if (result->clean >= 0) {
5174-
result->tree = parse_tree_indirect(&working_tree_oid);
5175-
if (!result->tree)
5176-
die(_("unable to read tree (%s)"),
5177-
oid_to_hex(&working_tree_oid));
5196+
if (!opt->mergeability_only) {
5197+
result->tree = parse_tree_indirect(&working_tree_oid);
5198+
if (!result->tree)
5199+
die(_("unable to read tree (%s)"),
5200+
oid_to_hex(&working_tree_oid));
5201+
}
51785202
/* existence of conflicted entries implies unclean */
51795203
result->clean &= strmap_empty(&opt->priv->conflicted);
51805204
}

merge-ort.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ struct merge_options {
8383
/* miscellaneous control options */
8484
const char *subtree_shift;
8585
unsigned renormalize : 1;
86+
unsigned mergeability_only : 1; /* exit early, write fewer objects */
8687
unsigned record_conflict_msgs_as_headers : 1;
8788
const char *msg_header_prefix;
8889

0 commit comments

Comments
 (0)