Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Non-zero start/stop values in XhcGetElapsedTicks

["Laszlo Ersek" <lersek@redhat.com>]

On 11/2/23 14:06, Pedro Falcato wrote:
> On Thu, Nov 2, 2023 at 11:28 AM Laszlo Ersek <lersek@redhat.com>
> wrote:
>>
>> On 11/1/23 02:12, Mike Maslenkin wrote:

[...]

>>> Just curious why this patch was broken by google groups.
>>>
>>> Some patches to edk2 and edk2-redfish-client have unintended line
>>> breaks marked with "=",  additional "0D" and additional "3D" to "="
>>> Web shows https://edk2.groups.io/g/devel/message/110434 exactly as
>>> it saved by mailer.
>>> What do sender should setup to avoid this?
>>
>> I recommend selecting base64 content-transfer-encoding, rather than
>> quode-printable. Base64 will ensure that the embedded CRLFs (which
>> are used in the edk2 source tree) survive intact, and also that
>> "git-am" can cleanly apply the patch (as saved from the mailing
>> list).
>>
>> Base64 is more robust than 8bit too. (If 8bit survived all mail
>> servers along the way, it would work fine as well.)
>>
>> ... According to my notes, git has always *ignored* the
>>
>> [sendemail]
>>         transferEncoding = base64
>>
>> stanza in my git config file. Which is why I have an alias around
>> git-send-email that open-codes
>>
>>   git send-email --transfer-encoding=base64 ...
>>
>> So that's what I recommend.
>>
>> (BTW, our "BaseTools/Scripts/SetupGit.py" script sets
>> "sendemail.transferEncoding=8bit", but that is problematic for two
>> reasons: (1) git ignores it anyway, per my records mentioned above,
>> (2) 8bit is inferior to base64 in practice, when it comes to CRLF
>> integrity across all email servers.)
>>
>> ... Side comment: I can apply quoted-printable-encoded patches as
>> well, from the list, but that's only because I manually transcode
>> them to 8bit, before passing them to git-am. I use the following
>> hairy script:
>>
>> ----------------------------
>> #!/bin/bash
>> set -e -u -C
>>
>> TMPD=$(mktemp -d)
>> trap 'rm -f -r -- "$TMPD"' EXIT
>>
>> cd "$TMPD"
>> tee input | dos2unix | csplit -s - '/^$/'
>> HEAD_LINES=$(wc -l < xx00)
>>
>> head -n "$HEAD_LINES" input \
>> | sed -r 's/^(Content-Transfer-Encoding: )quoted-printable/\18bit/'
>>
>> tail -n +$((HEAD_LINES + 1)) input \
>> | perl -p -e 'use MIME::QuotedPrint; $_=MIME::QuotedPrint::decode($_);' \
>> | unix2dos
>> ----------------------------
>>
>> (The perl command is from Paolo Bonzini.)
>
> Ooooooh, cool script!

Thanks :)

>>
>> Summary: send your patches with
>>
>>   git send-email --transfer-encoding=base64 ...
>
> I've been involved in EDK2 for the last 2.5 years and I still haven't
> found a consistent way to both send and apply patches :/
> I usually use 8bit.

I've now updated

  https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-24

with the following commit, at Mike Maslenkin's poking:

> commit 385073186afb8dd7cb8af4c9fcd96d86423fd9d3
> Author: Laszlo Ersek <lersek@redhat.com>
> Date:   Fri Nov 3 07:10:44 2023 +0100
>
>     Laszlo's unkempt git guide: improve git-send-email options
>
>     (1) Recent git-send-email auto-CC's email addresses from miscellaneous
>     "Whatever-by:" tags; suppress that logic as well. (What we really want is
>     to restrict the CC'ing logic to "bodycc" + "cccmd", but there is no way to
>     state that in a positive sense, so we need to suppress "everything else".)
>
>     (2) Embedded CRLFs are best protected with base64
>     content-transfer-encoding; add an according option. Quoted-printable
>     encoding tends to wreak havoc for both git-am and mailing list archives on
>     the web.
>
>     Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>
> diff --git a/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md b/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
> index 6f85c782a710..0ffe24a5acbd 100644
> --- a/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
> +++ b/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
> @@ -402,11 +402,13 @@ Contributor workflow
>       Time to mail-bomb the list! Do the following:
>
>       ```
> -     git send-email         \
> -       --suppress-cc=author \
> -       --suppress-cc=self   \
> -       --suppress-cc=cc     \
> -       --suppress-cc=sob    \
> +     git send-email               \
> +       --suppress-cc=author       \
> +       --suppress-cc=self         \
> +       --suppress-cc=cc           \
> +       --suppress-cc=sob          \
> +       --suppress-cc=misc-by      \
> +       --transfer-encoding=base64 \
>         *.patch
>       ```
>

For applying patches, I have

[core]
        whitespace = cr-at-eol
[am]
        messageid = true
        keepcr = true

Between these three settings, the two that are functionally important
for applying patches are "am.keepcr" and "core.whitespace". Those are
both listed at

  https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05

and included in "BaseTools/Scripts/SetupGit.py". The only remaining
issue that I've needed to deal with is decoding quoted-printable, which
I do with the above script.

On 11/2/23 14:06, Pedro Falcato wrote:
> FWIW, Rebecca started hosting a lore instance
> (https://openfw.io/edk2-devel/,

I didn't know. Thank you, Rebecca, for that! The more independent archives, the better!

> although it doesn't seem to be feeling too well atm) and I tried to
> get b4 to work, to see if most of this process could be nicely
> automated. Sadly, CRLF problems galore :(

Right, "applying patches from a web archive" is a separate question.

I see two ways out, if we're displeased with the current git-am
experience:

- Get rid of repository-level CRLFs (git is capable of automatic
  LF<->CRLF translation at commit and checkout, so for Windows users,
  the internal representation changing from CRLF to LF should not be
  disruptive). This would be similar, size-wise, to the big
  "uncrustification". The conversion commits could be suppressed for
  git-blame purposes (compare your own commit 6ded9f50c3aa, "edk2: Add
  .git-blame-ignore-revs file", 2023-04-16).

- Or else, move the review workflow to github altogether.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110608): https://edk2.groups.io/g/devel/message/110608
Mute This Topic: https://groups.io/mt/102301510/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-