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

[PATCH 26/37] merge-recursive: Avoid doubly merging rename/add conflict contents

From
Elijah Newren <newren@gmail.com>
Date
Sep 20, 2010, 08:28 UTC
Message-ID
<1284971350-30590-27-git-send-email-newren@gmail.com>
In-Reply-To
<1284971350-30590-1-git-send-email-newren@gmail.com>

When a commit moves A to B while another commit created B (or moved C to B), and these two different commits serve as different merge-bases for a later merge, c94736a (merge-recursive: don't segfault while handling rename clashes 2009-07-30) added some special code to avoid segfaults. Since that commit, the two versions of B are merged in place (which could be potentially conflicting) and the intermediate result is used as the virtual ancestor.

However, right before this special merge, try_merge was turned on, meaning that process_renames() would try an alternative merge that ignores the 'add' part of the conflict, and, if the merge is clean, store that as the new virtual ancestor. This could cause incorrect merging of criss-cross merges; it would typically result in just recording a slightly confusing merge base, but in some cases it could cause silent acceptance of one side of a merge as the final resolution when a conflict should have been flagged.

When we do a special merge for such a rename/add conflict between merge-bases, turn try_merge off to avoid an inappropriate second merge.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c                 |    1 +
 t/t6036-recursive-corner-cases.sh |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 5f528c1..178bbd8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1061,6 +1061,7 @@ static int process_renames(struct merge_options *o,
 						    mfi.sha,
 						    mfi.mode,
 						    ren1_dst);
+					try_merge = 0;
 				} else {
 					new_path = unique_path(o, ren1_dst, branch2);
 					output(o, 1, "Adding as %s instead", new_path);
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 6c2b2bf..a2e5c5c 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -120,7 +120,7 @@ test_expect_success 'setup criss-cross + rename merges with basic modification'
 	git tag R2
 '
 
-test_expect_failure 'merge criss-cross + rename merges with basic modification' '
+test_expect_success 'merge criss-cross + rename merges with basic modification' '
 	git reset --hard &&
 	git checkout L2^0 &&
 
@@ -202,7 +202,7 @@ test_expect_success 'setup differently handled merges of rename/add conflict' '
 	git tag E
 '
 
-test_expect_failure 'git detects differently handled merges conflict' '
+test_expect_success 'git detects differently handled merges conflict' '
 	git reset --hard &&
 	git checkout D^0 &&
 

-- 
1.7.3.271.g16009
Previous: Elijah NewrenNext: Elijah Newren
Message 33 of 46 in “Audit and fix corner case bugs in recursive merge”
  1. 00/37 Audit and fix corner case bugs in recursive mergeElijah Newren, Sep 20, 2010
  2. 01/37 t3030: Add a testcase for resolvable rename/add conflict with symlinksElijah Newren, Sep 20, 2010
  3. Johannes SixtSep 20, 2010
  4. Jakub NarebskiSep 20, 2010
  5. 02/37 merge-recursive: Restructure showing how to chain more process_* functionsElijah Newren, Sep 20, 2010
  6. 03/37 t6032: Add a test checking for excessive output from mergeElijah Newren, Sep 20, 2010
  7. 04/37 t6022: Add test combinations of {content conflict?, D/F conflict remains?}Elijah Newren, Sep 20, 2010
  8. 05/37 t6022: Add tests for reversing order of merges when D/F conflicts presentElijah Newren, Sep 20, 2010
  9. 06/37 t6022: Add tests with both rename source & dest involved in D/F conflictsElijah Newren, Sep 20, 2010
  10. 07/37 t6022: Add paired rename+D/F conflict: (two/file, one/file) -> (one, two)Elijah Newren, Sep 20, 2010
  11. 08/37 t6022: Add tests for rename/rename combined with D/F conflictsElijah Newren, Sep 20, 2010
  12. 09/37 t6020: Modernize style a bitElijah Newren, Sep 20, 2010
  13. Johannes SixtSep 20, 2010
  14. Elijah NewrenSep 20, 2010
  15. Junio C HamanoSep 22, 2010
  16. Elijah NewrenSep 22, 2010
  17. 10/37 t6020: Add a testcase for modify/delete + directory/file conflictElijah Newren, Sep 20, 2010
  18. 11/37 t6036: Test index and worktree state, not just that merge failsElijah Newren, Sep 20, 2010
  19. 12/37 t6036: Add a second testcase similar to the first but with content changesElijah Newren, Sep 20, 2010
  20. 13/37 t6036: Add testcase for undetected conflictElijah Newren, Sep 20, 2010
  21. 14/37 merge-recursive: Small code clarification -- variable name and commentsElijah Newren, Sep 20, 2010
  22. 15/37 merge-recursive: Rename conflict_rename_rename*() for clarityElijah Newren, Sep 20, 2010
  23. 16/37 merge-recursive: Nuke rename/directory conflict detectionElijah Newren, Sep 20, 2010
  24. 17/37 merge-recursive: Move rename/delete handling into dedicated functionElijah Newren, Sep 20, 2010
  25. 18/37 merge-recursive: Move delete/modify handling into dedicated functionElijah Newren, Sep 20, 2010
  26. 19/37 merge-recursive: Move process_entry's content merging into a functionElijah Newren, Sep 20, 2010
  27. 20/37 merge-recursive: New data structures for deferring of D/F conflictsElijah Newren, Sep 20, 2010
  28. 21/37 merge-recursive: New function to assist resolving renames in-core onlyElijah Newren, Sep 20, 2010
  29. 22/37 merge-recursive: Have process_entry() skip D/F or rename entriesElijah Newren, Sep 20, 2010
  30. 23/37 merge-recursive: Structure process_df_entry() to handle more casesElijah Newren, Sep 20, 2010
  31. 24/37 merge-recursive: Update conflict_rename_rename_1to2() call signatureElijah Newren, Sep 20, 2010
  32. 25/37 merge-recursive: Update merge_content() call signatureElijah Newren, Sep 20, 2010
  33. 26/37 merge-recursive: Avoid doubly merging rename/add conflict contentsElijah Newren, Sep 20, 2010
  34. 27/37 merge-recursive: Move handling of double rename of one file to twoElijah Newren, Sep 20, 2010
  35. 28/37 merge-recursive: Move handling of double rename of one file to other fileElijah Newren, Sep 20, 2010
  36. 29/37 merge-recursive: Delay handling of rename/delete conflictsElijah Newren, Sep 20, 2010
  37. 30/37 merge-recursive: Delay content merging for renamesElijah Newren, Sep 20, 2010
  38. 31/37 merge-recursive: Delay modify/delete conflicts if D/F conflict presentElijah Newren, Sep 20, 2010
  39. 32/37 conflict_rename_delete(): Check whether D/F conflicts are still presentElijah Newren, Sep 20, 2010
  40. 33/37 conflict_rename_rename_1to2(): Fix checks for presence of D/F conflictsElijah Newren, Sep 20, 2010
  41. 34/37 merge_content(): Check whether D/F conflicts are still presentElijah Newren, Sep 20, 2010
  42. 35/37 handle_delete_modify(): Check whether D/F conflicts are still presentElijah Newren, Sep 20, 2010
  43. 36/37 merge-recursive: Make room for directories in D/F conflictsElijah Newren, Sep 20, 2010
  44. Johannes SixtSep 20, 2010
  45. Elijah NewrenSep 20, 2010
  46. 37/37 merge-recursive: Remove redundant path clearing for D/F conflictsElijah Newren, Sep 20, 2010

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.