Simulate canonical merge #610

Author: codex

Makefile

@@ -1,8 +1,11 @@
 PYTHON_VERSION ?= 3.10
 VENV_DIR ?= .venv
 BASE_PYTHON ?= $(shell command -v python$(PYTHON_VERSION) 2>/dev/null || command -v python3 2>/dev/null || command -v python 2>/dev/null)
+# RUN_PYTHON is quoted at every use below: the checkout path may contain
+# spaces, and unquoted it word-splits in sh, which makes ensure-venv
+# misreport a perfectly good interpreter as a missing one.
 RUN_PYTHON = $(shell for dir in "$$VIRTUAL_ENV" "$$CONDA_PREFIX" "$(VENV_DIR)"; do if [ -n "$$dir" ] && [ -x "$$dir/bin/python" ]; then printf "%s/bin/python" "$$dir"; exit 0; elif [ -n "$$dir" ] && [ -x "$$dir/Scripts/python.exe" ]; then printf "%s/Scripts/python.exe" "$$dir"; exit 0; fi; done; if [ "$(OS)" = "Windows_NT" ]; then printf "%s/Scripts/python.exe" "$(VENV_DIR)"; else printf "%s/bin/python" "$(VENV_DIR)"; fi)
-PIP = $(RUN_PYTHON) -m pip
+PIP = "$(RUN_PYTHON)" -m pip
 
 .DEFAULT_GOAL := setup
 
@@ -36,13 +39,13 @@ ensure-venv:
 			exit 1; \
 		}; \
 	fi
-	@$(RUN_PYTHON) -c "import sys; required=tuple(map(int, '$(PYTHON_VERSION)'.split('.')[:2])); raise SystemExit(0 if sys.version_info[:2] >= required else 1)" || { \
+	@"$(RUN_PYTHON)" -c "import sys; required=tuple(map(int, '$(PYTHON_VERSION)'.split('.')[:2])); raise SystemExit(0 if sys.version_info[:2] >= required else 1)" || { \
 		echo "ERROR: OpenMontage requires Python $(PYTHON_VERSION)+."; \
-		echo "Current interpreter is $$($(RUN_PYTHON) -c 'import sys; print(\".\".join(map(str, sys.version_info[:3])))' 2>/dev/null || echo unavailable): $(RUN_PYTHON)"; \
+		echo "Current interpreter is $$("$(RUN_PYTHON)" -c 'import sys; print(\".\".join(map(str, sys.version_info[:3])))' 2>/dev/null || echo unavailable): $(RUN_PYTHON)"; \
 		echo "Activate a compatible environment or remove it so make can create $(VENV_DIR)."; \
 		exit 1; \
 	}
-	@$(RUN_PYTHON) -m pip --version >/dev/null 2>&1 || $(RUN_PYTHON) -m ensurepip --upgrade >/dev/null
+	@"$(RUN_PYTHON)" -m pip --version >/dev/null 2>&1 || "$(RUN_PYTHON)" -m ensurepip --upgrade >/dev/null
 
 venv: ensure-venv
 	@echo "==> Virtual environment ready."
@@ -70,9 +73,9 @@ setup: bootstrap
 	@echo "    Pulls the 'hyperframes' npm package into the local npx cache so the"
 	@echo "    first render doesn't pay a 30-60s cold-fetch penalty. ~20MB of disk."
 	@npx --yes hyperframes --version >/dev/null 2>&1 && echo "    HyperFrames CLI cached (npx)" || echo "  [skip] HyperFrames cache-warm failed — offline or npm unavailable; first render will fetch on demand"
-	@$(RUN_PYTHON) -c "from tools.video.hyperframes_compose import HyperFramesCompose; HyperFramesCompose._npm_resolve_cache=None; c=HyperFramesCompose()._runtime_check(); print(f'    HyperFrames runtime_available={c[\"runtime_available\"]}, npm={c.get(\"npm_package_version\") or c.get(\"npm_resolve_error\")}'); [print(f'    note: {r}') for r in c['reasons']]" || echo "  [skip] HyperFrames check failed — runtime can be set up later"
+	@"$(RUN_PYTHON)" -c "from tools.video.hyperframes_compose import HyperFramesCompose; HyperFramesCompose._npm_resolve_cache=None; c=HyperFramesCompose()._runtime_check(); print(f'    HyperFrames runtime_available={c[\"runtime_available\"]}, npm={c.get(\"npm_package_version\") or c.get(\"npm_resolve_error\")}'); [print(f'    note: {r}') for r in c['reasons']]" || echo "  [skip] HyperFrames check failed — runtime can be set up later"
 	@echo ""
-	$(RUN_PYTHON) -c "import shutil, os; e=os.path.exists('.env'); shutil.copy('.env.example','.env') if not e else None; print('==> Created .env from .env.example — add your API keys there.' if not e else '==> .env already exists — skipping.')"
+	"$(RUN_PYTHON)" -c "import shutil, os; e=os.path.exists('.env'); shutil.copy('.env.example','.env') if not e else None; print('==> Created .env from .env.example — add your API keys there.' if not e else '==> .env already exists — skipping.')"
 	@echo ""
 	@echo "Done! Open this project in your AI coding assistant and start creating."
 	@echo "  Optional: add API keys to .env to unlock cloud providers."
@@ -95,19 +98,19 @@ install-gpu: bootstrap
 # ---- Testing ----
 
 test: ensure-venv
-	$(RUN_PYTHON) -m pytest tests/ -v
+	"$(RUN_PYTHON)" -m pytest tests/ -v
 
 test-contracts: ensure-venv
-	$(RUN_PYTHON) -m pytest tests/contracts/ -v
+	"$(RUN_PYTHON)" -m pytest tests/contracts/ -v
 
 # ---- Utilities ----
 
 preflight: ensure-venv
-	$(RUN_PYTHON) -c "from tools.tool_registry import registry; import json; registry.discover(); print(json.dumps(registry.provider_menu(), indent=2))"
+	"$(RUN_PYTHON)" -c "from tools.tool_registry import registry; import json; registry.discover(); print(json.dumps(registry.provider_menu(), indent=2))"
 
 hyperframes-doctor: ensure-venv
 	@echo "==> Probing HyperFrames runtime (node/ffmpeg/npx + hyperframes doctor)..."
-	$(RUN_PYTHON) -c "from tools.video.hyperframes_compose import HyperFramesCompose; r=HyperFramesCompose().execute({'operation':'doctor'}); import json; print(json.dumps(r.data, indent=2)); print('OK' if r.success else f'FAIL: {r.error}')"
+	"$(RUN_PYTHON)" -c "from tools.video.hyperframes_compose import HyperFramesCompose; r=HyperFramesCompose().execute({'operation':'doctor'}); import json; print(json.dumps(r.data, indent=2)); print('OK' if r.success else f'FAIL: {r.error}')"
 
 hyperframes-warm:
 	@echo "==> Refreshing the HyperFrames npx cache to latest..."
@@ -119,16 +122,16 @@ demo: ensure-venv
 	@echo "==> Rendering zero-key demo videos (no API keys needed)..."
 	@echo "    These use only Remotion components — animated charts, text, data viz."
 	@echo ""
-	$(RUN_PYTHON) render_demo.py
+	"$(RUN_PYTHON)" render_demo.py
 
 demo-list: ensure-venv
-	$(RUN_PYTHON) render_demo.py --list
+	"$(RUN_PYTHON)" render_demo.py --list
 
 lint: ensure-venv
-	$(RUN_PYTHON) -m py_compile tools/base_tool.py
-	$(RUN_PYTHON) -m py_compile tools/tool_registry.py
-	$(RUN_PYTHON) -m py_compile tools/cost_tracker.py
-	$(RUN_PYTHON) -m py_compile tools/analysis/composition_validator.py
+	"$(RUN_PYTHON)" -m py_compile tools/base_tool.py
+	"$(RUN_PYTHON)" -m py_compile tools/tool_registry.py
+	"$(RUN_PYTHON)" -m py_compile tools/cost_tracker.py
+	"$(RUN_PYTHON)" -m py_compile tools/analysis/composition_validator.py
 
 clean:
 	$(BASE_PYTHON) -c "import pathlib, shutil; excluded=[pathlib.Path('$(VENV_DIR)'), pathlib.Path('venv')]; skip=lambda p: any(p == root or root in p.parents for root in excluded); roots=[p for p in pathlib.Path('.').rglob('__pycache__') if not skip(p)]; [shutil.rmtree(p) for p in roots]; files=[p for p in pathlib.Path('.').rglob('*.pyc') if not skip(p)]; [p.unlink() for p in files]"

tests/tools/test_makefile_run_python_quoting.py

@@ -0,0 +1,77 @@
+"""Regression tests for Makefile RUN_PYTHON quoting.
+
+A checkout path may contain spaces (a Windows "Program Files"-style directory,
+an external volume, or any user folder with a space in the name). RUN_PYTHON
+expands to that path, so every command-position use has to be quoted. Unquoted,
+sh word-splits it and ensure-venv fails while blaming the interpreter:
+
+    /bin/sh: /Volumes/Some Drive/repo/.venv/bin/python: not found
+    ERROR: OpenMontage requires Python 3.10+.
+    Current interpreter is unavailable: /Volumes/Some Drive/repo/.venv/bin/python
+
+The interpreter is fine. Every make target fails, and the reported cause is
+wrong, which sends people off to reinstall a Python they do not need.
+"""
+
+import os
+import re
+import shutil
+import stat
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+MAKEFILE = PROJECT_ROOT / "Makefile"
+
+# Command position: the start of a recipe line (optionally after @), after a
+# shell operator, inside a command substitution, or in the PIP recipe variable.
+# Display-only uses inside echo strings do not need quoting.
+COMMAND_POSITION = re.compile(r"(?:^\t@?|\|\| |&& |\$\$\(|^PIP = )\$\(RUN_PYTHON\)", re.M)
+
+
+def test_run_python_is_quoted_in_every_command_position():
+    text = MAKEFILE.read_text()
+    offenders = []
+    for match in COMMAND_POSITION.finditer(text):
+        line_no = text.count("\n", 0, match.start()) + 1
+        offenders.append(f"  Makefile:{line_no}: {text.splitlines()[line_no - 1].strip()[:90]}")
+
+    assert not offenders, (
+        "Unquoted $(RUN_PYTHON) in command position. The interpreter path can "
+        "contain spaces, so these word-split in sh:\n" + "\n".join(offenders)
+    )
+
+
+@pytest.mark.skipif(shutil.which("make") is None, reason="make is not installed")
+def test_ensure_venv_succeeds_when_the_venv_path_contains_spaces(tmp_path):
+    """End-to-end: run a real make target from a path with a space in it."""
+    workdir = tmp_path / "dir with spaces"
+    bindir = workdir / ".venv" / "bin"
+    bindir.mkdir(parents=True)
+    shutil.copy(MAKEFILE, workdir / "Makefile")
+
+    # A wrapper rather than a symlink, so `python -m pip` still resolves
+    # against the real environment instead of this empty directory.
+    fake_python = bindir / "python"
+    fake_python.write_text(f'#!/bin/sh\nexec "{sys.executable}" "$@"\n')
+    fake_python.chmod(fake_python.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
+
+    env = {**os.environ, "VIRTUAL_ENV": str(workdir / ".venv"), "CONDA_PREFIX": ""}
+    proc = subprocess.run(
+        ["make", "ensure-venv"],
+        cwd=workdir,
+        env=env,
+        capture_output=True,
+        text=True,
+        timeout=120,
+    )
+
+    assert proc.returncode == 0, (
+        f"make ensure-venv failed from a path containing spaces.\n"
+        f"stdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"
+    )
+    assert "No such file or directory" not in proc.stderr
+    assert "requires Python" not in proc.stdout