linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clang-tools: Print information when clang-tidy tool is missing
@ 2021-07-02 23:51 Maciej Falkowski
  2021-07-04  0:23 ` Nathan Chancellor
  2021-07-05 16:18 ` Masahiro Yamada
  0 siblings, 2 replies; 5+ messages in thread
From: Maciej Falkowski @ 2021-07-02 23:51 UTC (permalink / raw)
  To: natechancellor, ndesaulniers, masahiroy, michal.lkml, nhuck
  Cc: clang-built-linux, linux-kbuild, linux-kernel, maciej.falkowski9

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clang-tools: Print information when clang-tidy tool is missing
  2021-07-02 23:51 [PATCH] clang-tools: Print information when clang-tidy tool is missing Maciej Falkowski
@ 2021-07-04  0:23 ` Nathan Chancellor
  2021-07-05 16:18 ` Masahiro Yamada
  1 sibling, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2021-07-04  0:23 UTC (permalink / raw)
  To: Maciej Falkowski
  Cc: natechancellor, ndesaulniers, masahiroy, michal.lkml, nhuck,
	clang-built-linux, linux-kbuild, linux-kernel

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clang-tools: Print information when clang-tidy tool is missing
  2021-07-02 23:51 [PATCH] clang-tools: Print information when clang-tidy tool is missing Maciej Falkowski
  2021-07-04  0:23 ` Nathan Chancellor
@ 2021-07-05 16:18 ` Masahiro Yamada
  2021-08-07 11:01   ` [PATCH v2] " Maciej Falkowski
  1 sibling, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2021-07-05 16:18 UTC (permalink / raw)
  To: Maciej Falkowski
  Cc: Nathan Chancellor, Nick Desaulniers, Michal Marek,
	Nathan Huckleberry, clang-built-linux, Linux Kbuild mailing list,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] clang-tools: Print information when clang-tidy tool is missing
  2021-07-05 16:18 ` Masahiro Yamada
@ 2021-08-07 11:01   ` Maciej Falkowski
  2021-08-08 22:18     ` Nathan Chancellor
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej Falkowski @ 2021-08-07 11:01 UTC (permalink / raw)
  To: natechancellor, ndesaulniers, masahiroy, michal.lkml, nhuck
  Cc: clang-built-linux, linux-kbuild, linux-kernel, maciej.falkowski9

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] clang-tools: Print information when clang-tidy tool is missing
  2021-08-07 11:01   ` [PATCH v2] " Maciej Falkowski
@ 2021-08-08 22:18     ` Nathan Chancellor
  0 siblings, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2021-08-08 22:18 UTC (permalink / raw)
  To: Maciej Falkowski
  Cc: natechancellor, ndesaulniers, masahiroy, michal.lkml, nhuck,
	clang-built-linux, linux-kbuild, linux-kernel

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-08 22:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 23:51 [PATCH] clang-tools: Print information when clang-tidy tool is missing Maciej Falkowski
2021-07-04  0:23 ` Nathan Chancellor
2021-07-05 16:18 ` Masahiro Yamada
2021-08-07 11:01   ` [PATCH v2] " Maciej Falkowski
2021-08-08 22:18     ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).