On 09/23/14 23:35, Junio C Hamano wrote: > Laszlo Ersek <ler...@redhat.com> writes: > >> [...]
> The important thing to note here is that use of text/plain for > patches, if you want to have distinction between CRLF and LF in your > payload, is not designed to work over e-mails. That's good to know, thanks! > Now if we accept that this issue is coming from lossy nature of > transporting patches via e-mails, we would realize that the right > place to solve this is not in "apply"'s parsing of structural part > of the "diff" output (e.g. "diff --git ...", "rename from ..." or > "--- filename"), but the payload part (i.e. " " followed by context, > "-" followed by removed and "+" followed by added). I can agree with this, yes. > Removal of CR > by "am -> mailsplit -> mailinfo -> apply" callchain is not losing > any information, as the input does not have useful information to > let us answer "are the lines in this path supposed to end with > CRLF?" in the first place; "/dev/null\r" patch is barking up a wrong > tree. OK. > Our line-endings infrastructure (e.g. core.autocrlf configuration > variable, `eol` attribute) is all geared toward supporting cross > platform projects in that the BCP is to use LF line endings as the > canonical line endings for in-repository data and convert it to CRLF > for working tree files when necessary. We do not have direct > support (other than declaring contents for paths as "binary" aka "no > conversion") to use CRLF in in-repository data (and that is partly > deliberate). I see. The edk2 "mirror-of-svn" git repo is then somewhat "incompatible" with the original design. > But it is conceivable to enhance the attribute system to allow us to > mark certain paths (or "all paths", which is a trivial special case) > as using CRLF line endings in in-repository and in-working-tree. It > could be setting `eol` attribute to `both-crlf` or something. > > Then "am -> mailsplit -> mailinfo -> apply" chain could: > > * "mailsplit" and "mailinfo" does not have to do anything special, > other than stripping CR and make sure "apply" only sees LF > endings; > > * "apply" is taught a new option "--fix-mta-corruption" which > causes it to pay attention to the `eol` attribute set to > `both-crlf`, and when it reads a patch > > diff --git a/one b/one > --- one > +++ one > @@ -4,6 +4,6 @@ > common1 > common2 > -old1 > -old2 > +new1 > +new2 > common3 > common4 > > and notices that path "one" is marked as such, it pretends as if > the input were > > diff --git a/one b/one > --- one > +++ one > @@ -4,6 +4,6 @@ > common1^M > common2^M > -old1^M > -old2^M > +new1^M > +new2^M > common3^M > common4^M > > to compensate for possible breakage during the mail transit. > > * "am" is taught to pass "--fix-mta-corruption" to "apply" perhaps > by default. > > Because code paths that internally run "git am", e.g. "git rebase", > work on data that is *not* corrupt by mta (we generate diff and > apply ourselves), these places need to tell "am" not to use the > "--fix-mta-corruption" when running "apply". Thank you for taking the time to describe this. It does look like the by-the-book solution. Obviously, I can't implement it myself -- first, I have no experience with the git codebase, second, I have no time nor mandate to get familiar with it and to make a serious effort to improve it in this direction. (IOW "git" is a tool for my job, not my job.) The one-liner patch and this discussion is all I can manage -- I thought I'd take a stab at it myself rather than just "complain". If someone finds the time to implement and document this feature, a small part of the community will be very grateful. (Not much of a compensation for a corner case like this, admittedly.) Thanks, Laszlo -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
