From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.89.1633626118427356387 for ; Thu, 07 Oct 2021 10:01:58 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=ZbYRXmOR; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [10.0.0.19] (c-73-27-179-174.hsd1.fl.comcast.net [73.27.179.174]) by linux.microsoft.com (Postfix) with ESMTPSA id 22E4920B85E6; Thu, 7 Oct 2021 10:01:57 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 22E4920B85E6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1633626117; bh=R5PrfvXKsLc7xF33U/aLfK1YdCfU6Dh0JrpRT9iSuqo=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=ZbYRXmORmcanVDO3Hg+rAxstKSbtbb+FYAZc9i9UbyTeG5QYCMqV1IZKAkvTX91/X RJ7qu3BMstUeLAq9VqkRWZsLrQpQT3xpwi6YlAzKcOihUIlX4WMo6gVYFOdyAiS9Vl E4JHpgl95RU6bEa7Cwld1+VlgLALZu+oUWs0rdps= Subject: Re: [edk2-devel] Progress on getting Uncrustify working for EDK2? To: "Kinney, Michael D" , Andrew Fish , edk2-devel-groups-io , Michael Kubacki Cc: Leif Lindholm , =?UTF-8?Q?Marvin_H=c3=a4user?= , Rebecca Cran , Bret Barkelew 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> From: "Michael Kubacki" Message-ID: <0fb7eb63-9ef2-a4c9-ba81-f1c82149de0c@linux.microsoft.com> Date: Thu, 7 Oct 2021 13:01:56 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Thanks Mike, I will look into that. For others, I recently published an "uncrustify_poc_3" branch on my edk2=20 fork that demonstrates the latest set of results I've obtained with=20 Uncrustify tool changes and the configuration file. Most of the=20 edk2-specific issues have been mitigated and I was able to reach a=20 steady state of formatting results after two runs against the edk2 code=20 base. The results are starting to trend toward what might be "acceptable". I would note that the edk2 repository and, in particular, certain=20 packages like MdePkg and MdeModulePkg will have less substantial changes=20 than others as the coding style tends to be followed more consistently=20 there relative to many other repos. Here's a link to the branch: https://github.com/makubacki/edk2/tree/uncrustify_poc_3 There's a version here that attempts to add missing file and function=20 headers (same branch just with that commit at the top): https://github.com/makubacki/edk2/tree/uncrustify_poc_3_with_headers Here's an Uncrustify application PR that I'm currently using the=20 executable from for testing (it is in the PR build artifacts): https://dev.azure.com/projectmu/Uncrustify/_git/Uncrustify/pullrequest/24 I'm interested in the discussion around style changes though we might be=20 able to get away with less of those now. I am working on this in between=20 other work so I also appreciate anyone willing to help out in any way. Thanks, Michael On 10/7/2021 12:34 PM, Kinney, Michael D wrote: > Hi Michael, >=20 > I am looking at your uncrustify_poc_3 branch, and I see an unexpected cha= nge > related to use of IN/OUT in a function declaration. >=20 > File: MdeModulePkg\Logo\Logo.c >=20 > Before Uncrustify > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > EFI_STATUS > EFIAPI > GetImage ( > IN EDKII_PLATFORM_LOGO_PROTOCOL *This, > IN OUT UINT32 *Instance, > OUT EFI_IMAGE_INPUT *Image, > OUT EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE *Attribute, > OUT INTN *OffsetX, > OUT INTN *OffsetY > ) >=20 > After Uncrustify > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > GetImage ( > IN EDKII_PLATFORM_LOGO_PROTOCOL *This, > IN OUT UINT32 *Instance, > OUT EFI_IMAGE_INPUT *Image, > OUT EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE *Attribute, > OUT INTN *OffsetX, > OUT INTN *OffsetY > ) >=20 > It aligned the start of each parameter line with 2 space indent, but lost= the > alignment of the parameter names and did not do 2 spaces between the para= meter > type and parameter name. >=20 > Best regards, >=20 > Mike >=20 > ---------------------------------------------------------- >=20 > From: Andrew Fish > Sent: Thursday, October 7, 2021 9:04 AM > To: edk2-devel-groups-io > Cc: Leif Lindholm ; mikuback@linux.microsoft.com; Marv= in H=C3=A4user ; Rebecca Cran ; M= ichael Kubacki ; Bret Barkelew ; Kinney, Michael D ; Andrew Fi= sh > Subject: Re: [edk2-devel] Progress on getting Uncrustify working for EDK2= ? >=20 > On the git-blame front we might be able to build a recipe that walks past= the code style and line ending commits that would could add to an FAQ or W= iki. >=20 > For bonus points we could add a git command that does this. You can add a= Python git command to the repo by adding a git- script in the= path. So we could add a Python git command that skips blame of any of the = known Uncrustify or line ending type commits. Basically `git eblame` that w= orks 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 = build, hopeful I=E2=80=99ll be able to contribute that soon. My co-works ke= ep finding bugs so I=E2=80=99m not quite done. >=20 > Thanks, >=20 > Andrew Fish >=20 >=20 > On Oct 7, 2021, at 11:30 AM, Andrew Fish via http://groups.io wrote: >=20 >=20 >=20 >=20 > On Oct 7, 2021, at 6:48 AM, Leif Lindholm wrot= e: >=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: > =C2=A0- typedef\nEFIAPI > =C2=A0- 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 >=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: >=20 > 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 h= elp > 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/p= rojectmu/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 should= be > ready soon to initially deploy in Project Mu. Before doing so, I am tryin= g > 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 ru= n > 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 futu= re. > Once this configuration file is ready, we will baseline Project Mu code a= s > 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 p= ost > a "uncrustify_poc_3" branch soon with the results. >=20 > Regarding indentation, Marvin is right that Uncrustify cannot support edk= 2 > indentation style out-of-box. Some changes are made in that fork to handl= e > the 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 case a decision were made to change > the style. >=20 > Thanks, > Michael >=20 > On 8/16/2021 3:39 PM, Marvin H=C3=A4user wrote: >=20 > 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 this > 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 >=20 > cc devel@ . >=20 > On 8/16/21 1:33 PM, Rebecca Cran wrote: >=20 >=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-archiv= e.com/search?l=3Ddevel@edk2.groups.io&q=3Dsubject:%22Re%5C%3A+%5C%5Bedk2%5C= -devel%5C%5D+TianoCore+Community+Meeting+Minutes+%5C-+2%5C%2F4%22&o=3Dnewes= t&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