Re: Re: git apply --3way behaves abnormally when the patch contains binary changes.
- From
- lilinchao@oschina.cn <lilinchao@oschina.cn>
- Date
- Jul 28, 2021, 04:45 UTC
- Message-ID
- <9f510f56ef5e11eb90f70026b95c99cc@oschina.cn>
- 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.
Show 54 quoted lines
>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.