git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "lilinchao@oschina.cn" <lilinchao@oschina.cn>
To: "Junio C Hamano" <gitster@pobox.com>,
	"jerry@skydio.com" <jerry@skydio.com>
Cc: git <git@vger.kernel.org>
Subject: Re: Re: git apply --3way behaves abnormally when the patch contains binary changes.
Date: Wed, 28 Jul 2021 12:45:21 +0800	[thread overview]
Message-ID: <9f510f56ef5e11eb90f70026b95c99cc@oschina.cn> (raw)
In-Reply-To: 4eb90a4eef4011ebab68d4ae5272fd1139378@pobox.com

The defective test demo I provided is not that important(for me), the purpose of this is to
bring out our topic that "git apply -3" behaves differently on different Git version when
the patch is binary.
Maybe I would say this breaks backward compatibility, but the poor test demo didn't prove this. 
If anyone would like to see the incompatibility, he can run "git apply --index  -3 binary.diff" in command line in different Git version environment.

>Jerry Zhang <jerry@skydio.com> writes:
>
>>>         # 2. based on left_bin branch, make any change, and commit
>>>         git checkout -b right &&
>>>         cat bin.png bin.png > bin.png &&
>>>         git add bin.png &&
>>>         git commit -m "update binary file" &&
>>>
>>>         # 3. make patch
>>>         git diff --binary left..right >bin.diff &&
>>>         # apply --3way, and it will fail
>>>         test_must_fail git apply --index --3way bin.diff
>>> '
>>> "
>>>
>>> But  "git apply --index --3way bin.diff" will not faill on Git version 2.31.0.
>> Are you sure? I checked out to "commit
>> a5828ae6b52137b913b978e16cd2334482eb4c1f (HEAD, tag: v2.31.0)" and
>> rebuilt and ran your test snippet and it still failed.
>
>Isn't it just because the reproduction recipe is simply wrong?
>
>It says
>
>    * be on left branch and have a binary file
>    * be on right branch and have a modified binary file
>    * create a patch to take left to right
>
>Notice that we have a patch and we are still on the right branch.
>Of course, applying the patch to take us from left to right would
>fail from that state, but I _think_ the intent of the reproduction
>recipe was, after all of the above, do this here:
>
>    * switch to left branch and attempt to apply the patch.
>
>And the patch is meant to take us from left to right, and we are on
>pristine left, the application ought to cleanly succeed, no?
>
>"git apply bin.diff" would probably work correctly but I do not know
>offhand what the code after your change does with --3way enabled.
>
>We refuse to merge binary files, so I would not be surprised if we
>failed the 3way in this case (even though we _could_ fast-forward,
>it may not be worth complicating the --3way logic---nobody sane
>would say --3way when it is unnecessary) but after 3way fails, do we
>still correctly fall back to "straight application" like we do for
>text patches with your change?  Before your change, we would have
>first attempted the "straight application", which would succeed and
>wouldn't have hit "3way will refuse to merge binaries" at all.
>
>So, I do not think it is implausible that we are seeing a legit
>regression report.
>
>Thanks.

      parent reply	other threads:[~2021-07-28  4:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 14:07 git apply --3way behaves abnormally when the patch contains binary changes lilinchao
2021-07-27 22:44 ` Jerry Zhang
2021-07-28  1:08   ` Junio C Hamano
2021-07-28  1:37     ` Jerry Zhang
     [not found]   ` <4eb90a4eef4011ebab68d4ae5272fd1139378@pobox.com>
2021-07-28  4:45     ` lilinchao [this message]

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=9f510f56ef5e11eb90f70026b95c99cc@oschina.cn \
    --to=lilinchao@oschina.cn \
    --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).