All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: git@vger.kernel.org, jordan.l.justen@intel.com, matt.fleming@intel.com
Subject: Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"
Date: Tue, 23 Sep 2014 11:54:37 -0700	[thread overview]
Message-ID: <xmqq1tr2jhg2.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1411434583-27692-1-git-send-email-lersek@redhat.com> (Laszlo Ersek's message of "Tue, 23 Sep 2014 03:09:43 +0200")

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.

  reply	other threads:[~2014-09-23 18:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23  1:09 [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r" Laszlo Ersek
2014-09-23 18:54 ` Junio C Hamano [this message]
2014-09-23 19:31   ` Laszlo Ersek
2014-09-23 19:56     ` Junio C Hamano
2014-09-23 20:33       ` Laszlo Ersek
2014-09-23 20:40         ` Junio C Hamano
2014-09-23 20:57           ` Laszlo Ersek
2014-09-23 20:02     ` Junio C Hamano
2014-09-23 20:32       ` Laszlo Ersek
2014-09-23 20:35         ` Junio C Hamano
2014-09-23 20:49           ` Laszlo Ersek
2014-09-23 21:35             ` Junio C Hamano
2014-09-24 12:56               ` Laszlo Ersek
2014-09-24 17:55                 ` Junio C Hamano
2014-09-23 20:17   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1tr2jhg2.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jordan.l.justen@intel.com \
    --cc=lersek@redhat.com \
    --cc=matt.fleming@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.