For one single patch FILE containing both deletion and creation of the same file, applying with three way would fail, which should not. When git-apply processes one single patch FILE, patches inside it would be applied before write_out_results(), thus it may occur that one file being deleted but it is still in the index when applying a new patch, in this case, try_threeway() would find an old file thus causing merge conflict. To avoid this, git-apply should fall back to directly apply when it turns out to be such cases. Signed-off-by: Hongren (Zenithal) Zheng <i@xxxxxxxxxxx> --- apply.c | 13 ++++++++++++- t/t4108-apply-threeway.sh | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) More notes below: This patch is a bugfix hence it is based on branch `maint`. This bug is caused by a behavior change since 2.32 where git apply --3way would try 3-way first before directly apply. Interestingly, if the deletion patch and the addition patch are in two patch files, applying with three way would go on cleanly. As indicated in commit msg, if these two patches are in different patch files, write_out_results() would be called twice, unlike when they are in the same file, write_out_results() would be called altogether after all patches being applied. One way to fix this is to check for this kind of conditions, which is presented in this patch. A side note though, this kind of checks and fixes already exist as indicated by variable ok_if_exists in function check_patch(). See the comment around this variable for more info. This kind of fixes is really dark magic. Another way, which I do not adopt because it requires major refactor but it is more clean and understandable, is to change the way write_out_resultes() is called, namely instead of calling it after all patches being applied in one patch FILE, after each patch being applied, we write_out_result immediately thus deleting one file would immediately delete the file from the index. The man page of `patch` says: If the patch file contains more than one patch, patch tries to apply each of them as if they came from separate patch files. So I think this way is more standardized. However, as also indicated by comments around variable ok_if_exists in function check_patch(), consequtive patches in one file have special meanings as endowed by diff.c::run_diff() I do not know how to handle this, so I just send it as notes. More comment: this problem or this kind of fix may be related to https://lore.kernel.org/git/YR1OszUm08BMAE1N@xxxxxxxxxxxxxxxxxxxxxxx/ diff --git a/apply.c b/apply.c index 44bc31d6eb5b42d4077eff458246cde376cb6785..3fa96fcc781bdc27f66a35442f27972a0e84ea77 100644 --- a/apply.c +++ b/apply.c @@ -3558,8 +3558,19 @@ static int try_threeway(struct apply_state *state, char *img; struct image tmp_image; - /* No point falling back to 3-way merge in these cases */ + /* + * No point using 3-way merge in these cases + * + * For patch->is_new, if new_name does not exist in the index, + * we can directly apply; if new_name exists, + * according to ok_if_exists in check_patch(), + * there are cases where new_name gets deleted in previous patches + * BUT still exists in index, in this case, we can directly apply. + */ if (patch->is_delete || + (patch->is_new && + (index_name_pos(state->repo->index, patch->new_name, strlen(patch->new_name)) < 0 || + was_deleted(in_fn_table(state, patch->new_name)))) || S_ISGITLINK(patch->old_mode) || S_ISGITLINK(patch->new_mode)) return -1; diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index 65147efdea9a00e30d156e6f4d5d72a3987f230d..14bbb393430ed57a236d25aa568a0fdc6d221a6d 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -230,4 +230,24 @@ test_expect_success 'apply with --3way --cached and conflicts' ' test_cmp expect.diff actual.diff ' +test_expect_success 'apply delete then new patch with 3way' ' + git reset --hard main && + test_write_lines 1 > delnew && + git add delnew && + git commit -m "delnew" && + cat >delete-then-new.patch <<-\EOF && + --- a/delnew + +++ /dev/null + @@ -1 +0,0 @@ + -1 + --- /dev/null + +++ b/delnew + @@ -0,0 +1 @@ + +2 + EOF + + # Apply must succeed. + git apply --3way delete-then-new.patch +' + test_done -- 2.32.0
