Tighten artifact setup behavior

cli/python/base_setup/engine.py

@@ -83,7 +83,13 @@ def reconcile_manifest(
     artifacts = merge_artifacts(default_manifest.artifacts, manifest.artifacts)
     definitions = resolve_artifact_definitions(artifacts)
     if not manifest.artifacts:
-        ctx.log.info("Project '%s' declares no artifacts.", manifest.project_name)
+        if artifacts:
+            ctx.log.info(
+                "Project '%s' declares no artifacts; installing Base default artifacts only.",
+                manifest.project_name,
+            )
+        else:
+            ctx.log.info("Project '%s' has no artifacts to install.", manifest.project_name)
 
     for artifact, definition in zip(artifacts, definitions, strict=True):
         reconcile_artifact(ctx, definition, artifact.version, dry_run=dry_run)
@@ -149,6 +155,13 @@ def reconcile_homebrew_artifact(
     version: str,
     dry_run: bool,
 ) -> None:
+    if version != "latest":
+        raise ArtifactError(
+            "Homebrew artifact "
+            f"'{definition.name}' specifies version '{version}', but Base only supports "
+            "Homebrew artifact version 'latest' right now."
+        )
+
     command = ["brew", "install", definition.package]
     if dry_run:
         dry_run_command(ctx, command)
@@ -171,7 +184,7 @@ def reconcile_homebrew_artifact(
         definition.package,
         version,
     )
-    run_command(command)
+    run_command(ctx, command)
 
 
 def reconcile_python_artifact(
@@ -181,7 +194,7 @@ def reconcile_python_artifact(
     dry_run: bool,
 ) -> None:
     project = os.environ.get("BASE_PROJECT", "base")
-    venv_dir = Path.home() / ".base.d" / project / ".venv"
+    venv_dir = project_venv_dir(project)
     python_bin = venv_dir / "bin" / "python"
     requirement = f"{definition.package}=={version}" if version != "latest" else definition.package
 
@@ -200,7 +213,14 @@ def reconcile_python_artifact(
         venv.create(venv_dir, with_pip=True)
 
     ctx.log.info("Installing Python artifact '%s' into project virtual environment.", definition.name)
-    run_command([str(python_bin), "-m", "pip", "install", requirement])
+    run_command(ctx, [str(python_bin), "-m", "pip", "install", requirement])
+
+
+def project_venv_dir(project: str) -> Path:
+    override = os.environ.get("BASE_PROJECT_VENV_DIR")
+    if override:
+        return Path(override).expanduser()
+    return Path.home() / ".base.d" / project / ".venv"
 
 
 def python_artifact_installed(python_bin: Path, package: str, version: str) -> bool:
@@ -229,10 +249,15 @@ def run_check(command: list[str]) -> bool:
     return subprocess.run(command, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False).returncode == 0
 
 
-def run_command(command: list[str]) -> None:
-    completed = subprocess.run(command, check=False)
+def run_command(ctx: base_cli.Context, command: list[str]) -> None:
+    completed = subprocess.run(command, stderr=subprocess.PIPE, text=True, check=False)
     if completed.returncode:
-        raise ArtifactError(f"Command failed with exit {completed.returncode}: {format_command(command)}")
+        stderr = (completed.stderr or "").strip()
+        message = f"Command failed with exit {completed.returncode}: {format_command(command)}"
+        if stderr:
+            ctx.log.error("Command stderr: %s", stderr)
+            message = f"{message}\n{stderr}"
+        raise ArtifactError(message)
 
 
 def dry_run_command(ctx: base_cli.Context, command: list[str]) -> None:

cli/python/base_setup/tests/test_engine.py

@@ -9,6 +9,7 @@
 from pathlib import Path
 from unittest import mock
 
+from base_setup import engine
 from base_setup.engine import ArtifactError, main, merge_artifacts
 from base_setup.manifest import ArtifactRequest
 from base_setup.manifest import read_manifest
@@ -25,6 +26,12 @@ def run_engine(args: list[str]) -> tuple[int, str, str]:
     return status, stdout.getvalue(), stderr.getvalue()
 
 
+def fake_context() -> mock.Mock:
+    ctx = mock.Mock()
+    ctx.log = mock.Mock()
+    return ctx
+
+
 class ManifestTests(unittest.TestCase):
     def test_merge_artifacts_keeps_defaults_and_manifest_artifacts(self) -> None:
         merged = merge_artifacts(
@@ -126,7 +133,7 @@ def test_known_homebrew_artifact_dry_run_does_not_require_brew(self) -> None:
                         "artifacts:",
                         "  - type: tool",
                         "    name: terraform",
-                        "    version: \"1.8.5\"",
+                        "    version: latest",
                     ]
                 ),
                 encoding="utf-8",
@@ -137,6 +144,61 @@ def test_known_homebrew_artifact_dry_run_does_not_require_brew(self) -> None:
         self.assertEqual(status, 0)
         self.assertIn("[DRY-RUN] Would run: brew install terraform", stderr)
 
+    def test_homebrew_artifact_rejects_non_latest_version(self) -> None:
+        definition = get_artifact_definition("tool", "terraform")
+        self.assertIsNotNone(definition)
+
+        with self.assertRaisesRegex(ArtifactError, "only supports Homebrew artifact version 'latest'"):
+            engine.reconcile_homebrew_artifact(fake_context(), definition, "1.8.5", dry_run=True)
+
+    def test_homebrew_artifact_latest_invokes_brew_install(self) -> None:
+        definition = get_artifact_definition("tool", "terraform")
+        self.assertIsNotNone(definition)
+        ctx = fake_context()
+
+        with mock.patch("base_setup.engine.command_exists", return_value=True), mock.patch(
+            "base_setup.engine.run_check",
+            return_value=False,
+        ), mock.patch("base_setup.engine.run_command") as run_command:
+            engine.reconcile_homebrew_artifact(ctx, definition, "latest", dry_run=False)
+
+        run_command.assert_called_once_with(ctx, ["brew", "install", "terraform"])
+
+    def test_python_artifact_honors_project_venv_dir_override(self) -> None:
+        definition = get_artifact_definition("python-package", "requests")
+        self.assertIsNotNone(definition)
+        ctx = fake_context()
+
+        with tempfile.TemporaryDirectory() as tmpdir:
+            venv_dir = Path(tmpdir) / "custom-venv"
+            with mock.patch.dict(
+                os.environ,
+                {"BASE_PROJECT": "demo", "BASE_PROJECT_VENV_DIR": str(venv_dir)},
+            ), mock.patch("base_setup.engine.python_artifact_installed", return_value=False):
+                engine.reconcile_python_artifact(ctx, definition, "latest", dry_run=True)
+
+        info_messages = [call.args[0] % call.args[1:] for call in ctx.log.info.call_args_list]
+        self.assertIn(
+            f"[DRY-RUN] Would create project virtual environment at '{venv_dir}'.",
+            info_messages,
+        )
+        self.assertIn(
+            f"[DRY-RUN] Would run: {venv_dir}/bin/python -m pip install requests",
+            info_messages,
+        )
+
+    def test_run_command_includes_stderr_on_failure(self) -> None:
+        ctx = fake_context()
+
+        with mock.patch(
+            "base_setup.engine.subprocess.run",
+            return_value=mock.Mock(returncode=17, stderr="installer exploded\n"),
+        ):
+            with self.assertRaisesRegex(ArtifactError, "installer exploded"):
+                engine.run_command(ctx, ["installer", "--bad"])
+
+        ctx.log.error.assert_called_once_with("Command stderr: %s", "installer exploded")
+
     @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed")
     def test_project_argument_validates_manifest_project_name(self) -> None:
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -179,7 +241,7 @@ def test_empty_artifact_list_is_supported(self) -> None:
         self.assertEqual(manifest.artifacts, ())
 
     @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed")
-    def test_empty_artifact_list_logs_that_no_artifacts_are_declared(self) -> None:
+    def test_empty_artifact_list_logs_that_base_defaults_are_used(self) -> None:
         with tempfile.TemporaryDirectory() as tmpdir:
             manifest_path = Path(tmpdir) / "base_manifest.yaml"
             manifest_path.write_text(
@@ -197,7 +259,7 @@ def test_empty_artifact_list_logs_that_no_artifacts_are_declared(self) -> None:
             status, _stdout, stderr = run_engine(["--dry-run", "--manifest", str(manifest_path)])
 
         self.assertEqual(status, 0)
-        self.assertIn("Project 'demo' declares no artifacts.", stderr)
+        self.assertIn("Project 'demo' declares no artifacts; installing Base default artifacts only.", stderr)
 
 
 if __name__ == "__main__":