From 1aea3e7c4fef255aef5e34d6d12cc92baa951de7 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Wed, 29 Jul 2026 20:54:58 +0200 Subject: [PATCH] fix: repair dev modules and scoped release git trust --- tests/test_python_environment_sync.py | 70 ++++++ tests/test_release_git_state.py | 39 ++++ tests/test_release_integration.py | 2 +- tools/release/govoplan_release/git_state.py | 18 +- tools/release/govoplan_release/publisher.py | 2 + .../govoplan_release/repository_prepare.py | 11 +- .../govoplan_release/repository_push.py | 11 +- .../govoplan_release/repository_sync.py | 11 +- .../govoplan_release/repository_tag.py | 8 +- .../govoplan_release/source_provenance.py | 6 +- tools/repo/sync-python-environment.py | 208 +++++++++++++++++- 11 files changed, 356 insertions(+), 30 deletions(-) create mode 100644 tests/test_release_git_state.py diff --git a/tests/test_python_environment_sync.py b/tests/test_python_environment_sync.py index d50318d..46dd569 100644 --- a/tests/test_python_environment_sync.py +++ b/tests/test_python_environment_sync.py @@ -77,6 +77,76 @@ class PythonEnvironmentSyncTests(unittest.TestCase): self.assertIn(str(root / "govoplan-core"), command) self.assertIn(str(root / "govoplan-access"), command) + def test_missing_editable_distribution_is_not_hidden_by_current_stamp(self) -> None: + sync = load_sync_module() + with tempfile.TemporaryDirectory(prefix="govoplan-python-sync-") as directory: + root = Path(directory) + project_root = root / "govoplan-probe-missing" + project_root.mkdir() + (project_root / "pyproject.toml").write_text( + "\n".join( + ( + "[project]", + 'name = "govoplan-probe-definitely-not-installed"', + 'version = "0.1.0"', + "", + '[project.entry-points."govoplan.modules"]', + 'probe_missing = "govoplan_probe.backend.manifest:get_manifest"', + "", + ) + ), + encoding="utf-8", + ) + requirements = root / "requirements-dev.txt" + requirements.write_text("-e ./govoplan-probe-missing\n", encoding="utf-8") + entries = sync.local_requirement_entries(requirements) + + validation = sync.validate_local_installations(sys.executable, entries) + plan = sync.build_environment_repair_plan( + python=sys.executable, + stale_requirements=validation.stale_requirements, + ) + + self.assertFalse(validation.current) + self.assertEqual(validation.stale_requirements, entries) + self.assertIn("distribution is not installed", validation.issues[0]) + self.assertEqual(plan.mode, "Selective Python environment repair") + self.assertEqual(plan.commands[0][-2:], ("-e", str(project_root))) + + def test_declared_module_entry_points_are_part_of_environment_validation(self) -> None: + sync = load_sync_module() + with tempfile.TemporaryDirectory(prefix="govoplan-python-sync-") as directory: + root = Path(directory) + project_root = root / "govoplan-probe" + project_root.mkdir() + (project_root / "pyproject.toml").write_text( + "\n".join( + ( + "[project]", + 'name = "govoplan-probe"', + 'version = "0.1.0"', + "", + '[project.entry-points."govoplan.modules"]', + 'probe = "govoplan_probe.backend.manifest:get_manifest"', + "", + ) + ), + encoding="utf-8", + ) + requirements = root / "requirements-dev.txt" + requirements.write_text("-e ./govoplan-probe\n", encoding="utf-8") + + expectations = sync.local_installation_expectations( + sync.local_requirement_entries(requirements) + ) + + self.assertEqual(len(expectations), 1) + self.assertEqual(expectations[0].distribution, "govoplan-probe") + self.assertEqual( + expectations[0].entry_points, + (("govoplan.modules", "probe", "govoplan_probe.backend.manifest:get_manifest"),), + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_release_git_state.py b/tests/test_release_git_state.py new file mode 100644 index 0000000..29371ca --- /dev/null +++ b/tests/test_release_git_state.py @@ -0,0 +1,39 @@ +from __future__ import annotations + +from pathlib import Path +import subprocess +import sys +import unittest +from unittest.mock import patch + + +META_ROOT = Path(__file__).resolve().parents[1] +RELEASE_TOOLS_ROOT = META_ROOT / "tools" / "release" + +if str(RELEASE_TOOLS_ROOT) not in sys.path: + sys.path.insert(0, str(RELEASE_TOOLS_ROOT)) + +from govoplan_release import git_state # noqa: E402 + + +class ReleaseGitStateTests(unittest.TestCase): + def test_git_trusts_only_the_resolved_repository_for_each_command(self) -> None: + repository = Path("/workspace/../workspace/govoplan-core") + completed = subprocess.CompletedProcess([], 0, "", "") + + with patch.object(git_state.subprocess, "run", return_value=completed) as run: + result = git_state.git(repository, "status", "--porcelain") + + self.assertEqual(result.returncode, 0) + command = run.call_args.args[0] + self.assertIn(f"safe.directory={repository.resolve()}", command) + self.assertNotIn("safe.directory=*", command) + self.assertEqual(run.call_args.kwargs["cwd"], repository) + self.assertEqual( + run.call_args.kwargs["env"]["GIT_CONFIG_GLOBAL"], + "/dev/null", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_release_integration.py b/tests/test_release_integration.py index 576a852..62b69a1 100644 --- a/tests/test_release_integration.py +++ b/tests/test_release_integration.py @@ -150,7 +150,7 @@ def test_pytest_style_function(): checks = workflow.index("bash tools/checks/check-release-integration.sh") self.assertLess(install, checks) - self.assertIn("pytest>=8,<9", requirements) + self.assertIn("pytest>=9.0.3,<10", requirements) self.assertNotIn("requirements-release-tests.txt", (meta_root / "requirements-release.txt").read_text(encoding="utf-8")) diff --git a/tools/release/govoplan_release/git_state.py b/tools/release/govoplan_release/git_state.py index 3433a8e..e4fc9d0 100644 --- a/tools/release/govoplan_release/git_state.py +++ b/tools/release/govoplan_release/git_state.py @@ -164,9 +164,10 @@ def git_success(path: Path, *args: str, timeout: int = 10) -> bool: def git(path: Path, *args: str, timeout: int = 10) -> subprocess.CompletedProcess[str]: + command = scoped_git_command(path, *args) try: return subprocess.run( - ["/usr/bin/git", "-c", "core.hooksPath=/dev/null", *args], + command, cwd=path, check=False, text=True, @@ -176,7 +177,20 @@ def git(path: Path, *args: str, timeout: int = 10) -> subprocess.CompletedProces env=sanitized_git_environment(), ) except subprocess.TimeoutExpired as exc: - return subprocess.CompletedProcess(["/usr/bin/git", *args], 124, exc.stdout or "", exc.stderr or "git command timed out") + return subprocess.CompletedProcess(command, 124, exc.stdout or "", exc.stderr or "git command timed out") + + +def scoped_git_command(path: Path, *args: str) -> tuple[str, ...]: + """Trust exactly one resolved catalog checkout for one Git invocation.""" + + return ( + "/usr/bin/git", + "-c", + "core.hooksPath=/dev/null", + "-c", + f"safe.directory={path.resolve()}", + *args, + ) def sanitized_git_environment( diff --git a/tools/release/govoplan_release/publisher.py b/tools/release/govoplan_release/publisher.py index 3b5c34a..5d66512 100644 --- a/tools/release/govoplan_release/publisher.py +++ b/tools/release/govoplan_release/publisher.py @@ -1985,6 +1985,8 @@ def _git_bytes( "-C", str(path), "-c", + f"safe.directory={path.resolve()}", + "-c", "core.sharedRepository=0600", "-c", "core.fsmonitor=false", diff --git a/tools/release/govoplan_release/repository_prepare.py b/tools/release/govoplan_release/repository_prepare.py index 6ef50af..7642a70 100644 --- a/tools/release/govoplan_release/repository_prepare.py +++ b/tools/release/govoplan_release/repository_prepare.py @@ -5,7 +5,7 @@ from __future__ import annotations from pathlib import Path import subprocess -from .git_state import collect_repository_snapshot +from .git_state import collect_repository_snapshot, scoped_git_command from .repository_push import command_text, compact_output from .workspace import load_repository_specs, resolve_repo_path, resolve_workspace_root @@ -131,7 +131,12 @@ def resolve_message(*, repo: str, version: str | None, message: str | None) -> s def run(command: tuple[str, ...], *, cwd: Path) -> subprocess.CompletedProcess[str]: + effective_command = ( + scoped_git_command(cwd, *command[1:]) + if command and Path(command[0]).name == "git" + else command + ) try: - return subprocess.run(command, cwd=cwd, check=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=120) + return subprocess.run(effective_command, cwd=cwd, check=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=120) except subprocess.TimeoutExpired as exc: - return subprocess.CompletedProcess(command, 124, exc.stdout or "", exc.stderr or "git command timed out") + return subprocess.CompletedProcess(effective_command, 124, exc.stdout or "", exc.stderr or "git command timed out") diff --git a/tools/release/govoplan_release/repository_push.py b/tools/release/govoplan_release/repository_push.py index 1faf3d5..ff42774 100644 --- a/tools/release/govoplan_release/repository_push.py +++ b/tools/release/govoplan_release/repository_push.py @@ -6,7 +6,7 @@ from pathlib import Path import shlex import subprocess -from .git_state import collect_repository_snapshot +from .git_state import collect_repository_snapshot, scoped_git_command from .workspace import load_repository_specs, resolve_repo_path, resolve_workspace_root @@ -138,10 +138,15 @@ def command_text(command: tuple[str, ...]) -> str: def run(command: tuple[str, ...], *, cwd: Path) -> subprocess.CompletedProcess[str]: + effective_command = ( + scoped_git_command(cwd, *command[1:]) + if command and Path(command[0]).name == "git" + else command + ) try: - return subprocess.run(command, cwd=cwd, check=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=120) + return subprocess.run(effective_command, cwd=cwd, check=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=120) except subprocess.TimeoutExpired as exc: - return subprocess.CompletedProcess(command, 124, exc.stdout or "", exc.stderr or "git push timed out") + return subprocess.CompletedProcess(effective_command, 124, exc.stdout or "", exc.stderr or "git push timed out") def compact_output(value: str) -> str: diff --git a/tools/release/govoplan_release/repository_sync.py b/tools/release/govoplan_release/repository_sync.py index 125ce22..3886977 100644 --- a/tools/release/govoplan_release/repository_sync.py +++ b/tools/release/govoplan_release/repository_sync.py @@ -5,7 +5,7 @@ from __future__ import annotations from pathlib import Path import subprocess -from .git_state import collect_repository_snapshot +from .git_state import collect_repository_snapshot, scoped_git_command from .repository_push import command_text, compact_output from .workspace import load_repository_specs, resolve_repo_path, resolve_workspace_root @@ -93,7 +93,12 @@ def sync_repositories( def run(command: tuple[str, ...], *, cwd: Path) -> subprocess.CompletedProcess[str]: + effective_command = ( + scoped_git_command(cwd, *command[1:]) + if command and Path(command[0]).name == "git" + else command + ) try: - return subprocess.run(command, cwd=cwd, check=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=120) + return subprocess.run(effective_command, cwd=cwd, check=False, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=120) except subprocess.TimeoutExpired as exc: - return subprocess.CompletedProcess(command, 124, exc.stdout or "", exc.stderr or "git fetch timed out") + return subprocess.CompletedProcess(effective_command, 124, exc.stdout or "", exc.stderr or "git fetch timed out") diff --git a/tools/release/govoplan_release/repository_tag.py b/tools/release/govoplan_release/repository_tag.py index 3379987..785bf8b 100644 --- a/tools/release/govoplan_release/repository_tag.py +++ b/tools/release/govoplan_release/repository_tag.py @@ -13,6 +13,7 @@ from .git_state import ( collect_repository_snapshot, git_text, sanitized_git_environment, + scoped_git_command, ) from .model import RepositorySnapshot from .repository_push import command_text, compact_output @@ -479,12 +480,7 @@ def run( effective_command = command effective_environment = env if command and Path(command[0]).name == "git": - effective_command = ( - "/usr/bin/git", - "-c", - "core.hooksPath=/dev/null", - *command[1:], - ) + effective_command = scoped_git_command(cwd, *command[1:]) effective_environment = sanitized_git_environment(env) try: return subprocess.run( diff --git a/tools/release/govoplan_release/source_provenance.py b/tools/release/govoplan_release/source_provenance.py index e8854cb..536297b 100644 --- a/tools/release/govoplan_release/source_provenance.py +++ b/tools/release/govoplan_release/source_provenance.py @@ -16,6 +16,7 @@ from .git_state import ( collect_repository_snapshot, git_text, sanitized_git_environment, + scoped_git_command, ) from .repository_tag import RemoteTagResult, ref_commit, remote_tag_commit from .version_alignment import repository_version_issues @@ -417,9 +418,10 @@ def _git_file(path: Path, tag: str, name: str) -> str | None: def _git(path: Path, *args: str) -> subprocess.CompletedProcess[str]: + command = scoped_git_command(path, *args) try: return subprocess.run( - ("/usr/bin/git", "-c", "core.hooksPath=/dev/null", *args), + command, cwd=path, check=False, text=True, @@ -429,7 +431,7 @@ def _git(path: Path, *args: str) -> subprocess.CompletedProcess[str]: env=sanitized_git_environment(), ) except subprocess.TimeoutExpired as exc: - return subprocess.CompletedProcess(("/usr/bin/git", *args), 124, exc.stdout or "", exc.stderr or "Git command timed out") + return subprocess.CompletedProcess(command, 124, exc.stdout or "", exc.stderr or "Git command timed out") def _deduplicate(issues: list[SourceTagProvenanceIssue]) -> list[SourceTagProvenanceIssue]: diff --git a/tools/repo/sync-python-environment.py b/tools/repo/sync-python-environment.py index 47e7da5..0da7965 100644 --- a/tools/repo/sync-python-environment.py +++ b/tools/repo/sync-python-environment.py @@ -12,6 +12,7 @@ from pathlib import Path import shlex import subprocess import sys +import tomllib from typing import Any META_ROOT = Path(__file__).resolve().parents[2] @@ -49,6 +50,23 @@ class InstallPlan: warnings: tuple[str, ...] = () +@dataclass(frozen=True, slots=True) +class LocalInstallationExpectation: + requirement: RequirementEntry + distribution: str + entry_points: tuple[tuple[str, str, str], ...] + + +@dataclass(frozen=True, slots=True) +class EnvironmentValidation: + stale_requirements: tuple[RequirementEntry, ...] = () + issues: tuple[str, ...] = () + + @property + def current(self) -> bool: + return not self.stale_requirements + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument( @@ -85,8 +103,9 @@ def main() -> int: fingerprint = build_fingerprint(requirements=requirements, python=python, local_requirements=local_requirements) previous = load_stamp(stamp) current = previous.get("digest") == fingerprint["digest"] + environment = validate_local_installations(python, local_requirements) - if current and not args.force: + if current and environment.current and not args.force: payload = stamp_payload( requirements=requirements, python=python, @@ -107,20 +126,31 @@ def main() -> int: print(f"Python environment is current for {requirements.name}.") return 0 - print(f"Python environment metadata is stale for {requirements.name}.") + if current and not environment.current: + print(f"Python environment metadata is current for {requirements.name}, but installed packages need repair.") + else: + print(f"Python environment metadata is stale for {requirements.name}.") print(f"Tracked inputs: {len(fingerprint['inputs'])}") + for issue in environment.issues: + print(f"warning: {issue}", file=sys.stderr) if args.check: return 1 - plan = build_install_plan( - previous=previous, - fingerprint=fingerprint, - requirements=requirements, - python=python, - local_requirements=local_requirements, - force=args.force, - ) + if current and environment.stale_requirements and not args.force: + plan = build_environment_repair_plan( + python=python, + stale_requirements=environment.stale_requirements, + ) + else: + plan = build_install_plan( + previous=previous, + fingerprint=fingerprint, + requirements=requirements, + python=python, + local_requirements=local_requirements, + force=args.force, + ) print(f"{plan.mode}: {plan.reason}") for warning in plan.warnings: @@ -141,6 +171,13 @@ def main() -> int: for command in commands: subprocess.run(command, check=True) + repaired_environment = validate_local_installations(python, local_requirements) + if not repaired_environment.current: + for issue in repaired_environment.issues: + print(f"error: {issue}", file=sys.stderr) + print("Python environment synchronization did not install all local package metadata.", file=sys.stderr) + return 2 + write_stamp( stamp, stamp_payload( @@ -297,6 +334,127 @@ def build_install_plan( ) +def build_environment_repair_plan( + *, + python: str, + stale_requirements: tuple[RequirementEntry, ...], +) -> InstallPlan: + command = ( + python, + "-m", + "pip", + "install", + *( + argument + for requirement in stale_requirements + for argument in requirement.install_args + ), + ) + return InstallPlan( + "Selective Python environment repair", + f"reinstalling {len(stale_requirements)} missing or stale local distribution(s).", + (command,), + ) + + +def validate_local_installations( + python: str, + local_requirements: tuple[RequirementEntry, ...], +) -> EnvironmentValidation: + expectations = local_installation_expectations(local_requirements) + if not expectations: + return EnvironmentValidation() + + probe_input = [ + { + "key": expectation.requirement.key, + "distribution": expectation.distribution, + "entry_points": [ + {"group": group, "name": name, "value": value} + for group, name, value in expectation.entry_points + ], + } + for expectation in expectations + ] + try: + result = subprocess.run( + (python, "-c", _LOCAL_INSTALLATION_PROBE), + check=False, + text=True, + input=json.dumps(probe_input), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=20, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + return EnvironmentValidation( + stale_requirements=tuple(item.requirement for item in expectations), + issues=(f"could not inspect installed local distributions ({type(exc).__name__})",), + ) + + if result.returncode != 0: + detail = result.stderr.strip() or result.stdout.strip() or "installation probe failed" + return EnvironmentValidation( + stale_requirements=tuple(item.requirement for item in expectations), + issues=(f"could not inspect installed local distributions: {detail}",), + ) + try: + payload = json.loads(result.stdout) + except json.JSONDecodeError: + return EnvironmentValidation( + stale_requirements=tuple(item.requirement for item in expectations), + issues=("installed local distribution probe returned invalid JSON",), + ) + + raw_issues = payload.get("issues") if isinstance(payload, dict) else None + issues_by_key = raw_issues if isinstance(raw_issues, dict) else {} + stale = tuple( + expectation.requirement + for expectation in expectations + if expectation.requirement.key in issues_by_key + ) + issues = tuple( + f"{expectation.distribution}: {issues_by_key[expectation.requirement.key]}" + for expectation in expectations + if expectation.requirement.key in issues_by_key + ) + return EnvironmentValidation(stale_requirements=stale, issues=issues) + + +def local_installation_expectations( + local_requirements: tuple[RequirementEntry, ...], +) -> tuple[LocalInstallationExpectation, ...]: + expectations: list[LocalInstallationExpectation] = [] + for requirement in local_requirements: + if requirement.pyproject is None: + continue + with Path(requirement.pyproject).open("rb") as handle: + payload = tomllib.load(handle) + project = payload.get("project") + if not isinstance(project, dict): + continue + distribution = project.get("name") + if not isinstance(distribution, str) or not distribution.strip(): + continue + declared_entry_points: list[tuple[str, str, str]] = [] + entry_point_groups = project.get("entry-points") + if isinstance(entry_point_groups, dict): + for group, entries in entry_point_groups.items(): + if not isinstance(group, str) or not isinstance(entries, dict): + continue + for name, value in entries.items(): + if isinstance(name, str) and isinstance(value, str): + declared_entry_points.append((group, name, value)) + expectations.append( + LocalInstallationExpectation( + requirement=requirement, + distribution=distribution, + entry_points=tuple(sorted(declared_entry_points)), + ) + ) + return tuple(expectations) + + def requirement_delta_plan(previous_entries: object, current_entries: tuple[RequirementEntry, ...]) -> InstallPlan | None: parsed_previous = previous_requirement_entries(previous_entries) if parsed_previous is None: @@ -481,5 +639,35 @@ def format_command(command: tuple[str, ...]) -> str: return " ".join(shlex.quote(item) for item in command) +_LOCAL_INSTALLATION_PROBE = """ +import importlib.metadata +import json +import sys + +expectations = json.load(sys.stdin) +issues = {} +for expectation in expectations: + key = expectation["key"] + distribution_name = expectation["distribution"] + try: + distribution = importlib.metadata.distribution(distribution_name) + except importlib.metadata.PackageNotFoundError: + issues[key] = "distribution is not installed" + continue + installed = { + (entry_point.group, entry_point.name): entry_point.value + for entry_point in distribution.entry_points + } + mismatches = [] + for entry_point in expectation["entry_points"]: + identity = (entry_point["group"], entry_point["name"]) + if installed.get(identity) != entry_point["value"]: + mismatches.append(f"{identity[0]}:{identity[1]}") + if mismatches: + issues[key] = "entry-point metadata is missing or stale: " + ", ".join(mismatches) +print(json.dumps({"issues": issues}, sort_keys=True)) +""" + + if __name__ == "__main__": raise SystemExit(main())