Re: [edk2-devel] Progress on getting Uncrustify working for EDK2?

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 7204 bytes --]

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

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-<commandName> 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 works just like `git blame` but skips formatting changes. 

I’m working on a git-pgrep that only greps files used by a given build, hopeful I’ll be able to contribute that soon. My co-works keep finding bugs so I’m not quite done.

Thanks,

Andrew Fish

> On Oct 7, 2021, at 11:30 AM, Andrew Fish via groups.io <afish=apple.com@groups.io> wrote:
> 
> 
> 
>> On Oct 7, 2021, at 6:48 AM, Leif Lindholm <leif@nuviainc.com <mailto:leif@nuviainc.com>> wrote:
>> 
>> Hi Michael,
>> 
>> 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 rework cycles for
>> developers.
>> 
>> 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
>> 
>> 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.
>> 
>> 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 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?
> 
> 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 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/edk2/tree/uncrustify_poc_2 <https://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:
>>> https://dev.azure.com/projectmu/_git/Uncrustify
>>> 
>>> 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
>>> 
>>> 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 trying
>>> 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 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 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 so they can be viewed as a
>>> build artifact.
>>> 
>>> 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.
>>> 
>>> 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'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.
>>> 
>>> Thanks,
>>> Michael
>>> 
>>> On 8/16/2021 3:39 PM, Marvin Häuser wrote:
>>>> Hey Rebecca,
>>>> 
>>>> 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.
>>>> 
>>>> 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=devel@edk2.groups.io&q=subject:%22Re%5C%3A+%5C%5Bedk2%5C-devel%5C%5D+TianoCore+Community+Meeting+Minutes+%5C-+2%5C%2F4%22&o=newest&f=1
>>>>>> .
>>>>>> 
>>>>>> I was wondering if there's been 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 use 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
>>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>> 
>> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 43456 bytes --]