Skip to content

Fix another crazy rename assertion

Elijah Newren via GitGitGadget git 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest3 patches 5 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3t6429: update comment to mention correct toolt/t6429-merge-sequence-rename-caching.sh+7 -85 replies
2/3merge-ort: remove debugging crudmerge-ort.c+1 -10 replies
3/3merge-ort: fix failing merges in special corner casemerge-ort.c t/t6429-merge-sequence-rename-caching.sh+106 -10 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/3] Fix another crazy rename assertion

This fixes another special corner case that was being triggered at GitHub; the error being triggered is the same as what I submitted a fix for a few months ago, but the way it was triggered and the fix needed are different in this case. See the final commit message for details. The first two patches are just tiny cleanups I noticed while investigating the problem.

I will also note that I first came up with an alternative fix -- checking in use_cached_pairs() whether new_name was contained in the opt->priv->paths strmap, and if not, skipping to the next cached rename instead of adding it to pairs. That would also work, but it would mean that if a yet-subsequent commit after that did modify the old/file path, I think we'd have to re-detect the rename, which would hurt the effectiveness of the cached renames optimization. Simply avoiding using it in process_renames() allows it to avoid being forgotten (and since old/file is NOT modified, the upstream rename remains valid). Besides, this fix is nicely symmetrical to the check on !oldinfo, so it seems more aesthetic to me as well as helping us preserve performance.

Elijah Newren (3):
t6429: update comment to mention correct tool
merge-ort: remove debugging crud
merge-ort: fix failing merges in special corner case

 merge-ort.c                              | 31 +++++++-
 t/t6429-merge-sequence-rename-caching.sh | 93 ++++++++++++++++++++++--
 2 files changed, 114 insertions(+), 10 deletions(-)

base-commit: 4253630c6f07a4bdcc9aa62a50e26a4d466219d1
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1992%2Fnewren%2Ffix-another-crazy-rename-assertion-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1992/newren/fix-another-crazy-rename-assertion-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1992

Signature · 1 line
gitgitgadget