Merge pull request #256 from davidhewitt/error-quiet

respect `quiet` flag in `cargo metadata`

Author: davidhewitt

CHANGELOG.md

@@ -11,6 +11,8 @@
 
 ### Changed
 - `Exec` binding `RustExtension` with `script=True` is deprecated in favor of `RustBin`. [#248](https://github.com/PyO3/setuptools-rust/pull/248)
+- Errors while calling `cargo metadata` are now reported back to the user [#254](https://github.com/PyO3/setuptools-rust/pull/254)
+- `quiet` option will now suppress output of `cargo metadata`. [#256](https://github.com/PyO3/setuptools-rust/pull/256)
 
 ### Fixed
 - If the sysconfig for `BLDSHARED` has no flags, `setuptools-rust` won't crash anymore. [#241](https://github.com/PyO3/setuptools-rust/pull/241)

setuptools_rust/build.py

@@ -22,6 +22,7 @@
 
 from .command import RustCommand
 from .extension import RustBin, RustExtension, Strip
+from .private import format_called_process_error
 from .utils import (
     PyLimitedApi,
     binding_features,
@@ -139,13 +140,14 @@ def build_extension(
                 f"can't find Rust extension project file: {ext.path}"
             )
 
+        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)
+        target_dir = _base_cargo_target_dir(ext, quiet=quiet)
         if target_triple is not None:
             target_dir = os.path.join(target_dir, target_triple)
 
-        quiet = self.qbuild or ext.quiet
-        debug = self._is_debug_build(ext)
         cargo_args = self._cargo_args(
             ext=ext, target_triple=target_triple, release=not debug, quiet=quiet
         )
@@ -210,21 +212,12 @@ def build_extension(
 
         # Execute cargo
         try:
+            stderr = subprocess.PIPE if quiet else None
             output = subprocess.check_output(
-                command, env=env, encoding="latin-1", stderr=subprocess.PIPE
+                command, env=env, encoding="latin-1", stderr=stderr
             )
         except subprocess.CalledProcessError as e:
-            raise CompileError(
-                f"""
-                cargo failed with code: {e.returncode}
-
-                Output captured from stderr:
-                {e.stderr}
-
-                Output captured from stdout:
-                {e.stdout}
-                """
-            )
+            raise CompileError(format_called_process_error(e))
 
         except OSError:
             raise DistutilsExecError(
@@ -280,7 +273,7 @@ def build_extension(
             else:
                 dylib_ext = "so"
 
-            wildcard_so = "*{}.{}".format(ext.get_lib_name(), dylib_ext)
+            wildcard_so = "*{}.{}".format(ext.get_lib_name(quiet=quiet), dylib_ext)
 
             try:
                 dylib_paths.append(
@@ -711,13 +704,13 @@ def _prepare_build_environment(cross_lib: Optional[str]) -> Dict[str, str]:
     return env
 
 
-def _base_cargo_target_dir(ext: RustExtension) -> str:
+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()["target_directory"]
+    target_directory = ext._metadata(quiet=quiet)["target_directory"]
     assert isinstance(
         target_directory, str
     ), "expected cargo metadata to contain a string target directory"

setuptools_rust/extension.py

@@ -1,15 +1,17 @@
 import json
 import os
 import re
-import warnings
 import subprocess
+import warnings
 from distutils.errors import DistutilsSetupError
 from enum import IntEnum, auto
 from typing import Any, Dict, List, NewType, Optional, Union
 
 from semantic_version import SimpleSpec
 from typing_extensions import Literal
 
+from .private import format_called_process_error
+
 
 class Binding(IntEnum):
     """
@@ -167,9 +169,9 @@ def __init__(
                 DeprecationWarning,
             )
 
-    def get_lib_name(self) -> str:
+    def get_lib_name(self, *, quiet: bool) -> str:
         """Parse Cargo.toml to get the name of the shared library."""
-        metadata = self._metadata()
+        metadata = self._metadata(quiet=quiet)
         root_key = metadata["resolve"]["root"]
         [pkg] = [p for p in metadata["packages"] if p["id"] == root_key]
         name = pkg["targets"][0]["name"]
@@ -224,7 +226,7 @@ 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) -> "_CargoMetadata":
+    def _metadata(self, *, quiet: bool) -> "_CargoMetadata":
         """Returns cargo metedata for this extension package.
 
         Cached - will only execute cargo on first invocation.
@@ -242,21 +244,12 @@ def _metadata(self) -> "_CargoMetadata":
                 metadata_command.extend(self.cargo_manifest_args)
 
             try:
+                stderr = subprocess.PIPE if quiet else None
                 payload = subprocess.check_output(
-                    metadata_command, encoding="latin-1", stderr=subprocess.PIPE
+                    metadata_command, encoding="latin-1", stderr=stderr
                 )
             except subprocess.CalledProcessError as e:
-                raise DistutilsSetupError(
-                    f"""
-                    cargo metadata failed with code: {e.returncode}
-
-                    Output captured from stderr:
-                    {e.stderr}
-
-                    Output captured from stdout:
-                    {e.stdout}
-                    """
-                )
+                raise DistutilsSetupError(format_called_process_error(e))
             try:
                 self._cargo_metadata = json.loads(payload)
             except json.decoder.JSONDecodeError as e:

setuptools_rust/private.py

@@ -0,0 +1,33 @@
+import subprocess
+
+
+def format_called_process_error(e: subprocess.CalledProcessError) -> str:
+    """Helper to convert a CalledProcessError to an error message.
+
+    >>> format_called_process_error(subprocess.CalledProcessError(
+    ...     1, ['cargo', 'foo bar'], 'message', None
+    ... ))
+    "`cargo 'foo bar'` failed with code 1\\n-- Output captured from stdout:\\nmessage"
+    >>> 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}
+-- Output captured from stdout:
+{e.stdout}"""
+
+    if e.stderr is not None:
+        message += f"""
+-- Output captured from stderr:
+{e.stderr}"""
+
+    return message
+
+
+def _quote_whitespace(string: str) -> str:
+    if " " in string:
+        return f"'{string}'"
+    else:
+        return string