Re: [PATCH V2] git-apply: silence errors for success cases

[Jerry Zhang <jerry@skydio.com>]

On Tue, Jul 27, 2021 at 8:14 PM Jerry Zhang <jerry@skydio.com> wrote:
>
> Certain invocations of "git apply --3way"
> will print error messages 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.
>
> Only error messaging is affected and other
> behavior does not change.
>
> Signed-off-by: Jerry Zhang <jerry@skydio.com>
> ---
> V1->V2: rebase onto master and rerun tests
>
> I think I addressed previous comments. What
> are the next steps for this patch?
>
>  apply.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/apply.c b/apply.c
> index 44bc31d6eb..fb321c707b 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -3560,7 +3560,9 @@ static int try_threeway(struct apply_state *state,
>
>         /* 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 */
> --
> 2.32.0.1314.g6ed4fcc4cc
>
Any updates on this patch? I had addressed the previous comment in this
thread in a different patch "apply: adjust messages to account for
--3way changes"
that was already merged in a while ago.

I originally intended this to mainly fix print message behavior, but
while testing
I found that it also fixed a separately reported issue from this thread:
https://www.spinics.net/lists/git/msg421481.html.

I probably owe an update / refresher on why I originally submitted these
features in the first place. I've written a tool for branchless / patch stack
based code review and development w/github. The core of this is a mechanism
that can cherry-pick commits without touching the cache or working tree
by using "git apply --cached --3way" with a custom index file. We've been
using this internally with a custom built git for a while now and gotten pretty
good feedback. We'd like to open source this tool soon, and it'd be nice
if our open-source users (eventually) don't need to build git themselves :).