From 6c48ac76e8fd36550c3c8085e53dfd3fb73d7442 Mon Sep 17 00:00:00 2001 From: Matthew Broadway Date: Sat, 30 Nov 2024 21:41:03 +0000 Subject: [PATCH] upgrade to latest maturin. Fixed failing tests added added debug tools --- src/maturin_import_hook/__main__.py | 10 ++- src/maturin_import_hook/_resolve_project.py | 14 +++- src/maturin_import_hook/project_importer.py | 2 + src/maturin_import_hook/rust_file_importer.py | 2 + tests/README.md | 2 +- tests/maturin | 2 +- tests/package_resolver/Cargo.lock | 77 ++++++++----------- tests/package_resolver/Cargo.toml | 4 +- tests/package_resolver/src/main.rs | 13 +++- tests/requirements.txt | 2 +- tests/resolved.json | 3 +- tests/runner.py | 69 +++++++++++++---- tests/test_import_hook/common.py | 1 + .../packages/my_rust_module.rs | 2 +- .../packages/subpackage/my_rust_module.rs | 2 +- .../file_importer_helpers/reload_template.rs | 2 +- .../blank-project/Cargo.lock | 20 ++--- .../blank-project/Cargo.toml | 2 +- .../reload_template.rs | 12 +-- .../test_import_hook/test_project_importer.py | 69 ++++++++++++----- 20 files changed, 195 insertions(+), 115 deletions(-) diff --git a/src/maturin_import_hook/__main__.py b/src/maturin_import_hook/__main__.py index 6f9559d..999718f 100644 --- a/src/maturin_import_hook/__main__.py +++ b/src/maturin_import_hook/__main__.py @@ -191,10 +191,12 @@ def _main() -> None: install.add_argument( "--user", action="store_true", - help="whether to install into usercustomize.py instead of sitecustomize.py. " - "Note that usercustomize.py is shared between virtualenvs of the same interpreter version and is not loaded " - "unless the virtualenv is created with the `--system-site-packages` argument. Use `site info` to check " - "whether usercustomize.py is loaded the current interpreter.", + help=( + "whether to install into usercustomize.py instead of sitecustomize.py. " + "Note that usercustomize.py is shared between virtualenvs of the same interpreter version and is " + "not loaded unless the virtualenv is created with the `--system-site-packages` argument. " + "Use `site info` to check whether usercustomize.py is loaded the current interpreter." + ), ) uninstall = site_sub_actions.add_parser( diff --git a/src/maturin_import_hook/_resolve_project.py b/src/maturin_import_hook/_resolve_project.py index f1d8395..86b7d25 100644 --- a/src/maturin_import_hook/_resolve_project.py +++ b/src/maturin_import_hook/_resolve_project.py @@ -142,7 +142,7 @@ def all_path_dependencies(self) -> list[Path]: def _find_all_path_dependencies(immediate_path_dependencies: list[Path]) -> list[Path]: if not immediate_path_dependencies: return [] - all_path_dependencies = set() + all_path_dependencies: set[Path] = set() to_search = immediate_path_dependencies.copy() while to_search: dependency_project_dir = to_search.pop() @@ -170,6 +170,9 @@ def _resolve_project(project_dir: Path) -> MaturinProject: msg = "no pyproject.toml found" raise _ProjectResolveError(msg) pyproject = _TomlFile.load(pyproject_path) + if not _is_valid_pyproject(pyproject): + msg = "pyproject.toml is invalid (does not have required fields)" + raise _ProjectResolveError(msg) manifest_path = find_cargo_manifest(project_dir) if manifest_path is None: @@ -203,6 +206,13 @@ def _resolve_project(project_dir: Path) -> MaturinProject: ) +def _is_valid_pyproject(pyproject: _TomlFile) -> bool: + """in maturin serde is used to load into a `PyProjectToml` struct. + This function should match whether the toml would parse correctly""" + # it should be sufficient to check the required fields rather than match the serde parsing logic exactly + return pyproject.get_value(["build-system", "requires"], list) is not None + + def _resolve_rust_module(python_dir: Path, module_name: str) -> tuple[Path, Path, str]: """This follows the same logic as project_layout.rs (ProjectLayout::determine). @@ -244,7 +254,7 @@ def _resolve_module_name(pyproject: _TomlFile, cargo: _TomlFile) -> Optional[str def _get_immediate_path_dependencies(manifest_dir_path: Path, cargo: _TomlFile) -> list[Path]: - path_dependencies = [] + path_dependencies: list[Path] = [] for dependency in cargo.get_value_or_default(["dependencies"], dict, {}).values(): if isinstance(dependency, dict): relative_path: Any = dependency.get("path", None) diff --git a/src/maturin_import_hook/project_importer.py b/src/maturin_import_hook/project_importer.py index 4ab87f3..a133123 100644 --- a/src/maturin_import_hook/project_importer.py +++ b/src/maturin_import_hook/project_importer.py @@ -172,6 +172,8 @@ def find_spec( logger.info('rebuilt and loaded package "%s" in %.3fs', package_name, duration) else: logger.debug('loaded package "%s" in %.3fs', package_name, duration) + elif logger.isEnabledFor(logging.DEBUG): + logger.debug('%s did not find "%s"', type(self).__name__, package_name) return spec def _handle_reload(self, package_name: str, spec: ModuleSpec) -> ModuleSpec: diff --git a/src/maturin_import_hook/rust_file_importer.py b/src/maturin_import_hook/rust_file_importer.py index c50d0fd..fc45019 100644 --- a/src/maturin_import_hook/rust_file_importer.py +++ b/src/maturin_import_hook/rust_file_importer.py @@ -144,6 +144,8 @@ def find_spec( logger.info('rebuilt and loaded module "%s" in %.3fs', fullname, duration) else: logger.debug('loaded module "%s" in %.3fs', fullname, duration) + elif logger.isEnabledFor(logging.DEBUG): + logger.debug('%s did not find "%s"', type(self).__name__, fullname) return spec diff --git a/tests/README.md b/tests/README.md index 1551270..429e973 100644 --- a/tests/README.md +++ b/tests/README.md @@ -34,7 +34,7 @@ To update maturin: - update the submodule to the maturin commit you want to update to - re-run the `package_resolver` to update `resolved.json` (see `package_resolver/README.md` for instructions) - update `requirements.txt` to match the packages and versions installed by the maturin ci - (see `pip install` commands in `maturin/.github/workflows/test.yml`) + (see `pip` and `uv` commands in `maturin/.github/workflows/test.yml`) - check the `uniffi` package version listed in the `Cargo.toml` of any of the `uniffi-*` test crates and update `uniffi-bindgen` in `requirements.txt` to match. - check that no crates have been added to `test-crates` that should be excluded from the import hook tests. diff --git a/tests/maturin b/tests/maturin index aebaded..d896c62 160000 --- a/tests/maturin +++ b/tests/maturin @@ -1 +1 @@ -Subproject commit aebadedec43a92a1ba0fe94980c84f37122aa5b3 +Subproject commit d896c62f31ded33adca2f58819913e29b7950299 diff --git a/tests/package_resolver/Cargo.lock b/tests/package_resolver/Cargo.lock index cb6fa44..d1566a0 100644 --- a/tests/package_resolver/Cargo.lock +++ b/tests/package_resolver/Cargo.lock @@ -85,9 +85,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.81" +version = "1.0.93" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0952808a6c2afd1aa8947271f3a60f1a6763c7b912d210184c5149b5cf147247" +checksum = "4c95c10ba0b00a02636238b814946408b1322d5ac4760326e6fb8ec956d85775" [[package]] name = "autocfg" @@ -293,20 +293,20 @@ dependencies = [ [[package]] name = "cbindgen" -version = "0.26.0" +version = "0.27.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da6bc11b07529f16944307272d5bd9b22530bc7d05751717c9d416586cedab49" +checksum = "3fce8dd7fcfcbf3a0a87d8f515194b49d6135acab73e18bd380d1d93bb1a15eb" dependencies = [ "heck", - "indexmap 1.9.3", + "indexmap", "log", "proc-macro2", "quote", "serde", "serde_json", - "syn 1.0.109", + "syn 2.0.87", "tempfile", - "toml 0.5.11", + "toml", ] [[package]] @@ -348,7 +348,7 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8eebd66744a15ded14960ab4ccdbfb51ad3b81f51f3f04a80adac98c985396c9" dependencies = [ - "hashbrown 0.14.3", + "hashbrown", "stacker", ] @@ -659,7 +659,7 @@ version = "0.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d4c93f393add03d72bc10dd3dea43a1610ecb29e0c0a6459c70b53b82931adf" dependencies = [ - "goblin", + "goblin 0.8.0", ] [[package]] @@ -760,10 +760,15 @@ dependencies = [ ] [[package]] -name = "hashbrown" -version = "0.12.3" +name = "goblin" +version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +checksum = "53ab3f32d1d77146981dea5d6b1e8fe31eedcb7013e5e00d6ccd1259a4b4d923" +dependencies = [ + "log", + "plain", + "scroll", +] [[package]] name = "hashbrown" @@ -816,16 +821,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "indexmap" -version = "1.9.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" -dependencies = [ - "autocfg", - "hashbrown 0.12.3", -] - [[package]] name = "indexmap" version = "2.2.6" @@ -833,7 +828,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", - "hashbrown 0.14.3", + "hashbrown", "serde", ] @@ -897,7 +892,7 @@ checksum = "f88a93876d2485ede9c97d698c164cf5c024491908483964a998faae9705dea6" dependencies = [ "fs-err", "glob", - "goblin", + "goblin 0.8.0", ] [[package]] @@ -967,7 +962,7 @@ dependencies = [ [[package]] name = "maturin" -version = "1.7.4" +version = "1.7.6" dependencies = [ "anyhow", "base64 0.21.7", @@ -990,9 +985,9 @@ dependencies = [ "flate2", "fs-err", "glob", - "goblin", + "goblin 0.9.2", "ignore", - "indexmap 2.2.6", + "indexmap", "itertools 0.12.1", "lddtree", "minijinja", @@ -1020,7 +1015,7 @@ dependencies = [ "textwrap", "thiserror", "time", - "toml 0.8.12", + "toml", "toml_edit", "tracing", "tracing-subscriber", @@ -1055,9 +1050,9 @@ dependencies = [ [[package]] name = "minijinja" -version = "1.0.12" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6fe0ff215195a22884d867b547c70a0c4815cbbcc70991f281dca604b20d10ce" +checksum = "2c37e1b517d1dcd0e51dc36c4567b9d5a29262b3ec8da6cb5d35e27a8fb529b5" dependencies = [ "serde", ] @@ -1307,11 +1302,11 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef7061023bcb58a0fc4a4bbe9819c13b0dca7c2abc14da14f5ecc1532ab3a36a" dependencies = [ - "indexmap 2.2.6", + "indexmap", "pep440_rs", "pep508_rs", "serde", - "toml 0.8.12", + "toml", ] [[package]] @@ -1633,11 +1628,12 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.114" +version = "1.0.133" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5f09b1bd632ef549eaa9f60a1f8de742bdbc698e6cee2095fc84dde5f549ae0" +checksum = "c7fceb2473b9166b2294ef05efcb65a3db80803f0b03ef86a5fc88a2b85ee377" dependencies = [ "itoa", + "memchr", "ryu", "serde", ] @@ -1888,15 +1884,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" -[[package]] -name = "toml" -version = "0.5.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4f7f0dd8d50a853a531c426359045b1998f04219d88799810762cd4ad314234" -dependencies = [ - "serde", -] - [[package]] name = "toml" version = "0.8.12" @@ -1924,7 +1911,7 @@ version = "0.22.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e40bb779c5187258fd7aad0eb68cb8706a0a81fa712fbea808ab43c4b8374c4" dependencies = [ - "indexmap 2.2.6", + "indexmap", "serde", "serde_spanned", "toml_datetime", @@ -2421,7 +2408,7 @@ dependencies = [ "serde_json", "sha2", "tempfile", - "toml 0.8.12", + "toml", "tracing", "tracing-subscriber", "twox-hash", diff --git a/tests/package_resolver/Cargo.toml b/tests/package_resolver/Cargo.toml index 7b61825..f4b23a0 100644 --- a/tests/package_resolver/Cargo.toml +++ b/tests/package_resolver/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" publish = false [dependencies] -anyhow = "1.0.79" +anyhow = "1.0.93" maturin = { path = "../maturin" } serde = { version = "1.0.195", features = ["derive"] } -serde_json = "1.0.111" +serde_json = "1.0.133" diff --git a/tests/package_resolver/src/main.rs b/tests/package_resolver/src/main.rs index 492df23..5b95910 100644 --- a/tests/package_resolver/src/main.rs +++ b/tests/package_resolver/src/main.rs @@ -32,7 +32,7 @@ fn resolve_package(project_root: &Path) -> Result { let _cwd = TemporaryChdir::chdir(&project_root)?; let build_options: BuildOptions = Default::default(); - let build_context = build_options.into_build_context(false, false, false)?; + let build_context = build_options.into_build_context().build()?; let extension_module_dir = if build_context.project_layout.python_module.is_some() { Some(relative_path( &build_context.project_layout.rust_module, @@ -74,7 +74,16 @@ fn resolve_all_packages(test_crates_dir: &Path) -> Result { for path in entries { if path.join("pyproject.toml").exists() { let project_name = path.file_name().unwrap().to_str().unwrap().to_owned(); - resolved_packages.insert(project_name, resolve_package(&path).unwrap_or(Value::Null)); + println!("resolve '{}'", project_name); + match resolve_package(&path) { + Ok(value) => { + resolved_packages.insert(project_name, value); + } + Err(err) => { + println!("resolve failed with: {:?}", err); + resolved_packages.insert(project_name, Value::Null); + } + } } } Ok(Value::Object(resolved_packages)) diff --git a/tests/requirements.txt b/tests/requirements.txt index 4483a2c..702d4db 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,6 +1,6 @@ -e .. uv -maturin==1.7.4 +maturin==1.7.6 pytest junit2html uniffi-bindgen==0.28.0 diff --git a/tests/resolved.json b/tests/resolved.json index d009a4e..ec6ad2e 100644 --- a/tests/resolved.json +++ b/tests/resolved.json @@ -1,5 +1,5 @@ { - "commit": "aebadedec43a92a1ba0fe94980c84f37122aa5b3", + "commit": "d896c62f31ded33adca2f58819913e29b7950299", "crates": { "cffi-mixed": { "cargo_manifest_path": "Cargo.toml", @@ -155,6 +155,7 @@ "python_dir": ".", "python_module": null }, + "workspace-inverted-order": null, "wrong-python-source": { "cargo_manifest_path": "Cargo.toml", "extension_module_dir": null, diff --git a/tests/runner.py b/tests/runner.py index 1050b9a..747cbe9 100644 --- a/tests/runner.py +++ b/tests/runner.py @@ -8,6 +8,7 @@ import subprocess import sys from dataclasses import dataclass +from enum import Enum from pathlib import Path # ruff: noqa: INP001 @@ -19,14 +20,24 @@ logging.basicConfig(format="[%(name)s] [%(levelname)s] %(message)s", level=logging.DEBUG) +class PackageInstaller(Enum): + PIP = "pip" + UV = "uv" + + def __str__(self) -> str: + return self.value + + @dataclass class TestOptions: test_specification: str test_suite_name: str timeout: int max_failures: int | None + package_installer: PackageInstaller use_lld: bool profile: Path | None + maturin_debug: bool html_report: bool notify: bool @@ -49,7 +60,7 @@ def _run_tests_serial( report_path = workspace / "report.html" report_path.unlink(missing_ok=True) - venv = _create_test_venv(python, workspace / "venv") + venv = _create_test_venv(python, workspace / "venv", options.package_installer) try: _run_test_in_environment(venv, workspace / "cache", reports_dir / "results.xml", options) finally: @@ -78,6 +89,11 @@ def _run_test_in_environment( env["MATURIN_BUILD_DIR"] = str(cache_dir / "maturin_build_cache") env["CARGO_TARGET_DIR"] = str(cache_dir / "target") + env["MATURIN_IMPORT_HOOK_TEST_PACKAGE_INSTALLER"] = options.package_installer.value + + if options.maturin_debug: + env["RUST_LOG"] = "maturin=debug" + if options.use_lld: log.info("using lld") # https://stackoverflow.com/a/57817848 @@ -100,8 +116,8 @@ def _run_test_in_environment( sys.exit(proc.returncode) -def _pip_install_command(interpreter_path: Path) -> list[str]: - if shutil.which("uv") is not None: +def _package_install_command(interpreter_path: Path, package_installer: PackageInstaller) -> list[str]: + if package_installer == PackageInstaller.UV: log.info("using uv to install packages") return [ "uv", @@ -110,7 +126,7 @@ def _pip_install_command(interpreter_path: Path) -> list[str]: "--python", str(interpreter_path), ] - else: + elif package_installer == PackageInstaller.PIP: log.info("using pip to install packages") return [ str(interpreter_path), @@ -119,14 +135,16 @@ def _pip_install_command(interpreter_path: Path) -> list[str]: "install", "--disable-pip-version-check", ] + else: + raise ValueError(package_installer) -def _create_test_venv(python: Path, venv_dir: Path) -> VirtualEnv: - venv = VirtualEnv.create(venv_dir, python) +def _create_test_venv(python: Path, venv_dir: Path, package_installer: PackageInstaller) -> VirtualEnv: + venv = VirtualEnv.create(venv_dir, python, package_installer) log.info("installing test requirements into virtualenv") proc = subprocess.run( [ - *_pip_install_command(venv.interpreter_path), + *_package_install_command(venv.interpreter_path, package_installer), "-r", "requirements.txt", ], @@ -144,8 +162,10 @@ def _create_test_venv(python: Path, venv_dir: Path) -> VirtualEnv: return venv -def _create_virtual_env_command(interpreter_path: Path, venv_path: Path) -> list[str]: - if shutil.which("uv") is not None: +def _create_virtual_env_command( + interpreter_path: Path, venv_path: Path, package_installer: PackageInstaller +) -> list[str]: + if package_installer == PackageInstaller.UV: log.info("using uv to create virtual environments") return ["uv", "venv", "--seed", "--python", str(interpreter_path), str(venv_path)] elif shutil.which("virtualenv") is not None: @@ -156,8 +176,10 @@ def _create_virtual_env_command(interpreter_path: Path, venv_path: Path) -> list return [str(interpreter_path), "-m", "venv", str(venv_path)] -def _install_into_virtual_env_command(interpreter_path: Path, package_path: Path) -> list[str]: - if shutil.which("uv") is not None: +def _install_into_virtual_env_command( + interpreter_path: Path, package_path: Path, package_installer: PackageInstaller +) -> list[str]: + if package_installer == PackageInstaller.UV: log.info("using uv to install package as editable") return ["uv", "pip", "install", "--python", str(interpreter_path), "--editable", str(package_path)] else: @@ -166,23 +188,24 @@ def _install_into_virtual_env_command(interpreter_path: Path, package_path: Path class VirtualEnv: - def __init__(self, root: Path) -> None: + def __init__(self, root: Path, package_installer: PackageInstaller) -> None: self._root = root.resolve() self._is_windows = platform.system() == "Windows" + self._package_installer = package_installer @staticmethod - def create(root: Path, interpreter_path: Path) -> VirtualEnv: + def create(root: Path, interpreter_path: Path, package_installer: PackageInstaller) -> VirtualEnv: if root.exists(): log.info("removing virtualenv at %s", root) shutil.rmtree(root) if not interpreter_path.exists(): raise FileNotFoundError(interpreter_path) log.info("creating test virtualenv at '%s' from '%s'", root, interpreter_path) - cmd = _create_virtual_env_command(interpreter_path, root) + cmd = _create_virtual_env_command(interpreter_path, root, package_installer) proc = subprocess.run(cmd, capture_output=True, check=True) log.debug("%s", proc.stdout.decode()) assert root.is_dir() - return VirtualEnv(root) + return VirtualEnv(root, package_installer) @property def root_dir(self) -> Path: @@ -204,7 +227,7 @@ def interpreter_path(self) -> Path: return interpreter def install_editable_package(self, package_path: Path) -> None: - cmd = _install_into_virtual_env_command(self.interpreter_path, package_path) + cmd = _install_into_virtual_env_command(self.interpreter_path, package_path, self._package_installer) proc = subprocess.run(cmd, capture_output=True, check=True) log.debug("%s", proc.stdout.decode()) @@ -296,11 +319,23 @@ def main() -> None: default=True, help="send a notification when finished", ) + parser.add_argument( + "--installer", + choices=list(PackageInstaller), + type=PackageInstaller, + default=PackageInstaller.UV, + help="the package installer to use in the tests", + ) parser.add_argument( "--lld", action="store_true", help="use lld for linking (generally faster than the default).", ) + parser.add_argument( + "--maturin_debug", + action="store_true", + help="have maturin produce verbose logs", + ) parser.add_argument( "--profile", type=Path, @@ -321,8 +356,10 @@ def main() -> None: test_suite_name=args.name, timeout=args.timeout, max_failures=args.max_failures, + package_installer=args.installer, use_lld=args.lld, profile=args.profile, + maturin_debug=args.maturin_debug, html_report=args.html_report, notify=args.notify, ) diff --git a/tests/test_import_hook/common.py b/tests/test_import_hook/common.py index 127d9a5..fda11b3 100644 --- a/tests/test_import_hook/common.py +++ b/tests/test_import_hook/common.py @@ -31,6 +31,7 @@ "hello-world", # not imported as a python module (subprocess only) "license-test", # not imported as a python module (subprocess only) "pyo3-bin", # not imported as a python module (subprocess only) + "workspace-inverted-order", # this directory is not a maturin package, only the subdirectory } diff --git a/tests/test_import_hook/file_importer_helpers/packages/my_rust_module.rs b/tests/test_import_hook/file_importer_helpers/packages/my_rust_module.rs index a2261a6..89e799b 100644 --- a/tests/test_import_hook/file_importer_helpers/packages/my_rust_module.rs +++ b/tests/test_import_hook/file_importer_helpers/packages/my_rust_module.rs @@ -6,7 +6,7 @@ pub fn do_something(a: usize, b: usize) -> PyResult { } #[pymodule] -pub fn my_rust_module(_py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> { +pub fn my_rust_module(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_function(wrap_pyfunction!(do_something, m)?)?; Ok(()) } diff --git a/tests/test_import_hook/file_importer_helpers/packages/subpackage/my_rust_module.rs b/tests/test_import_hook/file_importer_helpers/packages/subpackage/my_rust_module.rs index f5189ad..67203a5 100644 --- a/tests/test_import_hook/file_importer_helpers/packages/subpackage/my_rust_module.rs +++ b/tests/test_import_hook/file_importer_helpers/packages/subpackage/my_rust_module.rs @@ -6,7 +6,7 @@ pub fn get_num() -> PyResult { } #[pymodule] -pub fn my_rust_module(_py: Python, m: &PyModule) -> PyResult<()> { +pub fn my_rust_module(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_function(wrap_pyfunction!(get_num, m)?)?; Ok(()) } diff --git a/tests/test_import_hook/file_importer_helpers/reload_template.rs b/tests/test_import_hook/file_importer_helpers/reload_template.rs index 2f3bb05..35a7fad 100644 --- a/tests/test_import_hook/file_importer_helpers/reload_template.rs +++ b/tests/test_import_hook/file_importer_helpers/reload_template.rs @@ -117,7 +117,7 @@ fn my_module(py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> { m.add("data_str", "foo")?; - let logging = PyModule::new(py, "logging")?; + let logging = PyModule::import(py, "logging")?; logging .getattr("info")? .call1(("my_module extension module initialised",))?; diff --git a/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.lock b/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.lock index 1d9c1e1..925777c 100644 --- a/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.lock +++ b/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.lock @@ -71,9 +71,9 @@ dependencies = [ [[package]] name = "pyo3" -version = "0.23.1" +version = "0.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ebb0c0cc0de9678e53be9ccf8a2ab53045e6e3a8be03393ceccc5e7396ccb40" +checksum = "f54b3d09cbdd1f8c20650b28e7b09e338881482f4aa908a5f61a00c98fba2690" dependencies = [ "cfg-if", "indoc", @@ -89,9 +89,9 @@ dependencies = [ [[package]] name = "pyo3-build-config" -version = "0.23.1" +version = "0.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "80e3ce69c4ec34476534b490e412b871ba03a82e35604c3dfb95fcb6bfb60c09" +checksum = "3015cf985888fe66cfb63ce0e321c603706cd541b7aec7ddd35c281390af45d8" dependencies = [ "once_cell", "target-lexicon", @@ -99,9 +99,9 @@ dependencies = [ [[package]] name = "pyo3-ffi" -version = "0.23.1" +version = "0.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b09f311c76b36dfd6dd6f7fa6f9f18e7e46a1c937110d283e80b12ba2468a75" +checksum = "6fca7cd8fd809b5ac4eefb89c1f98f7a7651d3739dfb341ca6980090f554c270" dependencies = [ "libc", "pyo3-build-config", @@ -109,9 +109,9 @@ dependencies = [ [[package]] name = "pyo3-macros" -version = "0.23.1" +version = "0.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd4f74086536d1e1deaff99ec0387481fb3325c82e4e48be0e75ab3d3fcb487a" +checksum = "34e657fa5379a79151b6ff5328d9216a84f55dc93b17b08e7c3609a969b73aa0" dependencies = [ "proc-macro2", "pyo3-macros-backend", @@ -121,9 +121,9 @@ dependencies = [ [[package]] name = "pyo3-macros-backend" -version = "0.23.1" +version = "0.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e77dfeb76b32bbf069144a5ea0a36176ab59c8db9ce28732d0f06f096bbfbc8" +checksum = "295548d5ffd95fd1981d2d3cf4458831b21d60af046b729b6fd143b0ba7aee2f" dependencies = [ "heck", "proc-macro2", diff --git a/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.toml b/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.toml index 9d583bd..72bb266 100644 --- a/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.toml +++ b/tests/test_import_hook/project_importer_helpers/blank-project/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" description = "" [dependencies] -pyo3 = { version = "0.23.1", features = ["extension-module"] } +pyo3 = { version = "0.23.2", features = ["extension-module"] } [lib] crate-type = ["cdylib"] diff --git a/tests/test_import_hook/project_importer_helpers/reload_template.rs b/tests/test_import_hook/project_importer_helpers/reload_template.rs index 23e3961..03cb9af 100644 --- a/tests/test_import_hook/project_importer_helpers/reload_template.rs +++ b/tests/test_import_hook/project_importer_helpers/reload_template.rs @@ -40,7 +40,7 @@ impl Integer { } fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyResult { - let logging = PyModule::import_bound(py, "logging")?; + let logging = PyModule::import(py, "logging")?; let message = format!( "comparing Integer instances {} and {}", self.name, other.name @@ -66,7 +66,7 @@ impl PicklableInteger { } fn __richcmp__(&self, other: &Self, op: CompareOp, py: Python<'_>) -> PyResult { - let logging = PyModule::import_bound(py, "logging")?; + let logging = PyModule::import(py, "logging")?; let message = format!( "comparing PicklableInteger instances {} and {}", self.name, other.name @@ -89,7 +89,7 @@ fn get_str() -> String { } fn register_child_module(py: Python<'_>, parent_module: &Bound<'_, PyModule>) -> PyResult<()> { - let child_module = PyModule::new_bound(py, "child")?; + let child_module = PyModule::new(py, "child")?; child_module.add_wrapped(wrap_pyfunction!(get_str))?; parent_module.add_submodule(&child_module)?; Ok(()) @@ -105,19 +105,19 @@ fn my_project(py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> { register_child_module(py, m)?; - let data = PyDict::new_bound(py); + let data = PyDict::new(py); data.set_item("foo", 123)?; m.add("data", data)?; if !m.hasattr("data_init_once")? { - let data = PyDict::new_bound(py); + let data = PyDict::new(py); data.set_item("foo", 123)?; m.add("data_init_once", data)?; } m.add("data_str", "foo")?; - let logging = PyModule::import_bound(py, "logging")?; + let logging = PyModule::import(py, "logging")?; logging .getattr("info")? .call1(("my_project extension module initialised",))?; diff --git a/tests/test_import_hook/test_project_importer.py b/tests/test_import_hook/test_project_importer.py index 981b10f..e91c094 100644 --- a/tests/test_import_hook/test_project_importer.py +++ b/tests/test_import_hook/test_project_importer.py @@ -8,9 +8,10 @@ import subprocess import sys from collections.abc import Iterator +from enum import Enum from pathlib import Path from textwrap import dedent -from typing import Optional +from typing import Any import pytest @@ -358,7 +359,7 @@ def test_concurrent_import(workspace: Path, initially_mixed: bool, mixed: bool) shutil.rmtree(project_dir) _get_project_copy(TEST_CRATES_DIR / project_name, project_dir) - args = {"python_script": check_installed_with_hook, "quiet": True} + args: dict[str, Any] = {"python_script": check_installed_with_hook, "quiet": True} if platform.system() == "Windows" and platform.python_implementation() == "PyPy": # workaround for https://github.com/pypy/pypy/issues/4917 @@ -1339,22 +1340,29 @@ def _rebuilt_message(project_name: str) -> str: return f'rebuilt and loaded package "{with_underscores(project_name)}"' -_UV_AVAILABLE: Optional[bool] = None +class PackageInstaller(Enum): + PIP = "pip" + UV = "uv" + def __str__(self) -> str: + return self.value -def uv_available() -> bool: - """whether the `uv` command is installed""" - global _UV_AVAILABLE - if _UV_AVAILABLE is None: - _UV_AVAILABLE = shutil.which("uv") is not None - return _UV_AVAILABLE + +def get_package_installer() -> PackageInstaller: + # set by `runner.py` + key = "MATURIN_IMPORT_HOOK_TEST_PACKAGE_INSTALLER" + if key not in os.environ: + msg = f"environment variable {key} not set" + raise RuntimeError(msg) + return PackageInstaller(os.environ[key]) def _uninstall(*project_names: str) -> None: log.info("uninstalling %s", sorted(project_names)) - if uv_available(): + installer = get_package_installer() + if installer == PackageInstaller.UV: cmd = ["uv", "pip", "uninstall", "--python", str(sys.executable), *project_names] - else: + elif installer == PackageInstaller.PIP: cmd = [ sys.executable, "-m", @@ -1364,13 +1372,16 @@ def _uninstall(*project_names: str) -> None: "-y", *project_names, ] + else: + raise ValueError(installer) subprocess.check_call(cmd) def _get_installed_package_names() -> set[str]: - if uv_available(): + installer = get_package_installer() + if installer == PackageInstaller.UV: cmd = ["uv", "pip", "list", "--python", sys.executable, "--format", "json"] - else: + elif installer == PackageInstaller.PIP: cmd = [ sys.executable, "-m", @@ -1379,6 +1390,8 @@ def _get_installed_package_names() -> set[str]: "list", "--format=json", ] + else: + raise ValueError(installer) packages = json.loads(subprocess.check_output(cmd).decode()) return {package["name"] for package in packages} @@ -1391,15 +1404,21 @@ def _install_editable(project_dir: Path) -> None: assert maturin_path is not None env = os.environ.copy() env["VIRTUAL_ENV"] = sys.exec_prefix - subprocess.check_call([maturin_path, "develop", "--uv"], cwd=project_dir, env=env) + args = [maturin_path, "develop"] + if get_package_installer() == PackageInstaller.UV: + args.append("--uv") + subprocess.check_call(args, cwd=project_dir, env=env) def _install_non_editable(project_dir: Path) -> None: log.info("installing %s in non-editable mode", project_dir.name) - if uv_available(): + installer = get_package_installer() + if installer == PackageInstaller.UV: cmd = ["uv", "pip", "install", "--python", sys.executable, str(project_dir)] - else: + elif installer == PackageInstaller.PIP: cmd = [sys.executable, "-m", "pip", "install", "--disable-pip-version-check", str(project_dir)] + else: + raise ValueError(installer) subprocess.check_call(cmd) @@ -1433,11 +1452,14 @@ def _is_editable_installed_correctly(project_name: str, project_dir: Path, is_mi installed_editable_with_direct_url, ) - if uv_available(): + installer = get_package_installer() + if installer == PackageInstaller.UV: # TODO(matt): use uv once the --files option is supported https://github.com/astral-sh/uv/issues/2526 cmd = [sys.executable, "-m", "pip", "show", "--disable-pip-version-check", "-f", project_name] - else: + elif installer == PackageInstaller.PIP: cmd = [sys.executable, "-m", "pip", "show", "--disable-pip-version-check", "-f", project_name] + else: + raise ValueError(installer) proc = subprocess.run( cmd, @@ -1450,14 +1472,21 @@ def _is_editable_installed_correctly(project_name: str, project_dir: Path, is_mi return installed_editable_with_direct_url and (installed_as_pth == is_mixed) -def _get_project_copy(project_dir: Path, output_path: Path) -> Path: - for relative_path in _get_relative_files_tracked_by_git(project_dir): +def _get_project_copy(project_dir: Path, output_path: Path, *, use_git: bool = True) -> Path: + paths = _get_relative_files_tracked_by_git(project_dir) if use_git else _get_relative_files(project_dir) + for relative_path in paths: output_file_path = output_path / relative_path output_file_path.parent.mkdir(parents=True, exist_ok=True) shutil.copy(project_dir / relative_path, output_file_path) return output_path +def _get_relative_files(root: Path) -> Iterator[Path]: + for p in root.rglob("*"): + if p.is_file(): + yield p.relative_to(root) + + def _get_relative_files_tracked_by_git(root: Path) -> Iterator[Path]: """This is used to ignore built artifacts to create a clean copy.""" output = subprocess.check_output(["git", "ls-tree", "--name-only", "-z", "-r", "HEAD"], cwd=root)