config: surface real make-uncompress failures, drop dead try/except

Previously uncompress_gds() called subprocess.run without check=True so its
except CalledProcessError block was dead code, and stderr/stdout went into
PIPE and were thrown away. When `make uncompress` failed (e.g. on Fargate
under disk pressure for a large repo) cf-precheck silently swallowed the
real error and emitted only the misleading downstream "A single valid GDS
was not found" message — see chipfoundry/cf-cli#20 where users blamed LFS.

Now: judge by post-condition (gds/*.gds files in the working tree) since
caravel-lite's Makefile legitimately exits non-zero on the mag/ step *after*
the gds/ step completes, and we don't want that benign failure to fail the
whole precheck. If gds/ ends up populated, log INFO with the file list +
make's exit code and continue. If empty, log CRITICAL with returncode +
stderr tail (4000 chars) + stdout tail (200 chars) so the operator can see
exactly why make couldn't produce the layouts. Fail with sys.exit(252).

Per .cursor/rules/no-fallback-code.mdc.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: jdicorpo

src/cf_precheck/config.py

@@ -13,16 +13,53 @@
 
 
 def uncompress_gds(project_path: Path, caravel_root: Path) -> None:
+    """Run the caravel-lite `uncompress` make target to gunzip gds/*.gds.gz.
+
+    Surfaces real failures instead of silently swallowing them. We deliberately
+    do NOT use ``check=True`` because the caravel-lite Makefile's ``uncompress``
+    target also touches ``mag/*.mag.gz``; if a project has both ``mag/foo.mag``
+    and ``mag/foo.mag.gz`` committed (common), gunzip exits non-zero on that
+    *after* the gds step has already produced ``gds/*.gds`` files. Failing the
+    whole precheck on that benign mag-step exit would be wrong. Instead we
+    judge by the post-condition cf-precheck actually cares about: at least one
+    ``gds/*.gds`` file in the working tree.
+    """
     cmd = f"make -f {caravel_root}/Makefile uncompress;"
-    try:
-        logging.info(f"Extracting compressed files in: {project_path}")
-        subprocess.run(
-            cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
-            shell=True, cwd=str(project_path),
+    logging.info(f"Extracting compressed files in: {project_path}")
+    result = subprocess.run(
+        cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
+        shell=True, cwd=str(project_path),
+    )
+
+    gds_dir = project_path / "gds"
+    produced_gds = sorted(gds_dir.glob("*.gds")) if gds_dir.is_dir() else []
+
+    if produced_gds:
+        names = ", ".join(p.name for p in produced_gds)
+        logging.info(
+            f"make uncompress produced {len(produced_gds)} gds/*.gds file(s): {names} "
+            f"(make exit {result.returncode})"
+        )
+        return
+
+    stderr_text = (result.stderr or b"").decode(errors="replace").strip()
+    stdout_text = (result.stdout or b"").decode(errors="replace").strip()
+    stderr_tail = stderr_text[-4000:] if stderr_text else "(empty)"
+    stdout_tail = stdout_text[-200:] if stdout_text else "(empty)"
+
+    if result.returncode != 0:
+        logging.critical(
+            "make uncompress failed and produced no gds/*.gds files "
+            f"(exit {result.returncode}). "
+            f"stderr tail: {stderr_tail} | stdout tail: {stdout_tail}"
+        )
+    else:
+        logging.critical(
+            "make uncompress reported success (exit 0) but produced no "
+            f"gds/*.gds files in {gds_dir}. stderr tail: {stderr_tail} | "
+            f"stdout tail: {stdout_tail}"
         )
-    except subprocess.CalledProcessError as error:
-        logging.critical(f"Make 'uncompress' error: {error}")
-        sys.exit(252)
+    sys.exit(252)
 
 
 def is_binary_file(filename: str | Path) -> bool:

tests/test_uncompress_gds.py

@@ -0,0 +1,108 @@
+"""Regression: uncompress_gds must surface real make failures, not swallow them.
+
+The previous implementation captured stderr/stdout into PIPE and threw them
+away, used ``subprocess.run`` without ``check=True`` (so its
+``except CalledProcessError`` block was dead code), and returned silently on
+ANY make failure. That meant Fargate-only failures (e.g. disk pressure during
+gunzip on a heavy repo) bubbled up as the misleading downstream error
+"A single valid GDS was not found" with no information about why. See
+chipfoundry/cf-cli#20.
+"""
+
+from __future__ import annotations
+
+import logging
+import subprocess
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+from cf_precheck.config import uncompress_gds
+
+
+def _completed(returncode: int, stderr: bytes = b"", stdout: bytes = b"") -> subprocess.CompletedProcess:
+    return subprocess.CompletedProcess(
+        args="make ... uncompress",
+        returncode=returncode,
+        stderr=stderr,
+        stdout=stdout,
+    )
+
+
+def test_success_when_gds_files_present_regardless_of_exit_code(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Caravel-lite Makefile legitimately exits 2 on the mag/ step after the
+    gds/ step succeeds. As long as gds/*.gds is on disk, treat as success."""
+    (tmp_path / "gds").mkdir()
+    (tmp_path / "gds" / "user_project_wrapper.gds").write_bytes(b"GDSII")
+    (tmp_path / "gds" / "other.gds").write_bytes(b"GDSII")
+
+    with patch.object(
+        subprocess, "run",
+        return_value=_completed(2, stderr=b"gzip: mag/user_project_wrapper.mag already exists"),
+    ), caplog.at_level(logging.INFO):
+        uncompress_gds(tmp_path, Path("/opt/caravel"))
+
+    info_msgs = [r.message for r in caplog.records if r.levelno == logging.INFO]
+    assert any("produced 2 gds/*.gds file" in m for m in info_msgs), info_msgs
+    assert any("user_project_wrapper.gds" in m for m in info_msgs), info_msgs
+    assert any("other.gds" in m for m in info_msgs), info_msgs
+    assert any("make exit 2" in m for m in info_msgs), info_msgs
+
+
+def test_critical_with_stderr_when_make_fails_and_no_gds(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Real make failure (e.g. disk full, missing tool) on a repo with no
+    pre-existing gds/*.gds — surface stderr + returncode, then exit 252."""
+    (tmp_path / "gds").mkdir()
+    stderr = b"gunzip: write error: No space left on device\n"
+
+    with patch.object(
+        subprocess, "run", return_value=_completed(2, stderr=stderr),
+    ), caplog.at_level(logging.CRITICAL):
+        with pytest.raises(SystemExit) as excinfo:
+            uncompress_gds(tmp_path, Path("/opt/caravel"))
+
+    assert excinfo.value.code == 252
+    crit_msgs = [r.message for r in caplog.records if r.levelno == logging.CRITICAL]
+    assert any("make uncompress failed" in m for m in crit_msgs), crit_msgs
+    assert any("No space left on device" in m for m in crit_msgs), crit_msgs
+    assert any("exit 2" in m for m in crit_msgs), crit_msgs
+
+
+def test_critical_when_make_succeeds_but_no_gds_produced(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """make exits 0 (e.g. no .gds.gz inputs to operate on) but no .gds files
+    appear: cf-precheck cannot proceed; surface a clear "produced no gds"
+    message instead of the misleading downstream "no valid GDS" error."""
+    (tmp_path / "gds").mkdir()
+
+    with patch.object(
+        subprocess, "run", return_value=_completed(0, stdout=b"Nothing to do for 'uncompress'."),
+    ), caplog.at_level(logging.CRITICAL):
+        with pytest.raises(SystemExit) as excinfo:
+            uncompress_gds(tmp_path, Path("/opt/caravel"))
+
+    assert excinfo.value.code == 252
+    crit_msgs = [r.message for r in caplog.records if r.levelno == logging.CRITICAL]
+    assert any("reported success" in m for m in crit_msgs), crit_msgs
+    assert any("produced no" in m for m in crit_msgs), crit_msgs
+
+
+def test_missing_gds_dir_treated_as_no_gds(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """If gds/ doesn't exist at all (malformed repo), still surface a clear
+    error rather than silently passing or KeyError'ing later."""
+    with patch.object(
+        subprocess, "run",
+        return_value=_completed(0, stdout=b"make: *** No rule to make target 'uncompress'."),
+    ), caplog.at_level(logging.CRITICAL):
+        with pytest.raises(SystemExit) as excinfo:
+            uncompress_gds(tmp_path, Path("/opt/caravel"))
+
+    assert excinfo.value.code == 252