git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 01/11] merge-ort: use STABLE_QSORT instead of QSORT where required

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 5, 2021, 00:55 UTC
Message-ID
<3ca16a5e3466914c1c7efa79843395a4dfc3c3f1.1614905738.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.973.git.git.1614905738.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>

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>
---
 merge-ort.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/merge-ort.c b/merge-ort.c
index 603d30c52170..5309488fd9d8 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2289,7 +2289,7 @@ static int detect_and_process_renames(struct merge_options *opt,
 	clean &= collect_renames(opt, &combined, MERGE_SIDE2,
 				 &renames->dir_renames[1],
 				 &renames->dir_renames[2]);
-	QSORT(combined.queue, combined.nr, compare_pairs);
+	STABLE_QSORT(combined.queue, combined.nr, compare_pairs);
 	trace2_region_leave("merge", "directory renames", opt->repo);
 
 	trace2_region_enter("merge", "process renames", opt->repo);
@@ -2415,6 +2415,7 @@ static void write_tree(struct object_id *result_oid,
 	 */
 	relevant_entries.items = versions->items + offset;
 	relevant_entries.nr = versions->nr - offset;
+	/* No need for STABLE_QSORT -- filenames must be unique */
 	QSORT(relevant_entries.items, relevant_entries.nr, tree_entry_order);
 
 	/* Pre-allocate some space in buf */
@@ -3190,6 +3191,11 @@ static int record_conflicted_index_entries(struct merge_options *opt,
 	 * entries we added to the end into their right locations.
 	 */
 	remove_marked_cache_entries(index, 1);
+	/*
+	 * No need for STABLE_QSORT -- cmp_cache_name_compare sorts primarily
+	 * on filename and secondarily on stage, and (name, stage #) are a
+	 * unique tuple.
+	 */
 	QSORT(index->cache, index->cache_nr, cmp_cache_name_compare);
 
 	return errs;
-- 
gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 2 of 44 in “Complete merge-ort implementation...almost”
  1. 00/11 Complete merge-ort implementation...almostElijah Newren via GitGitGadget, Mar 5, 2021
  2. 01/11 merge-ort: use STABLE_QSORT instead of QSORT where requiredElijah Newren via GitGitGadget, Mar 5, 2021
  3. 02/11 merge-ort: add a special minimal index just for renormalizationElijah Newren via GitGitGadget, Mar 5, 2021
  4. 03/11 merge-ort: add a function for initializing our special attr_indexElijah Newren via GitGitGadget, Mar 5, 2021
  5. Ævar Arnfjörð BjarmasonMar 8, 2021
  6. 04/11 merge-ort: have ll_merge() calls use the attr_index for renormalizationElijah Newren via GitGitGadget, Mar 5, 2021
  7. Ævar Arnfjörð BjarmasonMar 8, 2021
  8. Elijah NewrenMar 9, 2021
  9. 05/11 merge-ort: let renormalization change modify/delete into clean deleteElijah Newren via GitGitGadget, Mar 5, 2021
  10. Ævar Arnfjörð BjarmasonMar 8, 2021
  11. 06/11 merge-ort: support subtree shiftingElijah Newren via GitGitGadget, Mar 5, 2021
  12. 07/11 t6428: new test for SKIP_WORKTREE handling and conflictsElijah Newren via GitGitGadget, Mar 5, 2021
  13. Ævar Arnfjörð BjarmasonMar 8, 2021
  14. Elijah NewrenMar 8, 2021
  15. 08/11 merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entriesElijah Newren via GitGitGadget, Mar 5, 2021
  16. Ævar Arnfjörð BjarmasonMar 8, 2021
  17. Elijah NewrenMar 8, 2021
  18. 09/11 t: mark several submodule merging tests as fixed under merge-ortElijah Newren via GitGitGadget, Mar 5, 2021
  19. Ævar Arnfjörð BjarmasonMar 8, 2021
  20. 10/11 merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflictElijah Newren via GitGitGadget, Mar 5, 2021
  21. Ævar Arnfjörð BjarmasonMar 8, 2021
  22. Elijah NewrenMar 8, 2021
  23. 11/11 merge-recursive: add a bunch of FIXME comments documenting known bugsElijah Newren via GitGitGadget, Mar 5, 2021
  24. Ævar Arnfjörð BjarmasonMar 8, 2021
  25. Ævar Arnfjörð BjarmasonMar 8, 2021
  26. Elijah NewrenMar 8, 2021
  27. 00/10 Complete merge-ort implementation...almostElijah Newren via GitGitGadget, Mar 9, 2021
  28. 01/10 merge-ort: use STABLE_QSORT instead of QSORT where requiredElijah Newren via GitGitGadget, Mar 9, 2021
  29. 02/10 merge-ort: add a special minimal index just for renormalizationElijah Newren via GitGitGadget, Mar 9, 2021
  30. Derrick StoleeMar 11, 2021
  31. 05/10 merge-ort: support subtree shiftingElijah Newren via GitGitGadget, Mar 9, 2021
  32. 04/10 merge-ort: let renormalization change modify/delete into clean deleteElijah Newren via GitGitGadget, Mar 9, 2021
  33. 06/10 t6428: new test for SKIP_WORKTREE handling and conflictsElijah Newren via GitGitGadget, Mar 9, 2021
  34. Derrick StoleeMar 11, 2021
  35. 03/10 merge-ort: have ll_merge() use a special attr_index for renormalizationElijah Newren via GitGitGadget, Mar 9, 2021
  36. Derrick StoleeMar 11, 2021
  37. 07/10 merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entriesElijah Newren via GitGitGadget, Mar 9, 2021
  38. 09/10 merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflictElijah Newren via GitGitGadget, Mar 9, 2021
  39. 08/10 t: mark several submodule merging tests as fixed under merge-ortElijah Newren via GitGitGadget, Mar 9, 2021
  40. Derrick StoleeMar 11, 2021
  41. 10/10 merge-recursive: add a bunch of FIXME comments documenting known bugsElijah Newren via GitGitGadget, Mar 9, 2021
  42. Derrick StoleeMar 11, 2021
  43. Elijah NewrenMar 11, 2021
  44. Elijah NewrenMar 17, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.