From b0ab6ab3b9bcb2f51f3a1c857122e6b977fd2ac2 Mon Sep 17 00:00:00 2001 From: Ralf Gommers Date: Thu, 5 Dec 2024 12:34:08 +0100 Subject: [PATCH 1/2] ENH: support shared libraries inside packages This drops the use of staged installs with `--destdir` in favor of using `--prefix` for the install location. The problem is that RPATHs in extension modules end up pointing to what Meson thinks is the final install location (i.e., inside the prefix) rather than to the staging directory. `--destdir` seems meant for packaging, as an actual staging area, while for `spin` the final install directory is `build-install` (by default), there is no intent to later put this package into `/usr` or `C:\`. Hence using `--prefix` seems like the correct thing to do. The one test change here is to a test that was incorrect. `meson setup --prefix` expects an absolute path, and `/foobar` isn't a path that exists or can be created. Addresses the issue discussed in PR 238 - the `spin build` behavior before this change cannot be made to work for SciPy, because the internal shared library in `scipy.special` keeps breaking. It should also address the problem discussed in issue spin#176. --- spin/cmds/meson.py | 17 +++++------------ spin/tests/test_build_cmds.py | 3 ++- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/spin/cmds/meson.py b/spin/cmds/meson.py index ca47801..6756db1 100644 --- a/spin/cmds/meson.py +++ b/spin/cmds/meson.py @@ -231,11 +231,6 @@ def _check_coverage_tool_installation(coverage_type: GcovReportFormat, build_dir ) -if sys.platform.startswith("win"): - DEFAULT_PREFIX = "C:/" -else: - DEFAULT_PREFIX = "/usr" - build_dir_option = click.option( "-C", "--build-dir", @@ -266,10 +261,10 @@ def _check_coverage_tool_installation(coverage_type: GcovReportFormat, build_dir ) @click.option( "--prefix", - help="The build prefix, passed directly to meson.", + help="The build prefix as an absolute path, passed directly to meson.", type=str, metavar="PREFIX", - default=DEFAULT_PREFIX, + default="", ) @click.argument("meson_args", nargs=-1) @build_dir_option @@ -316,10 +311,12 @@ def build( Which can then be used to build (`spin-clang build`), to test (`spin-clang test ...`), etc. """ - abs_build_dir = os.path.abspath(build_dir) install_dir = _get_install_dir(build_dir) abs_install_dir = os.path.abspath(install_dir) + if not prefix: + prefix = abs_install_dir + cfg = get_config() distname = cfg.get("project.name", None) if distname and _is_editable_install_of_same_source(distname): @@ -385,10 +382,6 @@ def build( "--only-changed", "-C", build_dir, - "--destdir", - install_dir - if os.path.isabs(install_dir) - else os.path.relpath(abs_install_dir, abs_build_dir), ] + list(meson_install_args), output=(not quiet) and verbose, diff --git a/spin/tests/test_build_cmds.py b/spin/tests/test_build_cmds.py index a966346..35654ad 100644 --- a/spin/tests/test_build_cmds.py +++ b/spin/tests/test_build_cmds.py @@ -38,7 +38,8 @@ def test_debug_builds(example_pkg): def test_prefix_builds(example_pkg): """does spin build --prefix create a build-install directory with the correct structure?""" - spin("build", "--prefix=/foobar/") + prefix = Path.cwd() / "build-install" / "foobar" + spin("build", f"--prefix={prefix}") assert (Path("build-install") / Path("foobar")).exists() From e1cee14283eff7b02992579fbc01bdf2848e11bc Mon Sep 17 00:00:00 2001 From: Ralf Gommers Date: Thu, 5 Dec 2024 20:23:36 +0100 Subject: [PATCH 2/2] TST: add a shared library to the `example_pkg` package This tests using a shared library from a Python extension module, which is relevant for SciPy (scipy.special contains such a shared library). --- example_pkg/example_pkg/__init__.py | 20 +++++++++++-- example_pkg/example_pkg/_core.pyi | 1 + example_pkg/example_pkg/coremodule.c | 15 ++++++++++ example_pkg/example_pkg/meson.build | 8 ++++-- example_pkg/example_pkg/submodule/meson.build | 28 +++++++++++++++++++ example_pkg/example_pkg/submodule/shlib.c | 5 ++++ example_pkg/example_pkg/submodule/shlib.h | 3 ++ example_pkg/example_pkg/submodule/shlib_dll.h | 14 ++++++++++ example_pkg/meson.build | 3 +- 9 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 example_pkg/example_pkg/submodule/shlib.c create mode 100644 example_pkg/example_pkg/submodule/shlib.h create mode 100644 example_pkg/example_pkg/submodule/shlib_dll.h diff --git a/example_pkg/example_pkg/__init__.py b/example_pkg/example_pkg/__init__.py index 5a148b0..61c8790 100644 --- a/example_pkg/example_pkg/__init__.py +++ b/example_pkg/example_pkg/__init__.py @@ -1,4 +1,20 @@ -from ._core import echo +import os +import sys -__all__ = ["echo"] +__all__ = ["echo", "example_sum"] __version__ = "0.0.0dev0" + + +def _enable_sharedlib_loading(): + basedir = os.path.dirname(__file__) + subdir = os.path.join(basedir, "submodule") + if os.name == "nt": + os.add_dll_directory(subdir) + elif sys.platform == "cygwin": + os.environ["PATH"] = f'os.environ["PATH"]{os.pathsep}{subdir}' + + +_enable_sharedlib_loading() + + +from ._core import echo, example_sum # noqa: E402 diff --git a/example_pkg/example_pkg/_core.pyi b/example_pkg/example_pkg/_core.pyi index 9c2d05d..0dcd1b4 100644 --- a/example_pkg/example_pkg/_core.pyi +++ b/example_pkg/example_pkg/_core.pyi @@ -1 +1,2 @@ def echo(str) -> None: ... +def example_sum(a: int, b: int) -> int: ... diff --git a/example_pkg/example_pkg/coremodule.c b/example_pkg/example_pkg/coremodule.c index 1a0fd81..dd11ddb 100644 --- a/example_pkg/example_pkg/coremodule.c +++ b/example_pkg/example_pkg/coremodule.c @@ -1,6 +1,8 @@ #define PY_SSIZE_T_CLEAN #include +#include "shlib.h" + static PyObject * core_echo(PyObject *self, PyObject *args) { @@ -17,8 +19,21 @@ core_echo(PyObject *self, PyObject *args) return ret; } +static PyObject * +example_sum(PyObject *self, PyObject *args) +{ + int a, b; + if (!PyArg_ParseTuple(args, "ii", &a, &b)) { + return NULL; + } + + long result = sum(a, b); + return PyLong_FromLong(result); +} + static PyMethodDef CoreMethods[] = { {"echo", core_echo, METH_VARARGS, "Echo a string and return 42"}, + {"example_sum", example_sum, METH_VARARGS, "Sum up two integers"}, {NULL, NULL, 0, NULL} /* Sentinel */ }; diff --git a/example_pkg/example_pkg/meson.build b/example_pkg/example_pkg/meson.build index bdf2cf2..5e1ee2c 100644 --- a/example_pkg/example_pkg/meson.build +++ b/example_pkg/example_pkg/meson.build @@ -1,8 +1,13 @@ +subdir('submodule') + py.extension_module( '_core', 'coremodule.c', + include_directories: 'submodule', + dependencies: shlib_dep, install: true, - subdir: 'example_pkg' + subdir: 'example_pkg', + install_rpath: '$ORIGIN/submodule', ) python_sources = [ @@ -15,5 +20,4 @@ py.install_sources( subdir: 'example_pkg' ) -install_subdir('submodule', install_dir: py.get_install_dir() / 'example_pkg') install_subdir('tests', install_dir: py.get_install_dir() / 'example_pkg') diff --git a/example_pkg/example_pkg/submodule/meson.build b/example_pkg/example_pkg/submodule/meson.build index d287dc8..35bca24 100644 --- a/example_pkg/example_pkg/submodule/meson.build +++ b/example_pkg/example_pkg/submodule/meson.build @@ -1 +1,29 @@ +# Add a basic shared library, used from the Python extension module +# in ../coremodule.c +if meson.get_compiler('c').get_id() in ['msvc', 'clang-cl', 'intel-cl'] + export_dll_args = ['-DSHLIB_DLL_EXPORTS'] + import_dll_args = ['-DSHLIB_DLL_IMPORTS'] +else + export_dll_args = [] + import_dll_args = [] +endif + +shlib = shared_library( + 'shlib', + 'shlib.c', + c_args: export_dll_args, + install: true, + install_dir: py.get_install_dir() / 'example_pkg/submodule', +) + +shlib_dep = declare_dependency( + compile_args: import_dll_args, + link_with: shlib, +) + +py.install_sources( + ['__init__.py'], + subdir: 'example_pkg/submodule', +) + install_subdir('tests', install_dir: py.get_install_dir() / 'example_pkg/submodule') diff --git a/example_pkg/example_pkg/submodule/shlib.c b/example_pkg/example_pkg/submodule/shlib.c new file mode 100644 index 0000000..f68525f --- /dev/null +++ b/example_pkg/example_pkg/submodule/shlib.c @@ -0,0 +1,5 @@ +#include "shlib_dll.h" + +SHLIB_DLL int sum(int a, int b) { + return a + b; +} diff --git a/example_pkg/example_pkg/submodule/shlib.h b/example_pkg/example_pkg/submodule/shlib.h new file mode 100644 index 0000000..9221c01 --- /dev/null +++ b/example_pkg/example_pkg/submodule/shlib.h @@ -0,0 +1,3 @@ +#include "shlib_dll.h" + +SHLIB_DLL int sum(int a, int b); diff --git a/example_pkg/example_pkg/submodule/shlib_dll.h b/example_pkg/example_pkg/submodule/shlib_dll.h new file mode 100644 index 0000000..d9d2ec6 --- /dev/null +++ b/example_pkg/example_pkg/submodule/shlib_dll.h @@ -0,0 +1,14 @@ +#pragma once + +// Windows support. When building the `shlib` DLL, this macro expands to +// `__declspec(dllexport)` so we can annotate symbols appropriately as being +// exported. When used in headers consuming a DLL, this macro expands to +// `__declspec(dllimport)` so that consumers know the symbol is defined inside +// the DLL. In all other cases, the macro expands to nothing. +#if defined(SHLIB_DLL_EXPORTS) + #define SHLIB_DLL __declspec(dllexport) +#elif defined(SHLIB_DLL_IMPORTS) + #define SHLIB_DLL __declspec(dllimport) +#else + #define SHLIB_DLL +#endif diff --git a/example_pkg/meson.build b/example_pkg/meson.build index 514e5dd..a118783 100644 --- a/example_pkg/meson.build +++ b/example_pkg/meson.build @@ -13,8 +13,7 @@ project( cc = meson.get_compiler('c') -py_mod = import('python') -py = py_mod.find_installation(pure: false) +py = import('python').find_installation(pure: false) py_dep = py.dependency() subdir('example_pkg')