merge-ort: use STABLE_QSORT instead of QSORT where required

rename/rename conflict handling depends on the fact that if both sides renamed the same path, that the one on the MERGE_SIDE1 will appear first in the combined diff_queue_struct passed to process_renames(). Since we add all pairs from MERGE_SIDE1 to combined first, and then all pairs from MERGE_SIDE2, and then sort based on filename, this will only be true if the sort used is stable. This was found due to the fact that Mac, unlike Linux, apparently has a system-defined qsort that is not stable. While we are at it, review the other callers of QSORT and add comments about why they can remain as calls to QSORT instead of being modified to call STABLE_QSORT. Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Mar 20, 2021 at 00:03 UTC 72b3091040f8d2784225527ebecfc3c2a56fe196
1 file changed +7 -1
merge-ort.c
+7 -1
@@ -2405,7 +2405,7 @@ static int detect_and_process_renames(struct merge_options *opt,
2405 clean &= collect_renames(opt, &combined, MERGE_SIDE2,
2406 &renames->dir_renames[1],
2407 &renames->dir_renames[2]);
2408 - QSORT(combined.queue, combined.nr, compare_pairs);
2408 + STABLE_QSORT(combined.queue, combined.nr, compare_pairs);
2409 trace2_region_leave("merge", "directory renames", opt->repo);
2410
2411 trace2_region_enter("merge", "process renames", opt->repo);
@@ -2550,6 +2550,7 @@ static void write_tree(struct object_id *result_oid,
2550 */
2551 relevant_entries.items = versions->items + offset;
2552 relevant_entries.nr = versions->nr - offset;
2553 + /* No need for STABLE_QSORT -- filenames must be unique */
2554 QSORT(relevant_entries.items, relevant_entries.nr, tree_entry_order);
2555
2556 /* Pre-allocate some space in buf */
@@ -3325,6 +3326,11 @@ static int record_conflicted_index_entries(struct merge_options *opt,
3326 * entries we added to the end into their right locations.
3327 */
3328 remove_marked_cache_entries(index, 1);
3329 + /*
3330 + * No need for STABLE_QSORT -- cmp_cache_name_compare sorts primarily
3331 + * on filename and secondarily on stage, and (name, stage #) are a
3332 + * unique tuple.
3333 + */
3334 QSORT(index->cache, index->cache_nr, cmp_cache_name_compare);
3335
3336 return errs;