[PATCH v4] patman: By default don't pass "--no-tree" to checkpatch for linux

[PATCH v4] patman: By default don't pass "--no-tree" to checkpatch for linux

[Douglas Anderson]

When you pass "--no-tree" to checkpatch it disables some extra checks
that are important for Linux. Specifically I want checks like:

  warning: DT compatible string "boogie,woogie" appears un-documented
  check ./Documentation/devicetree/bindings/

Let's make the default for Linux to _not_ pass --no-tree. We'll have a
config option and command line flag to override.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Without BooleanOptionalAction (python 3.9), it's a little odd to make
the help text understandable while having the help text account for
the project settings when printing the default (which it can only
print as "True" or "False". It feels like no matter how you swing it
you've got to use a double-negative or use weird syntax like I did.

I removed previous review/testing tags since at least parts of this
patch changed in non-trivial ways.

This patch is now a singleton since the other patches in the series
landed or became irrelevant.

Changes in v4:
- Don't use BooleanOptionalAction

 tools/patman/checkpatch.py | 11 +++++++----
 tools/patman/control.py    |  7 ++++---
 tools/patman/main.py       |  6 ++++++
 tools/patman/settings.py   |  3 ++-
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 70ba561c2686..d1b902dd9627 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -186,7 +186,7 @@ def check_patch_parse(checkpatch_output, verbose=False):
     return result
 
 
-def check_patch(fname, verbose=False, show_types=False):
+def check_patch(fname, verbose=False, show_types=False, use_tree=False):
     """Run checkpatch.pl on a file and parse the results.
 
     Args:
@@ -194,6 +194,7 @@ def check_patch(fname, verbose=False, show_types=False):
         verbose: True to print out every line of the checkpatch output as it is
             parsed
         show_types: Tell checkpatch to show the type (number) of each message
+        use_tree (bool): If False we'll pass '--no-tree' to checkpatch.
 
     Returns:
         namedtuple containing:
@@ -210,7 +211,9 @@ def check_patch(fname, verbose=False, show_types=False):
             stdout: Full output of checkpatch
     """
     chk = find_check_patch()
-    args = [chk, '--no-tree']
+    args = [chk]
+    if not use_tree:
+        args.append('--no-tree')
     if show_types:
         args.append('--show-types')
     output = command.output(*args, fname, raise_on_error=False)
@@ -236,13 +239,13 @@ def get_warning_msg(col, msg_type, fname, line, msg):
     line_str = '' if line is None else '%d' % line
     return '%s:%s: %s: %s\n' % (fname, line_str, msg_type, msg)
 
-def check_patches(verbose, args):
+def check_patches(verbose, args, use_tree):
     '''Run the checkpatch.pl script on each patch'''
     error_count, warning_count, check_count = 0, 0, 0
     col = terminal.Color()
 
     for fname in args:
-        result = check_patch(fname, verbose)
+        result = check_patch(fname, verbose, use_tree=use_tree)
         if not result.ok:
             error_count += result.errors
             warning_count += result.warnings
diff --git a/tools/patman/control.py b/tools/patman/control.py
index b40382388e07..bf426cf7bcf4 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -64,7 +64,7 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
         patchstream.insert_cover_letter(cover_fname, series, to_do)
     return series, cover_fname, patch_files
 
-def check_patches(series, patch_files, run_checkpatch, verbose):
+def check_patches(series, patch_files, run_checkpatch, verbose, use_tree):
     """Run some checks on a set of patches
 
     This santiy-checks the patman tags like Series-version and runs the patches
@@ -77,6 +77,7 @@ def check_patches(series, patch_files, run_checkpatch, verbose):
         run_checkpatch (bool): True to run checkpatch.pl
         verbose (bool): True to print out every line of the checkpatch output as
             it is parsed
+        use_tree (bool): If False we'll pass '--no-tree' to checkpatch.
 
     Returns:
         bool: True if the patches had no errors, False if they did
@@ -86,7 +87,7 @@ def check_patches(series, patch_files, run_checkpatch, verbose):
 
     # Check the patches, and run them through 'git am' just to be sure
     if run_checkpatch:
-        ok = checkpatch.check_patches(verbose, patch_files)
+        ok = checkpatch.check_patches(verbose, patch_files, use_tree)
     else:
         ok = True
     return ok
@@ -165,7 +166,7 @@ def send(args):
         col, args.branch, args.count, args.start, args.end,
         args.ignore_binary, args.add_signoff)
     ok = check_patches(series, patch_files, args.check_patch,
-                       args.verbose)
+                       args.verbose, args.check_patch_use_tree)
 
     ok = ok and gitutil.check_suppress_cc_config()
 
diff --git a/tools/patman/main.py b/tools/patman/main.py
index d0c1a16b2b91..5fb78dc428ca 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -81,6 +81,12 @@ send.add_argument('--no-binary', action='store_true', dest='ignore_binary',
 send.add_argument('--no-check', action='store_false', dest='check_patch',
                   default=True,
                   help="Don't check for patch compliance")
+send.add_argument('--tree', dest='check_patch_use_tree', default=False,
+                  action='store_true',
+                  help=("Set `tree` to True. If `tree` is False then we'll "
+                  "pass '--no-tree' to checkpatch (default: tree=%(default)s)"))
+send.add_argument('--no-tree', dest='check_patch_use_tree',
+                  action='store_false', help="Set `tree` to False")
 send.add_argument('--no-tags', action='store_false', dest='process_tags',
                   default=True, help="Don't process subject tags as aliases")
 send.add_argument('--no-signoff', action='store_false', dest='add_signoff',
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 6c5f6c8ed992..cee18bf01a65 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -23,6 +23,7 @@ _default_settings = {
     "u-boot": {},
     "linux": {
         "process_tags": "False",
+        "check_patch_use_tree": "True",
     },
     "gcc": {
         "process_tags": "False",
@@ -71,7 +72,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
     >>> config = _ProjectConfigParser("linux")
     >>> config.readfp(StringIO(sample_config))
     >>> sorted((str(a), str(b)) for (a, b) in config.items("settings"))
-    [('am_hero', 'True'), ('process_tags', 'False')]
+    [('am_hero', 'True'), ('check_patch_use_tree', 'True'), ('process_tags', 'False')]
 
     # Check to make sure that settings works with unknown project.
     >>> config = _ProjectConfigParser("unknown")
-- 
2.37.0.170.g444d1eabd0-goog

Re: [PATCH v4] patman: By default don't pass "--no-tree" to checkpatch for linux

[Simon Glass]

On Tue, 19 Jul 2022 at 16:00, Douglas Anderson <dianders@chromium.org> wrote:
>
> When you pass "--no-tree" to checkpatch it disables some extra checks
> that are important for Linux. Specifically I want checks like:
>
>   warning: DT compatible string "boogie,woogie" appears un-documented
>   check ./Documentation/devicetree/bindings/
>
> Let's make the default for Linux to _not_ pass --no-tree. We'll have a
> config option and command line flag to override.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Without BooleanOptionalAction (python 3.9), it's a little odd to make
> the help text understandable while having the help text account for
> the project settings when printing the default (which it can only
> print as "True" or "False". It feels like no matter how you swing it
> you've got to use a double-negative or use weird syntax like I did.
>
> I removed previous review/testing tags since at least parts of this
> patch changed in non-trivial ways.
>
> This patch is now a singleton since the other patches in the series
> landed or became irrelevant.
>
> Changes in v4:
> - Don't use BooleanOptionalAction
>
>  tools/patman/checkpatch.py | 11 +++++++----
>  tools/patman/control.py    |  7 ++++---
>  tools/patman/main.py       |  6 ++++++
>  tools/patman/settings.py   |  3 ++-
>  4 files changed, 19 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v4] patman: By default don't pass "--no-tree" to checkpatch for linux

[Simon Glass]

On Tue, 19 Jul 2022 at 16:00, Douglas Anderson <dianders@chromium.org> wrote:
>
> When you pass "--no-tree" to checkpatch it disables some extra checks
> that are important for Linux. Specifically I want checks like:
>
>   warning: DT compatible string "boogie,woogie" appears un-documented
>   check ./Documentation/devicetree/bindings/
>
> Let's make the default for Linux to _not_ pass --no-tree. We'll have a
> config option and command line flag to override.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Without BooleanOptionalAction (python 3.9), it's a little odd to make
> the help text understandable while having the help text account for
> the project settings when printing the default (which it can only
> print as "True" or "False". It feels like no matter how you swing it
> you've got to use a double-negative or use weird syntax like I did.
>
> I removed previous review/testing tags since at least parts of this
> patch changed in non-trivial ways.
>
> This patch is now a singleton since the other patches in the series
> landed or became irrelevant.
>
> Changes in v4:
> - Don't use BooleanOptionalAction
>
>  tools/patman/checkpatch.py | 11 +++++++----
>  tools/patman/control.py    |  7 ++++---
>  tools/patman/main.py       |  6 ++++++
>  tools/patman/settings.py   |  3 ++-
>  4 files changed, 19 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!