git-apply{,mbox,patch} should default to --unidiff-zero

7 messages, 3 authors, 2016-06-15 · open the first message on its own page

git-apply{,mbox,patch} should default to --unidiff-zero

From: Adrian Bunk <hidden>
Date: 2016-06-15 22:43:19

I just ran into the following issue:

I sent someone a patch that purposefully contained a chunk without 
context, and git-apply of the recipient refused to apply it without
an explicit --unidiff-zero.

git-apply{,mbox,patch} should default to doing --unidiff-zero:
Generating a patch without context is something I have to do explicitely 
by giving "diff" an option or by manually editing the patch. I know 
about the dangers of having no context, but there are use cases where
I know that replacing and/or deleting one or more lines is safe even 
without context and where I want to avoid context e.g. for avoiding to 
clash with other patches.

Example use case:
Look at the file Documentation/feature-removal-schedule.txt in the Linux 
kernel. If I want to send someone two independent patches removing 
adjanced entries in this file, the patches can be applied in any order 
exactly as long as this chunk does not contain any context. Removing an 
entry from this file is obviously safe even without any context.

TIA
Adrian

BTW: Please Cc me on replies.

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: git-apply{,mbox,patch} should default to --unidiff-zero

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:19

Hi,

On Fri, 6 Jul 2007, Adrian Bunk wrote:
git-apply{,mbox,patch} should default to doing --unidiff-zero:
But is that not dangerous? At least now the committer has some safeguard 
against this kind of mistakes. Because you can easily introduce mistakes 
that way.

Ciao,
Dscho

Re: git-apply{,mbox,patch} should default to --unidiff-zero

From: Adrian Bunk <hidden>
Date: 2016-06-15 22:43:19

On Fri, Jul 06, 2007 at 02:18:46AM +0100, Johannes Schindelin wrote:
Hi,
Hi Johannes,
On Fri, 6 Jul 2007, Adrian Bunk wrote:
quoted
git-apply{,mbox,patch} should default to doing --unidiff-zero:
But is that not dangerous? At least now the committer has some safeguard 
against this kind of mistakes. Because you can easily introduce mistakes 
that way.
you are saying "easily".

Did you ever actually run into such a problem?

You must do something like "diff -U0" or manually editing patches for 
creating such patches, and that's very unusual.

And although GNU patch (which has a much bigger userbase than git) 
applies such patches without any warning I don't remember having ever 
seen what you call "easily".
Ciao,
Dscho
cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: git-apply{,mbox,patch} should default to --unidiff-zero

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:19

Hi,

On Fri, 6 Jul 2007, Adrian Bunk wrote:
On Fri, Jul 06, 2007 at 02:18:46AM +0100, Johannes Schindelin wrote:
quoted
On Fri, 6 Jul 2007, Adrian Bunk wrote:
quoted
git-apply{,mbox,patch} should default to doing --unidiff-zero:
But is that not dangerous? At least now the committer has some 
safeguard against this kind of mistakes. Because you can easily 
introduce mistakes that way.
you are saying "easily".

Did you ever actually run into such a problem?
Not yet, thankfully.
You must do something like "diff -U0" or manually editing patches for 
creating such patches, and that's very unusual.
The point is that the _committer_ is not necessarily involved in that 
business.

And "git apply" is strict for a reason. It catches possibly unwanted 
things much earlier than patch. I _want_ to be warned that somebody is 
introducing some code at a certain position, which might, or might not be 
correct. apply has no way to tell, since there is no context to at least 
minimally verify.
And although GNU patch (which has a much bigger userbase than git) 
applies such patches without any warning I don't remember having ever 
seen what you call "easily".
GNU patch is very sloppy. And I had to fix up quite a number of patches 
which were "successfully" applied, but did not do what they were supposed 
to do. The recent "GNU patch applies _indented_ _context_ diffs" fracass 
is only one example why I prefer git apply.

Unfortunately, I do not off-hand remember if I had to fix up a 
unified-zero patch that GNU patch applied, but I do know this:

	if "git am" learns to apply unified-zero by default, the first 
	thing I will do is patch it in my Git branch to _not_ do that. I 
	do _not_ want that. I want to be warned.

	I can still decide that it is probably okay, but I will make 
	_damned_ _well_ sure afterwards that it did something sensible. I 
	will _only_ apply such a scrutiny when git apply refused to apply 
	a unified-zero patch, and I decided to apply it nevertheless.

Ciao,
Dscho

Re: git-apply{,mbox,patch} should default to --unidiff-zero

From: Adrian Bunk <hidden>
Date: 2016-06-15 22:43:19

On Fri, Jul 06, 2007 at 02:51:07AM +0100, Johannes Schindelin wrote:
Hi,

On Fri, 6 Jul 2007, Adrian Bunk wrote:
quoted
On Fri, Jul 06, 2007 at 02:18:46AM +0100, Johannes Schindelin wrote:
quoted
On Fri, 6 Jul 2007, Adrian Bunk wrote:
quoted
git-apply{,mbox,patch} should default to doing --unidiff-zero:
But is that not dangerous? At least now the committer has some 
safeguard against this kind of mistakes. Because you can easily 
introduce mistakes that way.
you are saying "easily".

Did you ever actually run into such a problem?
Not yet, thankfully.
quoted
You must do something like "diff -U0" or manually editing patches for 
creating such patches, and that's very unusual.
The point is that the _committer_ is not necessarily involved in that 
business.

And "git apply" is strict for a reason. It catches possibly unwanted 
things much earlier than patch. I _want_ to be warned that somebody is 
introducing some code at a certain position, which might, or might not be 
correct. apply has no way to tell, since there is no context to at least 
minimally verify.
...
That's wrong.

My use cases are replacing or deleting lines.

In these cases there is context in the deleted lines that is already 
being verified even with --unidiff-zero.
Ciao,
Dscho
cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: git-apply{,mbox,patch} should default to --unidiff-zero

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:19

Hi,

On Fri, 6 Jul 2007, Adrian Bunk wrote:
On Fri, Jul 06, 2007 at 02:51:07AM +0100, Johannes Schindelin wrote:
quoted
On Fri, 6 Jul 2007, Adrian Bunk wrote:
quoted
You must do something like "diff -U0" or manually editing patches 
for creating such patches, and that's very unusual.
The point is that the _committer_ is not necessarily involved in that 
business.
BTW this still holds true, and you have not addressed that.  It really is 
a serious issue.  "git apply" is a committer's tool.  So it should help 
the committer.
quoted
And "git apply" is strict for a reason. It catches possibly unwanted 
things much earlier than patch. I _want_ to be warned that somebody is 
introducing some code at a certain position, which might, or might not 
be correct. apply has no way to tell, since there is no context to at 
least minimally verify.
...
That's wrong.

My use cases are replacing or deleting lines.
That is _your_ use case.
In these cases there is context in the deleted lines that is already 
being verified even with --unidiff-zero.
With --unidiff-zero, also _adding_ lines will be handled as if there were 
no problem.

Yes, in your case it fixes a problem.

Yet, in other cases it introduces a problem.

Okay?

Ciao,
Dscho

Re: git-apply{,mbox,patch} should default to --unidiff-zero

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:19


On Fri, 6 Jul 2007, Johannes Schindelin wrote:
With --unidiff-zero, also _adding_ lines will be handled as if there were 
no problem.

Yes, in your case it fixes a problem.

Yet, in other cases it introduces a problem.
Well, we could make the rule be that ew require --unidiff-zero only if 
there really is _no_ old data to verify in a hunk. No deleted lines, and 
no context around it.

Adrian has a point in that if there are lines to be deleted, that in 
itself is context, and then the strict behaviour of "git-apply" is 
arguably unnecessaily strict.

That said, I do absolutely _hate_ how GNU patch will basically apply 
random line noise without complaints. So git-apply is designed to be much 
stricter on _so_ many levels. The thing that I personally always really 
detested about GNU patch was how it would apply part of a patch, then fail 
half-way, and leave the partial patch applied!

git-apply is about a million times better than standard "patch", exactly 
because it tries to make sure that what it does makes sense, and you 
actually need to use explicit flags to make it do things that may be hard 
to undo or slightly questionable.

		Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help