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

["Leif Lindholm" <leif@nuviainc.com>]

Hi Mike,

On Tue, Nov 09, 2021 at 16:33:31 +0000, Michael D Kinney wrote:
> > >> 1. If virtually everyone will need Uncrustify, why cannot it be built along with BaseTools from a submodule? Especially
> > >> with the fork that makes sense, after that it depends on the upstream (it does not look too nice to me).
> > > No matter where uncrustify sources are hosted, developers can always choose to build the uncrustify tool locally.
> > > Providing release binaries for the tool may be simpler for some customers.
> > > Using release binaries from EDK II CI agents will reduce CI execution time.
> > 
> > My point was it'd be nice if it (optionally) "just worked", so
> > Uncrustify could be be built as part of the edk2 BaseTools build
> > process, or release binaries could be downloaded by some script, or
> > whatever really. I guess it could be the same logic as for the CI?
> 
> Setting up a development work environment also requires the installation of tools such as 
> compilers, NASM, IASL, etc.  These are not handled as part of BaseTools today.

No, but users are not required to install a specific, not-yet-upstream
version of a tool. This is the fundamental problem here.

Once the edk2 support is available in upstream uncrustify, there is a
substantial lag in how the updated version trickles down through
packaging systems.

> Supporting this indent style is one of the enhancements in the fork.  If we want to align
> to one of the indent styles supported by a wider array of source editors/IDEs, then that
> would require a change to the EDK II Coding Style Specification and approval from the
> EDK II community.

Yes. The above problem of getting the tool at the appropriate version
is why I previously stated my preference as being to change to coding
style so it matched what was already supported in upstream uncrustify.

Adding a version of the tool as a submodule of BaseTools would be  a
mitigation for the 5-year window introduced by *depending* on
not-yet-upstream support in a commonly used tool.

> > 6) Allow static function declarations.
> 
> I agree that static functions should be allowed.  Please add any comments you have to 
> the following Bugzilla.  If you have ideas on the specific spec updates required, then
> please provide a patch against the spec markdown.
> 
>     https://bugzilla.tianocore.org/show_bug.cgi?id=1766

I think that's a different issue, but otherwise agreed.

/
    Leif