diff mbox series

[b4,v2] ez: verify default checkpatch.pl arguments

Message ID 20240919-extend-checkpatch-v2-1-3557f177a942@linaro.org (mailing list archive)
State Under Review
Headers show
Series [b4,v2] ez: verify default checkpatch.pl arguments | expand

Commit Message

Manos Pitsidianakis Sept. 19, 2024, 6:42 a.m. UTC
For projects which use a scripts/checkpatch.pl workflow like the kernel
but do not sync its source [ever/frequently], like QEMU, some arguments
might not be available. This leads b4 prep --check to print the script's
error output.

This commit only uses arguments that exist in the script's help output.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
Changes in v2:
- Dropped patches that touched stuff that are fixed in upstream now
- Link to v1: https://patch.msgid.link/20240815-extend-checkpatch-v1-0-ab7f654be699@linaro.org
---
 src/b4/ez.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)


---
base-commit: dedf88cb947bab87c418b49d975df11f83621692
change-id: 20240815-extend-checkpatch-b2fe4b89da78

--
γαῖα πυρί μιχθήτω

Comments

Konstantin Ryabitsev Sept. 19, 2024, 11:25 a.m. UTC | #1
On Thu, Sep 19, 2024 at 09:42:45AM GMT, Manos Pitsidianakis wrote:
> -                ppcmds = [f'{checkpatch} -q --terse --no-summary --mailback --showfile']
> +                ecode, help_out, err = b4._run_command([checkpatch, "-h"], stdin=None, rundir=topdir)
> +                help_out = help_out.decode(errors='replace').strip() if help_out else ""

You should do this after `if ecode == 0`, since that's the only situation in
which you use this output. We also don't need to strip() or do the if/else
check, because Popen.communicate() should always return bytes.

> +                if ecode == 0:
> +                    ppcmds = f'{checkpatch}'

No need to f-string a single variable here.

> +                    for arg in ["q", "-terse", "-no-summary", "-mailback", "-showfile"]:
> +                        if f"-{arg} " in help_out:
> +                            ppcmds += f" -{arg}"
> +                    ppcmds += " -"
> +                    ppcmds = [ppcmds]
> +                else:
> +                    ppcmds = [f'{checkpatch} -q --terse --no-summary --mailback --showfile -']

I suggest you do the following that is a bit more python-idiomatic:
    
    defargs = ['--quiet', '--terse', '--no-summary', '--mailback', '--showfile']
    if ecode == 0:
        help_out = help_out.decode(errors='ignore')
        for defarg in list(defargs):
            if f' {defarg} ' not in help_out:
                defargs.remove(defarg)

    ppcmds = [f'{checkpatch} ' + ' '.join(defargs) + ' -']

Thanks for your help!

-K
diff mbox series

Patch

diff --git a/src/b4/ez.py b/src/b4/ez.py
index de3fa4287ad08c88b12956d47c4d1cc7bd03591c..1fdf0810a4378dee4ca0397b4983679f12a30d81 100644
--- a/src/b4/ez.py
+++ b/src/b4/ez.py
@@ -1680,7 +1680,17 @@  def get_check_cmds() -> Tuple[List[str], List[str]]:
         if topdir:
             checkpatch = os.path.join(topdir, 'scripts', 'checkpatch.pl')
             if os.access(checkpatch, os.X_OK):
-                ppcmds = [f'{checkpatch} -q --terse --no-summary --mailback --showfile']
+                ecode, help_out, err = b4._run_command([checkpatch, "-h"], stdin=None, rundir=topdir)
+                help_out = help_out.decode(errors='replace').strip() if help_out else ""
+                if ecode == 0:
+                    ppcmds = f'{checkpatch}'
+                    for arg in ["q", "-terse", "-no-summary", "-mailback", "-showfile"]:
+                        if f"-{arg} " in help_out:
+                            ppcmds += f" -{arg}"
+                    ppcmds += " -"
+                    ppcmds = [ppcmds]
+                else:
+                    ppcmds = [f'{checkpatch} -q --terse --no-summary --mailback --showfile -']
 
     # TODO: support for a whole-series check command, (pytest, etc)
     return ppcmds, scmds