{"thread":{"id":"8886","subject":"git-apply{,mbox,patch} should default to --unidiff-zero","startedAt":"2007-07-05T23:22:10Z","lastAt":"2007-07-06T12:49:51Z","messageCount":10,"participants":["Adrian Bunk","Johannes Schindelin","Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"46576","messageId":"20070705232210.GR3492@stusta.de","threadId":"8886","inReplyTo":null,"subject":"git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Adrian Bunk","fromEmail":"bunk@stusta.de","sentAt":"2007-07-05T23:22:10Z","receivedAt":"2007-07-05T23:22:10Z","isPatch":false,"sender":{"key":"bunk@stusta.de","avatar":null},"body":"I just ran into the following issue:\n\nI sent someone a patch that purposefully contained a chunk without \ncontext, and git-apply of the recipient refused to apply it without\nan explicit --unidiff-zero.\n\ngit-apply{,mbox,patch} should default to doing --unidiff-zero:\nGenerating a patch without context is something I have to do explicitely \nby giving \"diff\" an option or by manually editing the patch. I know \nabout the dangers of having no context, but there are use cases where\nI know that replacing and/or deleting one or more lines is safe even \nwithout context and where I want to avoid context e.g. for avoiding to \nclash with other patches.\n\nExample use case:\nLook at the file Documentation/feature-removal-schedule.txt in the Linux \nkernel. If I want to send someone two independent patches removing \nadjanced entries in this file, the patches can be applied in any order \nexactly as long as this chunk does not contain any context. Removing an \nentry from this file is obviously safe even without any context.\n\nTIA\nAdrian\n\nBTW: Please Cc me on replies.\n\n-- \n\n \"Is there not promise of rain?\" Ling Tan asked suddenly out\n of the darkness. There had been need of rain for many days.\n \"Only a promise,\" Lao Er said.\n Pearl S. Buck - Dragon Seed\n"},{"id":"46583","messageId":"Pine.LNX.4.64.0707060217460.9789@racer.site","threadId":"8886","inReplyTo":"20070705232210.GR3492@stusta.de","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-06T01:18:46Z","receivedAt":"2007-07-06T01:18:46Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jul 2007, Adrian Bunk wrote:\n\n> git-apply{,mbox,patch} should default to doing --unidiff-zero:\n\nBut is that not dangerous? At least now the committer has some safeguard \nagainst this kind of mistakes. Because you can easily introduce mistakes \nthat way.\n\nCiao,\nDscho\n"},{"id":"46585","messageId":"20070706014222.GK3492@stusta.de","threadId":"8886","inReplyTo":"Pine.LNX.4.64.0707060217460.9789@racer.site","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Adrian Bunk","fromEmail":"bunk@stusta.de","sentAt":"2007-07-06T01:42:23Z","receivedAt":"2007-07-06T01:42:23Z","isPatch":false,"sender":{"key":"bunk@stusta.de","avatar":null},"body":"On Fri, Jul 06, 2007 at 02:18:46AM +0100, Johannes Schindelin wrote:\n\n> Hi,\n\nHi Johannes,\n\n> On Fri, 6 Jul 2007, Adrian Bunk wrote:\n> \n> > git-apply{,mbox,patch} should default to doing --unidiff-zero:\n> \n> But is that not dangerous? At least now the committer has some safeguard \n> against this kind of mistakes. Because you can easily introduce mistakes \n> that way.\n\nyou are saying \"easily\".\n\nDid you ever actually run into such a problem?\n\nYou must do something like \"diff -U0\" or manually editing patches for \ncreating such patches, and that's very unusual.\n\nAnd although GNU patch (which has a much bigger userbase than git) \napplies such patches without any warning I don't remember having ever \nseen what you call \"easily\".\n\n> Ciao,\n> Dscho\n\ncu\nAdrian\n\n-- \n\n \"Is there not promise of rain?\" Ling Tan asked suddenly out\n of the darkness. There had been need of rain for many days.\n \"Only a promise,\" Lao Er said.\n Pearl S. Buck - Dragon Seed\n"},{"id":"46586","messageId":"Pine.LNX.4.64.0707060243110.4093@racer.site","threadId":"8886","inReplyTo":"20070706014222.GK3492@stusta.de","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-06T01:51:07Z","receivedAt":"2007-07-06T01:51:07Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jul 2007, Adrian Bunk wrote:\n\n> On Fri, Jul 06, 2007 at 02:18:46AM +0100, Johannes Schindelin wrote:\n> \n> > On Fri, 6 Jul 2007, Adrian Bunk wrote:\n> > \n> > > git-apply{,mbox,patch} should default to doing --unidiff-zero:\n> > \n> > But is that not dangerous? At least now the committer has some \n> > safeguard against this kind of mistakes. Because you can easily \n> > introduce mistakes that way.\n> \n> you are saying \"easily\".\n> \n> Did you ever actually run into such a problem?\n\nNot yet, thankfully.\n\n> You must do something like \"diff -U0\" or manually editing patches for \n> creating such patches, and that's very unusual.\n\nThe point is that the _committer_ is not necessarily involved in that \nbusiness.\n\nAnd \"git apply\" is strict for a reason. It catches possibly unwanted \nthings much earlier than patch. I _want_ to be warned that somebody is \nintroducing some code at a certain position, which might, or might not be \ncorrect. apply has no way to tell, since there is no context to at least \nminimally verify.\n\n> And although GNU patch (which has a much bigger userbase than git) \n> applies such patches without any warning I don't remember having ever \n> seen what you call \"easily\".\n\nGNU patch is very sloppy. And I had to fix up quite a number of patches \nwhich were \"successfully\" applied, but did not do what they were supposed \nto do. The recent \"GNU patch applies _indented_ _context_ diffs\" fracass \nis only one example why I prefer git apply.\n\nUnfortunately, I do not off-hand remember if I had to fix up a \nunified-zero patch that GNU patch applied, but I do know this:\n\n\tif \"git am\" learns to apply unified-zero by default, the first \n\tthing I will do is patch it in my Git branch to _not_ do that. I \n\tdo _not_ want that. I want to be warned.\n\n\tI can still decide that it is probably okay, but I will make \n\t_damned_ _well_ sure afterwards that it did something sensible. I \n\twill _only_ apply such a scrutiny when git apply refused to apply \n\ta unified-zero patch, and I decided to apply it nevertheless.\n\nCiao,\nDscho\n"},{"id":"46587","messageId":"20070706022629.GL3492@stusta.de","threadId":"8886","inReplyTo":"Pine.LNX.4.64.0707060243110.4093@racer.site","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Adrian Bunk","fromEmail":"bunk@stusta.de","sentAt":"2007-07-06T02:26:29Z","receivedAt":"2007-07-06T02:26:29Z","isPatch":false,"sender":{"key":"bunk@stusta.de","avatar":null},"body":"On Fri, Jul 06, 2007 at 02:51:07AM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Fri, 6 Jul 2007, Adrian Bunk wrote:\n> \n> > On Fri, Jul 06, 2007 at 02:18:46AM +0100, Johannes Schindelin wrote:\n> > \n> > > On Fri, 6 Jul 2007, Adrian Bunk wrote:\n> > > \n> > > > git-apply{,mbox,patch} should default to doing --unidiff-zero:\n> > > \n> > > But is that not dangerous? At least now the committer has some \n> > > safeguard against this kind of mistakes. Because you can easily \n> > > introduce mistakes that way.\n> > \n> > you are saying \"easily\".\n> > \n> > Did you ever actually run into such a problem?\n> \n> Not yet, thankfully.\n> \n> > You must do something like \"diff -U0\" or manually editing patches for \n> > creating such patches, and that's very unusual.\n> \n> The point is that the _committer_ is not necessarily involved in that \n> business.\n> \n> And \"git apply\" is strict for a reason. It catches possibly unwanted \n> things much earlier than patch. I _want_ to be warned that somebody is \n> introducing some code at a certain position, which might, or might not be \n> correct. apply has no way to tell, since there is no context to at least \n> minimally verify.\n>...\n\nThat's wrong.\n\nMy use cases are replacing or deleting lines.\n\nIn these cases there is context in the deleted lines that is already \nbeing verified even with --unidiff-zero.\n\n> Ciao,\n> Dscho\n\ncu\nAdrian\n\n-- \n\n \"Is there not promise of rain?\" Ling Tan asked suddenly out\n of the darkness. There had been need of rain for many days.\n \"Only a promise,\" Lao Er said.\n Pearl S. Buck - Dragon Seed\n"},{"id":"46589","messageId":"Pine.LNX.4.64.0707060413190.4093@racer.site","threadId":"8886","inReplyTo":"20070706022629.GL3492@stusta.de","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-06T03:16:08Z","receivedAt":"2007-07-06T03:16:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jul 2007, Adrian Bunk wrote:\n\n> On Fri, Jul 06, 2007 at 02:51:07AM +0100, Johannes Schindelin wrote:\n> > \n> > On Fri, 6 Jul 2007, Adrian Bunk wrote:\n> > \n> > > You must do something like \"diff -U0\" or manually editing patches \n> > > for creating such patches, and that's very unusual.\n> > \n> > The point is that the _committer_ is not necessarily involved in that \n> > business.\n\nBTW this still holds true, and you have not addressed that. It really is \na serious issue. \"git apply\" is a committer's tool. So it should help \nthe committer.\n\n> > And \"git apply\" is strict for a reason. It catches possibly unwanted \n> > things much earlier than patch. I _want_ to be warned that somebody is \n> > introducing some code at a certain position, which might, or might not \n> > be correct. apply has no way to tell, since there is no context to at \n> > least minimally verify.\n> >...\n> \n> That's wrong.\n> \n> My use cases are replacing or deleting lines.\n\nThat is _your_ use case.\n\n> In these cases there is context in the deleted lines that is already \n> being verified even with --unidiff-zero.\n\nWith --unidiff-zero, also _adding_ lines will be handled as if there were \nno problem.\n\nYes, in your case it fixes a problem.\n\nYet, in other cases it introduces a problem.\n\nOkay?\n\nCiao,\nDscho\n"},{"id":"46593","messageId":"alpine.LFD.0.98.0707052108070.9434@woody.linux-foundation.org","threadId":"8886","inReplyTo":"Pine.LNX.4.64.0707060413190.4093@racer.site","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-07-06T04:12:17Z","receivedAt":"2007-07-06T04:12:17Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 6 Jul 2007, Johannes Schindelin wrote:\n> \n> With --unidiff-zero, also _adding_ lines will be handled as if there were \n> no problem.\n> \n> Yes, in your case it fixes a problem.\n> \n> Yet, in other cases it introduces a problem.\n\nWell, we could make the rule be that ew require --unidiff-zero only if \nthere really is _no_ old data to verify in a hunk. No deleted lines, and \nno context around it.\n\nAdrian has a point in that if there are lines to be deleted, that in \nitself is context, and then the strict behaviour of \"git-apply\" is \narguably unnecessaily strict.\n\nThat said, I do absolutely _hate_ how GNU patch will basically apply \nrandom line noise without complaints. So git-apply is designed to be much \nstricter on _so_ many levels. The thing that I personally always really \ndetested about GNU patch was how it would apply part of a patch, then fail \nhalf-way, and leave the partial patch applied!\n\ngit-apply is about a million times better than standard \"patch\", exactly \nbecause it tries to make sure that what it does makes sense, and you \nactually need to use explicit flags to make it do things that may be hard \nto undo or slightly questionable.\n\n\t\tLinus\n"},{"id":"46595","messageId":"7vd4z6gkbk.fsf@assigned-by-dhcp.cox.net","threadId":"8886","inReplyTo":"alpine.LFD.0.98.0707052108070.9434@woody.linux-foundation.org","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-06T05:41:51Z","receivedAt":"2007-07-06T05:41:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Well, we could make the rule be that we require --unidiff-zero only if \n> there really is _no_ old data to verify in a hunk. No deleted lines, and \n> no context around it.\n\nThere are two things --unidiff-zero affects, because git-apply\nneeds to disable otherwise reasonable sanity checks it does for\nsafety:\n\n - When we see a patch that has only one hunk, and its change\n consists of only deletion, we can verify and complain if the\n header does not say it delete the file. Not so if the patch\n was created without \"diff -u0\". The same applies for \"only\n addition\" vs. \"creation of the file\".\n\n - When there is no leading context in the hunk, it usually has\n to match only at the beginning of the file (same for\n \"following context\" vs \"at the end of the file\"), and we do\n perform this sanity check. However, \"diff -u0\" patch needs\n to bypass it, as not having any context is the norm.\n\nBecause \"diff -u0\" is unusual, these sanity checks are disabled\nonly when the user explicitly says --unidiff-zero when applying.\n\n> Adrian has a point in that if there are lines to be deleted, that in \n> itself is context, and then the strict behaviour of \"git-apply\" is \n> arguably unnecessaily strict.\n\nNot really. That is true, unless you have two identical\ninstances of the group of lines being deleted, in which case you\ncannot safely tell which instance is to be removed.\n\n> That said, I do absolutely _hate_ how GNU patch will basically apply \n> random line noise without complaints. So git-apply is designed to be much \n> stricter on _so_ many levels. The thing that I personally always really \n> detested about GNU patch was how it would apply part of a patch, then fail \n> half-way, and leave the partial patch applied!\n>\n> git-apply is about a million times better than standard \"patch\", exactly \n> because it tries to make sure that what it does makes sense, and you \n> actually need to use explicit flags to make it do things that may be hard \n> to undo or slightly questionable.\n\nNo question about it.\n"},{"id":"46615","messageId":"20070706121441.GM3492@stusta.de","threadId":"8886","inReplyTo":"7vd4z6gkbk.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Adrian Bunk","fromEmail":"bunk@stusta.de","sentAt":"2007-07-06T12:14:41Z","receivedAt":"2007-07-06T12:14:41Z","isPatch":false,"sender":{"key":"bunk@stusta.de","avatar":null},"body":"On Thu, Jul 05, 2007 at 10:41:51PM -0700, Junio C Hamano wrote:\n> Linus Torvalds writes:\n>...\n> > Adrian has a point in that if there are lines to be deleted, that in \n> > itself is context, and then the strict behaviour of \"git-apply\" is \n> > arguably unnecessaily strict.\n> \n> Not really. That is true, unless you have two identical\n> instances of the group of lines being deleted, in which case you\n> cannot safely tell which instance is to be removed.\n>...\n\nThe interesting thing is that you can never safely tell it for any \namount of context - I've seen patches with three lines of context being \napplied at the wrong place simply because there were several matching \ncontexts.\n\ncu\nAdrian\n\n-- \n\n \"Is there not promise of rain?\" Ling Tan asked suddenly out\n of the darkness. There had been need of rain for many days.\n \"Only a promise,\" Lao Er said.\n Pearl S. Buck - Dragon Seed\n"},{"id":"46621","messageId":"Pine.LNX.4.64.0707061348050.4093@racer.site","threadId":"8886","inReplyTo":"20070706121441.GM3492@stusta.de","subject":"Re: git-apply{,mbox,patch} should default to --unidiff-zero","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-06T12:49:51Z","receivedAt":"2007-07-06T12:49:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Jul 2007, Adrian Bunk wrote:\n\n> On Thu, Jul 05, 2007 at 10:41:51PM -0700, Junio C Hamano wrote:\n> > Linus Torvalds writes:\n> >...\n> > > Adrian has a point in that if there are lines to be deleted, that in \n> > > itself is context, and then the strict behaviour of \"git-apply\" is \n> > > arguably unnecessaily strict.\n> > \n> > Not really. That is true, unless you have two identical instances of \n> > the group of lines being deleted, in which case you cannot safely tell \n> > which instance is to be removed.\n> >...\n> \n> The interesting thing is that you can never safely tell it for any \n> amount of context - I've seen patches with three lines of context being \n> applied at the wrong place simply because there were several matching \n> contexts.\n\nYes, that is right. You can never safely tell. But now you want to allow \neven less context by default. In which you can even \"more neverer\" safely \ntell. That is why I am disagreeing with that change.\n\nCiao,\nDscho\n"}]}