-
Notifications
You must be signed in to change notification settings - Fork 77
BUG: Use meson compile wrapper on Windows #371
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// SPDX-FileCopyrightText: 2023 The meson-python developers | ||
// | ||
// SPDX-License-Identifier: MIT | ||
|
||
#include <Python.h> | ||
|
||
#if defined _MSC_VER | ||
# define _COMPILER "msvc" | ||
#elif defined __clang__ | ||
# define _COMPILER "clang" | ||
#elif defined __GNUC__ | ||
# define _COMPILER "gcc" | ||
#else | ||
# define _COMPILER "unknown" | ||
#endif | ||
|
||
static PyObject* compiler(PyObject* self) | ||
{ | ||
return PyUnicode_FromString(_COMPILER); | ||
} | ||
|
||
static PyMethodDef methods[] = { | ||
{"compiler", (PyCFunction)compiler, METH_NOARGS, NULL}, | ||
{NULL, NULL, 0, NULL}, | ||
}; | ||
|
||
static struct PyModuleDef module = { | ||
PyModuleDef_HEAD_INIT, | ||
"detect_compiler", | ||
NULL, | ||
-1, | ||
methods, | ||
}; | ||
|
||
PyMODINIT_FUNC PyInit_detect_compiler(void) | ||
{ | ||
return PyModule_Create(&module); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# SPDX-FileCopyrightText: 2023 The meson-python developers | ||
# | ||
# SPDX-License-Identifier: MIT | ||
|
||
project( | ||
'detect-compiler', | ||
'c', | ||
version: '1.0', | ||
) | ||
|
||
py = import('python').find_installation() | ||
|
||
py.extension_module('detect_compiler', 'detect_compiler.c', install: true) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# SPDX-FileCopyrightText: 2023 The meson-python developers | ||
# | ||
# SPDX-License-Identifier: MIT | ||
|
||
[build-system] | ||
build-backend = 'mesonpy' | ||
requires = ['meson-python'] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
# | ||
# SPDX-License-Identifier: MIT | ||
|
||
import ast | ||
import os | ||
import platform | ||
import shutil | ||
import sys | ||
|
@@ -60,12 +62,26 @@ def test_unsupported_python_version(package_unsupported_python_version): | |
|
||
def test_user_args(package_user_args, tmp_path, monkeypatch): | ||
project_run = mesonpy.Project._run | ||
call_args_list = [] | ||
cmds = [] | ||
args = [] | ||
|
||
def wrapper(self, cmd): | ||
# intercept and filter out test arguments and forward the call | ||
call_args_list.append(tuple(cmd)) | ||
return project_run(self, [x for x in cmd if not x.startswith(('config-', 'cli-'))]) | ||
if cmd[:2] == ['meson', 'compile']: | ||
# when using meson compile instead of ninja directly, the | ||
# arguments needs to be unmarshalled from the form used to | ||
# pass them to the --ninja-args option | ||
assert cmd[-1].startswith('--ninja-args=') | ||
cmds.append(cmd[:2]) | ||
args.append(ast.literal_eval(cmd[-1].split('=')[1])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. slightly painful I guess, but fine in a test case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are you referring to? The not-so-nice part is marshaling the arguments into the format expected by Meson. Parsing them back is just a consequence of that. However, the code for either does not look that horrible. |
||
elif cmd[:1] == ['meson']: | ||
cmds.append(cmd[:2]) | ||
args.append(cmd[2:]) | ||
else: | ||
# direct ninja invocation | ||
cmds.append([os.path.basename(cmd[0])]) | ||
args.append(cmd[1:]) | ||
return project_run(self, [x for x in cmd if not x.startswith(('config-', 'cli-', '--ninja-args'))]) | ||
|
||
monkeypatch.setattr(mesonpy.Project, '_run', wrapper) | ||
|
||
|
@@ -79,19 +95,32 @@ def wrapper(self, cmd): | |
mesonpy.build_sdist(tmp_path, config_settings) | ||
mesonpy.build_wheel(tmp_path, config_settings) | ||
|
||
expected = [ | ||
# check that the right commands are executed, namely that 'meson | ||
# compile' is used on Windows rather than a 'ninja' direct | ||
# invocation. | ||
assert cmds == [ | ||
# sdist: calls to 'meson setup' and 'meson dist' | ||
('config-setup', 'cli-setup'), | ||
('config-dist', 'cli-dist'), | ||
['meson', 'setup'], | ||
['meson', 'dist'], | ||
# wheel: calls to 'meson setup', 'meson compile', and 'meson install' | ||
('config-setup', 'cli-setup'), | ||
('config-compile', 'cli-compile'), | ||
('config-install', 'cli-install'), | ||
['meson', 'setup'], | ||
['meson', 'compile'] if platform.system() == 'Windows' else ['ninja'], | ||
['meson', 'install'] | ||
] | ||
|
||
for expected_args, call_args in zip(expected, call_args_list): | ||
# check that the user options are passed to the invoked commands | ||
expected = [ | ||
# sdist: calls to 'meson setup' and 'meson dist' | ||
['config-setup', 'cli-setup'], | ||
['config-dist', 'cli-dist'], | ||
# wheel: calls to 'meson setup', 'meson compile', and 'meson install' | ||
['config-setup', 'cli-setup'], | ||
['config-compile', 'cli-compile'], | ||
['config-install', 'cli-install'], | ||
] | ||
for expected_args, cmd_args in zip(expected, args): | ||
for arg in expected_args: | ||
assert arg in call_args | ||
assert arg in cmd_args | ||
|
||
|
||
@pytest.mark.parametrize('package', ('top-level', 'meson-args')) | ||
|
@@ -190,3 +219,15 @@ def test_invalid_build_dir(package_pure, tmp_path, mocker): | |
assert meson.call_args_list[0].args[1][1] == 'setup' | ||
assert '--reconfigure' not in meson.call_args_list[0].args[1] | ||
project.build() | ||
|
||
|
||
@pytest.mark.skipif(not os.getenv('CI') or platform.system() != 'Windows', reason='Requires MSVC') | ||
def test_compiler(venv, package_detect_compiler, tmp_path): | ||
# Check that things are setup properly to use the MSVC compiler on | ||
# Windows. This effectively means running the compilation step | ||
# with 'meson compile' instead of 'ninja' on Windows. Run this | ||
# test only on CI where we know that MSVC is available. | ||
wheel = mesonpy.build_wheel(tmp_path, {'setup-args': ['--vsenv']}) | ||
venv.pip('install', os.fspath(tmp_path / wheel)) | ||
compiler = venv.python('-c', 'import detect_compiler; print(detect_compiler.compiler())').strip() | ||
assert compiler == 'msvc' |
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.
Is it actually only GCC/LLVM, or any other compiler? I'd expect the latter. If so, maybe rephrase like "When no other compiler known to Meson (e.g., GCC or Clang-cl) is found on ...."
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.
The same info is repeated under Examples, so if editing here then it needs editing further down too.
Uh oh!
There was an error while loading. Please reload this page.
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.
In terms of the code implementation, we quite literally check for
shutil.which
with any of cc, gcc, clang, clang-cl. The remaining autodetected default compilers for C are icl and pgcc, but icl ships with its own cl.exe (super "helpful" as noted by the code comments in detect.py) and meson will prefer cl.exe over pgcc anyway, if it can.The --vsenv code is not kept in sync with detect.py's list of default compiler search names, whether it "should" do so is another question (that I don't have an opinion on, to be clear. :D)
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 haven't found detailed documentation for the compiler auto detection. I had a look at the code, which behaves as @eli-schwartz describes https://github.com/mesonbuild/meson/blob/eb472a133f45826a246b40c3d775eebf098fd6b1/mesonbuild/utils/vsenv.py#L43-L51 I don't know how much of this is appropriate to document on the meson-python side, given that it is not explicitly documented and that I don't know how much of it is susceptible of change in Meson. I thought that GCC and LLVM covered the most popular cased and anyone using a less common compiler would know what they are doing.
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.
Thanks for the explantions both - looks like we're good then with the current docs.
agreed