{"thread":{"id":"5489","subject":"Why is there a --binary option needed for git-apply?","startedAt":"2006-09-05T18:40:53Z","lastAt":"2006-09-07T06:45:43Z","messageCount":3,"participants":["Carl Worth","Shawn Pearce","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"26366","messageId":"874pvmxikq.wl%cworth@cworth.org","threadId":"5489","inReplyTo":null,"subject":"Why is there a --binary option needed for git-apply?","fromName":"Carl Worth","fromEmail":"cworth@cworth.org","sentAt":"2006-09-05T18:40:53Z","receivedAt":"2006-09-05T18:40:53Z","isPatch":false,"sender":{"key":"cworth@cworth.org","avatar":"https://gravatar.com/avatar/3746dc28cde609bdbd7f939058356e7e2bbd16d21e32274df0725eb3d998bc5b?d=mp&s=160"},"body":"I was recently emailed a patch that introduced some new binary files\nto my repository. The patch included a very pleasant-looking chunk\nalong the lines of:\n\n\tdiff --git a/new/file.png b/new/file.png\n\tnew file mode 100644\n\tindex 0000000000000000000000000000000000000000..e7edd8141e15f5753ea94244e7315bd1341a8c05\n\tGIT binary patch\n\nI tried applying the patch with git-apply (1.4.2.rc2.gef1d9) and\nreceived an inscrutable error:\n\n\tfatal: patch with only garbage at line 90\n\nWhere line 90 happened to be the line after the first chunk.\n\nI was disappointed that the operation had failed and started guessing\nat problem causes (git version incompatibilities? MUA whitespace\nmunging?).\n\nShawn Pearce was kind enough to direct me to the --binary option for\ngit-apply which solved my problem. But that left me wondering why\ngit-apply requires this extra command-line option to do its\njob. Shouldn't git-apply simply apply the patch it is given?\n\nIf there is some reason for git-apply to only apply binary patches\nwhen under the duress of --binary, then at the very least it could use\na better error message explaining the situation.\n\n-Carl\n--\ncworth@redhat.com\n"},{"id":"26396","messageId":"20060906033803.GA30598@spearce.org","threadId":"5489","inReplyTo":"874pvmxikq.wl%cworth@cworth.org","subject":"Re: Why is there a --binary option needed for git-apply?","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-09-06T03:38:03Z","receivedAt":"2006-09-06T03:38:03Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Carl Worth <cworth@cworth.org> wrote:\n> Shawn Pearce was kind enough to direct me to the --binary option for\n> git-apply which solved my problem. But that left me wondering why\n> git-apply requires this extra command-line option to do its\n> job. Shouldn't git-apply simply apply the patch it is given?\n> \n> If there is some reason for git-apply to only apply binary patches\n> when under the duress of --binary, then at the very least it could use\n> a better error message explaining the situation.\n\nI see no reason why git-apply shouldn't always have --binary enabled.\nIf the patch contains full pre-image/post-image blob IDs and we have\nan exact match against the pre-image and we have the post-image\nin our tree it should just apply even if the user didn't supply\n--binary.  If the patch contains a binary delta and we have an\nexact match against the pre-image it should also just apply.\n\nBut if there's a binary hunk and we lack the full pre/post image blob\nIDs, we lack the post image and there's no detla, or the pre-image\ndoesn't exactly match then we should obviously still abort with a\nreasonable error message as there's no sane course of action to take.\n\n-- \nShawn.\n"},{"id":"26487","messageId":"7virk041k8.fsf@assigned-by-dhcp.cox.net","threadId":"5489","inReplyTo":"874pvmxikq.wl%cworth@cworth.org","subject":"Re: Why is there a --binary option needed for git-apply?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-07T06:45:43Z","receivedAt":"2006-09-07T06:45:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carl Worth <cworth@cworth.org> writes:\n\n> Shawn Pearce was kind enough to direct me to the --binary option for\n> git-apply which solved my problem. But that left me wondering why\n> git-apply requires this extra command-line option to do its\n> job. Shouldn't git-apply simply apply the patch it is given?\n\nThere is no reason it shouldn't.\n\nInitially it was done that way only because doing something\nunusual should be done under explicit user permission.  Binary\npatch is not \"unusual\" anymore, I guess ;-).\n\n-- >8 --\n[PATCH] Make apply --binary a no-op.\n\nHistorically we did not allow binary patch applied without an\nexplicit permission from the user, and this flag was the way to\ndo so.  This makes the flag a no-op by always allowing binary\npatch application.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n Documentation/git-apply.txt |   13 ++++---------\n builtin-apply.c             |   19 +++++--------------\n t/t4103-apply-binary.sh     |    4 ++--\n 3 files changed, 11 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex c76cfff..0a6f7b3 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -110,15 +110,10 @@ OPTIONS\n \tdeletion part but not addition part.\n \n --allow-binary-replacement, --binary::\n-\tWhen applying a patch, which is a git-enhanced patch\n-\tthat was prepared to record the pre- and post-image object\n-\tname in full, and the path being patched exactly matches\n-\tthe object the patch applies to (i.e. \"index\" line's\n-\tpre-image object name is what is in the working tree),\n-\tand the post-image object is available in the object\n-\tdatabase, use the post-image object as the patch\n-\tresult.  This allows binary files to be patched in a\n-\tvery limited way.\n+\tHistorically we did not allow binary patch applied\n+\twithout an explicit permission from the user, and this\n+\tflag was the way to do so.  Currently we always allow binary\n+\tpatch application, so this is a no-op.\n \n --exclude=<path-pattern>::\n \tDon't apply changes to files matching the given path pattern. This can\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 872c800..6e0864c 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -28,7 +28,6 @@ static int prefix_length = -1;\n static int newfd = -1;\n \n static int p_value = 1;\n-static int allow_binary_replacement;\n static int check_index;\n static int write_index;\n static int cached;\n@@ -1228,14 +1227,12 @@ static int parse_chunk(char *buffer, uns\n \t\t\t}\n \t\t}\n \n-\t\t/* Empty patch cannot be applied if:\n-\t\t * - it is a binary patch and we do not do binary_replace, or\n-\t\t * - text patch without metadata change\n+\t\t/* Empty patch cannot be applied if it is a text patch\n+\t\t * without metadata change.  A binary patch appears\n+\t\t * empty to us here.\n \t\t */\n \t\tif ((apply || check) &&\n-\t\t    (patch->is_binary\n-\t\t     ? !allow_binary_replacement\n-\t\t     : !metadata_changes(patch)))\n+\t\t    (!patch->is_binary && !metadata_changes(patch)))\n \t\t\tdie(\"patch with only garbage at line %d\", linenr);\n \t}\n \n@@ -1676,11 +1673,6 @@ static int apply_binary(struct buffer_de\n \tunsigned char hdr[50];\n \tint hdrlen;\n \n-\tif (!allow_binary_replacement)\n-\t\treturn error(\"cannot apply binary patch to '%s' \"\n-\t\t\t     \"without --allow-binary-replacement\",\n-\t\t\t     name);\n-\n \t/* For safety, we require patch index line to contain\n \t * full 40-byte textual SHA1 for old and new, at least for now.\n \t */\n@@ -2497,8 +2489,7 @@ int cmd_apply(int argc, const char **arg\n \t\t}\n \t\tif (!strcmp(arg, \"--allow-binary-replacement\") ||\n \t\t    !strcmp(arg, \"--binary\")) {\n-\t\t\tallow_binary_replacement = 1;\n-\t\t\tcontinue;\n+\t\t\tcontinue; /* now no-op */\n \t\t}\n \t\tif (!strcmp(arg, \"--numstat\")) {\n \t\t\tapply = 0;\ndiff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh\nindex ff05269..e2b1124 100755\n--- a/t/t4103-apply-binary.sh\n+++ b/t/t4103-apply-binary.sh\n@@ -94,11 +94,11 @@ test_expect_failure 'apply binary diff (\n \t'do_reset\n \t git-apply --index C.diff'\n \n-test_expect_failure 'apply binary diff without replacement -- should fail.' \\\n+test_expect_success 'apply binary diff without replacement.' \\\n \t'do_reset\n \t git-apply BF.diff'\n \n-test_expect_failure 'apply binary diff without replacement (copy) -- should fail.' \\\n+test_expect_success 'apply binary diff without replacement (copy).' \\\n \t'do_reset\n \t git-apply CF.diff'\n \n-- \n1.4.2.gfce41\n"}]}