being nice to patch(1)

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

being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

James's current git-scsi-misc has this commit in it:


commit a16efc1cbf0a9e5ea9f99ae98fb774b60d05c35b
Author: Kars de Jong [off-list ref]
Date:   Sun Jun 17 14:47:08 2007 +0200

[SCSI] 53c700: Amiga 4000T NCR53c710 SCSI
    
    New driver for the Amiga 4000T built-in NCR53c710 SCSI controller, using the
    53c700 SCSI core.
    
    Signed-off-by: Geert Uytterhoeven [off-list ref]
    Signed-off-by: James Bottomley [off-list ref]


When one pulls that diff out of git with `git-show' or whatever, it doesn't
work - patch(1) has a heart attack over the "53c700":


|commit f98754960a9b25057ad5f249f877b3d6fab889ce
|Author: FUJITA Tomonori [off-list ref]
|Date:   Mon May 14 20:25:31 2007 +0900
|
|    [SCSI] hptiop: convert to use the data buffer accessors
|    
|    - remove the unnecessary map_single path.
|    
|    - convert to use the new accessors for the sg lists and the
|    parameters.
|    
|    Jens Axboe [off-list ref] did the for_each_sg cleanup.
|    
|    Signed-off-by: FUJITA Tomonori [off-list ref]
|    Acked-by: HighPoint Linux Team [off-list ref]
|    Signed-off-by: James Bottomley [off-list ref]
|
|commit a16efc1cbf0a9e5ea9f99ae98fb774b60d05c35b
|Author: Kars de Jong [off-list ref]
|Date:   Sun Jun 17 14:47:08 2007 +0200
|
|    [SCSI] 53c700: Amiga 4000T NCR53c710 SCSI
|    
|    New driver for the Amiga 4000T built-in NCR53c710 SCSI controller, using the
--------------------------
File to patch: 




This I assume is because ^[ ]*<number>c<number> is a magic marker for
contextual diffs.

So...  if someone is feeling really, really, really bored one day, it would
be nice to teach git to somehow escape such patch-magic-patterns in the
changelog when emitting plain old patches.

Re: being nice to patch(1)

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


On Mon, 2 Jul 2007, Andrew Morton wrote:
James's current git-scsi-misc has this commit in it:

commit a16efc1cbf0a9e5ea9f99ae98fb774b60d05c35b
Author: Kars de Jong [off-list ref]
Date:   Sun Jun 17 14:47:08 2007 +0200

[SCSI] 53c700: Amiga 4000T NCR53c710 SCSI
    
    New driver for the Amiga 4000T built-in NCR53c710 SCSI controller, using the
    53c700 SCSI core.
    
    Signed-off-by: Geert Uytterhoeven [off-list ref]
    Signed-off-by: James Bottomley [off-list ref]


When one pulls that diff out of git with `git-show' or whatever, it doesn't
work - patch(1) has a heart attack over the "53c700":
There's really nothing git can do about this, this is a patch oddity about 
the free-form message. A really strange one too, because the line is 
literally four spaces followed by the 53c700, and the thing is, that's not 
even a valid olf-fashioned patch (_without_ the four spaces, I could see 
that "patch" might think that it's a really old ed-

I think you have two options:

 - tell patch to take it as a unified diff:

	git show | patch -p1 -u

   should work, since patch won't be trying to figure out what kind of 
   diff it is, and won't think that the 53c700 is some kind of odd ed 
   script.

 - suppress the free-form messages, by using (for example)

	git show --pretty=oneline | patch -p1

   and now "patch" doesn't get any random commit message except for the 
   first line (which always starts with the SHA1) and hopefully cannot 
   _possibly_ interpret that to be some strange patch format.

Or, of course, just use "git-apply" instead of patch to apply the thing.

			Linus

Re: being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

On Mon, 2 Jul 2007 14:16:16 -0700 (PDT)
Linus Torvalds [off-list ref] wrote:

On Mon, 2 Jul 2007, Andrew Morton wrote:
quoted
James's current git-scsi-misc has this commit in it:

commit a16efc1cbf0a9e5ea9f99ae98fb774b60d05c35b
Author: Kars de Jong [off-list ref]
Date:   Sun Jun 17 14:47:08 2007 +0200

[SCSI] 53c700: Amiga 4000T NCR53c710 SCSI
    
    New driver for the Amiga 4000T built-in NCR53c710 SCSI controller, using the
    53c700 SCSI core.
    
    Signed-off-by: Geert Uytterhoeven [off-list ref]
    Signed-off-by: James Bottomley [off-list ref]


When one pulls that diff out of git with `git-show' or whatever, it doesn't
work - patch(1) has a heart attack over the "53c700":
There's really nothing git can do about this, this is a patch oddity about 
the free-form message. A really strange one too, because the line is 
literally four spaces followed by the 53c700, and the thing is, that's not 
even a valid olf-fashioned patch (_without_ the four spaces, I could see 
that "patch" might think that it's a really old ed-

I think you have two options:

 - tell patch to take it as a unified diff:

	git show | patch -p1 -u

   should work, since patch won't be trying to figure out what kind of 
   diff it is, and won't think that the 53c700 is some kind of odd ed 
   script.
yup, `patch -u' fixes it up.
 - suppress the free-form messages, by using (for example)

	git show --pretty=oneline | patch -p1

   and now "patch" doesn't get any random commit message except for the 
   first line (which always starts with the SHA1) and hopefully cannot 
   _possibly_ interpret that to be some strange patch format.

Or, of course, just use "git-apply" instead of patch to apply the thing.
Thing is, changelog-followed-by-diff is a fairly standard format used by
quilt and other such toys.

Hopefully quilt is using -u so it won't encounter this oddity.

Re: being nice to patch(1)

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


On Mon, 2 Jul 2007, Andrew Morton wrote:
Thing is, changelog-followed-by-diff is a fairly standard format used by
quilt and other such toys.
Sure. And if a tool ends up eating the changelog as a diff, then that tool 
is broken. I really do think that this is a "patch" bug - I really don't 
think that that was a valid traditional diff with the four spaces at the 
head of the line.

Of course, if the changelog-followed-by-diff doesn't have any indentation 
or escaping at all, the changelog entry itself *could* actually have a 
real unified diff in it, and the tool would be unable to tell where the 
actual patch starts.

But at least "git show" and friends indent the changelog on purpose, 
exactly so that there is never any chance that there could be any real 
ambiguity, and this really was a "patch" bug as far as I can tell. 
Happily, one that is easy to work around, by just telling patch to always 
consider the patch a unified diff.

			Linus

Re: being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

On Mon, 2 Jul 2007 14:40:36 -0700 (PDT)
Linus Torvalds [off-list ref] wrote:

On Mon, 2 Jul 2007, Andrew Morton wrote:
quoted
Thing is, changelog-followed-by-diff is a fairly standard format used by
quilt and other such toys.
Sure. And if a tool ends up eating the changelog as a diff, then that tool 
is broken. I really do think that this is a "patch" bug - I really don't 
think that that was a valid traditional diff with the four spaces at the 
head of the line.

Of course, if the changelog-followed-by-diff doesn't have any indentation 
or escaping at all, the changelog entry itself *could* actually have a 
real unified diff in it, and the tool would be unable to tell where the 
actual patch starts.
erk, yes, sometimes people do like to quote a hunk of diff in the changelog
and yes, hell doth break loose.
But at least "git show" and friends indent the changelog on purpose, 
exactly so that there is never any chance that there could be any real 
ambiguity, and this really was a "patch" bug as far as I can tell. 
Happily, one that is easy to work around, by just telling patch to always 
consider the patch a unified diff.
I'm afraid indenting the changelog with leading spaces doesn't help -
patch(1) still tries to apply the diff.

I guess quilt-and-friends could (should) strip away all text prior to the
first ^--- before feeding to patch(1).  That would reliably remove all
git changelog text.

Re: being nice to patch(1)

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


On Mon, 2 Jul 2007, Andrew Morton wrote:
I'm afraid indenting the changelog with leading spaces doesn't help -
patch(1) still tries to apply the diff.
Oh wow. I didn't believe you, so I decided to test.

I shouldn't have doubted you.

That also explains why it reacted to that 53c700 even though it wasn't at 
the beginning of a line.

That really is a piece of crap.

People who think that basic programs like "patch" should DWIM stuff like 
that are incompetent. Yes, I can see how it can be "convenient", but 
dammit, whoever added that convenince feature really is a total moron.

At the very least it should be off by default, and controlled by some flag 
(ie "patch --dwim"). As it is, it's on by default, and I don't see any way 
at all to disable it (not in the man-page, and not googling the source 
with google code-search).

That's just incredibly broken.

I guess I shouldn't be surprised. The whole "things should be convenient, 
not safe" approach is shown by the default high fuzz-factor too. But at 
least that one you can disable.

It's positively microsoftian to make programs blindly be "convenient", 
with no thinking about what that means for security and safety of the end 
result.

So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things 
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit, 
"git-apply" really is the better program here.

		Linus

Re: [Quilt-dev] Re: being nice to patch(1)

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:43:19

On Tuesday 03 July 2007 02:28, Linus Torvalds wrote:
So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit,
"git-apply" really is the better program here.
I'm in bit of a conflict with choice one: when applying patches in an 
automated build process or similar, the likely way to do so is a simple loop 
over the series file. So the less magic when applying patches with quilt, the 
better.

Turning off the insane heuristic with patch -u will do well enough I hope. 
Quilt does not use that option by default because it also supports context 
diffs (some people / projects prefer them), but that can easily be customized 
in .quiltrc:

    QUILT_PATCH_OPTS=-u

Andreas

Re: [Quilt-dev] Re: being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

On Tue, 3 Jul 2007 15:34:46 +0200 Andreas Gruenbacher [off-list ref] wrote:
On Tuesday 03 July 2007 02:28, Linus Torvalds wrote:
quoted
So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit,
"git-apply" really is the better program here.
I'm in bit of a conflict with choice one: when applying patches in an 
automated build process or similar, the likely way to do so is a simple loop 
over the series file. So the less magic when applying patches with quilt, the 
better.

Turning off the insane heuristic with patch -u will do well enough I hope. 
Quilt does not use that option by default because it also supports context 
diffs (some people / projects prefer them), but that can easily be customized 
in .quiltrc:

    QUILT_PATCH_OPTS=-u
I guess one could try `patch -p1' and if that failed, `patch -p1 -u'.

But the problem is that patch will get stuck in interactive mode prompting
for a filename.  I've never actually worked how to make patch(1) just fail
rather than going interactive, not that I've tried terribly hard.  Any
hints there?

Thanks.

Re: [Quilt-dev] Re: being nice to patch(1)

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


On Tue, 3 Jul 2007, Andrew Morton wrote:
But the problem is that patch will get stuck in interactive mode prompting
for a filename.  I've never actually worked how to make patch(1) just fail
rather than going interactive, not that I've tried terribly hard.  Any
hints there?
"patch -t" (or "--batch") should do it, I suspect.

But even with "patch -tu -p1" you do seem to end up having patch notice 
indented patch fragments (ie things that obviously are *not* part of the 
patch, but some explanation).

		Linus

Re: [Quilt-dev] Re: being nice to patch(1)

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:43:19

On Tuesday 03 July 2007 17:49, Andrew Morton wrote:
I guess one could try `patch -p1' and if that failed, `patch -p1 -u'.
Hmm, I'll think about that, thanks.
But the problem is that patch will get stuck in interactive mode prompting
for a filename.  I've never actually worked how to make patch(1) just fail
rather than going interactive, not that I've tried terribly hard.  Any
hints there?
Patch -f will turn off those questions.

Andreas

Re: [Quilt-dev] Re: being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

On Tue, 3 Jul 2007 18:03:15 +0200 Andreas Gruenbacher [off-list ref] wrote:
On Tuesday 03 July 2007 17:49, Andrew Morton wrote:
quoted
I guess one could try `patch -p1' and if that failed, `patch -p1 -u'.
Hmm, I'll think about that, thanks.
quoted
But the problem is that patch will get stuck in interactive mode prompting
for a filename.  I've never actually worked how to make patch(1) just fail
rather than going interactive, not that I've tried terribly hard.  Any
hints there?
Patch -f will turn off those questions.
darnit, both `-f' and `-t' work.  Sigh.  I blame the manpage: too long ;)

Incidentally, the offending patch
(http://userweb.kernel.org/~akpm/git-scsi-misc.patch) sends patch(1) into
an infinite loop with `patch -p1 -f' and `patch -p1 -t'.  Presumably
it will do the same when that patch is offered to quilt...

Re: Re: being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

On Tue, 3 Jul 2007 18:03:15 +0200 Andreas Gruenbacher [off-list ref] wrote:
On Tuesday 03 July 2007 17:49, Andrew Morton wrote:
quoted
I guess one could try `patch -p1' and if that failed, `patch -p1 -u'.
Hmm, I'll think about that, thanks.
quoted
But the problem is that patch will get stuck in interactive mode prompting
for a filename.  I've never actually worked how to make patch(1) just fail
rather than going interactive, not that I've tried terribly hard.  Any
hints there?
Patch -f will turn off those questions.
darnit, both `-f' and `-t' work.  Sigh.  I blame the manpage: too long ;)

Incidentally, the offending patch
(http://userweb.kernel.org/~akpm/git-scsi-misc.patch) sends patch(1) into
an infinite loop with `patch -p1 -f' and `patch -p1 -t'.  Presumably
it will do the same when that patch is offered to quilt...

Re: Re: being nice to patch(1)

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-06-15 22:43:19

On Tue, 3 Jul 2007 15:34:46 +0200 Andreas Gruenbacher [off-list ref] wrote:
On Tuesday 03 July 2007 02:28, Linus Torvalds wrote:
quoted
So I would suggest that in quilt and other systems, you either:

 - strip all headers manually

 - forget about "patch", and use "git-apply" instead that does things
   right and doesn't screw up like this (and can do rename diffs etc too).

I guess the second choice generally isn't an option, but dammit,
"git-apply" really is the better program here.
I'm in bit of a conflict with choice one: when applying patches in an 
automated build process or similar, the likely way to do so is a simple loop 
over the series file. So the less magic when applying patches with quilt, the 
better.

Turning off the insane heuristic with patch -u will do well enough I hope. 
Quilt does not use that option by default because it also supports context 
diffs (some people / projects prefer them), but that can easily be customized 
in .quiltrc:

    QUILT_PATCH_OPTS=-u
I guess one could try `patch -p1' and if that failed, `patch -p1 -u'.

But the problem is that patch will get stuck in interactive mode prompting
for a filename.  I've never actually worked how to make patch(1) just fail
rather than going interactive, not that I've tried terribly hard.  Any
hints there?

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