From: Zenithal <i@zenithal.me>
To: Jerry Zhang <jerry@skydio.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH V4] git-apply: skip threeway in add / rename cases
Date: Wed, 22 Dec 2021 14:41:17 +0800 [thread overview]
Message-ID: <YcLIjXBdNmMvbqCj@Sun> (raw)
In-Reply-To: <20211217232902.7604-1-jerry@skydio.com>
On Fri, Dec 17, 2021 at 03:29:02PM -0800, Jerry Zhang wrote:
> 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.
next prev parent reply other threads:[~2021-12-22 6:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-17 22:43 [PATCH V3] git-apply: skip threeway in add / rename cases Jerry Zhang
2021-12-17 23:29 ` [PATCH V4] " Jerry Zhang
2021-12-22 6:41 ` Zenithal [this message]
2022-01-05 23:30 ` [PATCH V5] " Jerry Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YcLIjXBdNmMvbqCj@Sun \
--to=i@zenithal.me \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jerry@skydio.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).