On Tue, Feb 01, 2022 at 20:41:37 +0000, Soft Works wrote: > > On Tue, Feb 01, 2022 at 19:44:24 +0000, Soft Works wrote: > > > > On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote: > > > > > > > > In case anyone is wondering why patchwork fails to apply the second > > > > patch, > > > > this is probably once again because the patch updates one of FATE's ASS > > > > reference files which use CRLF line-endings. > > > > Locally git am applies both without a hitch for me on top of current > > > > master > > > > (and FATE passes after applying each patch). > > > > > > > > > You can add a .gitattributes file to tests/ref/fate/ which includes the > > > line > > > > > > sub-webvtt2 -diff > > > > > > Then your local git format-patch will create a binary diff for the file. > > > > Thanks for your suggestion. However, a binary diff would look like this > > which > > isn't great for seeing what's going on during review: > > > > [...] > > Yes, I know, but the question is probably what's more important.. > > ..that it can be applied or that the text is viewable?
It CAN be applied (as I've now written twice) and of course I verified this with the mail received from the list. > > Also as noted, locally plain `git am` has no issues applying the regular > > (non-binary) patch; > > Yes, because it hasn't been sent around via e-mail yet. SMTP requires > CRLF line endings, so essentially it depends on the receiving e-mail client > whether it outputs LF or CRLF. [...] The mail content is base64 encoded during client-server and server-server transfer. Perhaps the raw base64 text is using all CRLF line-endings, but this doesn't matter. Once decoded it will byte-for-byte match the format-patch I created locally except for the ffmpeg-devel footer and additional headers in the mail version — but git am will just ignore those. See the headers of the second patch's mail: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 For comparison, here are the same headers for your first reply: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit If you tried to send a patch with your mail client by pasting in the diff, then yes, presumably the line-endings would indeed get mangled. That's why doc/developer.texi tells you not to do this (instead recommending git send-email or binary (i.e. base64-encoded) attachments). > The most extreme example is sub-textenc (and there was another one iirc). > It has mixed line endings - some LF and some CRLF. At least at that point > there's no way out, because those endings will get unrecoverably lost > as soon as it is sent around via e-mail as text-diff. > > That's how I came to the binary diff as only working option. > (or you just don't send patches around via e-mail at all ;-) Mixed line-endings also aren't a problem with a base64-encoded mail body as eg git send-email will automatically use. There's one caveat specific to (ffmpeg's) patchwork I didn't know about before: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220116181655.6407-2-one...@oneric.de/ If I download the “diff” from the above page, CRLF and LF line-endings are preserved as they should be. However, the files served from the “mbox” and “series” buttons only contain LF line-endings. As everyone can test, the mail I sent evidently does use mixed CRLF and LF line-endings matching the file being patched (just like the result of the “diff” button). I believe this is an issue separate from patchwork using --no-keep-cr and should be fixed on patchwork's side. Tl;dr: Mixed line-endings in a diff are no technical issue, provided the patch is sent correctly. The issue is purely with patchwork, which: a) uses --no-keep-cr or similar presumably to protect against CRLF sneaking into regular source files. (This is a minor annoyance when CRLF is actually required, but otherwise harmless) b) mangles the line-endings for the “mbox” and “series” options but does the correct thing for the “diff” option. This is harmful and imho should be fixed by patchwork. Nonetheless, the patch can still be correctly applied from the _mail_ on the list. The patch on the list can easily be applied from the received mail by a plain `git am` or `git am --keep-cr` if the value of am.keepcr is set to false. (In my case am.keepcr is not set at all and plain git am does the right thing.) If someone wants to apply the patches and has trouble with getting the patch out of the mailclient I can on request resend the patch with a binary diff for the reference file. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
