Re: [PATCH V4] git-apply: skip threeway in add / rename cases
- From
- Zenithal <i@zenithal.me>
- Date
- Dec 22, 2021, 06:41 UTC
- Message-ID
- <YcLIjXBdNmMvbqCj@Sun>
- In-Reply-To
- <20211217232902.7604-1-jerry@skydio.com>
On Fri, Dec 17, 2021 at 03:29:02PM -0800, Jerry Zhang wrote:
Show 91 quoted lines
> Certain invocations of "git apply --3way"
> will attempt threeway and fail due to
> missing objects, even though git is able
> to fall back on apply_fragments and
> apply the patch successfully with a return
> value of 0. To fix, return early from
> try_threeway() in the following cases:
>
> When the patch is a rename and no lines have
> changed. In this case, "git diff" doesn't
> record the blob info, so 3way is neither
> possible nor necessary.
>
> When the patch is an addition and there is
> no add/add conflict, i.e. direct_to_threeway
> is false. In this case, threeway will fail
> since the preimage is not in cache, but isn't
> necessary anyway since there is no conflict.
>
> This fixes a few unecessary error prints
> when applying these kinds of patches with
> --3way.
>
> It also fixes a reported issue where applying
> a concatenation of several git produced patches
> will fail when those patches involve a deletion
> followed by creation of the same file. Added a
> test for this case too.
> (test provided by <i@zenithal.me>)
>
> Signed-off-by: Jerry Zhang <jerry@skydio.com>
> ---
> V3->V4:
> - Fix test bug where it wasn't actually
> exercising the correct failure mode.
>
> apply.c | 4 +++-
> t/t4108-apply-threeway.sh | 18 ++++++++++++++++++
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/apply.c b/apply.c
> index fed195250b..afc1c6510e 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -3580,11 +3580,13 @@ 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 */
> if (patch->is_delete ||
> - S_ISGITLINK(patch->old_mode) || S_ISGITLINK(patch->new_mode))
> + S_ISGITLINK(patch->old_mode) || S_ISGITLINK(patch->new_mode) ||
> + (patch->is_new && !patch->direct_to_threeway) ||
> + (patch->is_rename && !patch->lines_added && !patch->lines_deleted))
> return -1;
>
> /* Preimage the patch was prepared for */
> if (patch->is_new)
> write_object_file("", 0, blob_type, &pre_oid);
> diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
> index cc3aa3314a..c558282bc0 100755
> --- a/t/t4108-apply-threeway.sh
> +++ b/t/t4108-apply-threeway.sh
> @@ -273,6 +273,24 @@ test_expect_success 'apply full-index patch with 3way' '
>
> # Apply must succeed.
> git apply --3way --index bin.diff
> '
>
> +test_expect_success 'apply delete then new patch with 3way' '
> + git reset --hard main &&
> + test_write_lines 2 > delnew &&
> + git add delnew &&
> + git diff --cached >> new.patch &&
> + git reset --hard &&
> + test_write_lines 1 > delnew &&
> + git add delnew &&
> + git commit -m "delnew" &&
> + rm delnew &&
> + git diff >> delete-then-new.patch &&
> + cat new.patch >> delete-then-new.patch &&
> +
> + git checkout -- . &&
> + # Apply must succeed.
> + git apply --3way delete-then-new.patch
> +'
> +
> test_done
> --
> 2.32.0.1314.g6ed4fcc4cc
>This fully resolved the issue I mentioned in https://lore.kernel.org/git/YVmTKWlOFr+IwzzI@Sun/
Tested-by: Hongren (Zenithal) Zheng <i@zenithal.me>
Also, I would prefer a Reported-by: Hongren (Zenithal) Zheng <i@zenithal.me> tag or even Co-authored-by: Hongren (Zenithal) Zheng <i@zenithal.me> if you deem it appropriate.