-
Notifications
You must be signed in to change notification settings - Fork 75
ENH: raise SystemExit when calling meson fails #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This fixes the case when Meson errors out. However, the new configuration handling code raises |
@@ -649,7 +649,9 @@ def _get_config_key(self, key: str) -> Any: | |||
def _proc(self, *args: str) -> None: | |||
"""Invoke a subprocess.""" | |||
print('{cyan}{bold}+ {}{reset}'.format(' '.join(args), **_STYLES)) | |||
subprocess.check_call(list(args), env=self._env) | |||
r = subprocess.run(list(args), env=self._env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend always specifying check=
to subprocess.run
to clarify intent. There's a check (bugbear, perhaps?) that looks for this.
What is the status of this PR? People are reporting the failures directly to Meson itself now. As far as I can tell, this flawlessly solves the problem some of the time, and doesn't touch the code involved in some other times. It seems obviously correct (incremental improvements FTW) and solves real problems for people, so why delay in merging it? |
I am almost finished in #87, after that is in, I will merge this too and make a release. |
On user input error (either command line or configuration file) we now raise |
24a83e0
to
5d9a05d
Compare
I added handling of the exceptions raised for configuration errors. Please have a look. |
Yeah, it makes sense to release this first 👍 |
Fixes #228.