Move build system to meson-python#174
Conversation
ndgrigorian
requested review from
antonwolfy,
jharlow-intel and
xaleryb
as code owners
ndgrigorian
force-pushed
the
use-meson-build
branch
4 times, most recently
from
1f7006a to
e209e46
Compare
ndgrigorian
force-pushed
the
use-meson-build
branch
from
e209e46 to
dfc8f4c
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates mkl-service from a setuptools/setup.py-driven build to a meson-python build, updating conda recipes and CI workflows accordingly, and adding an mkl runtime dependency.
Changes:
- Remove
setup.pyand define extension builds/install layout inmeson.build(C + Cython extensions, Python sources, tests). - Switch
pyproject.tomlbuild backend tomesonpyand addmklto runtime dependencies. - Update conda recipes and GitHub Actions workflows to build/install via
pip/python -m buildinstead ofsetup.py.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Removed legacy setuptools build script. |
| pyproject.toml | Switched build backend to mesonpy, updated build requirements and runtime deps. |
| meson.build | Added Meson build definition for C/Cython extensions and installation layout. |
| mkl/_py_mkl_service.pyx | Added/introduced Cython wrapper implementation used by the public API. |
| conda-recipe/meta.yaml | Updated conda build requirements for Meson-based builds. |
| conda-recipe/build.sh | Updated wheel build/install flow (no setup.py). |
| conda-recipe/bld.bat | Removed setup.py clean step and MKLROOT usage. |
| conda-recipe-cf/meta.yaml | Updated conda-forge recipe host requirements for Meson-based builds. |
| conda-recipe-cf/build.sh | Switched to pip install . build/install flow. |
| conda-recipe-cf/bld.bat | Switched to pip install . build/install flow. |
| .github/workflows/build-with-standard-clang.yml | New CI job building with system clang. |
| .github/workflows/build-with-clang.yml | Updated IntelLLVM clang CI job to Meson-based install. |
| .github/workflows/build_pip.yml | New CI job testing editable install + (pre-)release NumPy in conda env. |
| 'mkl-service', | ||
| ['c', 'cython'], | ||
| version: run_command( | ||
| 'python', '-c', |
There was a problem hiding this comment.
run_command('python', ...) hardcodes the interpreter, which may not match Meson’s discovered Python (virtualenv/pyenv/Windows). Use the py installation found by import('python').find_installation() when computing the version to avoid mismatched interpreters.
| 'python', '-c', | |
| import('python').find_installation(pure: false), '-c', |
| strategy: | ||
| matrix: | ||
| python: ["3.10", "3.11", "3.12", "3.13", "3.14"] | ||
| numpy_version: ["numpy'>=2'"] |
There was a problem hiding this comment.
The matrix value numpy_version: ["numpy'>=2'"] includes embedded quotes and will be passed verbatim to pip install, which is not a valid requirement specifier. Use something like numpy>=2 (or just install numpy and control pre-releases separately).
| numpy_version: ["numpy'>=2'"] | |
| numpy_version: ["numpy>=2"] |
| strategy: | ||
| matrix: | ||
| python: ["3.10", "3.11", "3.12", "3.13"] | ||
| numpy_version: ["numpy'>=2'"] |
There was a problem hiding this comment.
The matrix value numpy_version: ["numpy'>=2'"] includes embedded quotes and will be passed verbatim to pip install, which is not a valid requirement specifier. Use something like numpy>=2.
| numpy_version: ["numpy'>=2'"] | |
| numpy_version: ["numpy>=2"] |
ndgrigorian
force-pushed
the
use-meson-build
branch
3 times, most recently
from
b3cd7c9 to
3fb7aff
Compare
ndgrigorian
force-pushed
the
use-meson-build
branch
from
ef09add to
ac6e9fc
Compare
add build instructions
remove unnecessary packages and add python 3.14 to clang build
ndgrigorian
force-pushed
the
use-meson-build
branch
from
ac6e9fc to
b39b64b
Compare
| ) | ||
|
|
||
| install_subdir( | ||
| 'mkl/tests', |
There was a problem hiding this comment.
That copies the entire directory. I confirmed mkl/tests/ contains AGENTS.md alongside test_mkl_service.py. The old setup.py only matched tests/*.py, so this is a regression that ships a dev file into the package.
| - name: Run mkl-service tests | ||
| run: | | ||
| pip install pytest | ||
| # mkl-service cannot be installed in editable mode, we need |
There was a problem hiding this comment.
We use editable-install in build_pip.yml and that works.
| "cmake" | ||
| ] | ||
|
|
||
| [project] |
There was a problem hiding this comment.
PR description claims: Also adds mkl as a dependency
But it seems not true, there is not run-time dependency on MKL implemented.
| ) | ||
|
|
||
| rpath = '' | ||
| if host_machine.system() == 'linux' |
There was a problem hiding this comment.
Do we need to consider RPATH for macOS also?
| line_length = 80 | ||
| profile = "black" | ||
|
|
||
| [tool.setuptools] |
There was a problem hiding this comment.
Is it true that [tool.setuptools], [tool.setuptools.dynamic], [tool.setuptools.package-data] are ignored under mesonpy and misleading (the dynamic-version table looks authoritative but isn't)?
| run: | | ||
| conda install mkl-devel mkl | ||
|
|
||
| - name: Build conda package |
| @@ -37,7 +37,7 @@ Higher-precedence file overrides; lower must not restate overridden guidance. | |||
| - Build/config: `pyproject.toml`, `setup.py` | |||
| - `_mkl_service.pyx` — Cython wrappers for MKL support functions | ||
| - `_mkl_service.pxd` — Cython declarations (C function signatures) | ||
| - `_py_mkl_service.pyx` — Cython wrappers for MKL support functions | ||
| - `_py_mkl_service.pxd` — Cython declarations (C function signatures) |
There was a problem hiding this comment.
| - `_py_mkl_service.pxd` — Cython declarations (C function signatures) | |
| - `_mkl_service.pxd` — Cython declarations (C function signatures) |
| - `_py_mkl_service.pxd` — Cython declarations (C function signatures) | ||
| - `_mklinitmodule.c` — C extension for Linux-side MKL runtime preloading/init | ||
| - `_init_helper.py` — Windows loading helper (DLL path setup in venv) | ||
| - `_version.py` — version string (dynamic via setuptools) |
| - **C init:** `_mklinitmodule.c` handles Linux preloading (`dlopen(..., RTLD_GLOBAL)`) for MKL runtime | ||
| - **Windows loading helper:** `_init_helper.py` handles DLL path setup in Windows venv | ||
| - **Python wrapper:** `__init__.py` imports `_py_mkl_service` (generated from `.pyx`) | ||
| - **Version:** `_version.py` (dynamic via setuptools) |
This PR proposes moving from
setuptoolstomeson-pythonas themkl-servicebuild systemmeson-pythonis already used by NumPy and allowssetup.pyto be removed (with its logic moved into themeson.buildscript)Also adds mkl as a dependency