@matheusbsbs / matheus-bagliano / commits / c5a6f65527

merge-ort: add modify/delete handling and delayed output processing

The focus here is on adding a path_msg() which will queue up warning/conflict/notice messages about the merge for later processing, storing these in a pathname -> strbuf map. It might seem like a big change, but it really just is: * declaration of necessary map with some comments * initialization and recording of data * a bunch of code to iterate over the map at print/free time * at least one caller in order to avoid an error about having an unused function (which we provide in the form of implementing modify/delete conflict handling). At this stage, it is probably not clear why I am opting for delayed output processing. There are multiple reasons: 1. Merges are supposed to abort if they would overwrite dirty changes in the working tree. We cannot correctly determine whether changes would be overwritten until both rename detection has occurred and full processing of entries with the renames has finalized. Warning/conflict/notice messages come up at intermediate codepaths along the way, so unless we want spurious conflict/warning messages being printed when the merge will be aborted anyway, we need to save these messages and only print them when relevant. 2. There can be multiple messages for a single path, and we want all messages for a give path to appear together instead of having them grouped by conflict/warning type. This was a problem already with merge-recursive.c but became even more important due to the splitting apart of conflict types as discussed in the commit message for 1f3c9ba707 ("t6425: be more flexible with rename/delete conflict messages", 2020-08-10) 3. Some callers might want to avoid showing the output in certain cases, such as if the end result is a clean merge. Rebases have typically done this. 4. Some callers might not want the output to go to stdout or even stderr, but might want to do something else with it entirely. For example, a --remerge-diff option to `git show` or `git log -p` that remerges on the fly and diffs merge commits against the remerged version would benefit from stdout/stderr not being written to in the standard form. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Dec 3, 2020 at 15:59 UTC c5a6f65527aa3b6f5d7cf25437a88d8727ab0646

1 file changed +98 -2

merge-ort.c

+98 -2

@@ -104,6 +104,15 @@ struct merge_options_internal {

104 */

105 struct string_list paths_to_free;

106

107 + /*

108 + * output: special messages and conflict notices for various paths

109 + *

110 + * This is a map of pathnames (a subset of the keys in "paths" above)

111 + * to strbufs. It gathers various warning/conflict/notice messages

112 + * for later processing.

113 + */

114 + struct strmap output;

115 +

116 /*

117 * current_dir_name: temporary var used in collect_merge_info_callback()

118 *

@@ -274,6 +283,27 @@ static void clear_internal_opts(struct merge_options_internal *opti,

283 opti->paths_to_free.strdup_strings = 1;

284 string_list_clear(&opti->paths_to_free, 0);

285 opti->paths_to_free.strdup_strings = 0;

286 +

287 + if (!reinitialize) {

288 + struct hashmap_iter iter;

289 + struct strmap_entry *e;

290 +

291 + /* Release and free each strbuf found in output */

292 + strmap_for_each_entry(&opti->output, &iter, e) {

293 + struct strbuf *sb = e->value;

294 + strbuf_release(sb);

295 + /*

296 + * While strictly speaking we don't need to free(sb)

297 + * here because we could pass free_values=1 when

298 + * calling strmap_clear() on opti->output, that would

299 + * require strmap_clear to do another

300 + * strmap_for_each_entry() loop, so we just free it

301 + * while we're iterating anyway.

302 + */

303 + free(sb);

304 + }

305 + strmap_clear(&opti->output, 0);

306 + }

307 }

308

309 static int err(struct merge_options *opt, const char *err, ...)

@@ -292,6 +322,27 @@ static int err(struct merge_options *opt, const char *err, ...)

322 return -1;

323 }

324

325 +__attribute__((format (printf, 4, 5)))

326 +static void path_msg(struct merge_options *opt,

327 + const char *path,

328 + int omittable_hint, /* skippable under --remerge-diff */

329 + const char *fmt, ...)

330 +{

331 + va_list ap;

332 + struct strbuf *sb = strmap_get(&opt->priv->output, path);

333 + if (!sb) {

334 + sb = xmalloc(sizeof(*sb));

335 + strbuf_init(sb, 0);

336 + strmap_put(&opt->priv->output, path, sb);

337 + }

338 +

339 + va_start(ap, fmt);

340 + strbuf_vaddf(sb, fmt, ap);

341 + va_end(ap);

342 +

343 + strbuf_addch(sb, '\n');

344 +}

345 +

346 /*** Function Grouping: functions related to collect_merge_info() ***/

347

348 static void setup_path_info(struct merge_options *opt,

@@ -973,7 +1024,23 @@ static void process_entry(struct merge_options *opt,

1024 (void)handle_content_merge;

1025 } else if (ci->filemask == 3 || ci->filemask == 5) {

1026 /* Modify/delete */

976 - die("Not yet implemented.");

1027 + const char *modify_branch, *delete_branch;

1028 + int side = (ci->filemask == 5) ? 2 : 1;

1029 + int index = opt->priv->call_depth ? 0 : side;

1030 +

1031 + ci->merged.result.mode = ci->stages[index].mode;

1032 + oidcpy(&ci->merged.result.oid, &ci->stages[index].oid);

1033 + ci->merged.clean = 0;

1034 +

1035 + modify_branch = (side == 1) ? opt->branch1 : opt->branch2;

1036 + delete_branch = (side == 1) ? opt->branch2 : opt->branch1;

1037 +

1038 + path_msg(opt, path, 0,

1039 + _("CONFLICT (modify/delete): %s deleted in %s "

1040 + "and modified in %s. Version %s of %s left "

1041 + "in tree."),

1042 + path, delete_branch, modify_branch,

1043 + modify_branch, path);

1044 } else if (ci->filemask == 2 || ci->filemask == 4) {

1045 /* Added on one side */

1046 int side = (ci->filemask == 4) ? 2 : 1;

@@ -1240,7 +1307,29 @@ void merge_switch_to_result(struct merge_options *opt,

1307 }

1308

1309 if (display_update_msgs) {

1243 - /* TODO: print out CONFLICT and other informational messages. */

1310 + struct merge_options_internal *opti = result->priv;

1311 + struct hashmap_iter iter;

1312 + struct strmap_entry *e;

1313 + struct string_list olist = STRING_LIST_INIT_NODUP;

1314 + int i;

1315 +

1316 + /* Hack to pre-allocate olist to the desired size */

1317 + ALLOC_GROW(olist.items, strmap_get_size(&opti->output),

1318 + olist.alloc);

1319 +

1320 + /* Put every entry from output into olist, then sort */

1321 + strmap_for_each_entry(&opti->output, &iter, e) {

1322 + string_list_append(&olist, e->key)->util = e->value;

1323 + }

1324 + string_list_sort(&olist);

1325 +

1326 + /* Iterate over the items, printing them */

1327 + for (i = 0; i < olist.nr; ++i) {

1328 + struct strbuf *sb = olist.items[i].util;

1329 +

1330 + printf("%s", sb->buf);

1331 + }

1332 + string_list_clear(&olist, 0);

1333 }

1334

1335 merge_finalize(opt, result);

@@ -1307,6 +1396,13 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)

1396 strmap_init_with_options(&opt->priv->paths, NULL, 0);

1397 strmap_init_with_options(&opt->priv->conflicted, NULL, 0);

1398 string_list_init(&opt->priv->paths_to_free, 0);

1399 +

1400 + /*

1401 + * keys & strbufs in output will sometimes need to outlive "paths",

1402 + * so it will have a copy of relevant keys. It's probably a small

1403 + * subset of the overall paths that have special output.

1404 + */

1405 + strmap_init(&opt->priv->output);

1406 }

1407

1408 /*** Function Grouping: merge_incore_*() and their internal variants ***/