Reputation: 133079
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
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 hunksReported-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 tolinelen()
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 astrbuf
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
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
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