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

[Laszlo Ersek <lersek@redhat.com>]

On 09/23/14 22:35, Junio C Hamano wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> 
>> On 09/23/14 22:02, Junio C Hamano wrote:
>>> Laszlo Ersek <lersek@redhat.com> writes:
>>>
>>>> On 09/23/14 20:54, Junio C Hamano wrote:
>>>> ...
>>>>> 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.
>>>>
>>>> I agree with your analysis. It is indeed the MTA...
>>>>>    - CR and LF MUST only occur together as CRLF; they MUST NOT appear
>>>>>      independently in the body.
>>>>
>>>> But why is this situation "not workable"? The same happens with *all*
>>>> patches that people mail around, it's just not visible to them, because
>>>> git-am strips all CRs indiscriminately.
>>>
>>> It is not "git am" or "git apply" that "strips all CRs
>>> indiscriminately".  I just tried to apply 0001-add-f2 without
>>> letting your MTA/MUA corrupt it on "master" branch in the repository
>>> you prepared that patch from, i.e.
>>>
>>> 	git checkout master^0 ;# go back
>>>         git am 0001-add-f2* ;# apply that "+hello world\r\n" patch
>>>         git diff branch ;# nothing
>>
>> When you did this, was am.keepcr=true in effect?
> 
> I actually briefly scratched my head but realized when I saw it work
> "as expected" without me passing "--keep-cr" to "am" myself.
> 
> But I did that experiment in the repository created by following
> your reproduction recipe, in which it had these:
> 
>   git config core.whitespace cr-at-eol
>   git config am.keepcr true
> 
> so yes I had keepcr set.

Thank you for confirming, I expected so.

Because in this case the test doesn't refute my claim that "git-am
strips all CRs indiscriminately".

Git-am *does* strip all CRs indiscriminately, undoing the CRs that the
email servers / clients introduce. Your above test worked out because
you prevented git-am from stripping the CRs, with the keepcr=true
setting. If you turn that off, then either your git am command won't
succeed (because it will run into context conflicts due to different
line endings -- although not in this example), or the final git-diff
will report differences.

In summary:
- the email infrastructure turns all line terminators into CRLFs
- git-am strips these by default, from source code lines and from git
  diff header lines alike,
- this is fine for repos that store files with \n terminators,
- not fine for repos with embedded \r\n terminators -- the default
  stripping behavior of git-am breaks the source code in that case
  (runs into conflicts with existing files, and creates new files with
  wrong line endings)
- if you set am.keepcr=true, then the source code remains intact (no
  conflicts for existing files), but new files cannot be created,
  because the /dev/null\r header lines are rejected.

Thanks
Laszlo