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

["Michael Kubacki" <mikuback@linux.microsoft.com>]

That is borrowed from a pattern used in other plugins. Example:
https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/SpellCheck/SpellCheck.py#L160-#L161

I don't mind updating the plugins to do something different if that is 
desired, but perhaps that should be a separate change.

Regards,
Michael

On 3/22/2022 12:14 PM, Michael D Kinney wrote:
> 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
> 
> 
> 
> 
> 
>