kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

Merge branch 'en/ort-rename-another-fix' 

Yet another corner case fix around renames in the "ort" merge
strategy.

* en/ort-rename-another-fix:
  merge-ort: fix failing merges in special corner case
  merge-ort: remove debugging crud
  t6429: update comment to mention correct tool
main
Junio C Hamano 2025-11-26 10:32:40 -08:00
parent 42bf8a534b a562d90a35
commit 716e871d50
2 changed files with 114 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
merge-ort.c
View File

@ -2912,6 +2912,32 @@ static int process_renames(struct merge_options *opt,
if (!oldinfo || oldinfo->merged.clean)
continue;

/*
* Rename caching from a previous commit might give us an
* irrelevant rename for the current commit.
*
* Imagine:
* foo/A -> bar/A
* was a cached rename for the upstream side from the
* previous commit (without the directories being renamed),
* but the next commit being replayed
* * does NOT add or delete files
* * does NOT have directory renames
* * does NOT modify any files under bar/
* * does NOT modify foo/A
* * DOES modify other files under foo/ (otherwise the
* !oldinfo check above would have already exited for
* us)
* In such a case, our trivial directory resolution will
* have already merged bar/, and our attempt to process
* the cached
* foo/A -> bar/A
* would be counterproductive, and lack the necessary
* information anyway. Skip such renames.
*/
if (!newinfo)
continue;

/*
* diff_filepairs have copies of pathnames, thus we have to
* use standard 'strcmp()' (negated) instead of '=='.
@ -3438,7 +3464,7 @@ static int collect_renames(struct merge_options *opt,
continue;
}
if (opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_NONE &&
p->status == 'R' && 1) {
p->status == 'R') {
possibly_cache_new_pair(renames, p, side_index, NULL);
goto skip_directory_renames;
}
@ -5118,7 +5144,8 @@ static void merge_check_renames_reusable(struct merge_options *opt,
* optimization" comment near that case).
*
* This could be revisited in the future; see the commit message
* where this comment was added for some possible pointers.
* where this comment was added for some possible pointers, or the
* later commit where this comment was added.
*/
if (opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_NONE) {
renames->cached_pairs_valid_side = 0; /* neither side valid */

93
t/t6429-merge-sequence-rename-caching.sh
View File

@ -11,14 +11,13 @@ test_description="remember regular & dir renames in sequence of merges"
# sure that we are triggering rename caching rather than rename
# bypassing.
#
# NOTE 2: this testfile uses 'test-tool fast-rebase' instead of either
# cherry-pick or rebase. sequencer.c is only superficially
# integrated with merge-ort; it calls merge_switch_to_result()
# after EACH merge, which updates the index and working copy AND
# throws away the cached results (because merge_switch_to_result()
# is only supposed to be called at the end of the sequence).
# Integrating them more deeply is a big task, so for now the tests
# use 'test-tool fast-rebase'.
# NOTE 2: this testfile uses replay instead of either cherry-pick or rebase.
# sequencer.c is only superficially integrated with merge-ort; it
# calls merge_switch_to_result() after EACH merge, which updates the
# index and working copy AND throws away the cached results (because
# merge_switch_to_result() is only supposed to be called at the end
# of the sequence). Integrating them more deeply is a big task, so
# for now the tests use 'git replay'.
#


@ -769,4 +768,82 @@ test_expect_success 'avoid assuming we detected renames' '
)
'

#
# In the following testcase:
# Base: olddir/{valuesX_1, valuesY_1, valuesZ_1}
# other/content
# Upstream: rename olddir/valuesX_1 -> newdir/valuesX_2
# Topic_1: modify olddir/valuesX_1 -> olddir/valuesX_3
# Topic_2: modify olddir/valuesY,
# modify other/content
# Expected Pick1: olddir/{valuesY, valuesZ}, newdir/valuesX, other/content
# Expected Pick2: olddir/{valuesY, valuesZ}, newdir/valuesX, other/content
#
# This testcase presents no problems for git traditionally, but the fact that
# olddir/valuesX -> newdir/valuesX
# gets cached after the first pick presents a problem for the second commit to
# be replayed, because it appears to be an irrelevant rename, so the trivial
# directory resolution will resolve newdir/ without recursing into it, giving
# us no way to apply the cached rename to anything.
#
test_expect_success 'rename a file, use it on first pick, but irrelevant on second' '
git init rename_a_file_use_it_once_irrelevant_on_second &&
(
cd rename_a_file_use_it_once_irrelevant_on_second &&

mkdir olddir/ other/ &&
test_seq 3 8 >olddir/valuesX &&
test_seq 3 8 >olddir/valuesY &&
test_seq 3 8 >olddir/valuesZ &&
printf "%s\n" A B C D E F G >other/content &&
git add olddir other &&
git commit -m orig &&

git branch upstream &&
git branch topic &&

git switch upstream &&
test_seq 1 8 >olddir/valuesX &&
git add olddir &&
mkdir newdir &&
git mv olddir/valuesX newdir &&
git commit -m "Renamed (and modified) olddir/valuesX into newdir/" &&

git switch topic &&

test_seq 3 10 >olddir/valuesX &&
git add olddir &&
git commit -m A &&

test_seq 1 8 >olddir/valuesY &&
printf "%s\n" A B C D E F G H I >other/content &&
git add olddir/valuesY other &&
git commit -m B &&

#
# Actual testing; mostly we want to verify that we do not hit
# git: merge-ort.c:3032: process_renames: Assertion `newinfo && !newinfo->merged.clean` failed.
#

git switch upstream &&
git config merge.directoryRenames true &&

git replay --onto HEAD upstream~1..topic >out &&

#
# ...but we may as well check that the replay gave us a reasonable result
#

git update-ref --stdin <out &&
git checkout topic &&

git ls-files >tracked &&
test_line_count = 4 tracked &&
test_path_is_file newdir/valuesX &&
test_path_is_file olddir/valuesY &&
test_path_is_file olddir/valuesZ &&
test_path_is_file other/content
)
'

test_done