On Tue, Jul 27, 2021 at 6:08 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Jerry Zhang <jerry@xxxxxxxxxx> 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. Ah yep it's exactly as you say. I'll add the fix and a couple of test cases into a new patch. > > So, I do not think it is implausible that we are seeing a legit > regression report. > > Thanks.
