Re: [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support

["Michael D Kinney" <michael.d.kinney@intel.com>]

Michael,

One comment below.

Mike

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Monday, March 21, 2022 6:58 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Sean Brogan
> <sean.brogan@microsoft.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>
> Subject: [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3880
> 
> Currently UncrustifyCheck.py provides the following exclusion
> options:
> 
> 1. Override the type of files UncrustifyCheck operates against by
> default (.c and .h files). Using the "IgnoreStandardPaths"
> configuration option.
> 
> 2. By default, UncrustifyCheck skips files in git submodules and
> ignored by git (the "SkipGitExclusions" configuration option can
> override this behavior).
> 
> The goal of UncrustifyCheck is to provide consistent formatting
> across the codebase. In some rare circumstances, maintainers might
> need to exclude a specific file (or file pattern) within their
> package. For example, a small set of auto-generated files from
> another repository.
> 
> This change adds a new configuration option that can be specified
> in a package CI YAML file to describe a list of files within the
> package that should be ignored by UncrustifyCheck.
> 
> The configuration option is called "IgnoreFiles" and it uses similar
> syntax to git ignore to ignore a list of files.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  .pytool/Plugin/UncrustifyCheck/Readme.md          |  7 ++++++
>  .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 24 ++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/.pytool/Plugin/UncrustifyCheck/Readme.md b/.pytool/Plugin/UncrustifyCheck/Readme.md
> index 0c46fd241a7a..efe7a573e4fa 100644
> --- a/.pytool/Plugin/UncrustifyCheck/Readme.md
> +++ b/.pytool/Plugin/UncrustifyCheck/Readme.md
> @@ -41,6 +41,7 @@ The plugin can be configured with a few optional configuration options.
>        "AdditionalIncludePaths": [], # Additional paths to check formatting (wildcards supported).
>        "AuditOnly": False,           # Don't fail the build if there are errors.  Just log them.
>        "ConfigFilePath": "",         # Custom path to an Uncrustify config file.
> +      "IgnoreFiles": [],            # A list of file patterns to ignore.
>        "IgnoreStandardPaths": [],    # Standard Plugin defined paths that should be ignored.
>        "OutputFileDiffs": True,      # Output chunks of formatting diffs in the test case log.
>                                      # This can significantly slow down the plugin on very large packages.
> @@ -67,6 +68,12 @@ the test as skipped. This allows visibility into the failures without breaking t
> 
>  When specified in the config file, this is a package relative path to the Uncrustify configuration file.
> 
> +### `IgnoreFiles`
> +
> +This option supports .gitignore file and folder matching strings including wildcards.
> +
> +The files specified by this configuration option will not be processed by Uncrustify.
> +
>  ### `IgnoreStandardPaths`
> 
>  This plugin by default will check the below standard paths. A package configuration file can specify any of these paths
> diff --git a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> index dc133fecc4b2..e8370381032f 100644
> --- a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> +++ b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> @@ -13,11 +13,13 @@ import os
>  import pathlib
>  import shutil
>  import timeit
> +from collections.abc import Callable
>  from edk2toolext.environment import version_aggregator
>  from edk2toolext.environment.plugin_manager import PluginManager
>  from edk2toolext.environment.plugintypes.ci_build_plugin import ICiBuildPlugin
>  from edk2toolext.environment.plugintypes.uefi_helper_plugin import HelperFunctions
>  from edk2toolext.environment.var_dict import VarDict
> +from edk2toollib.gitignore_parser import parse_gitignore_lines
>  from edk2toollib.log.junit_report_format import JunitReportTestCase
>  from edk2toollib.uefi.edk2.path_utilities import Edk2Path
>  from edk2toollib.utility_functions import  RunCmd
> @@ -273,6 +275,15 @@ class UncrustifyCheck(ICiBuildPlugin):
>              f"-c {self._app_config_file} -F {self._app_input_file_path} --if-changed --suffix
> {UncrustifyCheck.FORMATTED_FILE_EXTENSION}", outstream=output)
>          self._app_output = output.getvalue().strip().splitlines()
> 
> +    def _get_files_ignored_in_config(self) -> Callable[[str], bool]:
> +        """"
> +        Returns a function that returns true if a given file string path is ignored in the plugin configuration file and false
> otherwise.
> +        """
> +        ignored_files = []
> +        if "IgnoreFiles" in self._package_config:
> +            ignored_files = self._package_config["IgnoreFiles"]
> +        return parse_gitignore_lines(ignored_files, os.path.join(self._abs_workspace_path, "nofile.txt"),
> self._abs_workspace_path)

What is the purpose of "nofile.txt" here?  If this a param that is optional/ignored?  If so, there
are better python methods to do that.

> +
>      def _get_git_ignored_paths(self) -> List[str]:
>          """"
>          Returns a list of file absolute path strings to all files ignored in this git repository.
> @@ -464,6 +475,19 @@ class UncrustifyCheck(ICiBuildPlugin):
>              self._abs_file_paths_to_format.extend(
>                  [str(path.resolve()) for path in pathlib.Path(self._abs_package_path).rglob(path)])
> 
> +        # Remove files ignore in the plugin configuration file
> +        plugin_ignored_files = list(filter(self._get_files_ignored_in_config(), self._abs_file_paths_to_format))
> +
> +        if plugin_ignored_files:
> +            logging.info(
> +                f"{self._package_name} file count before plugin ignore file exclusion: {len(self._abs_file_paths_to_format)}")
> +            for path in plugin_ignored_files:
> +                if path in self._abs_file_paths_to_format:
> +                    logging.info(f"  File ignored in plugin config file: {path}")
> +                    self._abs_file_paths_to_format.remove(path)
> +            logging.info(
> +                f"{self._package_name} file count after plugin ignore file exclusion: {len(self._abs_file_paths_to_format)}")
> +
>          if not "SkipGitExclusions" in self._package_config or not self._package_config["SkipGitExclusions"]:
>              # Remove files ignored by git
>              logging.info(
> --
> 2.28.0.windows.1