diff --git a/CHANGELOG.md b/CHANGELOG.md index 1baa89c0..ce64d15b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,15 @@ # Changelog -## 1.4.1 (2022-07-05) +## Unreleased +### Changed +- Locate cdylib artifacts by handling messages from cargo instead of searching target dir (fixes build on MSYS2). [#267](https://github.com/PyO3/setuptools-rust/pull/267) + +## 1.4.1 (2022-07-05) ### Fixed - Fix crash when checking Rust version. [#263](https://github.com/PyO3/setuptools-rust/pull/263) ## 1.4.0 (2022-07-05) - ### Packaging - Increase minimum `setuptools` version to 62.4. [#246](https://github.com/PyO3/setuptools-rust/pull/246) @@ -25,7 +28,6 @@ - If the sysconfig for `BLDSHARED` has no flags, `setuptools-rust` won't crash anymore. [#241](https://github.com/PyO3/setuptools-rust/pull/241) ## 1.3.0 (2022-04-26) - ### Packaging - Increase minimum `setuptools` version to 58. [#222](https://github.com/PyO3/setuptools-rust/pull/222) diff --git a/examples/namespace_package/Cargo.toml b/examples/namespace_package/Cargo.toml index 3b855e53..d6969f86 100644 --- a/examples/namespace_package/Cargo.toml +++ b/examples/namespace_package/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition = "2018" [lib] -crate-type = ["cdylib"] +crate-type = ["cdylib", "rlib"] [dependencies] pyo3 = { version = "0.16.5", features = ["extension-module"] } diff --git a/setuptools_rust/_utils.py b/setuptools_rust/_utils.py index f5325ba3..b6d9400b 100644 --- a/setuptools_rust/_utils.py +++ b/setuptools_rust/_utils.py @@ -1,9 +1,17 @@ import subprocess -def format_called_process_error(e: subprocess.CalledProcessError) -> str: +def format_called_process_error( + e: subprocess.CalledProcessError, + *, + include_stdout: bool = True, + include_stderr: bool = True, +) -> str: """Helper to convert a CalledProcessError to an error message. + If `include_stdout` or `include_stderr` are True (the default), the + respective output stream will be added to the error message (if + present in the exception). >>> format_called_process_error(subprocess.CalledProcessError( ... 777, ['ls', '-la'], None, None @@ -14,18 +22,22 @@ def format_called_process_error(e: subprocess.CalledProcessError) -> str: ... )) "`cargo 'foo bar'` failed with code 1\\n-- Output captured from stdout:\\nmessage" >>> format_called_process_error(subprocess.CalledProcessError( + ... 1, ['cargo', 'foo bar'], 'message', None + ... ), include_stdout=False) + "`cargo 'foo bar'` failed with code 1" + >>> format_called_process_error(subprocess.CalledProcessError( ... -1, ['cargo'], 'stdout', 'stderr' ... )) '`cargo` failed with code -1\\n-- Output captured from stdout:\\nstdout\\n-- Output captured from stderr:\\nstderr' """ command = " ".join(_quote_whitespace(arg) for arg in e.cmd) message = f"`{command}` failed with code {e.returncode}" - if e.stdout is not None: + if include_stdout and e.stdout is not None: message += f""" -- Output captured from stdout: {e.stdout}""" - if e.stderr is not None: + if include_stderr and e.stderr is not None: message += f""" -- Output captured from stderr: {e.stderr}""" diff --git a/setuptools_rust/build.py b/setuptools_rust/build.py index 4fe594bd..27c112cd 100644 --- a/setuptools_rust/build.py +++ b/setuptools_rust/build.py @@ -1,6 +1,7 @@ from __future__ import annotations import glob +import json import os import platform import shutil @@ -15,7 +16,8 @@ DistutilsPlatformError, ) from distutils.sysconfig import get_config_var -from typing import Dict, List, NamedTuple, Optional, Set, Tuple, cast +from pathlib import Path +from typing import Dict, Iterable, List, NamedTuple, Optional, Set, Tuple, cast from setuptools.command.build import build as CommandBuild # type: ignore[import] from setuptools.command.build_ext import build_ext as CommandBuildExt @@ -139,11 +141,6 @@ def build_extension( quiet = self.qbuild or ext.quiet debug = self._is_debug_build(ext) - # Find where to put the temporary build files created by `cargo` - target_dir = _base_cargo_target_dir(ext, quiet=quiet) - if target_triple is not None: - target_dir = os.path.join(target_dir, target_triple) - cargo_args = self._cargo_args( ext=ext, target_triple=target_triple, release=not debug, quiet=quiet ) @@ -154,7 +151,14 @@ def build_extension( rustflags.extend(["-C", "linker=" + linker]) if ext._uses_exec_binding(): - command = [self.cargo, "build", "--manifest-path", ext.path, *cargo_args] + command = [ + self.cargo, + "build", + "--manifest-path", + ext.path, + "--message-format=json-render-diagnostics", + *cargo_args, + ] else: rustc_args = [ @@ -184,6 +188,7 @@ def build_extension( self.cargo, "rustc", "--lib", + "--message-format=json-render-diagnostics", "--manifest-path", ext.path, *cargo_args, @@ -209,13 +214,17 @@ def build_extension( try: # If quiet, capture all output and only show it in the exception # If not quiet, forward all cargo output to stderr - stdout = subprocess.PIPE if quiet else sys.stderr.fileno() stderr = subprocess.PIPE if quiet else None - subprocess.run( - command, env=env, stdout=stdout, stderr=stderr, text=True, check=True + cargo_messages = subprocess.check_output( + command, + env=env, + stderr=stderr, + text=True, ) except subprocess.CalledProcessError as e: - raise CompileError(format_called_process_error(e)) + # Don't include stdout in the formatted error as it is a huge dump + # of cargo json lines which aren't helpful for the end user. + raise CompileError(format_called_process_error(e, include_stdout=False)) except OSError: raise DistutilsExecError( @@ -226,60 +235,64 @@ def build_extension( # Find the shared library that cargo hopefully produced and copy # it into the build directory as if it were produced by build_ext. - profile = ext.get_cargo_profile() - if profile: - # https://doc.rust-lang.org/cargo/reference/profiles.html - if profile in {"dev", "test"}: - profile_dir = "debug" - elif profile == "bench": - profile_dir = "release" - else: - profile_dir = profile - else: - profile_dir = "debug" if debug else "release" - artifacts_dir = os.path.join(target_dir, profile_dir) dylib_paths = [] + package_id = ext.metadata(quiet=quiet)["resolve"]["root"] if ext._uses_exec_binding(): + # Find artifact from cargo messages + artifacts = _find_cargo_artifacts( + cargo_messages.splitlines(), + package_id=package_id, + kind="bin", + ) for name, dest in ext.target.items(): if not name: name = dest.split(".")[-1] - exe = sysconfig.get_config_var("EXE") - if exe is not None: - name += exe - path = os.path.join(artifacts_dir, name) - if os.access(path, os.X_OK): - dylib_paths.append(_BuiltModule(dest, path)) - else: + try: + artifact_path = next( + artifact + for artifact in artifacts + if Path(artifact).with_suffix("").name == name + ) + except StopIteration: raise DistutilsExecError( - "Rust build failed; " - f"unable to find executable '{name}' in '{artifacts_dir}'" + f"Rust build failed; unable to locate executable '{name}'" ) - else: - platform = sysconfig.get_platform() - if "win" in platform: - dylib_ext = "dll" - elif platform.startswith("macosx"): - dylib_ext = "dylib" - elif "wasm32" in platform: - dylib_ext = "wasm" - else: - dylib_ext = "so" - - wildcard_so = "*{}.{}".format(ext.get_lib_name(quiet=quiet), dylib_ext) - try: - dylib_paths.append( - _BuiltModule( - ext.name, - next(glob.iglob(os.path.join(artifacts_dir, wildcard_so))), + if os.environ.get("CARGO") == "cross": + artifact_path = _replace_cross_target_dir( + artifact_path, ext, quiet=quiet ) + + dylib_paths.append(_BuiltModule(dest, artifact_path)) + else: + # Find artifact from cargo messages + artifacts = tuple( + _find_cargo_artifacts( + cargo_messages.splitlines(), + package_id=package_id, + kind="cdylib", ) - except StopIteration: + ) + if len(artifacts) == 0: + raise DistutilsExecError( + "Rust build failed; unable to find any build artifacts" + ) + elif len(artifacts) > 1: raise DistutilsExecError( - f"Rust build failed; unable to find any {wildcard_so} in {artifacts_dir}" + f"Rust build failed; expected only one build artifact but found {artifacts}" + ) + + artifact_path = artifacts[0] + + if os.environ.get("CARGO") == "cross": + artifact_path = _replace_cross_target_dir( + artifact_path, ext, quiet=quiet ) + + # guaranteed to be just one element after checks above + dylib_paths.append(_BuiltModule(ext.name, artifact_path)) return dylib_paths def install_extension( @@ -668,23 +681,9 @@ def _prepare_build_environment(cross_lib: Optional[str]) -> Dict[str, str]: if cross_lib: env.setdefault("PYO3_CROSS_LIB_DIR", cross_lib) - env.pop("CARGO", None) return env -def _base_cargo_target_dir(ext: RustExtension, *, quiet: bool) -> str: - """Returns the root target directory cargo will use. - - If --target is passed to cargo in the command line, the target directory - will have the target appended as a child. - """ - target_directory = ext._metadata(quiet=quiet)["target_directory"] - assert isinstance( - target_directory, str - ), "expected cargo metadata to contain a string target directory" - return target_directory - - def _is_py_limited_api( ext_setting: Literal["auto", True, False], wheel_setting: Optional[_PyLimitedApi], @@ -771,3 +770,64 @@ def _split_platform_and_extension(ext_path: str) -> Tuple[str, str, str]: # rust.cpython-38-x86_64-linux-gnu to (rust, .cpython-38-x86_64-linux-gnu) ext_path, platform_tag = os.path.splitext(ext_path) return (ext_path, platform_tag, extension) + + +def _find_cargo_artifacts( + cargo_messages: List[str], + *, + package_id: str, + kind: str, +) -> Iterable[str]: + """Identifies cargo artifacts built for the given `package_id` from the + provided cargo_messages. + + >>> list(_find_cargo_artifacts( + ... [ + ... '{"some_irrelevant_message": []}', + ... '{"reason":"compiler-artifact","package_id":"some_id","target":{"kind":["cdylib"]},"filenames":["/some/path/baz.so"]}', + ... '{"reason":"compiler-artifact","package_id":"some_id","target":{"kind":["cdylib", "rlib"]},"filenames":["/file/two/baz.dylib", "/file/two/baz.rlib"]}', + ... '{"reason":"compiler-artifact","package_id":"some_other_id","target":{"kind":["cdylib"]},"filenames":["/not/this.so"]}', + ... ], + ... package_id="some_id", + ... kind="cdylib", + ... )) + ['/some/path/baz.so', '/file/two/baz.dylib'] + >>> list(_find_cargo_artifacts( + ... [ + ... '{"some_irrelevant_message": []}', + ... '{"reason":"compiler-artifact","package_id":"some_id","target":{"kind":["cdylib"]},"filenames":["/some/path/baz.so"]}', + ... '{"reason":"compiler-artifact","package_id":"some_id","target":{"kind":["cdylib", "rlib"]},"filenames":["/file/two/baz.dylib", "/file/two/baz.rlib"]}', + ... '{"reason":"compiler-artifact","package_id":"some_other_id","target":{"kind":["cdylib"]},"filenames":["/not/this.so"]}', + ... ], + ... package_id="some_id", + ... kind="rlib", + ... )) + ['/file/two/baz.rlib'] + """ + for message in cargo_messages: + # only bother parsing messages that look like a match + if "compiler-artifact" in message and package_id in message and kind in message: + parsed = json.loads(message) + # verify the message is correct + if ( + parsed.get("reason") == "compiler-artifact" + and parsed.get("package_id") == package_id + ): + for artifact_kind, filename in zip( + parsed["target"]["kind"], parsed["filenames"] + ): + if artifact_kind == kind: + yield filename + + +def _replace_cross_target_dir(path: str, ext: RustExtension, *, quiet: bool) -> str: + """Replaces target director from `cross` docker build with the correct + local path. + + Cross artifact messages and metadata contain paths from inside the + dockerfile; invoking `cargo metadata` we can work out the correct local + target directory. + """ + cross_target_dir = ext._metadata(cargo="cross", quiet=quiet)["target_directory"] + local_target_dir = ext._metadata(cargo="cargo", quiet=quiet)["target_directory"] + return path.replace(cross_target_dir, local_target_dir) diff --git a/setuptools_rust/extension.py b/setuptools_rust/extension.py index e846abaa..4534e582 100644 --- a/setuptools_rust/extension.py +++ b/setuptools_rust/extension.py @@ -5,7 +5,8 @@ import warnings from distutils.errors import DistutilsSetupError from enum import IntEnum, auto -from typing import Any, Dict, List, NewType, Optional, Sequence, Union +from functools import lru_cache +from typing import Any, Dict, List, NewType, Optional, Sequence, Union, cast from semantic_version import SimpleSpec from typing_extensions import Literal @@ -156,8 +157,6 @@ def __init__( self.optional = optional self.py_limited_api = py_limited_api - self._cargo_metadata: Optional[_CargoMetadata] = None - if native: warnings.warn( "`native` is deprecated, set RUSTFLAGS=-Ctarget-cpu=native instead.", @@ -231,42 +230,45 @@ def install_script(self, module_name: str, exe_path: str) -> None: with open(file, "w") as f: f.write(_SCRIPT_TEMPLATE.format(executable=repr(executable))) - def _metadata(self, *, quiet: bool) -> "_CargoMetadata": + def metadata(self, *, quiet: bool) -> "CargoMetadata": """Returns cargo metadata for this extension package. Cached - will only execute cargo on first invocation. """ - if self._cargo_metadata is None: - metadata_command = [ - "cargo", - "metadata", - "--manifest-path", - self.path, - "--format-version", - "1", - ] - if self.cargo_manifest_args: - metadata_command.extend(self.cargo_manifest_args) - - try: - # If quiet, capture stderr and only show it on exceptions - # If not quiet, let stderr be inherited - stderr = subprocess.PIPE if quiet else None - payload = subprocess.check_output( - metadata_command, stderr=stderr, encoding="latin-1" - ) - except subprocess.CalledProcessError as e: - raise DistutilsSetupError(format_called_process_error(e)) - try: - self._cargo_metadata = json.loads(payload) - except json.decoder.JSONDecodeError as e: - raise DistutilsSetupError( - f""" - Error parsing output of cargo metadata as json; received: - {payload} - """ - ) from e - return self._cargo_metadata + + return self._metadata(os.environ.get("CARGO", "cargo"), quiet) + + @lru_cache() + def _metadata(self, cargo: str, quiet: bool) -> "CargoMetadata": + metadata_command = [ + cargo, + "metadata", + "--manifest-path", + self.path, + "--format-version", + "1", + ] + if self.cargo_manifest_args: + metadata_command.extend(self.cargo_manifest_args) + + try: + # If quiet, capture stderr and only show it on exceptions + # If not quiet, let stderr be inherited + stderr = subprocess.PIPE if quiet else None + payload = subprocess.check_output( + metadata_command, stderr=stderr, encoding="latin-1" + ) + except subprocess.CalledProcessError as e: + raise DistutilsSetupError(format_called_process_error(e)) + try: + return cast(CargoMetadata, json.loads(payload)) + except json.decoder.JSONDecodeError as e: + raise DistutilsSetupError( + f""" + Error parsing output of cargo metadata as json; received: + {payload} + """ + ) from e def _uses_exec_binding(self) -> bool: return self.binding == Binding.Exec @@ -332,7 +334,7 @@ def entry_points(self) -> List[str]: return [] -_CargoMetadata = NewType("_CargoMetadata", Dict[str, Any]) +CargoMetadata = NewType("CargoMetadata", Dict[str, Any]) def _script_name(executable: str) -> str: diff --git a/tests/test_extension.py b/tests/test_extension.py index c23fcbc9..786d6aac 100644 --- a/tests/test_extension.py +++ b/tests/test_extension.py @@ -18,7 +18,7 @@ def hello_world_bin() -> RustBin: def test_metadata_contents(hello_world_bin: RustBin) -> None: - metadata = hello_world_bin._metadata(quiet=False) + metadata = hello_world_bin.metadata(quiet=False) assert "target_directory" in metadata @@ -28,13 +28,13 @@ def test_metadata_cargo_log( monkeypatch.setenv("CARGO_LOG", "trace") # With quiet unset, no stdout, plenty of logging stderr - hello_world_bin._metadata(quiet=False) + hello_world_bin.metadata(quiet=False) captured = capfd.readouterr() assert captured.out == "" assert "TRACE cargo::util::config" in captured.err # With quiet set, nothing will be printed - hello_world_bin._metadata(quiet=True) + hello_world_bin.metadata(quiet=True) captured = capfd.readouterr() assert captured.out == "" assert captured.err == ""