On Fri, Oct 14, 2022 at 2:12 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Jerry Zhang via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Jerry Zhang <Jerry@xxxxxxxxxx> > > > > "git patch-id" currently doesn't produce correct output if the > > incoming diff has any binary files. Add logic to > > get_one_patchid to handle the different possible styles of binary > > diff. This attempts to keep resulting patch-ids identical to what > > would be produced by the counterpart logic in diff.c, that is it > > produces the id by hashing the a and b oids in succession. > > I thought I saw that a previous step touched diff.c to change how > patch ID for a binary diff is computed to match what patch-id > command computes? Now we also have to change patch-id? In the end > output from both may match, but which one between diff and patch-id > have we standardised on? Er yeah let me see if I can simplify. Before: Internal patch-id w/ unstable + binary was correct Internal patch-id w/ stable + binary was broken builtin patch-id w/ binary was broken After: Internal patch-id w/ unstable + binary is correct Internal patch-id w/ stable + binary is now correct builtin patch-id w/ binary is now correct So the "standard" actually came from the one working case from "before", which was the diff.c logic + unstable. I based all new logic on that because it seemed reasonable and correct. Since "internal w/unstable" is never exposed externally, it's perhaps true that i could have invented a totally new format and standardized on that. Hashing the oids in succession is pretty much representative of a binary patch though, so I don't think there's much to be improved on. > > Puzzled...
