{"thread":{"id":"43218","subject":"Re: Problem with git-apply?","startedAt":"2006-11-04T07:23:49Z","lastAt":"2006-11-04T18:33:06Z","messageCount":11,"participants":["Kevin Shanahan","Junio C Hamano","Shawn Pearce","Jakub Narebski","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"296021","messageId":"20061104072349.GA19667@cubit","threadId":"43218","inReplyTo":null,"subject":"Problem with git-apply?","fromName":"Kevin Shanahan","fromEmail":"kmshanah@disenchant.net","sentAt":"2006-11-04T07:23:49Z","receivedAt":"2006-11-04T07:23:49Z","isPatch":false,"sender":{"key":"kmshanah@disenchant.net","avatar":null},"body":"Hi,\n\nI seem to be having problems using git-apply to apply any patch which\ncreates a new file. Unfortunately it's been a few weeks since I last\nused git here locally, but this seems like some behaviour change since\nthe last version I was using. I'm currently using version 1.4.3.3 from\nDebian Sid. This little test script demonstrates the problem I'm\nhaving:\n\n#!/bin/sh\n\nmkdir a b repo\necho foo > b/foo\ndiff -urN a b > test.diff\n\n(\n    cd repo\n    echo bar > bar # need something to init the db\n    git init-db\n    git add .\n    git commit -a -m \"Test Commit\"\n\n    git apply ../test.diff\n    git commit -a -m \"Test Commit (file added)\"\n)\n\nHere, this outputs for me:\n\ndefaulting to local storage area\nCommitting initial tree ee314a31b622b027c10981acaed7903a3607dbd4\nerror: foo: No such file or directory\nnothing to commit\n\nHas something broken or am I doing something wrong here?\n\nThanks,\n"},{"id":"296805","messageId":"20061104073000.GB9422@spearce.org","threadId":"43218","inReplyTo":"20061104072349.GA19667@cubit","subject":"Re: Problem with git-apply?","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-11-04T07:30:00Z","receivedAt":"2006-11-04T07:30:00Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Kevin Shanahan <kmshanah@disenchant.net> wrote:\n>     git apply ../test.diff\n\nTry:\n\n      git apply --index ../test.diff\n\ninstead.  The --index is needed to record the new file in the index;\notherwise it won't actually be added during commit.\n\n-- \n"},{"id":"295575","messageId":"20061104080751.GB19667@cubit","threadId":"43218","inReplyTo":"20061104072349.GA19667@cubit","subject":"Re: Problem with git-apply?","fromName":"Kevin Shanahan","fromEmail":"kmshanah@disenchant.net","sentAt":"2006-11-04T08:07:51Z","receivedAt":"2006-11-04T08:07:51Z","isPatch":false,"sender":{"key":"kmshanah@disenchant.net","avatar":null},"body":"On Sat, Nov 04, 2006 at 05:53:49PM +1030, Kevin Shanahan wrote:\n> I seem to be having problems using git-apply to apply any patch which\n> creates a new file. Unfortunately it's been a few weeks since I last\n> used git here locally, but this seems like some behaviour change since\n> the last version I was using. I'm currently using version 1.4.3.3 from\n> Debian Sid. This little test script demonstrates the problem I'm\n> having:\n\nSorry, I probably should have said to CC me, as I'm not subscribed.\n\nThanks for your reply Shawn, but --index doesn't seem to make any\ndifference. If it helps, I can confirm that the script works fine with\nversion 1.4.1.1.\n\n> #!/bin/sh\n> \n> mkdir a b repo\n> echo foo > b/foo\n> diff -urN a b > test.diff\n> \n> (\n>     cd repo\n>     echo bar > bar # need something to init the db\n>     git init-db\n>     git add .\n>     git commit -a -m \"Test Commit\"\n> \n>     git apply ../test.diff\n>     git commit -a -m \"Test Commit (file added)\"\n> )\n> \n> Here, this outputs for me:\n> \n> defaulting to local storage area\n> Committing initial tree ee314a31b622b027c10981acaed7903a3607dbd4\n> error: foo: No such file or directory\n> nothing to commit\n> \n> Has something broken or am I doing something wrong here?\n> \n> Thanks,\n"},{"id":"297960","messageId":"eihmas$8sj$1@sea.gmane.org","threadId":"43218","inReplyTo":"20061104072349.GA19667@cubit","subject":"Re: Problem with git-apply?","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-04T09:26:13Z","receivedAt":"2006-11-04T09:26:13Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Kevin Shanahan wrote:\n\n> I seem to be having problems using git-apply to apply any patch which\n> creates a new file. Unfortunately it's been a few weeks since I last\n> used git here locally, but this seems like some behaviour change since\n> the last version I was using. I'm currently using version 1.4.3.3 from\n> Debian Sid. This little test script demonstrates the problem I'm\n> having:\n> \n> #!/bin/sh\n> \n> mkdir a b repo\n> echo foo > b/foo\n> diff -urN a b > test.diff\n\nWhich produces the following diff:\n\ndiff -urN a/foo b/foo\n--- a/foo       1970-01-01 01:00:00.000000000 +0100\n+++ b/foo       2006-11-04 10:05:04.000000000 +0100\n@@ -0,0 +1 @@\n+foo\n \n> (\n>     cd repo\n>     echo bar > bar # need something to init the db\n>     git init-db\n>     git add .\n>     git commit -a -m \"Test Commit\"\n> \n>     git apply ../test.diff\n>     git commit -a -m \"Test Commit (file added)\"\n> )\n> \n> Here, this outputs for me:\n> \n> defaulting to local storage area\n> Committing initial tree ee314a31b622b027c10981acaed7903a3607dbd4\n> error: foo: No such file or directory\n> nothing to commit\n\nThis I think is a bug, or rather misfeature in git-apply (or at least\ninconsistency between GNU diff and git patch format). But if you change\nfrom-file line from \"--- a/foo\" to \"--- /dev/null\" then git-apply happily\napplies creation patch and creates file.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"294832","messageId":"7v8xir4k3w.fsf@assigned-by-dhcp.cox.net","threadId":"43218","inReplyTo":"20061104072349.GA19667@cubit","subject":"Re: Problem with git-apply?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-04T09:49:39Z","receivedAt":"2006-11-04T09:49:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Shanahan <kmshanah@disenchant.net> writes:\n\n> #!/bin/sh\n>\n> mkdir a b repo\n> echo foo > b/foo\n> diff -urN a b > test.diff\n\nIt is *very* surprising that this issue did not come up earlier,\ngiven that we used to use GNU diff internally to generate our\nown diff.\n\nIf you cat the test.diff file, you will see \"a/foo\" and \"b/foo\",\nnot \"/dev/null\".\n\nThe problem appears that GNU diff _never_ uses \"--- /dev/null\"\nor \"+++ /dev/null\" to indicate creation or deletion of the file,\nbut the \"traditional patch parser\" builtin-apply has assumed\nthat is what the traditional diff output from day one.  Where we\ngot that idea is mystery to me (this is Linus's code), but I\nsuspect it is what other SCMs did.\n\nWith the GNU diff output, the only way to guess it is a file\ncreation patch is to notice that the patch has only a single\nhunk that inserts into -0,0 (this tests that the file is\noriginally empty), and it is followed by a HT and the UNIX epoch\n(i.e. 1970-01-01 00:00:00 GMT) timestamp in localtime + offset\nformat (the latter is a guess to tell creation from modification\nto a file that was originally empty).  This means that we do not\nhave a way to tell if it was a file that had UNIX epoch\ntimestamp that was modified or if this is a creation.\n\nHaving to parse the localtime + offset and compare it with UNIX\nepoch is already crazy, but the saddest part is that this is\nquite GNU diff specific right now, and I'd rather not to require\nthe trailing HT + timestamp (which is now being written down in\nnew POSIX) to the \"traditional patch\" input format.\n\nProbably a reasonable compromise is to treat a non-git patch\n(i.e. the ones that does not begin with \"diff --git\") that\ntouches an empty file as a file creation patch, but I need to\nlook at the code to see how much damage such a change needs to\ncause.  If I remember collectly it wants to decide if it is a\ncreation patch or modification patch upfront, so it may not be \nas trivial as it sounds.  We'll see.\n"},{"id":"294824","messageId":"7v4ptf4jmj.fsf@assigned-by-dhcp.cox.net","threadId":"43218","inReplyTo":"7v8xir4k3w.fsf@assigned-by-dhcp.cox.net","subject":"Re: Problem with git-apply?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-04T10:00:04Z","receivedAt":"2006-11-04T10:00:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Kevin Shanahan <kmshanah@disenchant.net> writes:\n>\n>> #!/bin/sh\n>>\n>> mkdir a b repo\n>> echo foo > b/foo\n>> diff -urN a b > test.diff\n>\n> It is *very* surprising that this issue did not come up earlier,\n> given that we used to use GNU diff internally to generate our\n> own diff.\n>\n> If you cat the test.diff file, you will see \"a/foo\" and \"b/foo\",\n> not \"/dev/null\".\n>\n> The problem appears that GNU diff _never_ uses \"--- /dev/null\"\n> or \"+++ /dev/null\" to indicate creation or deletion of the file,\n> but the \"traditional patch parser\" builtin-apply has assumed\n> that is what the traditional diff output from day one.  Where we\n> got that idea is mystery to me (this is Linus's code), but I\n> suspect it is what other SCMs did.\n\n*BLUSH*  A prime example of \"you should not speak before\nthinking\".\n\nPlease forget everything I said.  The patch parsing is just fine\nwith or without \"/dev/null\".  This must be a recent breakage\naround write_out_one_result().  Will take a look.\n"},{"id":"294293","messageId":"20061104101200.GC19667@cubit","threadId":"43218","inReplyTo":"eihmas$8sj$1@sea.gmane.org","subject":"Re: Problem with git-apply?","fromName":"Kevin Shanahan","fromEmail":"kmshanah@disenchant.net","sentAt":"2006-11-04T10:12:00Z","receivedAt":"2006-11-04T10:12:00Z","isPatch":false,"sender":{"key":"kmshanah@disenchant.net","avatar":null},"body":"On Sat, Nov 04, 2006 at 10:26:13AM +0100, Jakub Narebski wrote:\n> This I think is a bug, or rather misfeature in git-apply (or at least\n> inconsistency between GNU diff and git patch format). But if you change\n> from-file line from \"--- a/foo\" to \"--- /dev/null\" then git-apply happily\n> applies creation patch and creates file.\n\nThanks. At least that gives me a workaround for now.\n\nCheers,\n"},{"id":"294681","messageId":"7vu01f34bo.fsf@assigned-by-dhcp.cox.net","threadId":"43218","inReplyTo":"7v4ptf4jmj.fsf@assigned-by-dhcp.cox.net","subject":"Re: Problem with git-apply?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-04T10:15:55Z","receivedAt":"2006-11-04T10:15:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> *BLUSH*  A prime example of \"you should not speak before\n> thinking\".\n>\n> Please forget everything I said.  The patch parsing is just fine\n> with or without \"/dev/null\".  This must be a recent breakage\n> around write_out_one_result().  Will take a look.\n\n\nActually it was a small problem in the patch parsing code.  Can\nyou give this a try?\n\n---\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 11397f5..db7cdce 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1043,10 +1043,14 @@ static int parse_single_patch(char *line\n \t\t * then not having oldlines means the patch is creation,\n \t\t * and not having newlines means the patch is deletion.\n \t\t */\n-\t\tif (patch->is_new < 0 && !oldlines)\n+\t\tif (patch->is_new < 0 && !oldlines) {\n \t\t\tpatch->is_new = 1;\n-\t\tif (patch->is_delete < 0 && !newlines)\n+\t\t\tpatch->old_name = NULL;\n+\t\t}\n+\t\tif (patch->is_delete < 0 && !newlines) {\n \t\t\tpatch->is_delete = 1;\n+\t\t\tpatch->new_name = NULL;\n+\t\t}\n \t}\n \n \tif (0 < patch->is_new && oldlines)\n"},{"id":"294056","messageId":"20061104104913.GD19667@cubit","threadId":"43218","inReplyTo":"7vu01f34bo.fsf@assigned-by-dhcp.cox.net","subject":"Re: Problem with git-apply?","fromName":"Kevin Shanahan","fromEmail":"kmshanah@disenchant.net","sentAt":"2006-11-04T10:49:13Z","receivedAt":"2006-11-04T10:49:13Z","isPatch":false,"sender":{"key":"kmshanah@disenchant.net","avatar":null},"body":"On Sat, Nov 04, 2006 at 02:15:55AM -0800, Junio C Hamano wrote:\n> Actually it was a small problem in the patch parsing code.  Can\n> you give this a try?\n\nYes, this patch works for me.\n\nCheers,\nKev.\n\n> ---\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index 11397f5..db7cdce 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -1043,10 +1043,14 @@ static int parse_single_patch(char *line\n>  \t\t * then not having oldlines means the patch is creation,\n>  \t\t * and not having newlines means the patch is deletion.\n>  \t\t */\n> -\t\tif (patch->is_new < 0 && !oldlines)\n> +\t\tif (patch->is_new < 0 && !oldlines) {\n>  \t\t\tpatch->is_new = 1;\n> -\t\tif (patch->is_delete < 0 && !newlines)\n> +\t\t\tpatch->old_name = NULL;\n> +\t\t}\n> +\t\tif (patch->is_delete < 0 && !newlines) {\n>  \t\t\tpatch->is_delete = 1;\n> +\t\t\tpatch->new_name = NULL;\n> +\t\t}\n>  \t}\n>  \n>  \tif (0 < patch->is_new && oldlines)\n"},{"id":"298741","messageId":"Pine.LNX.4.64.0611040821290.25218@g5.osdl.org","threadId":"43218","inReplyTo":"7v8xir4k3w.fsf@assigned-by-dhcp.cox.net","subject":"Re: Problem with git-apply?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-11-04T16:27:19Z","receivedAt":"2006-11-04T16:27:19Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 4 Nov 2006, Junio C Hamano wrote:\n> \n> The problem appears that GNU diff _never_ uses \"--- /dev/null\" or \"+++ \n> /dev/null\" to indicate creation or deletion of the file, but the \n> \"traditional patch parser\" builtin-apply has assumed that is what the \n> traditional diff output from day one.  Where we got that idea is mystery \n> to me (this is Linus's code), but I suspect it is what other SCMs did.\n\nNo, the original code used to trigger a \"create\" diff on\n - source was /dev/null\nOR\n - source had zero patches.\n\nIt used to have code like\n\n        if (patch->is_new < 0) {\n                patch->is_new =  !oldlines;\n                if (!oldlines)\n                        patch->old_name = NULL;\n        }\n        if (patch->is_delete < 0) {\n                patch->is_delete = !newlines;\n                if (!newlines)\n                        patch->new_name = NULL;\n        }\n\nand I think the person who broke it was you ;)\n\nAccording to git-pickaxe, the buggy commit is 4be60962.\n\nYou should know by now that I never have bugs. \n\n"},{"id":"296391","messageId":"7vzmb7vz8d.fsf@assigned-by-dhcp.cox.net","threadId":"43218","inReplyTo":"Pine.LNX.4.64.0611040821290.25218@g5.osdl.org","subject":"Re: Problem with git-apply?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-04T18:33:06Z","receivedAt":"2006-11-04T18:33:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> On Sat, 4 Nov 2006, Junio C Hamano wrote:\n>> \n>> The problem appears that GNU diff _never_ uses \"--- /dev/null\" or \"+++ \n>> /dev/null\" to indicate creation or deletion of the file, but the \n>> \"traditional patch parser\" builtin-apply has assumed that is what the \n>> traditional diff output from day one.  Where we got that idea is mystery \n>> to me (this is Linus's code), but I suspect it is what other SCMs did.\n>\n> No, the original code used to trigger a \"create\" diff on\n>  - source was /dev/null\n> OR\n>  - source had zero patches.\n>\n> It used to have code like\n>...\n> and I think the person who broke it was you ;)\n\nSorry, if you followed the threads by now you know I know that.\nA few messages down there is a fix for the breakage caused by\n4be60962, which I'll have in 'maint'.\n\nI think it is a time for a 1.4.3.4; these are queued since\nwe did 1.4.3.3.\n\nAndy Parkins (2):\n  Minor grammar fixes for git-diff-index.txt\n  git-clone documentation didn't mention --origin as equivalent of -o\n\nChristian Couder (3):\n  Remove --syslog in git-daemon inetd documentation examples.\n  Documentation: add upload-archive service to git-daemon.\n  Documentation: add git in /etc/services.\n\nEdgar Toernig (1):\n  Use memmove instead of memcpy for overlapping areas\n\nJ. Bruce Fields (1):\n  Documentation: updates to \"Everyday GIT\"\n\nJakub Narebski (3):\n  diff-format.txt: Combined diff format documentation supplement\n  diff-format.txt: Correct information about pathnames quoting in patch format\n  gitweb: Check git base URLs before generating URL from it\n\nJan Harkes (1):\n  Continue traversal when rev-list --unpacked finds a packed commit.\n\nJohannes Schindelin (1):\n  link_temp_to_file: call adjust_shared_perm() only when we created the directory\n\nJunio C Hamano (9):\n  Documentation: clarify refname disambiguation rules.\n  combine-diff: a few more finishing touches.\n  combine-diff: fix hunk_comment_line logic.\n  combine-diff: honour --no-commit-id\n  Surround \"#define DEBUG 0\" with \"#ifndef DEBUG..#endif\"\n  quote.c: ensure the same quoting across platforms.\n  revision traversal: --unpacked does not limit commit list anymore.\n  link_temp_to_file: don't leave the path truncated on adjust_shared_perm failure\n  apply: handle \"traditional\" creation/deletion diff correctly.\n\nNicolas Pitre (1):\n  pack-objects doesn't create random pack names\n\nRene Scharfe (1):\n  git-cherry: document limit and add diagram\n\n"}]}