From 4b46bd326bca6f2220a304c9832365b73748d545 Mon Sep 17 00:00:00 2001 From: Anthony Printup <92564080+anthonyprintup@users.noreply.github.com> Date: Sun, 4 Oct 2026 15:34:00 +0200 Subject: [PATCH] fix(cmake): preserve nested pip-bootstrap diagnostics Run venv through a small wrapper that catches bootstrap subprocess failures before the standard CLI discards their captured output. Preserve the selected interpreter, isolation, platform defaults, timeout, and existing rollback path. Redact URLs before truncation, remove control characters, bound each captured stream to 16,384 characters, and report the original child exit status without printing the child command or environment. Install the helper with the CMake package, document the diagnostic behavior, and cover real nested child failures, missing modules, rollback refusal, credential redaction, and successful environment creation. Extend the Windows Python 3.12-3.14 CI matrix with the new regressions. Validated focused tests on Windows with Python 3.12 and 3.14, normal and relocated-install provisioning, transaction/documentation tests, Ruff, and the private-path guard. Fixes #51 --- .github/workflows/cmake-integration.yml | 4 +- CMakeLists.txt | 1 + cmake/ProtocyteCreateEnvironment.py | 61 +++++++++++ cmake/ProtocyteFunctions.cmake | 16 ++- docs/wiki/Troubleshooting.md | 9 ++ tests/test_cmake.py | 128 ++++++++++++++++++++++++ tests/test_create_environment.py | 128 ++++++++++++++++++++++++ 7 files changed, 343 insertions(+), 4 deletions(-) create mode 100644 cmake/ProtocyteCreateEnvironment.py create mode 100644 tests/test_create_environment.py diff --git a/.github/workflows/cmake-integration.yml b/.github/workflows/cmake-integration.yml index 2bfba5d..86b9309 100644 --- a/.github/workflows/cmake-integration.yml +++ b/.github/workflows/cmake-integration.yml @@ -48,7 +48,9 @@ jobs: - name: Verify managed provisioning, nested reuse, and diagnostics run: > - uv run pytest -q -m cmake_integration + uv run pytest -q -o "addopts=" + tests/test_create_environment.py + tests/test_cmake.py::test_managed_environment_preserves_bootstrap_failure_and_rolls_back tests/test_cmake.py::test_managed_host_tools_support_nested_reuse_and_stage_specific_diagnostics fetchcontent-linux: diff --git a/CMakeLists.txt b/CMakeLists.txt index dc3518e..a5f66b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -178,6 +178,7 @@ if(PROTOCYTE_INSTALL) "${CMAKE_CURRENT_LIST_DIR}/cmake/ProtocyteFunctions.cmake" "${CMAKE_CURRENT_LIST_DIR}/cmake/ProtocyteProcess.cmake" "${CMAKE_CURRENT_LIST_DIR}/cmake/ProtocyteManagedEnvironment.py" + "${CMAKE_CURRENT_LIST_DIR}/cmake/ProtocyteCreateEnvironment.py" "${CMAKE_CURRENT_LIST_DIR}/.github/scripts/owned_transactions.py" "${CMAKE_CURRENT_LIST_DIR}/cmake/ProtocyteOutputSafety.cmake" "${CMAKE_CURRENT_BINARY_DIR}/cmake/protocyteConfig.cmake" diff --git a/cmake/ProtocyteCreateEnvironment.py b/cmake/ProtocyteCreateEnvironment.py new file mode 100644 index 0000000..eaff04a --- /dev/null +++ b/cmake/ProtocyteCreateEnvironment.py @@ -0,0 +1,61 @@ +"""Run venv's CLI entry point without discarding pip-bootstrap diagnostics. + +CMake invokes this with the selected host Python and -I, and owns the timeout +and transaction rollback. Keep venv's platform/version defaults in one place. +""" + +from __future__ import annotations + +import re +import subprocess +import sys + + +_MAX_DIAGNOSTIC_CHARS = 16 * 1024 +_URL = re.compile(r"\b[a-z][a-z0-9+.-]*://\S+", re.IGNORECASE) +_TRUNCATION = "\n[... bootstrap diagnostic truncated ...]\n" + + +def _diagnostic(value: bytes | str | None) -> str: + if value is None: + return "" + text = ( + value.decode("utf-8", errors="replace") if isinstance(value, bytes) else value + ) + text = text.replace("\r\n", "\n").replace("\r", "\n") + # Redact whole URLs, including path/query tokens, before truncating so a + # boundary cannot expose a credential. Never print the child command or env. + text = _URL.sub("", text) + text = "".join(c if c.isprintable() or c in "\n\t" else "?" for c in text) + if len(text) > _MAX_DIAGNOSTIC_CHARS: + head = (_MAX_DIAGNOSTIC_CHARS - len(_TRUNCATION)) // 2 + tail = _MAX_DIAGNOSTIC_CHARS - len(_TRUNCATION) - head + text = text[:head] + _TRUNCATION + text[-tail:] + return text + + +def main() -> int: + try: + import venv + + # venv.__main__ catches this exception and prints only str(error), + # losing the output captured by EnvBuilder._call_new_python. + venv.main() + except subprocess.CalledProcessError as error: + print( + f"Python environment bootstrap subprocess failed (exit code {error.returncode}).", + file=sys.stderr, + ) + print(_diagnostic(error.output), file=sys.stderr) + if error.stderr is not None: + print(_diagnostic(error.stderr), file=sys.stderr) + return error.returncode if error.returncode > 0 else 1 + except Exception as error: + print(_diagnostic(f"{type(error).__name__}: {error}"), file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.stderr.reconfigure(errors="backslashreplace") + sys.exit(main()) diff --git a/cmake/ProtocyteFunctions.cmake b/cmake/ProtocyteFunctions.cmake index c5df376..2622863 100644 --- a/cmake/ProtocyteFunctions.cmake +++ b/cmake/ProtocyteFunctions.cmake @@ -10,6 +10,11 @@ set_property( PROPERTY PROTOCYTE_INTERNAL_MANAGED_ENVIRONMENT_HELPER "${CMAKE_CURRENT_LIST_DIR}/ProtocyteManagedEnvironment.py" ) +set_property( + GLOBAL + PROPERTY PROTOCYTE_INTERNAL_CREATE_ENVIRONMENT_HELPER + "${CMAKE_CURRENT_LIST_DIR}/ProtocyteCreateEnvironment.py" +) function(_protocyte_configure_python_environment_root) if(NOT DEFINED PROTOCYTE_PYTHON_ENV_ROOT OR "${PROTOCYTE_PYTHON_ENV_ROOT}" STREQUAL "") @@ -2561,7 +2566,12 @@ function(_protocyte_ensure_python_environment out_python out_plugin) else() set(protocyte_disabled_pip_config "/dev/null") endif() - set(venv_arguments -m venv "${protocyte_python_staging}") + get_property( + protocyte_create_environment_helper + GLOBAL + PROPERTY PROTOCYTE_INTERNAL_CREATE_ENVIRONMENT_HELPER + ) + set(venv_arguments -I "${protocyte_create_environment_helper}" "${protocyte_python_staging}") execute_process( COMMAND "${CMAKE_COMMAND}" -E env @@ -2580,7 +2590,7 @@ function(_protocyte_ensure_python_environment out_python out_plugin) "--unset=PYTHONHOME" "PIP_ISOLATED=0" "PIP_CONFIG_FILE=${protocyte_disabled_pip_config}" - "${Python3_EXECUTABLE}" -I ${venv_arguments} + "${Python3_EXECUTABLE}" ${venv_arguments} RESULT_VARIABLE venv_result OUTPUT_VARIABLE venv_output ERROR_VARIABLE venv_error @@ -2600,7 +2610,7 @@ function(_protocyte_ensure_python_environment out_python out_plugin) string(APPEND venv_error "\nFailed to restore the previous environment: ${rollback_output}") endif() _protocyte_python_provisioning_error( - "create the virtual environment; ensure the selected Python provides venv and ensurepip" + "create the virtual environment and bootstrap pip" "\"${Python3_EXECUTABLE}\" ${venv_command}" "${venv_result}" "${venv_output}" diff --git a/docs/wiki/Troubleshooting.md b/docs/wiki/Troubleshooting.md index 3272f1e..830d902 100644 --- a/docs/wiki/Troubleshooting.md +++ b/docs/wiki/Troubleshooting.md @@ -22,6 +22,15 @@ An interpreter without `venv` or `ensurepip` cannot provision Protocyte's manage ## Managed Python Provisioning Fails +If virtual-environment creation or pip bootstrapping fails, the CMake error +includes the failing stage, selected interpreter, managed paths, and captured +child-process diagnostics. Bootstrap diagnostics redact URLs and retain up to +16,384 characters per stream, keeping both the beginning and end of longer +output. A subprocess failure alone does not mean `venv` or `ensurepip` is missing; +look for an explicit missing-module error or the underlying child failure. +Protocyte attempts rollback before reporting the error; any rollback failure is +reported alongside the original diagnostic, preserving unverified environments. + Check: - the selected interpreter can create virtual environments; diff --git a/tests/test_cmake.py b/tests/test_cmake.py index 1ad6015..adfd97d 100644 --- a/tests/test_cmake.py +++ b/tests/test_cmake.py @@ -3565,6 +3565,7 @@ def test_fetchcontent_can_explicitly_enable_protocyte_install( assert any(prefix.rglob("ProtocyteGenerate.cmake")) assert any(prefix.rglob("ProtocyteProcess.cmake")) assert any(prefix.rglob("ProtocyteManagedEnvironment.py")) + assert any(prefix.rglob("ProtocyteCreateEnvironment.py")) assert any(prefix.rglob("owned_transactions.py")) assert (prefix / "share/protocyte/python/pyproject.toml").is_file() @@ -13092,6 +13093,133 @@ def test_cmake_fingerprint_inputs_trigger_automatic_reconfiguration( assert updated_fingerprint != initial_fingerprint +@pytest.mark.parametrize("failure", ["nested-child", "missing-ensurepip"]) +@pytest.mark.parametrize("existing_environment", [False, True]) +def test_managed_environment_preserves_bootstrap_failure_and_rolls_back( + tmp_path: Path, failure: str, existing_environment: bool +) -> None: + repo_root = Path(__file__).resolve().parents[1] + helper = repo_root / "cmake/ProtocyteCreateEnvironment.py" + driver = tmp_path / "injected-bootstrap.py" + if failure == "nested-child": + child_code = ( + "import os, sys; " + "print('SYNTHETIC_' + 'BOOTSTRAP_' + 'FAILURE_12345', flush=True); " + "print(os.environ['PIP_INDEX_URL'], flush=True); " + "print('noise' * 5000, flush=True); " + "print('nested stderr detail', file=sys.stderr); sys.exit(17)" + ) + else: + child_code = ( + "import sys, importlib.abc; " + 'exec("class MissingEnsurepip(importlib.abc.MetaPathFinder):\\n' + " def find_spec(self, fullname, *args):\\n" + " if fullname == 'ensurepip':\\n" + ' raise ModuleNotFoundError(\\"No module named \'ensurepip\'\\", name=fullname)"); ' + "sys.meta_path.insert(0, MissingEnsurepip()); import ensurepip" + ) + driver.write_text( + "\n".join( + [ + "import os, runpy, subprocess, sys, venv", + "assert sys.flags.isolated", + "assert not any(name in os.environ for name in " + "('PIP_TARGET', 'PIP_PREFIX', 'PIP_ROOT', 'PIP_USER', 'PYTHONUSERBASE'))", + "class InjectedBuilder(venv.EnvBuilder):", + " def _setup_pip(self, context):", + " self._call_new_python(context, '-I', '-c', " + f"{child_code!r}, stderr=subprocess.STDOUT)", + "venv.EnvBuilder = InjectedBuilder", + f"runpy.run_path({str(helper)!r}, run_name='__main__')", + "", + ] + ), + encoding="utf-8", + ) + environment_root = tmp_path / "managed-environments" + source_dir = tmp_path / "consumer" + build_dir = _write_managed_environment_consumer(source_dir, environment_root) + cmake_file = source_dir / "CMakeLists.txt" + cmake_file.write_text( + cmake_file.read_text(encoding="utf-8").replace( + "protocyte_get_host_tools(", + "\n".join( + [ + "set_property(GLOBAL PROPERTY PROTOCYTE_INTERNAL_CREATE_ENVIRONMENT_HELPER", + f' "{driver.as_posix()}")', + "find_package(Python3 3.12 COMPONENTS Interpreter REQUIRED)", + "_protocyte_python_environment_fingerprint(fingerprint", + ' "${PROTOCYTE_PYTHON_PROJECT_ROOT}" "${PROTOCYTE_PYTHON_CONSTRAINTS}"', + ' "${Python3_EXECUTABLE}" "${Python3_VERSION}")', + 'string(SUBSTRING "${fingerprint}" 0 16 short_fingerprint)', + 'set(previous "${PROTOCYTE_PYTHON_ENV_ROOT}/${short_fingerprint}")', + f"if({'TRUE' if existing_environment else 'FALSE'})", + ' file(MAKE_DIRECTORY "${previous}")', + ' file(WRITE "${previous}/.protocyte-ready" "${fingerprint}\\n")', + ' file(WRITE "${previous}/retained.txt" "previous environment\\n")', + "endif()", + "protocyte_get_host_tools(", + ] + ), + ), + encoding="utf-8", + ) + environment = _managed_environment_process_environment() + environment.update( + PIP_NO_INDEX="1", + PIP_INDEX_URL="https://test-user:private-password@example.invalid/private-path?key=private-query", + PROTOCYTE_TRANSACTION_STATE_DIR=str(tmp_path / "transaction-state"), + PIP_TARGET=str(tmp_path / "forbidden-target"), + PIP_PREFIX=str(tmp_path / "forbidden-prefix"), + PIP_ROOT=str(tmp_path / "forbidden-root"), + PIP_USER="1", + PYTHONUSERBASE=str(tmp_path / "forbidden-user-base"), + ) + result = _configure_managed_environment(source_dir, build_dir, env=environment) + output = result.stdout + result.stderr + assert result.returncode != 0 + assert "create the virtual environment and bootstrap pip" in " ".join( + output.split() + ) + if failure == "nested-child": + assert "Exit code: 17" in output + assert "SYNTHETIC_BOOTSTRAP_FAILURE_12345" in output + assert "nested stderr detail" in output + assert "bootstrap diagnostic truncated" in output + assert "" in output + assert "No module named" not in output + for secret in ( + "private-password", + "private-path", + "private-query", + "example.invalid", + ): + assert secret not in output + else: + assert "Exit code: 1" in output + assert "No module named 'ensurepip'" in output + assert "SYNTHETIC_BOOTSTRAP_FAILURE_12345" not in output + assert len(output) < 24 * 1024 + if existing_environment: + # An unverified destination cannot be overwritten by rollback. Both + # that refusal and the original bootstrap failure must remain visible. + assert "Failed to restore the previous environment" in output + assert "refusing to restore over an unverified" in output + previous = _published_managed_environment(environment_root) + assert (previous / "retained.txt").read_text( + encoding="utf-8" + ) == "previous environment\n" + assert {path.name for path in previous.iterdir()} == { + ".protocyte-ready", + "retained.txt", + } + else: + assert "Failed to restore the previous environment" not in output + assert not any(path.is_dir() for path in environment_root.iterdir()) + _assert_no_managed_environment_transaction_leftovers(environment_root) + assert not list(tmp_path.glob("forbidden-*")) + + def test_cmake_provisioning_error_reports_the_failed_command(tmp_path: Path) -> None: repo_root = Path(__file__).resolve().parents[1] cmake_script = tmp_path / "provisioning_error.cmake" diff --git a/tests/test_create_environment.py b/tests/test_create_environment.py new file mode 100644 index 0000000..22c9ee8 --- /dev/null +++ b/tests/test_create_environment.py @@ -0,0 +1,128 @@ +import builtins +import importlib.util +import os +import subprocess +import sys +import venv +from pathlib import Path + +import pytest + + +_HELPER = Path(__file__).resolve().parents[1] / "cmake/ProtocyteCreateEnvironment.py" +_SPEC = importlib.util.spec_from_file_location("protocyte_create_environment", _HELPER) +assert _SPEC is not None and _SPEC.loader is not None +bootstrap = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(bootstrap) + + +@pytest.mark.parametrize("as_bytes", [False, True]) +def test_bootstrap_diagnostics_are_sanitized_and_bounded( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], as_bytes: bool +) -> None: + output = ( + "first diagnostic\n" + "https://user:password@example.invalid/private-token?key=query-secret\n" + "https://user:quoted'password@example.invalid/index\n" + "https://example.invalid/" + + "token" * 5000 + + "\n" + + "noise\n" * 5000 + + "last diagnostic\n" + ) + error_output = "stderr detail\nHTTPS://other:secret@example.invalid/index\n\x1b\x00" + + def fail() -> None: + raise subprocess.CalledProcessError( + 17, + ["command-must-not-leak"], + output=output.encode() if as_bytes else output, + stderr=error_output.encode() + b"\xff" if as_bytes else error_output, + ) + + monkeypatch.setattr(venv, "main", fail) + assert bootstrap.main() == 17 + captured = capsys.readouterr() + assert captured.out == "" + assert "exit code 17" in captured.err + assert "first diagnostic" in captured.err + assert "last diagnostic" in captured.err + assert "stderr detail" in captured.err + assert "bootstrap diagnostic truncated" in captured.err + assert "" in captured.err + assert len(captured.err) < 17 * 1024 + for private in ( + "password", + "quoted", + "private-token", + "query-secret", + "token", + "other:secret", + "example.invalid", + "command-must-not-leak", + "\x1b", + "\x00", + ): + assert private not in captured.err + + +def test_missing_venv_is_reported_separately( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + original_import = builtins.__import__ + + def without_venv(name, *args, **kwargs): + if name == "venv": + raise ModuleNotFoundError("No module named 'venv'", name="venv") + return original_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", without_venv) + assert bootstrap.main() == 1 + captured = capsys.readouterr() + assert "ModuleNotFoundError: No module named 'venv'" in captured.err + assert "bootstrap subprocess failed" not in captured.err + + +def test_create_environment_preserves_venv_defaults(tmp_path: Path) -> None: + destination = tmp_path / "environment with spaces" + environment = os.environ.copy() + environment.update( + PIP_NO_INDEX="1", + PIP_CONFIG_FILE=os.devnull, + PYTHONHOME=str(tmp_path / "invalid-home"), + PYTHONPATH=str(tmp_path / "invalid-import-path"), + ) + result = subprocess.run( + [sys.executable, "-I", str(_HELPER), str(destination)], + check=False, + capture_output=True, + text=True, + env=environment, + timeout=120, + ) + assert result.returncode == 0, result.stdout + result.stderr + python = destination / ("Scripts/python.exe" if os.name == "nt" else "bin/python") + probe = subprocess.run( + [ + str(python), + "-I", + "-c", + "import pip, sys; print(sys.prefix); print(sys.base_prefix); print(pip.__file__)", + ], + check=True, + capture_output=True, + text=True, + env=environment, + timeout=30, + ) + prefix, base, pip_path = probe.stdout.splitlines() + assert Path(prefix).resolve() == destination.resolve() + assert Path(base).resolve() == Path(sys.base_prefix).resolve() + assert Path(pip_path).is_relative_to(destination) + assert "include-system-site-packages = false" in ( + destination / "pyvenv.cfg" + ).read_text(encoding="utf-8") + assert python.is_symlink() == (os.name != "nt") + assert (python.parent / "activate").is_file() + if sys.version_info >= (3, 13): + assert (destination / ".gitignore").is_file()