meson
meson copied to clipboard
mesonbuild
More robust exe_exist function
The documentation for subprocess.run at https://docs.python.org/3/library/subprocess.html#popen-constructor has a warning, pointing to using shutil.which() instead of subprocess.run for detecting if exe files exists on the path.
I have experienced problems with detecting llvm-cov, when running as non-root in a container. In this situation subprocess.run would throw a Permission denied exception if llvm-cov is not on the path.
The commit message is definitely broken in and of itself. It says nothing at all, whereas the GitHub web UI has significantly more detail that is mandatory for future readers of the code to understand why this change was made.
In terms of the actual code change, if exe_exists is a wrapper function that immediately calls shutil.which with no other function body code, then it would make more sense to delete the function and, everywhere it is used, replace it with direct use of shutil.which. But it seems strange to get Permission Denied, anyway...
The commit message is definitely broken in and of itself. It says nothing at all, whereas the GitHub web UI has significantly more detail that is mandatory for future readers of the code to understand why this change was made.
In terms of the actual code change, if exe_exists is a wrapper function that immediately calls shutil.which with no other function body code, then it would make more sense to delete the function and, everywhere it is used, replace it with direct use of shutil.which. But it seems strange to get Permission Denied, anyway...
It is definitely strange, and I haven't been able to figure out why. Anyway, I'd be happy to remove the function call and use which() instead. And of course put some more info in the commit.
From what I can tell, the logic to call the program with --version dates back all the way to c3c9a31a5a51ea2b2c7b560262a5af0d372d52ab which contained inline versions of that logic.
git grep exe_exists
6 hits including the function definition.
git grep shutil.which
43 hits, none of which are the definition since it is in the stdlib.
I wonder if exe_exists simply exists because no one ever thought to change it.
@eli-schwartz I have updated the pr to getrid of the function and added info i the commit message. Let me know what you think.
FWICT exe_exists is now called only from unit tests. If that is the case then let's just delete the whole thing.
I wonder if exe_exists simply exists because no one ever thought to change it.
Quite possible. Or that this was working around some bug that existed 10 years ago but no longer does.
