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

Upgrade maturin #9

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
rev: v5.0.0
hooks:
- id: check-yaml
- id: check-toml
- id: end-of-file-fixer
- id: trailing-whitespace
- id: mixed-line-ending
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.9
rev: v0.7.4
hooks:
- id: ruff-format
- id: ruff
args: [ --fix ]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.10.0
rev: v1.13.0
hooks:
# note: mypy runs in an isolated environment and so has no access to third party packages
- id: mypy
Expand All @@ -25,6 +26,6 @@ repos:
hooks:
- id: codespell
- repo: https://github.com/igorshubovych/markdownlint-cli
rev: v0.41.0
rev: v0.42.0
hooks:
- id: markdownlint-fix
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ ignore = [
"RET505", # superfluous-else-return
"S101", # assert
"S301", # suspicious-pickle-usage
"S311", # suspicious-non-cryptographic-random-usage
"S324", # hashlib-insecure-hash-function
"S603", # subprocess-without-shell-equals-true
"S607", # start-process-with-partial-path
Expand Down
31 changes: 20 additions & 11 deletions src/maturin_import_hook/_resolve_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def load(path: Path) -> "_TomlFile":
data = tomllib.load(f)
return _TomlFile(path, data)

@staticmethod
def from_string(path: Path, data_str: str) -> "_TomlFile":
return _TomlFile(path, tomllib.loads(data_str))

def get_value_or_default(self, keys: list[str], required_type: type[_T], default: _T) -> _T:
value = self.get_value(keys, required_type)
return default if value is None else value
Expand Down Expand Up @@ -56,30 +60,35 @@ def get_value(self, keys: list[str], required_type: type[_T]) -> Optional[_T]:

def find_cargo_manifest(project_dir: Path) -> Optional[Path]:
pyproject_path = project_dir / "pyproject.toml"
if pyproject_path.exists():
pyproject = _TomlFile.load(pyproject_path)
relative_manifest_path = pyproject.get_value(["tool", "maturin", "manifest-path"], str)
if relative_manifest_path is not None:
return project_dir / relative_manifest_path
if pyproject_path.is_file():
pyproject_data = pyproject_path.read_text()
if "manifest-path" in pyproject_data:
pyproject = _TomlFile.from_string(pyproject_path, pyproject_data)
relative_manifest_path = pyproject.get_value(["tool", "maturin", "manifest-path"], str)
if relative_manifest_path is not None:
return project_dir / relative_manifest_path

manifest_path = project_dir / "Cargo.toml"
if manifest_path.exists():
if manifest_path.is_file():
return manifest_path
manifest_path = project_dir / "rust/Cargo.toml"
if manifest_path.exists():
if manifest_path.is_file():
return manifest_path
return None


def is_maybe_maturin_project(project_dir: Path) -> bool:
def is_maybe_maturin_project(directory: Path) -> bool:
"""note: this function does not check if this really is a maturin project for simplicity."""
return (project_dir / "pyproject.toml").exists() and find_cargo_manifest(project_dir) is not None
return (directory / "pyproject.toml").is_file() and find_cargo_manifest(directory) is not None


class ProjectResolver:
def __init__(self) -> None:
self._resolved_project_cache: dict[Path, Optional[MaturinProject]] = {}

def clear_cache(self) -> None:
self._resolved_project_cache.clear()

def resolve(self, project_dir: Path) -> Optional["MaturinProject"]:
if project_dir not in self._resolved_project_cache:
resolved = None
Expand Down Expand Up @@ -238,8 +247,8 @@ def _get_immediate_path_dependencies(manifest_dir_path: Path, cargo: _TomlFile)
path_dependencies = []
for dependency in cargo.get_value_or_default(["dependencies"], dict, {}).values():
if isinstance(dependency, dict):
relative_path = dependency.get("path", None)
if relative_path is not None:
relative_path: Any = dependency.get("path", None)
if relative_path is not None and isinstance(relative_path, str):
path_dependencies.append((manifest_dir_path / relative_path).resolve())
return path_dependencies

Expand Down
21 changes: 20 additions & 1 deletion src/maturin_import_hook/project_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import urllib.request
from abc import ABC, abstractmethod
from collections.abc import Iterator, Sequence
from functools import lru_cache
from importlib.machinery import ExtensionFileLoader, ModuleSpec, PathFinder
from pathlib import Path
from types import ModuleType
Expand Down Expand Up @@ -102,6 +103,12 @@ def find_maturin(self) -> Path:
self._maturin_path = find_maturin((1, 5, 0), (2, 0, 0))
return self._maturin_path

def invalidate_caches(self) -> None:
"""called by `importlib.invalidate_caches()`"""
logger.info("clearing cache")
self._resolver.clear_cache()
_find_maturin_project_above.cache_clear()

def find_spec(
self,
fullname: str,
Expand Down Expand Up @@ -373,17 +380,29 @@ def _is_editable_installed_package(project_dir: Path, package_name: str) -> bool
return False


@lru_cache(maxsize=4096)
def _find_maturin_project_above(path: Path) -> Optional[Path]:
for search_path in itertools.chain((path,), path.parents):
if is_maybe_maturin_project(search_path):
return search_path
return None


def _find_dist_info_path(directory: Path, package_name: str) -> Optional[Path]:
try:
names = os.listdir(directory)
except FileNotFoundError:
return None
for name in names:
if name.startswith(package_name) and name.endswith(".dist-info"):
return Path(directory / name)
return None


def _load_dist_info(
path: Path, package_name: str, *, require_project_target: bool = True
) -> tuple[Optional[Path], bool]:
dist_info_path = next(path.glob(f"{package_name}-*.dist-info"), None)
dist_info_path = _find_dist_info_path(path, package_name)
if dist_info_path is None:
return None, False
try:
Expand Down
7 changes: 4 additions & 3 deletions src/maturin_import_hook/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def to_args(self) -> list[str]:
class MaturinBuildSettings(MaturinSettings):
"""settings for `maturin build`."""

skip_auditwheel: bool = False
auditwheel: Optional[str] = None
mbway marked this conversation as resolved.
Show resolved Hide resolved
zig: bool = False

@staticmethod
Expand All @@ -108,8 +108,9 @@ def supported_commands() -> set[str]:

def to_args(self) -> list[str]:
args = []
if self.skip_auditwheel:
args.append("--skip-auditwheel")
if self.auditwheel is not None:
args.append("--auditwheel")
args.append(self.auditwheel)
if self.zig:
args.append("--zig")
args.extend(super().to_args())
Expand Down
19 changes: 18 additions & 1 deletion tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@ 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 used by the maturin ci (`.github/workflows.test.yml`)
- update `requirements.txt` to match the packages and versions installed by the maturin ci
(see `pip install` 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.
If so, add them to `IGNORED_TEST_CRATES` in `common.py`
- upgrade the test packages in `test_import_hook/*_helpers`
- check what version of `pyo3` is used by the command: `maturin new --bindings pyo3`
- update the version check in the import hook to ensure it allows using the new version
- run the tests to ensure everything still works

## Notes

Expand Down Expand Up @@ -66,6 +70,19 @@ Connect to the debugger, eg [using vscode](https://code.visualstudio.com/docs/py
Note: set `CLEAR_WORKSPACE = False` in `common.py` if you want to prevent the temporary files generated during the test
from being cleared.

### Benchmarking

The `create_benchmark_data.py` script creates a directory with many python packages to represent a worst case scenario.
Run the script then run `venv/bin/python run.py` from the created directory.

One way of obtaining profiling information is to run:

```sh
venv/bin/python -m cProfile -o profile.prof run.py
pyprof2calltree -i profile.prof -o profile.log
kcachegrind profile.log
```

### Caching

sccache is a tool for caching build artifacts to speed up compilation. Unfortunately, it is currently useless for these
Expand Down
Empty file added tests/__init__.py
Empty file.
131 changes: 131 additions & 0 deletions tests/create_benchmark_data.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import argparse
import logging
import random
import string
import sys
import textwrap
from dataclasses import dataclass
from pathlib import Path

from runner import VirtualEnv

script_dir = Path(__file__).resolve().parent
repo_root = script_dir.parent

log = logging.getLogger("runner")
logging.basicConfig(format="[%(name)s] [%(levelname)s] %(message)s", level=logging.DEBUG)


@dataclass
class BenchmarkConfig:
seed: int
filename_length: int
depth: int
num_python_editable_packages: int

@staticmethod
def default() -> "BenchmarkConfig":
return BenchmarkConfig(
seed=0,
filename_length=10,
depth=10,
num_python_editable_packages=100,
)


def random_name(rng: random.Random, length: int) -> str:
return "".join(rng.choices(string.ascii_lowercase, k=length))


def random_path(rng: random.Random, root: Path, depth: int, name_length: int) -> Path:
path = root
for _ in range(depth):
path = path / random_name(rng, name_length)
return path


def create_python_package(root: Path) -> tuple[str, Path]:
root.mkdir(parents=True, exist_ok=False)
src_dir = root / "src" / root.name
src_dir.mkdir(parents=True)
(src_dir / "__init__.py").write_text(
textwrap.dedent(f"""\
def get_name():
return "{root.name}"
""")
)
(root / "pyproject.toml").write_text(
textwrap.dedent(f"""\
[project]
name = "{root.name}"
version = "0.1.0"

[tool.setuptools.packages.find]
where = ["src"]

[build-system]
requires = ["setuptools", "wheel"]
build-backend = "setuptools.build_meta"
""")
)
return root.name, src_dir


def create_benchmark_environment(root: Path, config: BenchmarkConfig) -> None:
rng = random.Random(config.seed)

log.info("creating benchmark environment at %s", root)
root.mkdir(parents=True, exist_ok=False)
venv = VirtualEnv.create(root / "venv", Path(sys.executable))

venv.install_editable_package(repo_root)

python_package_names = []
python_package_paths = []

packages_root = random_path(rng, root, config.depth, config.filename_length)
name, src_dir = create_python_package(packages_root)
python_package_names.append(name)
python_package_paths.append(src_dir)

for _ in range(config.num_python_editable_packages):
path = random_path(rng, packages_root, config.depth, config.filename_length)
name, src_dir = create_python_package(path)
python_package_names.append(name)
python_package_paths.append(src_dir)

python_package_paths_str = ", ".join(f'"{path.parent}"' for path in python_package_paths)
import_python_packages = "\n".join(f"import {name}" for name in python_package_names)
(root / "run.py").write_text(f"""\
import time
import logging
import sys
import maturin_import_hook

sys.path.extend([{python_package_paths_str}])

# logging.basicConfig(format='%(asctime)s %(name)s [%(levelname)s] %(message)s', level=logging.DEBUG)
# maturin_import_hook.reset_logger()

maturin_import_hook.install()

start = time.perf_counter()

{import_python_packages}

end = time.perf_counter()
print(f'took {{end - start:.6f}}s')
""")


def main() -> None:
parser = argparse.ArgumentParser()
parser.add_argument("root", type=Path, help="the location to write the benchmark data to")
args = parser.parse_args()

config = BenchmarkConfig.default()
create_benchmark_environment(args.root, config)


if __name__ == "__main__":
main()
2 changes: 1 addition & 1 deletion tests/maturin
Submodule maturin updated 127 files
Loading
Loading