2021-07-02 23:55:31

by Maciej Falkowski

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

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 <[email protected]>
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


2021-07-04 00:23:53

by Nathan Chancellor

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

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 <[email protected]>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1342

Reviewed-by: Nathan Chancellor <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>

> ---
> 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

2021-07-05 16:20:40

by Masahiro Yamada

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

On Sat, Jul 3, 2021 at 8:51 AM Maciej Falkowski
<[email protected]> 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 <[email protected]>
> 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/[email protected]/


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 [email protected].
> 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

2021-08-07 11:02:44

by Maciej Falkowski

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

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 <[email protected]>
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

2021-08-08 22:19:16

by Nathan Chancellor

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

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 <[email protected]>
> 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 <[email protected]>

> ---
> 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