[PATCH] clang-tools: Print information when clang-tidy tool is missing

[PATCH] clang-tools: Print information when clang-tidy tool is missing

[Maciej Falkowski]

When clang-tidy tool is missing in the system, the FileNotFoundError
exception is raised in the program reporting a stack trace to the user:

$ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/usr/lib64/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib64/python3.8/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "./scripts/clang-tools/run-clang-tools.py", line 54, in run_analysis
    p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
  File "/usr/lib64/python3.8/subprocess.py", line 489, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'clang-tidy'
"""

The patch adds more user-friendly information about missing tool by
checking the presence of clang-tidy using `command -v` at the beginning
of the script:

$ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
Command 'clang-tidy' is missing in the system

Signed-off-by: Maciej Falkowski <maciej.falkowski9@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1342
---
 scripts/clang-tools/run-clang-tools.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
index fa7655c7cec0..d34eaf5a0ee5 100755
--- a/scripts/clang-tools/run-clang-tools.py
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -60,6 +60,11 @@ def run_analysis(entry):
 
 
 def main():
+    exitcode = subprocess.getstatusoutput('command -v clang-tidy')[0]
+    if exitcode == 1:
+        print("Command 'clang-tidy' is missing in the system", file=sys.stderr)
+        sys.exit(127)
+
     args = parse_arguments()
 
     lock = multiprocessing.Lock()
-- 
2.26.3

Re: [PATCH] clang-tools: Print information when clang-tidy tool is missing

[Nathan Chancellor]

On Sat, Jul 03, 2021 at 01:51:20AM +0200, Maciej Falkowski wrote:
> When clang-tidy tool is missing in the system, the FileNotFoundError
> exception is raised in the program reporting a stack trace to the user:
> 
> $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> multiprocessing.pool.RemoteTraceback:
> """
> Traceback (most recent call last):
>   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 125, in worker
>     result = (True, func(*args, **kwds))
>   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 48, in mapstar
>     return list(map(*args))
>   File "./scripts/clang-tools/run-clang-tools.py", line 54, in run_analysis
>     p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
>   File "/usr/lib64/python3.8/subprocess.py", line 489, in run
>     with Popen(*popenargs, **kwargs) as process:
>   File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
>     self._execute_child(args, executable, preexec_fn, close_fds,
>   File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
>     raise child_exception_type(errno_num, err_msg, err_filename)
> FileNotFoundError: [Errno 2] No such file or directory: 'clang-tidy'
> """
> 
> The patch adds more user-friendly information about missing tool by
> checking the presence of clang-tidy using `command -v` at the beginning
> of the script:
> 
> $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> Command 'clang-tidy' is missing in the system
> 
> Signed-off-by: Maciej Falkowski <maciej.falkowski9@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1342

Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/clang-tools/run-clang-tools.py | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> index fa7655c7cec0..d34eaf5a0ee5 100755
> --- a/scripts/clang-tools/run-clang-tools.py
> +++ b/scripts/clang-tools/run-clang-tools.py
> @@ -60,6 +60,11 @@ def run_analysis(entry):
>  
>  
>  def main():
> +    exitcode = subprocess.getstatusoutput('command -v clang-tidy')[0]
> +    if exitcode == 1:
> +        print("Command 'clang-tidy' is missing in the system", file=sys.stderr)
> +        sys.exit(127)
> +
>      args = parse_arguments()
>  
>      lock = multiprocessing.Lock()
> -- 
> 2.26.3

Re: [PATCH] clang-tools: Print information when clang-tidy tool is missing

[Masahiro Yamada]

On Sat, Jul 3, 2021 at 8:51 AM Maciej Falkowski
<maciej.falkowski9@gmail.com> wrote:
>
> When clang-tidy tool is missing in the system, the FileNotFoundError
> exception is raised in the program reporting a stack trace to the user:
>
> $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> multiprocessing.pool.RemoteTraceback:
> """
> Traceback (most recent call last):
>   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 125, in worker
>     result = (True, func(*args, **kwds))
>   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 48, in mapstar
>     return list(map(*args))
>   File "./scripts/clang-tools/run-clang-tools.py", line 54, in run_analysis
>     p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
>   File "/usr/lib64/python3.8/subprocess.py", line 489, in run
>     with Popen(*popenargs, **kwargs) as process:
>   File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
>     self._execute_child(args, executable, preexec_fn, close_fds,
>   File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
>     raise child_exception_type(errno_num, err_msg, err_filename)
> FileNotFoundError: [Errno 2] No such file or directory: 'clang-tidy'
> """
>
> The patch adds more user-friendly information about missing tool by
> checking the presence of clang-tidy using `command -v` at the beginning
> of the script:
>
> $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> Command 'clang-tidy' is missing in the system
>
> Signed-off-by: Maciej Falkowski <maciej.falkowski9@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1342
> ---
>  scripts/clang-tools/run-clang-tools.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> index fa7655c7cec0..d34eaf5a0ee5 100755
> --- a/scripts/clang-tools/run-clang-tools.py
> +++ b/scripts/clang-tools/run-clang-tools.py
> @@ -60,6 +60,11 @@ def run_analysis(entry):
>
>
>  def main():
> +    exitcode = subprocess.getstatusoutput('command -v clang-tidy')[0]
> +    if exitcode == 1:
> +        print("Command 'clang-tidy' is missing in the system", file=sys.stderr)
> +        sys.exit(127)



I like the first answer in this link:
https://stackoverflow.com/questions/82831/how-do-i-check-whether-a-file-exists-without-exceptions

"If the reason you're checking is so you can do something like
if file_exists: open_it(), it's safer to use a try around the attempt
to open it. Checking and then opening risks the file being deleted
or moved or something between when you check and when you try to open it."



Generally, I believe that Python's taste is:

   try:
        f = open("my-file")
   except:
        [ error handling ]


rather than:

    if not os.path.exists("my-file"):
           [ error handling ]
    f = open("my-file")



With the same logic applied,
if you like to display your custom error message here,
more Python-ish code might be:


    try:
         [ run clang-tidy ]
    except FileNotFoundError:
         print("Command 'clang-tidy' is missing in the system", file=sys.stderr)
         sys.exit(127)
    except:
         [ handle other errors ]



I often see, "I observed Python's backtrace, so let's suppress it"
for clang-tools scripts.

For example,
https://lore.kernel.org/lkml/20210520231821.12272-2-maciej.falkowski9@gmail.com/


If you believe "our custom error messages are better than
the default ones from Python", do you want to insert
ones to every code that can fail?

I do not think so.








> +
>      args = parse_arguments()
>
>      lock = multiprocessing.Lock()
> --
> 2.26.3
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210702235120.7023-1-maciej.falkowski9%40gmail.com.



--
Best Regards
Masahiro Yamada

[PATCH v2] clang-tools: Print information when clang-tidy tool is missing

[Maciej Falkowski]

When clang-tidy tool is missing in the system, the FileNotFoundError
exception is raised in the program reporting a stack trace to the user:

$ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/usr/lib64/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib64/python3.8/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "./scripts/clang-tools/run-clang-tools.py", line 54, in run_analysis
    p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
  File "/usr/lib64/python3.8/subprocess.py", line 489, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'clang-tidy'
"""

The patch adds more user-friendly information about missing tool:

$ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
Command 'clang-tidy' is missing in the system

Signed-off-by: Maciej Falkowski <maciej.falkowski9@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1342
---
Hi Masahiro,

Thank you for your feedback!
I am sorry that I haven't replied for so long.

I agree with your point, based on this I would like
to propose a second version of the patch.

changes in v2:
 - Solution has changed from LBYL style to EAFP

Best regards,
Maciej Falkowski
---
 scripts/clang-tools/run-clang-tools.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
index fa7655c7cec0..27ebe2f2069a 100755
--- a/scripts/clang-tools/run-clang-tools.py
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -67,7 +67,14 @@ def main():
     # Read JSON data into the datastore variable
     with open(args.path, "r") as f:
         datastore = json.load(f)
-        pool.map(run_analysis, datastore)
+        try:
+            pool.map(run_analysis, datastore)
+        except FileNotFoundError as err:
+            if err.filename == 'clang-tidy':
+                print("Command 'clang-tidy' is missing in the system", file=sys.stderr)
+                sys.exit(127)
+            else:
+                raise err
 
 
 if __name__ == "__main__":
-- 
2.26.3

Re: [PATCH v2] clang-tools: Print information when clang-tidy tool is missing

[Nathan Chancellor]

On Sat, Aug 07, 2021 at 01:01:16PM +0200, Maciej Falkowski wrote:
> When clang-tidy tool is missing in the system, the FileNotFoundError
> exception is raised in the program reporting a stack trace to the user:
> 
> $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> multiprocessing.pool.RemoteTraceback:
> """
> Traceback (most recent call last):
>   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 125, in worker
>     result = (True, func(*args, **kwds))
>   File "/usr/lib64/python3.8/multiprocessing/pool.py", line 48, in mapstar
>     return list(map(*args))
>   File "./scripts/clang-tools/run-clang-tools.py", line 54, in run_analysis
>     p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
>   File "/usr/lib64/python3.8/subprocess.py", line 489, in run
>     with Popen(*popenargs, **kwargs) as process:
>   File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
>     self._execute_child(args, executable, preexec_fn, close_fds,
>   File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
>     raise child_exception_type(errno_num, err_msg, err_filename)
> FileNotFoundError: [Errno 2] No such file or directory: 'clang-tidy'
> """
> 
> The patch adds more user-friendly information about missing tool:
> 
> $ ./scripts/clang-tools/run-clang-tools.py clang-tidy ./compile_commands.json
> Command 'clang-tidy' is missing in the system
> 
> Signed-off-by: Maciej Falkowski <maciej.falkowski9@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1342

LGTM, I think this is much better than the stacktrace output as above as
it is easier for someone who is not familiar with these scrips to act
on.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> Hi Masahiro,
> 
> Thank you for your feedback!
> I am sorry that I haven't replied for so long.
> 
> I agree with your point, based on this I would like
> to propose a second version of the patch.
> 
> changes in v2:
>  - Solution has changed from LBYL style to EAFP
> 
> Best regards,
> Maciej Falkowski
> ---
>  scripts/clang-tools/run-clang-tools.py | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> index fa7655c7cec0..27ebe2f2069a 100755
> --- a/scripts/clang-tools/run-clang-tools.py
> +++ b/scripts/clang-tools/run-clang-tools.py
> @@ -67,7 +67,14 @@ def main():
>      # Read JSON data into the datastore variable
>      with open(args.path, "r") as f:
>          datastore = json.load(f)
> -        pool.map(run_analysis, datastore)
> +        try:
> +            pool.map(run_analysis, datastore)
> +        except FileNotFoundError as err:
> +            if err.filename == 'clang-tidy':
> +                print("Command 'clang-tidy' is missing in the system", file=sys.stderr)
> +                sys.exit(127)
> +            else:
> +                raise err
>  
>  
>  if __name__ == "__main__":
> -- 
> 2.26.3