From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from rn-mailsvcp-ppex-lapp34.apple.com (rn-mailsvcp-ppex-lapp34.apple.com [17.179.253.43]) by mx.groups.io with SMTP id smtpd.web08.13511.1633620630976100447 for ; Thu, 07 Oct 2021 08:30:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=HE899lfZ; spf=pass (domain: apple.com, ip: 17.179.253.43, mailfrom: afish@apple.com) Received: from pps.filterd (rn-mailsvcp-ppex-lapp34.rno.apple.com [127.0.0.1]) by rn-mailsvcp-ppex-lapp34.rno.apple.com (8.16.1.2/8.16.1.2) with SMTP id 197FDbak010031; Thu, 7 Oct 2021 08:30:28 -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=2pTHAo3c1IFuxVfh7mqENF7ULfC49INtmTZKN/fK/w4=; b=HE899lfZxPDMGJLXrVewxKmkHihvmfhulNAhMeb/P3thEtwpbhq0dsJkhd7BCh7MVm3l VpM3fkE3QQWtXEBHBjcCIgZrapUhj0iJblKkmREeVvoolK3v9LhByUrfdQYSPAM71JmV 9BzPi+g9C5U4/efl0/37y4iep47GXEXBkERApN2MactVBRjUhSlDusdA5zEFMTxEq4Yb CXnfOIPZ0z+URVMl9Z+2xNZLOVq6ieVbFo2MSW5QQ9LrGq0C1jdqVW5FADGzvCMoRM2f xbxK367EhH9SoL+yr3ZhePQYTMGKYafM/5MnvoO/4LyhQO1CPBChzdZqpbhEi1TWNWAi nA== Received: from ma-mailsvcp-mta-lapp02.corp.apple.com (ma-mailsvcp-mta-lapp02.corp.apple.com [10.226.18.134]) by rn-mailsvcp-ppex-lapp34.rno.apple.com with ESMTP id 3bek92tuq1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Thu, 07 Oct 2021 08:30:28 -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 <0R0M00TVC5QRT400@ma-mailsvcp-mta-lapp02.corp.apple.com>; Thu, 07 Oct 2021 08:30:28 -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 <0R0M007005KXFM00@ma-mailsvcp-mmp-lapp02.apple.com>; Thu, 07 Oct 2021 08:30:27 -0700 (PDT) X-Va-A: X-Va-T-CD: 004664a286f28cb677a60481ed4243ce X-Va-E-CD: 574fda742400e238a02c5048d028c5dd X-Va-R-CD: b4e96f8f3d52136df210f2e6a101920e X-Va-CD: 0 X-Va-ID: 61375850-8776-4782-9391-06976b6dc8c3 X-V-A: X-V-T-CD: 004664a286f28cb677a60481ed4243ce X-V-E-CD: 574fda742400e238a02c5048d028c5dd X-V-R-CD: b4e96f8f3d52136df210f2e6a101920e X-V-CD: 0 X-V-ID: 085b2515-267e-4ac1-a6df-d03c5bf844f4 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 <0R0M00QF75QJI100@ma-mailsvcp-mmp-lapp02.apple.com>; Thu, 07 Oct 2021 08:30:27 -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 11:30:19 -0400 In-reply-to: <20211007104813.wa4rmfsqgcpvnzwt@leviathan> Cc: mikuback@linux.microsoft.com, mhaeuser@posteo.de, rebecca@nuviainc.com, Michael Kubacki , Bret Barkelew , Mike Kinney To: edk2-devel-groups-io , Leif Lindholm 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> 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=_AEC09A0B-4717-4A9B-93AE-7E935C190717" --Apple-Mail=_AEC09A0B-4717-4A9B-93AE-7E935C190717 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > 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? Can we disable the rule that does not like the DEBUG macro? I seem to remem= ber clang nags about some strange () usage, so maybe we would not lose that= much? Thanks, Andrew Fish >=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)-Fork= -Readme >>=20 >> That said, I have also finished a CI plugin to run Uncrustify that shoul= d be >> ready soon to initially deploy in Project Mu. Before doing so, I am tryi= ng >> 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 rule >> that reformats code because it exceeds a specified column width on one r= un >> 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 b= e >> tweaked in a more conservative approach that can be tightened in the fut= ure. >> 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 against >> 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 ed= k2 >> indentation style out-of-box. Some changes are made in that fork to hand= le >> the formatting. At this point, it can handle the indentation in the case= s >> I've seen. Uncrustify does potentially give us the ability to massively >> deploy changes across the codebase in case a decision were made to chang= e >> 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 entire >>> codebase could be reformatted at once with a good tool setup. While thi= s >>> 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-ar= chive.com/search?l=3Ddevel@edk2.groups.io&q=3Dsubject:%22Re%5C%3A+%5C%5Bedk= 2%5C-devel%5C%5D+TianoCore+Community+Meeting+Minutes+%5C-+2%5C%2F4%22&o=3Dn= ewest&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 --Apple-Mail=_AEC09A0B-4717-4A9B-93AE-7E935C190717 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On Oct 7, 202= 1, at 6:48 AM, Leif Lindholm <leif@nuviainc.com> wrote:

Hi Micha= el,

Apologies, I've owed you a response (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
substantially, as well as cutting down on number of re= work cycles for
develop= ers.

Looking at the changes to (well, the comments in) uncr= ustify, this
seems to b= e constrained to:
- Ne= wline after '(' for multi-line function calls.
- Dealing with "(("/"))" for DEBUG macros.- Function pointer typedefs:
 - typedef\nEFIAPI
 - closing parenthe= ses indentation

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<= br style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 1= 2px; font-style: normal; font-variant-caps: normal; font-weight: normal; le= tter-spacing: normal; text-align: start; text-indent: 0px; text-transform: = none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0p= x; text-decoration: none;" class=3D"">of its quirks that are substan= tial enough that they require not just
custom configuration but actual new function added to exist= ing code
conformance to= ols, this would be an excellent point to sanitise the
coding style instead.

Taki= ng these in order:

Newline after '('
-----------------
I think we already reached a level of flexibilit= y 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
requ= ired instead.

DEBUG macro parentheses
-----------------------
How does uncrustify treat DEBUG macros with= out this modification?
= Do we start getting everything turned into multi-level indented
multi-line statements without this= change?

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

Tha= nks,

Andrew Fish


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

Best Regards,

Leif

On Mon, Aug 16, 2021 at 16:00:38 -0400, Michael Kubacki wrote= :
The edk2= branch was created here:
https://github.com/makubacki/ed= k2/tree/uncrustify_poc_2

We have a Project= Mu fork with custom changes to the Uncrustify tool to help
c= omply with EDK II formatting here:
https://dev.azure.com/proj= ectmu/_git/Uncrustify

The latest information a= bout the status and how to experiment with the
configuration = file and the tool are in that fork: https://dev.azure.com/projectmu/Uncrust= ify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme

That said, I have also finished a CI plugin to run Unc= rustify that should be
ready soon to initially deploy in Proj= ect Mu. Before doing so, I am trying
to settle on an initial = configuration file that less strictly but more
reliably forma= ts the code than in the examples in those branches. For
examp= le, remove heuristics that when run against the same set of code
multiple times can produce different results. An example would be a r= ule
that reformats code because it exceeds a specified column= width on one run
but on the next run that reformatted code t= riggers a different rule to
further align the code and so on.= At least initially, some rules might be
tweaked in a more co= nservative approach that can be tightened in the future.
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 = against
modified files and if there's any changes, indicating= a formatting
deviation, the diff chunks are saved in a log s= o they can be viewed as a
build artifact.

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

Regarding indentation, Marvin is right that = Uncrustify cannot support edk2
indentation style out-of-box. = Some changes are made in that fork to handle
the formatting. = At this point, it can handle the indentation in the cases
I'v= e seen. Uncrustify does potentially give us the ability to massively
deploy changes across the codebase in case a decision were made to = change
the style.

Thanks,
Michael

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

I think even Uncrustify has issues with= the EDK II indentation style.
You might want to check the UE= FI Talkbox Discord server, I had a brief
chat with Michael ab= out it there. I don't think realistically any tool
supports E= DK II's indentation style however, so I'd propose it is
chang= ed. This could be for new submissions only, or actually the entire
codebase could be reformatted at once with a good tool setup. While t= his
screws with git blame, the (to my understanding) decided = on CRLF -> LF
change does that anyway, so at least two evi= ls 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 a= cross https://www.mail-archive.com/search?l=3Ddevel@edk2.groups.io&q=3D= subject:%22Re%5C%3A+%5C%5Bedk2%5C-devel%5C%5D+TianoCore+Community+Meeting+M= inutes+%5C-+2%5C%2F4%22&o=3Dnewest&f=3D1
.

I was wondering if there's been any progress on it tha= t I could
check out?


Michael Kubacki: in that message, you said:

"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."


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


-- 
Rebecca Cran















--Apple-Mail=_AEC09A0B-4717-4A9B-93AE-7E935C190717--