From 1fa2b38fd9e716a2ea5507ab4881a7b96d145d19 Mon Sep 17 00:00:00 2001 From: Matthew Broadway Date: Mon, 28 Aug 2023 18:23:05 +0100 Subject: [PATCH] support more maturin flags and properly support parsing output with color --- maturin/import_hook/_building.py | 17 +++- maturin/import_hook/project_importer.py | 2 +- maturin/import_hook/rust_file_importer.py | 2 +- maturin/import_hook/settings.py | 99 +++++++++++++++++-- tests/import_hook/common.py | 9 ++ .../rebuild_on_settings_change_helper.py | 2 +- tests/import_hook/test_project_importer.py | 3 + tests/import_hook/test_rust_file_importer.py | 10 +- tests/import_hook/test_utilities.py | 90 ++++++++++++++++- 9 files changed, 219 insertions(+), 15 deletions(-) diff --git a/maturin/import_hook/_building.py b/maturin/import_hook/_building.py index fee488650..66c943d48 100644 --- a/maturin/import_hook/_building.py +++ b/maturin/import_hook/_building.py @@ -162,6 +162,9 @@ def build_wheel( output_dir: Path, settings: MaturinSettings, ) -> str: + if "build" not in settings.supported_commands(): + msg = f'provided {type(settings).__name__} does not support the "build" command' + raise ImportError(msg) success, output = _run_maturin( [ "build", @@ -186,6 +189,11 @@ def develop_build_project( args = ["develop", "--manifest-path", str(manifest_path)] if skip_install: args.append("--skip-install") + if "develop" not in settings.supported_commands(): + msg = ( + f'provided {type(settings).__name__} does not support the "develop" command' + ) + raise ImportError(msg) args.extend(settings.to_args()) success, output = _run_maturin(args) if not success: @@ -211,7 +219,11 @@ def _run_maturin(args: list[str]) -> Tuple[bool, str]: logger.error("maturin output:\n%s", output) return False, output if logger.isEnabledFor(logging.DEBUG): - logger.debug("maturin output:\n%s", output) + logger.debug( + "maturin output (has warnings: %r):\n%s", + maturin_output_has_warnings(output), + output, + ) return True, output @@ -242,6 +254,5 @@ def _find_single_file(dir_path: Path, extension: Optional[str]) -> Optional[Path def maturin_output_has_warnings(output: str) -> bool: return ( - re.search(r"warning: `.*` \((lib|bin)\) generated [0-9]+ warnings?", output) - is not None + re.search(r"`.*` \((lib|bin)\) generated [0-9]+ warnings?", output) is not None ) diff --git a/maturin/import_hook/project_importer.py b/maturin/import_hook/project_importer.py index 6dd3825f8..ab4d79f59 100644 --- a/maturin/import_hook/project_importer.py +++ b/maturin/import_hook/project_importer.py @@ -79,7 +79,7 @@ def _get_settings(self, module_path: str, source_path: Path) -> MaturinSettings: elif isinstance(self._settings, MaturinSettingsProvider): return self._settings.get_settings(module_path, source_path) else: - return MaturinSettings() + return MaturinSettings.default() def find_spec( self, diff --git a/maturin/import_hook/rust_file_importer.py b/maturin/import_hook/rust_file_importer.py index 8600f5102..1bccb5fee 100644 --- a/maturin/import_hook/rust_file_importer.py +++ b/maturin/import_hook/rust_file_importer.py @@ -49,7 +49,7 @@ def _get_settings(self, module_path: str, source_path: Path) -> MaturinSettings: elif isinstance(self._settings, MaturinSettingsProvider): return self._settings.get_settings(module_path, source_path) else: - return MaturinSettings() + return MaturinSettings.default() def find_spec( self, diff --git a/maturin/import_hook/settings.py b/maturin/import_hook/settings.py index a667164d3..67bef6c21 100644 --- a/maturin/import_hook/settings.py +++ b/maturin/import_hook/settings.py @@ -1,29 +1,49 @@ from abc import ABC, abstractmethod from dataclasses import dataclass from pathlib import Path -from typing import List, Optional +from typing import List, Optional, Dict, Set -__all__ = ["MaturinSettings", "MaturinSettingsProvider"] +__all__ = [ + "MaturinSettings", + "MaturinBuildSettings", + "MaturinDevelopSettings", + "MaturinSettingsProvider", +] @dataclass class MaturinSettings: + """Settings common to `maturin build` and `maturin develop`""" + release: bool = False strip: bool = False quiet: bool = False jobs: Optional[int] = None + profile: Optional[str] = None features: Optional[List[str]] = None all_features: bool = False no_default_features: bool = False + target: Optional[str] = None + ignore_rust_version: bool = False + color: Optional[bool] = None frozen: bool = False locked: bool = False offline: bool = False + config: Optional[Dict[str, str]] = None + unstable_flags: Optional[List[str]] = None verbose: int = 0 + rustc_flags: Optional[List[str]] = None + + @staticmethod + def supported_commands() -> Set[str]: + return {"build", "develop"} - def __post_init__(self) -> None: - if self.verbose not in (0, 1, 2): - msg = f"invalid verbose value: {self.verbose}" - raise ValueError(msg) + @staticmethod + def default() -> "MaturinSettings": + """MaturinSettings() sets no flags but default() corresponds to some sensible defaults""" + return MaturinSettings( + color=True, + ) def to_args(self) -> List[str]: args = [] @@ -36,6 +56,9 @@ def to_args(self) -> List[str]: if self.jobs is not None: args.append("--jobs") args.append(str(self.jobs)) + if self.profile is not None: + args.append("--profile") + args.append(self.profile) if self.features: args.append("--features") args.append(",".join(self.features)) @@ -43,14 +66,78 @@ def to_args(self) -> List[str]: args.append("--all-features") if self.no_default_features: args.append("--no-default-features") + if self.target is not None: + args.append("--target") + args.append(self.target) + if self.ignore_rust_version: + args.append("--ignore-rust-version") + if self.color is not None: + args.append("--color") + if self.color: + args.append("always") + else: + args.append("never") if self.frozen: args.append("--frozen") if self.locked: args.append("--locked") if self.offline: args.append("--offline") + if self.config is not None: + for key, value in self.config.items(): + args.append("--config") + args.append(f"{key}={value}") + if self.unstable_flags is not None: + for flag in self.unstable_flags: + args.append("-Z") + args.append(flag) if self.verbose > 0: args.append("-{}".format("v" * self.verbose)) + if self.rustc_flags is not None: + args.extend(self.rustc_flags) + return args + + +@dataclass +class MaturinBuildSettings(MaturinSettings): + """settings for `maturin build`""" + + skip_auditwheel: bool = False + zig: bool = False + + @staticmethod + def supported_commands() -> Set[str]: + return {"build"} + + def to_args(self) -> List[str]: + args = [] + if self.skip_auditwheel: + args.append("--skip-auditwheel") + if self.zig: + args.append("--zig") + args.extend(super().to_args()) + return args + + +@dataclass +class MaturinDevelopSettings(MaturinSettings): + """settings for `maturin develop`""" + + extras: Optional[List[str]] = None + skip_install: bool = False + + @staticmethod + def supported_commands() -> Set[str]: + return {"develop"} + + def to_args(self) -> List[str]: + args = [] + if self.extras is not None: + args.append("--extras") + args.append(",".join(self.extras)) + if self.skip_install: + args.append("--skip-install") + args.extend(super().to_args()) return args diff --git a/tests/import_hook/common.py b/tests/import_hook/common.py index 28a9b8be1..cd9b8e385 100644 --- a/tests/import_hook/common.py +++ b/tests/import_hook/common.py @@ -1,4 +1,5 @@ import os +import re import shutil import site import subprocess @@ -238,3 +239,11 @@ def create_project_from_blank_template( f"from .{package_name} import *" ) return project_dir + + +def remove_ansii_escape_characters(text: str) -> str: + """Remove escape characters (eg used to color terminal output) from the given string + + based on: https://stackoverflow.com/a/14693789 + """ + return re.sub(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])", "", text) diff --git a/tests/import_hook/rust_file_import/rebuild_on_settings_change_helper.py b/tests/import_hook/rust_file_import/rebuild_on_settings_change_helper.py index aa0b2a023..9b6aff61f 100644 --- a/tests/import_hook/rust_file_import/rebuild_on_settings_change_helper.py +++ b/tests/import_hook/rust_file_import/rebuild_on_settings_change_helper.py @@ -18,7 +18,7 @@ def get_settings(self, module_path: str, source_path: Path) -> MaturinSettings: return MaturinSettings(features=["pyo3/extension-module", "large_number"]) else: print(f"building {module_path} with default settings") - return MaturinSettings() + return MaturinSettings.default() import_hook.install(settings=CustomSettingsProvider()) diff --git a/tests/import_hook/test_project_importer.py b/tests/import_hook/test_project_importer.py index cc77b1b4c..773f8dafc 100644 --- a/tests/import_hook/test_project_importer.py +++ b/tests/import_hook/test_project_importer.py @@ -23,6 +23,7 @@ test_crates, uninstall, with_underscores, + remove_ansii_escape_characters, ) """ @@ -679,6 +680,7 @@ def test_default_compile_warning(self, workspace: Path, is_mixed: bool) -> None: ) output1, _ = run_python_code(self.loader_script) + output1 = remove_ansii_escape_characters(output1) pattern = ( 'building "test_project"\n' 'maturin.import_hook \\[WARNING\\] build of "test_project" succeeded with warnings:\n' @@ -694,6 +696,7 @@ def test_default_compile_warning(self, workspace: Path, is_mixed: bool) -> None: ) output2, _ = run_python_code(self.loader_script) + output2 = remove_ansii_escape_characters(output2) pattern = ( 'maturin.import_hook \\[WARNING\\] the last build of "test_project" succeeded with warnings:\n' ".*" diff --git a/tests/import_hook/test_rust_file_importer.py b/tests/import_hook/test_rust_file_importer.py index 960ebcc0c..e2259241c 100644 --- a/tests/import_hook/test_rust_file_importer.py +++ b/tests/import_hook/test_rust_file_importer.py @@ -7,7 +7,13 @@ import pytest -from .common import log, run_python, script_dir, test_crates +from .common import ( + log, + run_python, + script_dir, + test_crates, + remove_ansii_escape_characters, +) """ These tests ensure the correct functioning of the rust file importer import hook. @@ -309,6 +315,7 @@ def test_default_compile_warning(self, workspace: Path) -> None: ) output1, _ = run_python([str(py_path)], workspace) + output1 = remove_ansii_escape_characters(output1) pattern = ( 'building "my_script"\n' 'maturin.import_hook \\[WARNING\\] build of "my_script" succeeded with warnings:\n' @@ -324,6 +331,7 @@ def test_default_compile_warning(self, workspace: Path) -> None: ) output2, _ = run_python([str(py_path)], workspace) + output2 = remove_ansii_escape_characters(output2) pattern = ( 'maturin.import_hook \\[WARNING\\] the last build of "my_script" succeeded with warnings:\n' ".*" diff --git a/tests/import_hook/test_utilities.py b/tests/import_hook/test_utilities.py index bae61cf04..2172d6d5c 100644 --- a/tests/import_hook/test_utilities.py +++ b/tests/import_hook/test_utilities.py @@ -12,18 +12,24 @@ import pytest -from maturin.import_hook._building import BuildCache, BuildStatus, LockNotHeldError +from maturin.import_hook import MaturinSettings +from maturin.import_hook._building import ( + BuildCache, + BuildStatus, + LockNotHeldError, +) from maturin.import_hook._file_lock import AtomicOpenLock, FileLock from maturin.import_hook._resolve_project import ProjectResolveError, _resolve_project from maturin.import_hook.project_importer import ( _get_installed_package_mtime, _get_project_mtime, ) +from maturin.import_hook.settings import MaturinDevelopSettings, MaturinBuildSettings from .common import log, test_crates # set this to be able to run these tests without going through run.rs each time -SAVED_RESOLVED_PACKAGES_PATH: Optional[Path] = None +SAVED_RESOLVED_PACKAGES_PATH: Optional[Path] = Path("/w/resolved.json") if SAVED_RESOLVED_PACKAGES_PATH is not None: if "RESOLVED_PACKAGES_PATH" in os.environ: @@ -31,6 +37,86 @@ os.environ["RESOLVED_PACKAGES_PATH"] = str(SAVED_RESOLVED_PACKAGES_PATH) +def test_settings() -> None: + assert MaturinSettings().to_args() == [] + assert MaturinBuildSettings().to_args() == [] + assert MaturinDevelopSettings().to_args() == [] + + settings = MaturinSettings( + release=True, + strip=True, + quiet=True, + jobs=1, + profile="profile1", + features=["feature1", "feature2"], + all_features=True, + no_default_features=True, + target="target1", + ignore_rust_version=True, + color=True, + frozen=True, + locked=True, + offline=True, + config={"key1": "value1", "key2": "value2"}, + unstable_flags=["unstable1", "unstable2"], + verbose=2, + rustc_flags=["flag1", "flag2"], + ) + # fmt: off + assert settings.to_args() == [ + '--release', + '--strip', + '--quiet', + '--jobs', '1', + '--profile', 'profile1', + '--features', 'feature1,feature2', + '--all-features', + '--no-default-features', + '--target', 'target1', + '--ignore-rust-version', + '--color', 'always', + '--frozen', + '--locked', + '--offline', + '--config', 'key1=value1', + '--config', 'key2=value2', + '-Z', 'unstable1', + '-Z', 'unstable2', + '-vv', + 'flag1', + 'flag2', + ] + # fmt: on + + build_settings = MaturinBuildSettings( + skip_auditwheel=True, zig=True, color=False, rustc_flags=["flag1", "flag2"] + ) + assert build_settings.to_args() == [ + "--skip-auditwheel", + "--zig", + "--color", + "never", + "flag1", + "flag2", + ] + + develop_settings = MaturinDevelopSettings( + extras=["extra1", "extra2"], + skip_install=True, + color=False, + rustc_flags=["flag1", "flag2"], + ) + assert develop_settings.to_args() == [ + "--extras", + "extra1,extra2", + "--skip-install", + "--color", + "never", + "flag1", + "flag2", + ] + + class TestFileLock: @staticmethod def _create_lock(path: Path, timeout_seconds: float, fallback: bool) -> FileLock: