Merge pull request #268 from davidhewitt/more-quiet

quiet: clarify implementation

Author: davidhewitt

.github/workflows/ci.yml

@@ -48,6 +48,13 @@ jobs:
         with:
           python-version: "3.x"
 
+      - name: Install Rust toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          profile: minimal
+          default: true
+
       - run: pip install nox
 
       - run: nox -s test

setuptools_rust/_utils.py

@@ -4,6 +4,11 @@
 def format_called_process_error(e: subprocess.CalledProcessError) -> str:
     """Helper to convert a CalledProcessError to an error message.
 
+
+    >>> format_called_process_error(subprocess.CalledProcessError(
+    ...     777, ['ls', '-la'], None, None
+    ... ))
+    '`ls -la` failed with code 777'
     >>> format_called_process_error(subprocess.CalledProcessError(
     ...     1, ['cargo', 'foo bar'], 'message', None
     ... ))
@@ -14,7 +19,9 @@ def format_called_process_error(e: subprocess.CalledProcessError) -> str:
     '`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}
+    message = f"`{command}` failed with code {e.returncode}"
+    if e.stdout is not None:
+        message += f"""
 -- Output captured from stdout:
 {e.stdout}"""
 

setuptools_rust/build.py

@@ -207,8 +207,13 @@ def build_extension(
 
         # Execute cargo
         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
-            output = subprocess.check_output(command, env=env, stderr=stderr, text=True)
+            subprocess.run(
+                command, env=env, stdout=stdout, stderr=stderr, text=True, check=True
+            )
         except subprocess.CalledProcessError as e:
             raise CompileError(format_called_process_error(e))
 
@@ -218,10 +223,6 @@ def build_extension(
                 "requires Rust to be installed and cargo to be on the PATH"
             )
 
-        if not quiet:
-            if output:
-                print(output, file=sys.stderr)
-
         # Find the shared library that cargo hopefully produced and copy
         # it into the build directory as if it were produced by build_ext.
 

setuptools_rust/extension.py

@@ -232,7 +232,7 @@ def install_script(self, module_name: str, exe_path: str) -> None:
                 f.write(_SCRIPT_TEMPLATE.format(executable=repr(executable)))
 
     def _metadata(self, *, quiet: bool) -> "_CargoMetadata":
-        """Returns cargo metedata for this extension package.
+        """Returns cargo metadata for this extension package.
 
         Cached - will only execute cargo on first invocation.
         """
@@ -249,6 +249,8 @@ def _metadata(self, *, quiet: bool) -> "_CargoMetadata":
                 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"

tests/test_extension.py

@@ -0,0 +1,40 @@
+from pathlib import Path
+
+import pytest
+from pytest import CaptureFixture, MonkeyPatch
+
+from setuptools_rust.extension import RustBin
+
+
+@pytest.fixture()
+def hello_world_bin() -> RustBin:
+    setuptools_rust_dir = Path(__file__).parent.parent
+    return RustBin(
+        "hello-world",
+        path=(
+            setuptools_rust_dir / "examples" / "hello-world" / "Cargo.toml"
+        ).as_posix(),
+    )
+
+
+def test_metadata_contents(hello_world_bin: RustBin) -> None:
+    metadata = hello_world_bin._metadata(quiet=False)
+    assert "target_directory" in metadata
+
+
+def test_metadata_cargo_log(
+    capfd: CaptureFixture, monkeypatch: MonkeyPatch, hello_world_bin: RustBin
+) -> None:
+    monkeypatch.setenv("CARGO_LOG", "trace")
+
+    # With quiet unset, no stdout, plenty of logging stderr
+    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)
+    captured = capfd.readouterr()
+    assert captured.out == ""
+    assert captured.err == ""