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

[PATCH v2 0/6] Small new merge-ort features, prepping for deletion of merge-recursive.[ch]

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 13, 2025, 02:46 UTC
Message-ID
<pull.1875.v2.git.1741834001.gitgitgadget@gmail.com>
In-Reply-To
<pull.1875.git.1741362522.gitgitgadget@gmail.com>

I've got 19 patches covering the work needed to prep for and allow us to delete merge-recursive.[ch], and remap 'recursive' to 'ort', including some clean-up along the way. I've tried to divide it up into some smaller patch series.

These 6 patches are the first of those series. Breakdown:
 * The first 3 patches provide small new features to allow us to convert
   later callers
 * Patches 4-5 document and fix a bug with directory rename detection being
   turned off (since git am always turned off directory rename detection,
   this is a prerequisite for converting git am)
 * Patch 6 converts git am from using merge_recursive_generic() to use the
   new merge_ort_generic().
Changes since v1:
 * Patch 1: Added an explanation in the commit message about the one
   difference between merge_recursive_generic() and merge_ort_generic() --
   the label for the ancestor commit
 * Patch 2: Linked the relevant discussion in the commit message, fixed a
   style issue, and added a testcase
 * Patch 3: Since the opt->verbosity stuff was an unwanted carryover (due to
   being partially public API), move the tweak to the merge-ort-wrappers to
   avoid promoting it
 * Added 3 more patches, so folks can see one of the callers of
   merge_ort_generic().
Elijah Newren (5):
  merge-ort: add new merge_ort_generic() function
  merge-ort: allow rename detection to be disabled
  merge-ort: support having merge verbosity be set to 0
  merge-ort: fix merge.directoryRenames=false
  am: switch from merge_recursive_generic() to merge_ort_generic()
Johannes Schindelin (1):
  t3650: document bug when directory renames are turned off
 Documentation/merge-strategies.adoc | 12 ++---
 builtin/am.c                        |  5 +-
 merge-ort-wrappers.c                | 72 ++++++++++++++++++++++++++++-
 merge-ort-wrappers.h                | 12 +++++
 merge-ort.c                         | 53 ++++++++++++++++++---
 merge-ort.h                         |  5 ++
 t/t3650-replay-basics.sh            | 22 +++++++++
 t/t4151-am-abort.sh                 |  2 +-
 t/t4255-am-submodule.sh             |  1 -
 t/t4301-merge-tree-write-tree.sh    |  6 +++
 t/t6427-diff3-conflict-markers.sh   |  2 +-
 11 files changed, 172 insertions(+), 20 deletions(-)
base-commit: a36e024e989f4d35f35987a60e3af8022cac3420
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1875%2Fnewren%2Fendit-new-features-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1875/newren/endit-new-features-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1875
Range-diff vs v1:
 1:  9f73e54224d ! 1:  c2f2e3e0cd6 merge-ort: add new merge_ort_generic() function
     @@ Commit message
          equivalent for the final entry point, so we can switch callers to
          use it and remove merge-recursive.[ch].
      
     +    While porting it over, finally fix the issue with the label for the
     +    ancestor (used when merge.conflictStyle=diff3 as a conflict label).
     +    merge-recursive.c has traditionally not allowed callers to set that
     +    label, but I have found that problematic for years.
     +
     +    (Side note: This function was initially part of the merge-ort rewrite,
     +    but reviewers questioned the ancestor label funnyness which I was
     +    never really happy with anyway.  It resulted in me jettisoning it and
     +    hoping at the time that I would eventually be able to force the existing
     +    callers to use some other API.  That worked with `git stash`, as per
     +    874cf2a60444 (stash: apply stash using 'merge_ort_nonrecursive()',
     +    2022-05-10), but this API is the most reasonable one for `git am` and
     +    `git merge-recursive`, if we can just allow them some freedom over the
     +    ancestor label.)
     +
     +    The merge_recursive_generic() function did not know whether it was being
     +    invoked by `git stash`, `git merge-recursive`, or `git am`, and the
     +    choice of meaningful ancestor label, when there is a unique ancestor,
     +    varies for these different callers:
     +
     +      * git am: ancestor is a constructed "fake ancestor" that user knows
     +                nothing about and has no access to.  (And is different than
     +                the normal thing we mean by a "virtual merge base" which is
     +                the merging of merge bases.)
     +      * git merge-recursive: ancestor might be a tree, but at least it
     +                             was one specified by the user (if they invoked
     +                             merge-recursive directly)
     +      * git stash: ancestor was the commit serving as the stash base
     +
     +    Thus, using a label like "constructed merge base" (as
     +    merge_recursive_generic() does) presupposes that `git am` is the only
     +    caller; it is incorrect for other callers.  This label has thrown me off
     +    more than once.  Allow the caller to override when there is a unique
     +    merge base.
     +
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## merge-ort-wrappers.c ##
 2:  4292b22723f ! 2:  f401a8e0967 merge-ort: allow rename detection to be disabled
     @@ Commit message
          longer with the option disabled seems unlikely to help surface such
          issues at this point.  Also, there has been at least one request to
          allow rename detection to be disabled for behavioral rather than
     -    performance reasons, so let's start heeding the config and command line
     -    settings.
     +    performance reasons (see the thread including
     +    https://lore.kernel.org/git/CABPp-BG-Nx6SCxxkGXn_Fwd2wseifMFND8eddvWxiZVZk0zRaA@mail.gmail.com/
     +    ), so let's start heeding the config and command line settings.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     @@ merge-ort.c: static int detect_and_process_renames(struct merge_options *opt)
       
       	if (!possible_renames(renames))
       		goto cleanup;
     -+	if (opt->detect_renames == 0) {
     ++	if (!opt->detect_renames) {
      +		renames->redo_after_renames = 0;
      +		renames->cached_pairs_valid_side = 0;
      +		goto cleanup;
     @@ merge-ort.c: static int detect_and_process_renames(struct merge_options *opt)
       
       	trace2_region_enter("merge", "regular renames", opt->repo);
       	detection_run |= detect_regular_renames(opt, MERGE_SIDE1);
     +
     + ## t/t4301-merge-tree-write-tree.sh ##
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Clean merge' '
     + 	test_cmp expect actual
     + '
     + 
     ++# Repeat the previous test, but turn off rename detection
     ++test_expect_success 'Failed merge without rename detection' '
     ++	test_must_fail git -c diff.renames=false merge-tree --write-tree side1 side3 >out &&
     ++	grep "CONFLICT (modify/delete): numbers deleted" out
     ++'
     ++
     + test_expect_success 'Content merge and a few conflicts' '
     + 	git checkout side1^0 &&
     + 	test_must_fail git merge side2 &&
 3:  c2a2be336e0 < -:  ----------- merge-ort: support having merge verbosity be set to 0
 -:  ----------- > 3:  a508b0a0fe2 merge-ort: support having merge verbosity be set to 0
 -:  ----------- > 4:  fefda4add11 t3650: document bug when directory renames are turned off
 -:  ----------- > 5:  b25225c3cab merge-ort: fix merge.directoryRenames=false
 -:  ----------- > 6:  3f4b74eb3b9 am: switch from merge_recursive_generic() to merge_ort_generic()
-- 
gitgitgadget
Previous: Taylor BlauNext: Elijah Newren via GitGitGadget
Message 18 of 25 in “Small new merge-ort features, prepping for deletion of merge-recursive.[ch]”
  1. 0/3 Small new merge-ort features, prepping for deletion of merge-recursive.[ch]Elijah Newren via GitGitGadget, Mar 7, 2025
  2. 1/3 merge-ort: add new merge_ort_generic() functionElijah Newren via GitGitGadget, Mar 7, 2025
  3. Patrick SteinhardtMar 12, 2025
  4. Taylor BlauMar 12, 2025
  5. Elijah NewrenMar 12, 2025
  6. 2/3 merge-ort: allow rename detection to be disabledElijah Newren via GitGitGadget, Mar 7, 2025
  7. Patrick SteinhardtMar 12, 2025
  8. Taylor BlauMar 12, 2025
  9. Elijah NewrenMar 12, 2025
  10. Taylor BlauMar 12, 2025
  11. Jeff KingMar 13, 2025
  12. 3/3 merge-ort: support having merge verbosity be set to 0Elijah Newren via GitGitGadget, Mar 7, 2025
  13. Taylor BlauMar 12, 2025
  14. Elijah NewrenMar 12, 2025
  15. Taylor BlauMar 12, 2025
  16. Patrick SteinhardtMar 12, 2025
  17. Taylor BlauMar 12, 2025
  18. 0/6 Small new merge-ort features, prepping for deletion of merge-recursive.[ch]Elijah Newren via GitGitGadget, Mar 13, 2025
  19. 1/6 merge-ort: add new merge_ort_generic() functionElijah Newren via GitGitGadget, Mar 13, 2025
  20. 3/6 merge-ort: support having merge verbosity be set to 0Elijah Newren via GitGitGadget, Mar 13, 2025
  21. 2/6 merge-ort: allow rename detection to be disabledElijah Newren via GitGitGadget, Mar 13, 2025
  22. 4/6 t3650: document bug when directory renames are turned offJohannes Schindelin via GitGitGadget, Mar 13, 2025
  23. 5/6 merge-ort: fix merge.directoryRenames=falseElijah Newren via GitGitGadget, Mar 13, 2025
  24. 6/6 am: switch from merge_recursive_generic() to merge_ort_generic()Elijah Newren via GitGitGadget, Mar 13, 2025
  25. Taylor BlauMar 17, 2025

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.