Skip to content
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

Raise if a postconf script fails #12115

Merged
merged 2 commits into from
Feb 11, 2024
Merged

Conversation

akihikodaki
Copy link
Contributor

Raise MesonException if a postconf script fails to let the user know about the failure.

@akihikodaki
Copy link
Contributor Author

I investigated the CI failure and it found test cases/common/139 mesonintrospect from scripts has been broken on Windows.

The arguments in the MESONINTROSPECT environment variable are escaped with the join_args function in mesonbuild/utils/universal.py for postconf scripts, and on Windows it emits a string that is incompatible with shlex.split() used in check_introspection.py. Even more oddly, the arguments in MESONINTROSPECT are always escaped with shlex.quote() in the other situations so that problem is specific to postconf scripts.

Here are possible ways to fix the bug:

  1. Use shlex.quote() instead of join_args to escape the arguments in MESONINTROSPECT for postconf scripts. This breaks scripts that pass MESONINTROSPECT to Windows cmd.
  2. Always use join_args to escape the arguments in MESONINTROSPECT and make join_args emit a string that is compatible with shlex.split(). More concretely, make it replace \ with \\. This adds extra verbosity to MESONINTROSPECT.
  3. Always use join_args to escape the arguments in MESONINTROSPECT and make check_introspection.py pass it to cmd on Windows. This abandons compatibility with shlex.split() and makes it harder to use MESONINTROSPECT in Python scripts.

mesonbuild/backend/backends.py Outdated Show resolved Hide resolved
@xclaesse
Copy link
Member

Always use join_args to escape the arguments in MESONINTROSPECT and make check_introspection.py pass it to cmd on Windows. This abandons compatibility with shlex.split() and makes it harder to use MESONINTROSPECT in Python scripts.

I think we should do that for now because this PR is unrelated to that issue. You could then open a separate issue to discuss the quoting problem, because I think it's larger than MESONINTROSPECT, it affects every places we quote args to be used by external scripts, how are we expecting 3rd party to use those values.

@xclaesse
Copy link
Member

LGTM, if you can just make that CI pass.

@eli-schwartz
Copy link
Member

Even more oddly, the arguments in MESONINTROSPECT are always escaped with shlex.quote() in the other situations so that problem is specific to postconf scripts.

This was changed for postconf in commit 522392e. Arguably, that's a bug and we should use shlex.quote...

@xclaesse xclaesse modified the milestones: 1.3.0, 1.4.0 Oct 17, 2023
joukewitteveen added a commit to joukewitteveen/meson that referenced this pull request Feb 3, 2024
The type of quoting was changed in 522392e to one that is suitable for
use with cmd.exe on Windows. However, the documentation states that the
type of quoting in MESONINTROSPECT is compatible with shlex.split() and
elsewhere in the code, the same variable is still quoted with
shlex.quote(). As mostly identified in mesonbuild#12148, there are a few choices:
1. Use shlex.quote() consistently and support Python but not cmd.exe.
2. Use join_args and support cmd.exe but not Python.
3. Use join_args and support splitting through the mesonbuild Python library.

This commit implements the first option and reverts part of 522392e.

Regression testing is implemented in mesonbuild#12115.

Fixes mesonbuild#12148
joukewitteveen added a commit to joukewitteveen/meson that referenced this pull request Feb 3, 2024
The type of quoting was changed in 522392e to one that is suitable for
use with cmd.exe on Windows. However, the documentation states that the
type of quoting in MESONINTROSPECT is compatible with shlex.split() and
elsewhere in the code, the same variable is still quoted with
shlex.quote(). As mostly identified in mesonbuild#12148, there are a few choices:
1. Use shlex.quote() consistently and support Python but not cmd.exe.
2. Use join_args and support cmd.exe but not Python.
3. Use join_args and support splitting through the mesonbuild Python library.

This commit implements the first option and reverts part of 522392e.

Regression testing is implemented in mesonbuild#12115.

Fixes mesonbuild#12148
joukewitteveen added a commit to joukewitteveen/meson that referenced this pull request Feb 10, 2024
The type of quoting was changed in 522392e to one that is suitable for
use with cmd.exe on Windows. However, the documentation states that the
type of quoting in MESONINTROSPECT is compatible with shlex.split() and
elsewhere in the code, the same variable is still quoted with
shlex.quote(). As mostly identified in mesonbuild#12148, there are a few choices:
1. Use shlex.quote() consistently and support Python but not cmd.exe.
2. Use join_args and support cmd.exe but not Python.
3. Use join_args and support splitting through the mesonbuild Python library.

This commit implements the first option and reverts part of 522392e.

Regression testing is implemented in mesonbuild#12115.

Fixes mesonbuild#12148
eli-schwartz pushed a commit that referenced this pull request Feb 11, 2024
The type of quoting was changed in 522392e to one that is suitable for
use with cmd.exe on Windows. However, the documentation states that the
type of quoting in MESONINTROSPECT is compatible with shlex.split() and
elsewhere in the code, the same variable is still quoted with
shlex.quote(). As mostly identified in #12148, there are a few choices:
1. Use shlex.quote() consistently and support Python but not cmd.exe.
2. Use join_args and support cmd.exe but not Python.
3. Use join_args and support splitting through the mesonbuild Python library.

This commit implements the first option and reverts part of 522392e.

Regression testing is implemented in #12115.

Fixes #12148
@joukewitteveen
Copy link
Contributor

Now that #12807 is merged, rebasing the current pull request should fix CI.

Raise MesonException if a postconf script fails to let the user know
about the failure.
@nirbheek nirbheek merged commit adf09b8 into mesonbuild:master Feb 11, 2024
35 checks passed
soumyaDghosh pushed a commit to soumyaDghosh/meson that referenced this pull request Jun 4, 2024
The type of quoting was changed in 522392e to one that is suitable for
use with cmd.exe on Windows. However, the documentation states that the
type of quoting in MESONINTROSPECT is compatible with shlex.split() and
elsewhere in the code, the same variable is still quoted with
shlex.quote(). As mostly identified in mesonbuild#12148, there are a few choices:
1. Use shlex.quote() consistently and support Python but not cmd.exe.
2. Use join_args and support cmd.exe but not Python.
3. Use join_args and support splitting through the mesonbuild Python library.

This commit implements the first option and reverts part of 522392e.

Regression testing is implemented in mesonbuild#12115.

Fixes mesonbuild#12148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants