Mecki
Mecki

Reputation: 133079

Is git cherry-pick actually the same as git show + git apply?

Could it be that

git cherry-pick <rev>

is in fact exactly the same as

git show <rev> | git apply -3 -

???

Since running either one causes exactly the same result for me and if I knew they are indeed equivalent, I could get a much better understanding of how cherry-picking with GIT actually works, as it doesn't always give me the result I'd expect, but that's probably because the sole change cannot be applied and cherry-picking falls back to a three-way-merge (that's what -3 does on the second command) and that would explain the unexpected result.

Upvotes: 1

Views: 143

Answers (3)

VonC
VonC

Reputation: 1326784

Note: for binary files, git apply would not work anyway, not before Git 2.34 (Q4 2021): "git apply"(man) miscounted the bytes and failed to read to the end of binary hunks.

See commit 46d723c (09 Aug 2021) by Jeff King (peff).
(Merged by Junio C Hamano -- gitster -- in commit 7e3b9d1, 30 Aug 2021)

apply: keep buffer/size pair in sync when parsing binary hunks

Reported-by: Xingman Chen
Signed-off-by: Jeff King

We parse through binary hunks by looping through the buffer with code like:

llen = linelen(buffer, size);

...do something with the line...

buffer += llen;
size -= llen;

However, before we enter the loop, there is one call that increments "buffer" but forgets to decrement "size".
As a result, our "size" is off by the length of that line, and subsequent calls to linelen() may look past the end of the buffer for a newline.

The fix is easy: we just need to decrement size as we do elsewhere.

This bug goes all the way back to 0660626 ("binary diff: further updates.", 2006-05-05, Git v1.4.1-rc1 -- merge).
Presumably nobody noticed because it only triggers if the patch is corrupted, and even then we are often "saved" by luck.
We use a strbuf to store the incoming patch, so we overallocate there, plus we add a 16-byte run of NULs as slop for memory comparisons.
So if this happened accidentally, the common case is that we'd just read a few uninitialized bytes from the end of the strbuf before producing the expected "this patch is corrupted" error complaint.

However, it is possible to carefully construct a case which reads off the end of the buffer.
The included test does so.
It will pass both before and after this patch when run normally, but using a tool like ASan shows that we get an out-of-bounds read before this patch, but not after.

Upvotes: 0

torek
torek

Reputation: 489083

Besides Edmundo's answer, which is right and I've upvoted it, there are a bunch of other niggling details, including ephemient's comment about binary files (git show needs --binary to produce a binary patch) and that you may need --full-index as well, in some rare cases where an abbreviated Index: line is insufficient.1

Also, of course, git apply (with or without -3) does not mind a dirty index and work-tree, while git cherry-pick requires a clean index and work-tree, unless you add -n. And, if you omit -n, git cherry-pick goes on to make a commit, which git apply never does.

All that said, though, yes, these two should be functionally equivalent for normal cases.


1To get this to occur, the blob hashes have to be non-unique upon abbreviation. This was made automatically-correct in Git version 1.7.2, though if you force a particular abbreviation length with core.abbrev or a command line flag, you can still get non-unique hash abbreviations. Adding --full-index ensures that you get unique ones.

(If you were to save a diff for a long period, add a bunch of new objects to the repository, and then try to git apply -3 the diff, it's possible for abbreviated Index: lines that were unambiguous before to become ambiguous now.)

Upvotes: 2

eftshift0
eftshift0

Reputation: 30267

Technically speaking, git cherry-pick works like a git merge where, instead of looking for the "best revision that is present on both branches" to check for differences and merge them, the "common revision" for the merging logic is forced to be the parent of the revision you are cherry-picking.

Upvotes: 3

Related Questions