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

Michael Kubacki posted 4 patches 3 years, 10 months ago
There is a newer version of this series
[edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
Posted by Michael Kubacki 3 years, 10 months ago
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)
+
     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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87811): https://edk2.groups.io/g/devel/message/87811
Mute This Topic: https://groups.io/mt/89944184/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
Posted by Michael D Kinney 3 years, 10 months ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87840): https://edk2.groups.io/g/devel/message/87840
Mute This Topic: https://groups.io/mt/89944184/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
Posted by Michael Kubacki 3 years, 10 months ago
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
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88340): https://edk2.groups.io/g/devel/message/88340
Mute This Topic: https://groups.io/mt/89944184/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
Posted by Michael D Kinney 3 years, 10 months ago
Can you explain how this filename is used?  Perhaps add a comment in this series.

If this pattern is used in several places, then I agree that can be cleaned up in a different patch series.

Mike 


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Friday, April 1, 2022 9:07 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
> 
> 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
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88349): https://edk2.groups.io/g/devel/message/88349
Mute This Topic: https://groups.io/mt/89944184/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
Posted by Sean 3 years, 10 months ago
In regard to the "nofile.txt"
That parameter is used for reporting which file specified the ignore list.  This was a feature of the gitignore parser that was integrated but that "report" is never used.  Thus, in the additional api I added where I enabled the ignore content in a parameter rather than file, I "shimmed" it using nofile.txt as an indicator that this came from parameter.

Hope that helps explain why.
Thanks
Sean


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88357): https://edk2.groups.io/g/devel/message/88357
Mute This Topic: https://groups.io/mt/89944184/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support
Posted by Michael D Kinney 3 years, 10 months ago
Hi Sean,

Perhaps in the future this can be converted to an option python parameter with a default value of None?

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
Sent: Friday, April 1, 2022 10:45 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 2/4] .pytool/Plugin/UncrustifyCheck: Add ignore file support

In regard to the "nofile.txt"
That parameter is used for reporting which file specified the ignore list.  This was a feature of the gitignore parser that was integrated but that "report" is never used.  Thus, in the additional api I added where I enabled the ignore content in a parameter rather than file, I "shimmed" it using nofile.txt as an indicator that this came from parameter.

Hope that helps explain why.
Thanks
Sean



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88381): https://edk2.groups.io/g/devel/message/88381
Mute This Topic: https://groups.io/mt/89944184/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-