From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from rn-mailsvcp-ppex-lapp35.apple.com (rn-mailsvcp-ppex-lapp35.apple.com [17.179.253.44]) by mx.groups.io with SMTP id smtpd.web09.454.1633622653174148432 for ; Thu, 07 Oct 2021 09:04:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=kWWnPO9R; spf=pass (domain: apple.com, ip: 17.179.253.44, mailfrom: afish@apple.com) Received: from pps.filterd (rn-mailsvcp-ppex-lapp35.rno.apple.com [127.0.0.1]) by rn-mailsvcp-ppex-lapp35.rno.apple.com (8.16.1.2/8.16.1.2) with SMTP id 197G3QEX020714; Thu, 7 Oct 2021 09:04:10 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=Mk/9JX3w4gc1EpX3SeoH8SI/ZYFLenWuPfplHRj1AnA=; b=kWWnPO9RKdu2vlNJmKLfwNeEQM1oGHJHxfJxIcKGw3CqN/VkfQobCzssmDaLr0v5pZBf F1GUwt+tlNgvAQj6LFmt7C9o4IL4Hf2RFQJWj/FXps4qYTETbw1S6X9fv++PV40echMt PgmLDyJuNiL4EVuskQBI+w9hvq0WeMDvcsslxA4KzB1cMNRKD7bX5YUZRvUldu2MQF8x jH/+Fgxut9LgJ7uNFkHwlxnRaU1obexsXMu3Hrs94MgXCCvGmoLzxjaq96a+YmHgTuII VMr5GLzrKDrWwT5Iw/2te76Ledmu4A/CdIrMVDOgdMkOmSgqWo+8LpLeOm1vvrqO6KNS 1w== Received: from ma-mailsvcp-mta-lapp02.corp.apple.com (ma-mailsvcp-mta-lapp02.corp.apple.com [10.226.18.134]) by rn-mailsvcp-ppex-lapp35.rno.apple.com with ESMTP id 3bejy8a6cm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Thu, 07 Oct 2021 09:04:10 -0700 Received: from ma-mailsvcp-mmp-lapp02.apple.com (ma-mailsvcp-mmp-lapp02.apple.com [17.32.222.15]) by ma-mailsvcp-mta-lapp02.corp.apple.com (Oracle Communications Messaging Server 8.1.0.12.20210903 64bit (built Sep 3 2021)) with ESMTPS id <0R0M00S7A7AYVG30@ma-mailsvcp-mta-lapp02.corp.apple.com>; Thu, 07 Oct 2021 09:04:10 -0700 (PDT) Received: from process_milters-daemon.ma-mailsvcp-mmp-lapp02.apple.com by ma-mailsvcp-mmp-lapp02.apple.com (Oracle Communications Messaging Server 8.1.0.9.20210415 64bit (built Apr 15 2021)) id <0R0M00C0079Y3W00@ma-mailsvcp-mmp-lapp02.apple.com>; Thu, 07 Oct 2021 09:04:10 -0700 (PDT) X-Va-A: X-Va-T-CD: 5975dd1eaec8696b379f33739df9e0a8 X-Va-E-CD: 574fda742400e238a02c5048d028c5dd X-Va-R-CD: b4e96f8f3d52136df210f2e6a101920e X-Va-CD: 0 X-Va-ID: c9efae3e-c7ad-4c5b-819c-91b19a347b72 X-V-A: X-V-T-CD: 5975dd1eaec8696b379f33739df9e0a8 X-V-E-CD: 574fda742400e238a02c5048d028c5dd X-V-R-CD: b4e96f8f3d52136df210f2e6a101920e X-V-CD: 0 X-V-ID: aefb8945-f896-4114-b64f-46c7ce9983e3 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790 definitions=2021-10-07_02:2021-10-07,2021-10-07 signatures=0 Received: from [17.234.148.236] (unknown [17.234.148.236]) by ma-mailsvcp-mmp-lapp02.apple.com (Oracle Communications Messaging Server 8.1.0.9.20210415 64bit (built Apr 15 2021)) with ESMTPSA id <0R0M00KG47AO7D00@ma-mailsvcp-mmp-lapp02.apple.com>; Thu, 07 Oct 2021 09:04:09 -0700 (PDT) From: "Andrew Fish" Message-id: MIME-version: 1.0 (Mac OS X Mail 14.0 \(3654.20.0.2.1\)) Subject: Re: [edk2-devel] Progress on getting Uncrustify working for EDK2? Date: Thu, 07 Oct 2021 12:03:59 -0400 In-reply-to: <16ABC94F0E9D1AC5.21629@groups.io> Cc: Leif Lindholm , mikuback@linux.microsoft.com, =?utf-8?Q?Marvin_H=C3=A4user?= , Rebecca Cran , Michael Kubacki , Bret Barkelew , Mike Kinney , Andrew Fish To: edk2-devel-groups-io References: <07a6ecff-f7bf-083a-f24d-246ca6c7988b@nuviainc.com> <2679bfa3-b4ec-d8e9-7e56-54ebe42d9001@posteo.de> <9fe0f984-db9d-9aec-0b44-5d30791a2855@linux.microsoft.com> <20211007104813.wa4rmfsqgcpvnzwt@leviathan> <16ABC94F0E9D1AC5.21629@groups.io> X-Mailer: Apple Mail (2.3654.20.0.2.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790 definitions=2021-10-07_02:2021-10-07,2021-10-07 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_77417F23-4091-4CB7-9557-FD3FF82E3404" --Apple-Mail=_77417F23-4091-4CB7-9557-FD3FF82E3404 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 On the git-blame front we might be able to build a recipe that walks past t= he code style and line ending commits that would could add to an FAQ or Wik= i. For bonus points we could add a git command that does this. You can add a P= ython git command to the repo by adding a git- script in the p= ath. So we could add a Python git command that skips blame of any of the kn= own Uncrustify or line ending type commits. Basically `git eblame` that wor= ks just like `git blame` but skips formatting changes.=20 I=E2=80=99m working on a git-pgrep that only greps files used by a given bu= ild, hopeful I=E2=80=99ll be able to contribute that soon. My co-works keep= finding bugs so I=E2=80=99m not quite done. Thanks, Andrew Fish > On Oct 7, 2021, at 11:30 AM, Andrew Fish via groups.io wrote: >=20 >=20 >=20 >> On Oct 7, 2021, at 6:48 AM, Leif Lindholm > wrote: >>=20 >> Hi Michael, >>=20 >> Apologies, I've owed you a response (promised off-list) for a while >> now. >>=20 >> First, let me say I hugely appreciate this effort. Apart from aligning >> the codebase(s), this will reduce manual reviewing effort >> substantially, as well as cutting down on number of rework cycles for >> developers. >>=20 >> Looking at the changes to (well, the comments in) uncrustify, this >> seems to be constrained to: >> - Newline after '(' for multi-line function calls. >> - Dealing with "(("/"))" for DEBUG macros. >> - Function pointer typedefs: >> - typedef\nEFIAPI >> - closing parentheses indentation >>=20 >> I don't think I've made any secret over the years that I am not a >> massive fan of the EDK2 coding style in general. So I think for any >> of its quirks that are substantial enough that they require not just >> custom configuration but actual new function added to existing code >> conformance tools, this would be an excellent point to sanitise the >> coding style instead. >>=20 >> Taking these in order: >>=20 >> Newline after '(' >> ----------------- >> I think we already reached a level of flexibility around this, where >> we don't actually enforce this (or single argument per >> line). Personally, I'd be happy to update the coding style as >> required instead. >>=20 >> DEBUG macro parentheses >> ----------------------- >> How does uncrustify treat DEBUG macros without this modification? >> Do we start getting everything turned into multi-level indented >> multi-line statements without this change? >=20 > Can we disable the rule that does not like the DEBUG macro? I seem to rem= ember clang nags about some strange () usage, so maybe we would not lose th= at much? >=20 > Thanks, >=20 > Andrew Fish >=20 >>=20 >> Function pointer typedefs: >> -------------------------- >> I don't see that function pointer typedefs need to rigidly follow the >> same pattern as the declaration of functions implementing them. Could >> we update the coding style (if needed) instead? >>=20 >> Best Regards, >>=20 >> Leif >>=20 >> On Mon, Aug 16, 2021 at 16:00:38 -0400, Michael Kubacki wrote: >>> The edk2 branch was created here: >>> https://github.com/makubacki/edk2/tree/uncrustify_poc_2 >>>=20 >>> We have a Project Mu fork with custom changes to the Uncrustify tool to= help >>> comply with EDK II formatting here: >>> https://dev.azure.com/projectmu/_git/Uncrustify >>>=20 >>> The latest information about the status and how to experiment with the >>> configuration file and the tool are in that fork: https://dev.azure.com= /projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-For= k-Readme >>>=20 >>> That said, I have also finished a CI plugin to run Uncrustify that shou= ld be >>> ready soon to initially deploy in Project Mu. Before doing so, I am try= ing >>> to settle on an initial configuration file that less strictly but more >>> reliably formats the code than in the examples in those branches. For >>> example, remove heuristics that when run against the same set of code >>> multiple times can produce different results. An example would be a rul= e >>> that reformats code because it exceeds a specified column width on one = run >>> but on the next run that reformatted code triggers a different rule to >>> further align the code and so on. At least initially, some rules might = be >>> tweaked in a more conservative approach that can be tightened in the fu= ture. >>> Once this configuration file is ready, we will baseline Project Mu code= as >>> an example and turn on the plugin. The CI plugin runs Uncrustify agains= t >>> modified files and if there's any changes, indicating a formatting >>> deviation, the diff chunks are saved in a log so they can be viewed as = a >>> build artifact. >>>=20 >>> I am making progress on the updated config file and I should be able to= post >>> a "uncrustify_poc_3" branch soon with the results. >>>=20 >>> Regarding indentation, Marvin is right that Uncrustify cannot support e= dk2 >>> indentation style out-of-box. Some changes are made in that fork to han= dle >>> the formatting. At this point, it can handle the indentation in the cas= es >>> I've seen. Uncrustify does potentially give us the ability to massively >>> deploy changes across the codebase in case a decision were made to chan= ge >>> the style. >>>=20 >>> Thanks, >>> Michael >>>=20 >>> On 8/16/2021 3:39 PM, Marvin H=C3=A4user wrote: >>>> Hey Rebecca, >>>>=20 >>>> I think even Uncrustify has issues with the EDK II indentation style. >>>> You might want to check the UEFI Talkbox Discord server, I had a brief >>>> chat with Michael about it there. I don't think realistically any tool >>>> supports EDK II's indentation style however, so I'd propose it is >>>> changed. This could be for new submissions only, or actually the entir= e >>>> codebase could be reformatted at once with a good tool setup. While th= is >>>> screws with git blame, the (to my understanding) decided on CRLF -> LF >>>> change does that anyway, so at least two evils could be dealt with in >>>> one go really. >>>>=20 >>>> Best regards, >>>> Marvin >>>>=20 >>>> On 16/08/2021 21:34, Rebecca Cran wrote: >>>>>=20 >>>>> cc devel@ . >>>>>=20 >>>>> On 8/16/21 1:33 PM, Rebecca Cran wrote: >>>>>>=20 >>>>>> I noticed a message on Twitter about an idea of using Uncrustify >>>>>> for EDK2 instead of the ECC tool, and came across https://www.mail-a= rchive.com/search?l=3Ddevel@edk2.groups.io&q=3Dsubject:%22Re%5C%3A+%5C%5Bed= k2%5C-devel%5C%5D+TianoCore+Community+Meeting+Minutes+%5C-+2%5C%2F4%22&o=3D= newest&f=3D1 >>>>>> . >>>>>>=20 >>>>>> I was wondering if there's been any progress on it that I could >>>>>> check out? >>>>>>=20 >>>>>>=20 >>>>>> Michael Kubacki: in that message, you said: >>>>>>=20 >>>>>> "I'm planning to put up a branch that we can use as a reference >>>>>> for a conversation around uncrustify in the next couple of >>>>>> weeks." >>>>>>=20 >>>>>>=20 >>>>>> Did you end up creating that branch, and if so could you provide >>>>>> a link to it please? >>>>>>=20 >>>>>>=20 >>>>>> --=20 >>>>>> Rebecca Cran >>>>>>=20 >>>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>=20 >>>=20 >>>=20 >>>=20 >>>=20 >>>=20 >>=20 >>=20 >=20 >=20 --Apple-Mail=_77417F23-4091-4CB7-9557-FD3FF82E3404 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 On the git-blame front we = might be able to build a recipe that walks past the code style and line end= ing commits that would could add to an FAQ or Wiki.

For bonus points we could add a git command th= at does this. You can add a Python git command to the repo by adding a git-= <commandName> script in the path. So we could add a Python git comman= d that skips blame of any of the known Uncrustify or line ending type commi= ts. Basically `git eblame` that works just like `git blame` but skips forma= tting changes. 

I=E2=80=99m working on a git-pgrep that only greps files used by a gi= ven build, hopeful I=E2=80=99ll be able to contribute that soon. My co-work= s keep finding bugs so I=E2=80=99m not quite done.
Thanks,

Andrew Fish

On Oct 7, 2021, at 11:30 A= M, Andrew Fish via groups.io &l= t;afish=3Dapple.c= om@groups.io> wrote:



On Oct 7, 2021, at 6:48 AM, = Leif Lindholm <leif@nuvi= ainc.com> wrote:

Hi Michael,
<= br class=3D"" style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; f= ont-size: 12px; font-style: normal; font-variant-caps: normal; font-weight:= normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-= transform: none; white-space: normal; word-spacing: 0px; -webkit-text-strok= e-width: 0px; text-decoration: none;">Apologies, I've owed you a res= ponse (promised off-list) for a while
now.

First, let me say I hugely appreciate = this effort. Apart from aligning
the codebase(s), this will reduce manual reviewing effort<= br class=3D"" style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; f= ont-size: 12px; font-style: normal; font-variant-caps: normal; font-weight:= normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-= transform: none; white-space: normal; word-spacing: 0px; -webkit-text-strok= e-width: 0px; text-decoration: none;">substantially, as well as cutt= ing down on number of rework cycles for
developers.
<= br class=3D"" style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; f= ont-size: 12px; font-style: normal; font-variant-caps: normal; font-weight:= normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-= transform: none; white-space: normal; word-spacing: 0px; -webkit-text-strok= e-width: 0px; text-decoration: none;">Looking at the changes to (wel= l, the comments in) uncrustify, this
seems to be constrained to:
- Newline after '(' for multi-line function calls.
- Dealing with "(("/"))" for= DEBUG macros.
- Functi= on pointer typedefs:
&n= bsp;- typedef\nEFIAPI
&= nbsp;- closing parentheses indentation

I don't think I've m= ade any secret over the years that I am not a
massive fan of the EDK2 coding style in general. So = I think for any
of its = quirks that are substantial enough that they require not just
custom configuration but actual new = function added to existing code
conformance tools, this would be an excellent point to sanitise th= e
coding style instead.=

Taking these in order:

Newline after '('
-----------------=
I think we already reached a = level of flexibility around this, where
we don't actually enforce this (or single argument per
line). Personally, I'd be = happy to update the coding style as
required instead.

DEBUG macro parentheses
-----------------------
How does uncrustify treat = DEBUG macros without this modification?
Do we start getting everything turned into multi-level ind= ented
multi-line stat= ements without this change?

Can w= e disable the rule that does not like the DEBUG macro? I seem to remember c= lang nags about some strange () usage, so maybe we would not lose that much= ?

Thanks,

Andrew Fish


Function pointer typedefs:
--------------------------
I don't see that function pointer = typedefs need to rigidly follow the
same pattern as the declaration of functions implementing them= . Could
we update the c= oding style (if needed) instead?

Best Regards,

Leif

On Mon, Aug 16, 2021 at 16:00:38 -0400, Michael K= ubacki wrote:
The edk2 branch was created here:
htt= ps://github.com/makubacki/edk2/tree/uncrustify_poc_2

We have a Project Mu fork with custom changes to the Uncrustify = tool to help
comply with EDK II formatting here:
ht= tps://dev.azure.com/projectmu/_git/Uncrustify

The latest information about the status and how to experiment with th= e
configuration file and the tool are in that fork: https://d= ev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-= (EDK-II)-Fork-Readme

That said, I have also fi= nished a CI plugin to run Uncrustify that should be
ready soo= n to initially deploy in Project Mu. Before doing so, I am trying
to settle on an initial configuration file that less strictly but mor= e
reliably formats the code than in the examples in those bra= nches. For
example, remove heuristics that when run against t= he same set of code
multiple times can produce different resu= lts. An example would be a rule
that reformats code because i= t exceeds a specified column width on one run
but on the next= run that reformatted code triggers a different rule to
furth= er align the code and so on. At least initially, some rules might be
tweaked in a more conservative approach that can be tightened in th= e future.
Once this configuration file is ready, we will base= line Project Mu code as
an example and turn on the plugin. Th= e CI plugin runs Uncrustify against
modified files and if the= re's any changes, indicating a formatting
deviation, the diff= chunks are saved in a log so they can be viewed as a
build a= rtifact.

I am making progress on the updated c= onfig file and I should be able to post
a "uncrustify_poc_3" = branch soon with the results.

Regarding indent= ation, Marvin is right that Uncrustify cannot support edk2
in= dentation style out-of-box. Some changes are made in that fork to handlethe formatting. At this point, it can handle the indentation in= the cases
I've seen. Uncrustify does potentially give us the= ability to massively
deploy changes across the codebase in c= ase a decision were made to change
the style.
<= br class=3D"">Thanks,
Michael

On= 8/16/2021 3:39 PM, Marvin H=C3=A4user wrote:
Hey Rebecca,

I think eve= n Uncrustify has issues with the EDK II indentation style.
Yo= u might want to check the UEFI Talkbox Discord server, I had a brief
chat with Michael about it there. I don't think realistically any t= ool
supports EDK II's indentation style however, so I'd propo= se it is
changed. This could be for new submissions only, or = actually the entire
codebase could be reformatted at once wit= h a good tool setup. While this
screws with git blame, the (t= o my understanding) decided on CRLF -> LF
change does that= anyway, so at least two evils could be dealt with in
one go = really.

Best regards,
Marvin

On 16/08/2021 21:34, Rebecca Cran wrote:

cc devel@ .

On 8/16/21 1:33 PM, Rebecca Cran wrote:

I noticed a message = on Twitter about an idea of using Uncrustify
for EDK2 instead= of the ECC tool, and came across https://www.mail-archive.com/search?l=3Dd= evel@edk2.groups.io&q=3Dsubject:%22Re%5C%3A+%5C%5Bedk2%5C-devel%5C%5D+T= ianoCore+Community+Meeting+Minutes+%5C-+2%5C%2F4%22&o=3Dnewest&f=3D= 1
.

I was wondering if there's b= een any progress on it that I could
check out?
=

Michael Kubacki: in that message, you said:
"I'm planning to put up a branch that we can us= e as a reference
for a conversation around uncrustify in the = next couple of
weeks."


Did you end up creating that branch, and if so could you provide
a link to it please?


= -- 
Rebecca= Cran











<= br class=3D"">



--Apple-Mail=_77417F23-4091-4CB7-9557-FD3FF82E3404--