Re: [PATCH for-maint] apply: gitdiff_verify_name(): accept "/dev/null\r"
- From
- Laszlo Ersek <lersek@redhat.com>
- Date
- Sep 23, 2014, 20:49 UTC
- Message-ID
- <5421DCE3.9090500@redhat.com>
- In-Reply-To
- <xmqqfvfihy7i.fsf@gitster.dls.corp.google.com>
On 09/23/14 22:35, Junio C Hamano wrote:
Show 49 quoted lines
> 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