Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 23, 2014, 18:54 UTC
Message-ID: <xmqq1tr2jhg2.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1411434583-27692-1-git-send-email-lersek@redhat.com>

Laszlo Ersek <lersek@redhat.com> writes:

>   git format-patch master..branch1

The output from this has these (excerpt from "od -xc" output):

0000360       f   2  \n  \n   d   i   f   f       -   -   g   i   t
           6620    0a32    640a    6669    2066    2d2d    6967    2074
0000400   a   /   f   2       b   /   f   2  \n   n   e   w       f   i
           2f61    3266    6220    662f    0a32    656e    2077    6966
0000420   l   e       m   o   d   e       1   0   0   6   4   4  \n   i
           656c    6d20    646f    2065    3031    3630    3434    690a
0000440   n   d   e   x       0   0   0   0   0   0   0   .   .   f   3
           646e    7865    3020    3030    3030    3030    2e2e    3366
0000460   5   d   3   e   6  \n   -   -   -       /   d   e   v   /   n
           6435    6533    0a36    2d2d    202d    642f    7665    6e2f
0000500   u   l   l  \n   +   +   +       b   /   f   2  \n   @   @
           6c75    0a6c    2b2b    202b    2f62    3266    400a    2040
0000520   -   0   ,   0       +   1       @   @  \n   +   h   e   l   l
           302d    302c    2b20    2031    4040    2b0a    6568    6c6c
0000540   o       w   o   r   l   d  \r  \n   -   -      \n   2   .   1
           206f    6f77    6c72    0d64    2d0a    202d    320a    312e

The structural parts of the diff, including "--- /dev/null" line, are all terminated by "\n" (as they should be), and the only CR appears in the message is at the end of "+hello world" line.

So I do not think apply should need to loosen its sanity check and take a random whitespace after the "/dev/null" as a valid "this is a creation event for the path" marker (e.g. "--- /dev/null whoa"?).

is_dev_null() is used to in the fallback code path that parses traditional patch output (e.g. GNU diff) which throws random cruft (e.g. timestamp) after the /dev/null marker, e.g.

    $ diff -u /dev/null f2
    --- /dev/null   2014-09-17 18:22:57.995111003 -0700
    +++ f2  2014-09-23 11:37:09.000000000 -0700
    @@ -0,0 +1 @@
    +hello world

and we'd be hesitant to allow that kind of looseness for Git patches where we know we end the line after the "/dev/null" marker.

> 3. In the reviewer / tester / maintainer role, save the patch from your
> email client to a local file. Assume that your email client does not
> corrupt the patch when saving it.

Perhaps compare this saved file with the output from the above format-patch to see where things got broken?

SMTP transport may be CRLF-unsafe, so I have a suspicion that it may turn out that what you are trying to do might be an equilvalent of

	git format-patch ... |
        # first lose all \r\n
        dos2unix | 
	# then make everything \r\n
        unix2dos |
        # and apply
        git am

which is not workable in the first place.  I dunno.