public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"michael.kubacki@outlook.com" <michael.kubacki@outlook.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Andrew Fish <afish@apple.com>, Leif Lindholm <leif@nuviainc.com>,
	"Liming Gao" <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [edk2-wiki][PATCH v2 1/1] Add Uncrustify formatting overview
Date: Fri, 10 Dec 2021 01:07:06 +0000	[thread overview]
Message-ID: <CO1PR11MB492912E6CFE6BF484ED68B45D2719@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MWHPR07MB3440AC7A05E9A59AD0F2DB48E9709@MWHPR07MB3440.namprd07.prod.outlook.com>

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Wednesday, December 8, 2021 4:08 PM
> To: devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Liming Gao <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [edk2-wiki][PATCH v2 1/1] Add Uncrustify formatting overview
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> This wiki page describes how Uncrustify is used in EDK II and how
> to install and use Uncrustify during the EDK II code development
> process.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
> 
> Notes:
>     v2 changes:
> 
>     - Add UncrustifyCheck CI plugin readme and config file links
>     - Update manual usage instructions to include Linux instructions
>     - Clean up manual usage instructions
>     - Minor formatting changes
> 
>  EDK-II-Code-Formatting.md | 247 ++++++++++++++++++++
>  1 file changed, 247 insertions(+)
> 
> diff --git a/EDK-II-Code-Formatting.md b/EDK-II-Code-Formatting.md
> new file mode 100644
> index 000000000000..e671663e5dcd
> --- /dev/null
> +++ b/EDK-II-Code-Formatting.md
> @@ -0,0 +1,247 @@
> +# EDK II Code Formatting
> +
> +To better achieve the goals of the [EDK II C Coding Standards Specification](https://edk2-docs.gitbook.io/edk-ii-c-
> coding-standards-specification/),
> +EDK II code formatting is automated using a source code beautifier called Uncrustify. Uncrustify is compatible with
> +C/C++ in addition to other languages. In EDK II, it is used to format C language source code.
> +
> +## Uncrustify in the edk2 Repository
> +
> +The Uncrustify collateral in the edk2 repository contains all of the resources needed to get the Uncrustify application
> +and run it with the same settings as other developers.
> +
> +> Uncrustify is automatically run against code submitted in edk2 as a continuous integration (CI) plugin called
> +"UncrustifyCheck". The plugin is available in the following directory:
> +[.pytool/Plugin/UncrustifyCheck](https://github.com/tianocore/edk2/tree/master/.pytool/Plugin/UncrustifyCheck)
> +
> +The UncrustifyCheck plugin in edk2 contains the following files used to check code for compliance to the coding
> +standard:
> +
> +* **default_file_header.txt** - A text file containing a template that is placed at the top of files missing a file
> +  header.
> +* **default_function_header.txt** - A text file containing a template that is placed above functions that are missing
> +  a function header.
> +* **Readme.md** - A file that contains details about how the plugin works and how to use it.
> +* **uncrustify_ext_dep.yml** - An "external dependency" file that is used to get the current version of the Uncrustify
> +  application used by the plugin. This file contains the NuGet feed URL and the version currently used.
> +* **uncrustify_plug_in.yaml** - A file that contains information to describe the plugin to the build environment.
> +* **uncrustify.cfg** - A file used by the Uncrustify application to control how it formats code. If you want to tweak
> +  particular formatting details, this is the place to start.
> +* **UncrustifyCheck.py** - The actual Python file that is the CI plugin. Like all CI plugins, this plugin can be run
> +  in local CI and server CI.
> +
> +## EDK II Uncrustify Fork
> +
> +Due to nuances in the way EDK II formats code, some changes were made to the upstream Uncrustify application. These
> +were changes that could not be made purely through the Uncrustify configuration file. For more details about the fork,
> +please visit that project overview in the link below.
> +
> +* Uncrustify upstream repository: https://github.com/uncrustify/uncrustify
> +* Uncrustify EDK II fork repository (in Project Mu): https://dev.azure.com/projectmu/Uncrustify
> +
> +## Developer Workflow
> +
> +Developers must install Uncrustify and run the application against their code before sending patch review emails or
> +submitting pull requests. Pull requests run against EDK II CI which includes the UncrustifyCheck CI plugin.
> +
> +Fortunately, Uncrustify can be installed quickly, you can format your code quickly locally, and you can verify the
> +code against the UncrustifyCheck CI plugin before sending it to others.
> +
> +The recommended flow is:
> +
> +1. Clone the edk2 source code repository
> +2. Use the `stuart*` commands to pull the Uncrustify application into the edk2 workspace
> +3. Set up the ability to run Uncrustify locally (for example, using the Visual Studio Code Uncrustify plugin)
> +4. Make and test code changes
> +5. Run EDK II CI locally to verify UncrustifyCheck passes
> +6. Send the code patch to the EDK II mailing list
> +
> +## Installing Uncrustify
> +
> +Uncrustify is a portable executable that is built in the [EDK II Uncrustify fork
> repository](https://dev.azure.com/projectmu/Uncrustify)
> +and ultimately published into a NuGet feed in that fork project.
> +
> +### Recommended Installation: In edk2 repository
> +
> +It is strongly recommended to follow this flow. It sets up the workspace to work with local CI and automatically gets
> +the current supported version of the application.
> +
> +The Uncrustify tool is installed automatically when the Pytools environment is used and the `stuart*` commands are run
> +to complete environment setup. Review the edk2
> [.pytool/Readme.md](https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-locally)
> +file for details on `stuart` and this overall flow.
> +
> +After running the `stuart_update` command, the Uncrustify application content should be brought down into
> +`.pytool\Plugin\UncrustifyCheck\mu-uncrustify-release_extdep` in your edk2 workspace. The contents of this directory
> +now represent the contents of the NuGet package and it should contain a Linux and Windows executable of the
> +application.
> +
> +### Manual Installation: Download from the fork project
> +
> +The release pipeline in the EDK II Uncrustify fork project contains the build information for each release. Each build
> +in this pipeline represents a release. By going to a specific build, the details mapping the build to source code
> +(such as the branch and commit) are present.
> +
> +The build content is published as a NuGet package to a NuGet feed. This is the same feed, the recommended installation
> +instructions automatically pull from. The NuGet feed is available in the
> ["Artifacts"](https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=feed&feed=mu_uncrustify)
> +section of the fork project. If you hover/click on a specific package entry (e.g. "mu-uncrustify-release"), a set of
> +three ellipsis will appear. Click the ellipsis and a context menu will appear. The NuGet package can be downloaded
> +by clicking "Download \<x.y.z\>".
> +
> +Once downloaded, the `.nupkg` file can be treated as a zip file. If the file is opened as a zip file, the executable
> +can be found in the `mu-uncrustify-<debug/release>` directory.
> +
> +## How to Run Uncrustify
> +
> +Once Uncrustify is installed, you can run the application in a number of ways. In all cases, you should be using the
> +Uncrustify application built from the [Uncrustify EDK II fork](https://dev.azure.com/projectmu/Uncrustify) and the
> +[Uncrustify configuration
> file](https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg)
> +currently checked into edk2.
> +
> +### Recommended Usage: Visual Studio (VS) Code Plugin
> +
> +The Visual Studio Code plugin provides a way to seamlessly run Uncrustify against code at anytime in the editor and
> +the configuration details are set once in the editor configuration file.
> +
> +1. Install the Uncrustify VS Code extension:
> +   > Name: Uncrustify
> +     Id: zachflower.uncrustify
> +     Description: Code format using uncrustify
> +     Publisher: Zachary Flower
> +     VS Marketplace Link: <https://marketplace.visualstudio.com/items?itemName=zachflower.uncrustify>
> +
> +2. Configure the Uncrustify plugin for your local setup by adding the following to your VS Code settings.json file:
> +   (Windows example)
> +
> +   ```json
> +   "uncrustify.configPath.windows": "path_to_your_config_file",
> +   "uncrustify.executablePath.windows": "path_to_your_uncrustify_executable"
> +   ```
> +
> +   > Windows Example:
> +   >
> +   >```json
> +   >"uncrustify.configPath.windows": "D:/src/edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg",
> +   >"uncrustify.executablePath.windows": "D:/src/edk2/.pytool/Plugin/UncrustifyCheck/mu-uncrustify-
> release_extdep/Windows-x86/uncrustify.exe"
> +   >```
> +
> +3. Open a C source code file, _`Ctrl+Shift+P` -> Format Document With... -> Configure Default Formatter -> Uncrustify_
> +
> +4. Then, _`Ctrl+Shift+P` -> Format Document_ any time you would like to format your source code file with Uncrustify
> +
> +### Manual Usage: Run in a Terminal
> +
> +These instructions are written for Windows 10. These activities could be further automated into a high-level script
> +but that has not been done yet.
> +
> +1. Generate a list of the files to run against. This example generates a recursive list of all .c and .h files.
> +
> +   * It is recommended to run this in cleanly cloned edk2 repo without submodules to prevent submodule files
> +     (such as Brotli files in MdeModulePkg) from getting included in the file list (if you are running against all
> +     files). Including all files will significantly increase the amount of time Uncrustify takes to run.
> +
> +   * Sample Powershell command to recursively write all .c and .h files in a given package to a text file:
> +
> +     ```powershell
> +     Get-ChildItem -Path .\MdePkg\* -Include *.c, *.h -Recurse -Force | %{$_.fullname} | Out-File
> +     -FilePath .\MdePkgFiles.txt -Encoding utf8
> +     ```
> +
> +   > **WARNING** Powershell will put the UTF-8 BOM at the beginning of the output file. Uncrustify does not recognize the
> BOM
> +   and it should be removed before passing the file as input to Uncrustify. If it is not removed, Uncrustify will
> +   not read the first file path in the text file properly which will cause the file to not be formatted.
> +
> +2. Run Uncrustify using the generated text file as input
> +
> +   The following assume you move the EDK II Uncrustify configuration file to the directory .uncrustify in your edk2
> +   workspace.
> +
> +   ```shell
> +   uncrustify.exe -c .\.pytool\Plugin\UncrustifyCheck\uncrustify.cfg -F MdePkgFiles.txt --replace --no-backup --if-
> changed
> +   ```
> +
> +   > *Note:* When testing a configuration change, it is sometimes useful to run Uncrustify against a particular file
> +     and check the debug output to understand what rule was applied and why it was applied. The command shows an
> +     example of how to run the configuration file `uncrustify.cfg` against the source file `VariableSmm.c` where the
> +     file is forced to be treated as C, the debug output is written to `uncrustify_debug.txt` and the log severity level
> +     is set to "all".
> +
> +   ```shell
> +   uncrustify.exe -c .\.pytool\Plugin\UncrustifyCheck\uncrustify.cfg -f
> .\MdeModulePkg\Universal\Variable\RuntimeDxe\VariableSmm.c -o output.c -l C -p uncrustify_debug.txt -L A
> 2>verbose_debug.txt
> +   ```
> +
> +Uncrustify will update the source files in-place (with the commands given). This allows you to diff the results with
> +git. From here, you can iteratively tweak the configuration file and check the results until your satisfied with the
> +outcome.
> +
> +## Uncrustify in CI
> +
> +The UncrustifyCheck CI plugin that will verify formatting on the server can be run locally. It is recommended to run
> +local CI to verify the patch submission will pass CI on the server.
> +
> +This can be done using the `stuart_ci_build` command.
> +
> +> Tip: To quickly only run UncrustifyCheck, remove the other plugin directories from your local `.pytool` directory
> +  and, of course, add them back when you're done.
> +
> +Here's an example of running UncrustifyCheck against MdeModulePkg from the root of an edk2 workspace:
> +
> +`stuart_ci_build -c .pytool/CISettings.py -p MdeModulePkg`
> +
> +If a file has a formatting error, it will be noted in the output from `stuart_ci_build`. This is visible in the
> +terminal output in local CI and the build output log in server CI.
> +
> +Read the [UncrustifyCheck
> Readme.md](https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/Readme.md)
> +to understand more about how the plugin can be configured for CI.
> +
> +## Extra Reading: Tracing History Across the Uncrustify Changes
> +
> +It might be helpful to view the entire history rewritten with Uncrustify formatting on every commit. For example, an
> +alternate version of the edk2 repository that serves as "documentation" with the entire history re-written.
> +
> +A tool called git-filter-repo can be used to perform this transformation and runs in a reasonable period of time
> +(a few hours):
> +
> +* <https://github.com/newren/git-filter-repo>
> +* <https://github.com/newren/git-filter-repo/blob/main/contrib/filter-repo-demos/lint-history>
> +
> +The following steps can be used to perform this transformation. This is the Windows process. A Linux process will be
> +added in the future.
> +
> +  > **WARNING**  This operation modifies (rewrites) all the commits in the local  copy of the repo.  Do not perform
> +                 these steps on a local repo you are using for active development.
> +
> +1. Clone edk2 into a new directory (see **WARNING**)
> +
> +   ```shell
> +   git clone https://github.com/tianocore/edk2.git edk2-uncrustified
> +   cd edk2-uncrustified
> +   ```
> +
> +2. Setup python virtual env, install pytools, and run stuart commands to setup build environment which includes
> +   installing uncrustify tools. See [Running CI Locally](https://github.com/tianocore/edk2/tree/master/.pytool#running-
> ci-locally).
> +
> +3. Make a backup copy of the plugin UncrustifyCheck outside WORKSPACE. (e.g. C:\Temp\UncrustifyCheck) so the
> +   Uncrustify executable and EDK II specific Uncrustify configuration file are available when working with a branch
> +   that does not have those tools in its scope.
> +
> +   ```shell
> +   xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck
> +   ```
> +
> +4. Use lint-history.py from git-filter-repo examples
> +
> +   * [git-filter-repo](https://github.com/newren/git-filter-repo)
> +   * [lint-history](https://github.com/newren/git-filter-repo/blob/main/contrib/filter-repo-demos/lint-history)
> +
> +Line #127 - Add try except around subprocess.check_call() with except being pass. This is required because there are a
> +few commits of C files in the edk2 repo that have incorrect C syntax and do not build with a C compiler and break the
> +Uncrustify parser. Skip reformat of C files that can not be parsed by uncrustify. These rare instances are addressed in
> +the commit that fixes the C syntax error.
> +
> +Run this slightly modified version of lint-history. Include only .c/.h files and exclude directories that start with
> +`Tools` or `BaseTools`. This step took about 2.2 hours on a laptop.
> +
> +```shell
> +lint-history.py
> +    --relevant "return (not filename.startswith(b'Tools') and not filename.startswith(b'BaseTools') and
> (filename.endswith(b'.c') or filename.endswith(b'.h')))"
> +    c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\mu-uncrustify-release_extdep\\Windows-x86\\uncrustify.exe -c
> c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\uncrustify.cfg --replace --no-backup --if-changed
> +```
> --
> 2.28.0.windows.1
> 
> 
> 
> 
> 


  reply	other threads:[~2021-12-10  1:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09  0:07 [edk2-wiki][PATCH v2 1/1] Add Uncrustify formatting overview Michael Kubacki
2021-12-10  1:07 ` Michael D Kinney [this message]
2021-12-10  1:26   ` [edk2-devel] " Michael D Kinney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO1PR11MB492912E6CFE6BF484ED68B45D2719@CO1PR11MB4929.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox