diff --git a/CHANGELOG.md b/CHANGELOG.md index 09c55d10e..1afef15e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Pin `Path.home()` under unit tests via a session-scoped autouse conftest fixture, fixing 56 Windows runner failures on the new `windows-2025-vs2026` GitHub-hosted image where `USERPROFILE`/`HOMEDRIVE`+`HOMEPATH` are not seeded for pytest workers; also patch the `_check_and_notify_updates` import binding in the disabled-self-update test so it no longer races on the version-check cache. (#1270) +- `apm install` now works on macOS git 2.53.0 (Homebrew): bare-cache commands switch to `--git-dir` to satisfy the `safe.bareRepository=explicit` default; fetched SHAs are pinned as synthetic refs so `git clone --local --shared` no longer silently omits them. (#1268) ## [0.13.0] - 2026-05-11 diff --git a/src/apm_cli/deps/bare_cache.py b/src/apm_cli/deps/bare_cache.py index 8c786936b..4cc9d42bd 100644 --- a/src/apm_cli/deps/bare_cache.py +++ b/src/apm_cli/deps/bare_cache.py @@ -25,6 +25,7 @@ import logging import os +import re import subprocess from collections.abc import Callable from pathlib import Path @@ -76,7 +77,7 @@ def _scrub_bare_remote_url(bare_path: Path, git_exe: str, env: dict[str, str]) - """ try: result = subprocess.run( - [git_exe, "-C", str(bare_path), "remote", "set-url", "origin", "redacted://"], + [git_exe, "--git-dir", str(bare_path), "remote", "set-url", "origin", "redacted://"], env=env, check=False, capture_output=True, @@ -197,13 +198,13 @@ def _bare_action(url: str, env: dict[str, str], target: Path) -> None: capture_output=True, ) subprocess.run( - [git_exe, "-C", str(target), "remote", "add", "origin", url], + [git_exe, "--git-dir", str(target), "remote", "add", "origin", url], env=env, check=True, capture_output=True, ) subprocess.run( - [git_exe, "-C", str(target), "fetch", "--depth=1", "origin", ref], + [git_exe, "--git-dir", str(target), "fetch", "--depth=1", "origin", ref], env=env, check=True, capture_output=True, @@ -214,7 +215,7 @@ def _bare_action(url: str, env: dict[str, str], target: Path) -> None: # Without update-ref, consumer's `git rev-parse HEAD` # is ambiguous. See 6.18. subprocess.run( - [git_exe, "-C", str(target), "update-ref", "HEAD", ref], + [git_exe, "--git-dir", str(target), "update-ref", "HEAD", ref], env=env, check=True, capture_output=True, @@ -244,7 +245,7 @@ def _bare_action(url: str, env: dict[str, str], target: Path) -> None: full_sha_result = subprocess.run( [ git_exe, - "-C", + "--git-dir", str(target), "rev-parse", "--verify", @@ -257,7 +258,7 @@ def _bare_action(url: str, env: dict[str, str], target: Path) -> None: ) full_sha = full_sha_result.stdout.strip() subprocess.run( - [git_exe, "-C", str(target), "update-ref", "HEAD", full_sha], + [git_exe, "--git-dir", str(target), "update-ref", "HEAD", full_sha], env=env, check=True, capture_output=True, @@ -351,6 +352,7 @@ def fetch_sha_into_bare( def _rev_parse_present() -> bool: """Return True if sha is already reachable in the bare.""" try: + # no env= needed -- purely local git plumbing, no network access result = subprocess.run( [ git_exe, @@ -367,6 +369,58 @@ def _rev_parse_present() -> bool: except Exception: return False + def _pin_sha_as_head_ref() -> None: + """Add refs/heads/apm-pin- so the SHA is reachable via git-clone. + + ``git clone --local --shared`` from a *shallow* bare ignores + ``--shared`` and falls back to the upload-pack protocol, which + only transfers objects reachable from advertised refs. A SHA + fetched by :func:`fetch_sha_into_bare` is inserted into the + object store but is *not* referenced by any ref, so the clone + silently omits it and subsequent ``git checkout `` fails. + + Creating a synthetic ``refs/heads/apm-pin-*`` ref makes the + commit reachable via the default ``refs/heads/*`` refspec, so + upload-pack includes it. Best-effort: a failure here is logged + at DEBUG level and does not abort the install (the fallback is + a fresh bare clone for the pinned package). + """ + if not re.fullmatch(r"[0-9a-f]{40}", sha): + _log.debug( + "fetch_sha_into_bare: sha %r is not a valid 40-char hex SHA, skipping pin ref", + sha, + ) + return + ref_name = f"refs/heads/apm-pin-{sha[:12]}" + try: + # no env= needed -- purely local git plumbing, no network access + result = subprocess.run( + [git_exe, "--git-dir", str(bare_path), "update-ref", ref_name, sha], + capture_output=True, + timeout=10, + ) + if result.returncode == 0: + _log.debug( + "fetch_sha_into_bare: pinned %s as %s in %s", + sha[:12], + ref_name, + bare_path, + ) + else: + _log.debug( + "fetch_sha_into_bare: update-ref exited %d for %s in %s", + result.returncode, + sha[:12], + bare_path, + ) + except Exception as exc: + _log.debug( + "fetch_sha_into_bare: could not create pin ref for %s in %s: %s", + sha[:12], + bare_path, + exc, + ) + def _scrub_fetch_head() -> None: """Truncate FETCH_HEAD to remove the token-embedded URL written by fetch.""" fetch_head = bare_path / "FETCH_HEAD" @@ -385,6 +439,7 @@ def _scrub_fetch_head() -> None: _log.debug("fetch_sha_into_bare: checking if %s is present in %s", sha[:12], bare_path) if _rev_parse_present(): _log.debug("fetch_sha_into_bare: SHA %s already present, skipping fetch", sha[:12]) + _pin_sha_as_head_ref() return True # Step 2: shallow fetch by full SHA (only for full 40-char SHAs). @@ -395,7 +450,7 @@ def _scrub_fetch_head() -> None: def _fetch_action_sha(url: str, env: dict[str, str], target: Path) -> None: subprocess.run( - [git_exe, "-C", str(bare_path), "fetch", "--depth=1", url, sha], + [git_exe, "--git-dir", str(target), "fetch", "--depth=1", url, sha], env=env, check=True, capture_output=True, @@ -412,6 +467,7 @@ def _fetch_action_sha(url: str, env: dict[str, str], target: Path) -> None: _scrub_fetch_head() if _rev_parse_present(): _log.debug("fetch_sha_into_bare: shallow fetch of %s succeeded", sha[:12]) + _pin_sha_as_head_ref() return True except subprocess.CalledProcessError as exc: stderr_text = "" @@ -437,7 +493,7 @@ def _fetch_action_sha(url: str, env: dict[str, str], target: Path) -> None: def _fetch_action_broad(url: str, env: dict[str, str], target: Path) -> None: subprocess.run( - [git_exe, "-C", str(bare_path), "fetch", f"--depth={broad_depth}", url], + [git_exe, "--git-dir", str(target), "fetch", f"--depth={broad_depth}", url], env=env, check=True, capture_output=True, @@ -454,6 +510,7 @@ def _fetch_action_broad(url: str, env: dict[str, str], target: Path) -> None: _scrub_fetch_head() if _rev_parse_present(): _log.debug("fetch_sha_into_bare: broad fetch succeeded, %s now present", sha[:12]) + _pin_sha_as_head_ref() return True except subprocess.CalledProcessError as exc: stderr_text = "" diff --git a/tests/integration/test_bare_cache_integration.py b/tests/integration/test_bare_cache_integration.py new file mode 100644 index 000000000..10e33f2a4 --- /dev/null +++ b/tests/integration/test_bare_cache_integration.py @@ -0,0 +1,186 @@ +"""Integration tests for bare_cache.fetch_sha_into_bare with real git.""" + +from __future__ import annotations + +import subprocess +from collections.abc import Callable +from pathlib import Path +from typing import Any + +from apm_cli.deps.bare_cache import fetch_sha_into_bare +from apm_cli.models.apm_package import DependencyReference + + +def _git(cwd: Path, *args: str) -> str: + result = subprocess.run( + ["git", *args], + cwd=cwd, + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() + + +def _make_execute(src: Path) -> Callable[..., None]: + """Return an execute_transport_plan that runs the clone_action locally.""" + + def execute_transport_plan( + url: str, + target: Path, + *, + dep_ref: DependencyReference, + clone_action: Callable[..., None], + **kwargs: Any, + ) -> None: + # Use the local src path as the URL so no network access is needed + clone_action(url=str(src), env={}, target=target) + + return execute_transport_plan + + +class TestFetchShaIntoBareIntegration: + """Real-git integration tests for fetch_sha_into_bare.""" + + def test_fetch_pins_ref_in_real_bare_repo(self, tmp_path: Path) -> None: + """fetch_sha_into_bare creates refs/heads/apm-pin- in a real bare.""" + # 1. Create a source repo with 2 commits + src = tmp_path / "source" + src.mkdir() + _git(src, "init", "--initial-branch=main") + _git(src, "config", "user.email", "test@test.com") + _git(src, "config", "user.name", "Test") + (src / "file1.txt").write_text("commit 1\n") + _git(src, "add", ".") + _git(src, "commit", "-m", "commit 1") + first_sha = _git(src, "rev-parse", "HEAD") + + (src / "file2.txt").write_text("commit 2\n") + _git(src, "add", ".") + _git(src, "commit", "-m", "commit 2") + + # 2. Create a shallow bare clone (depth=1, only HEAD). + # Use file:// URL to force smart-HTTP-like transport that actually respects + # --depth (plain local paths use hardlink transport that copies all packs). + bare = tmp_path / "bare" + file_url = src.as_uri() + subprocess.run( + ["git", "clone", "--bare", "--depth=1", file_url, str(bare)], + check=True, + capture_output=True, + ) + + # Verify first_sha is NOT in the bare (depth=1 via file:// properly excludes parents) + verify = subprocess.run( + ["git", "--git-dir", str(bare), "rev-parse", "--verify", f"{first_sha}^{{commit}}"], + capture_output=True, + ) + assert verify.returncode != 0, "first_sha should NOT be in depth=1 bare" + + # 3. Call fetch_sha_into_bare -- execute_transport_plan calls the fetch + # action with the local src path so no actual network access is needed. + dep_ref = DependencyReference.parse("owner/repo/sub#main") + result = fetch_sha_into_bare( + _make_execute(src), + file_url, + bare, + first_sha, + dep_ref=dep_ref, + ) + + # 4. Assert success + assert result is True + + # 5. Assert pin ref exists + refs = subprocess.run( + [ + "git", + "--git-dir", + str(bare), + "for-each-ref", + "--format=%(refname)", + "refs/heads/apm-pin-*", + ], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + expected_ref = f"refs/heads/apm-pin-{first_sha[:12]}" + assert expected_ref in refs, f"Expected {expected_ref} in refs, got: {refs!r}" + + # 6. Assert SHA is now accessible + verify2 = subprocess.run( + ["git", "--git-dir", str(bare), "rev-parse", "--verify", f"{first_sha}^{{commit}}"], + capture_output=True, + ) + assert verify2.returncode == 0, "first_sha should now be accessible after fetch" + + def test_already_present_sha_gets_pinned(self, tmp_path: Path) -> None: + """When SHA is already present (full clone), execute is skipped and pin ref is created.""" + # Create source repo with 1 commit + src = tmp_path / "source" + src.mkdir() + _git(src, "init", "--initial-branch=main") + _git(src, "config", "user.email", "test@test.com") + _git(src, "config", "user.name", "Test") + (src / "file.txt").write_text("content\n") + _git(src, "add", ".") + _git(src, "commit", "-m", "initial") + sha = _git(src, "rev-parse", "HEAD") + + # Full bare clone (SHA is present) + bare = tmp_path / "bare" + subprocess.run( + ["git", "clone", "--bare", str(src), str(bare)], + check=True, + capture_output=True, + ) + + # Verify SHA IS already in the full bare + verify = subprocess.run( + ["git", "--git-dir", str(bare), "rev-parse", "--verify", f"{sha}^{{commit}}"], + capture_output=True, + ) + assert verify.returncode == 0, "SHA should be present in full bare" + + dep_ref = DependencyReference.parse("owner/repo/sub#main") + execute_calls: list[int] = [] + + def counting_execute( + url: str, + target: Path, + *, + dep_ref: DependencyReference, + clone_action: Callable[..., None], + **kwargs: Any, + ) -> None: + execute_calls.append(1) + + result = fetch_sha_into_bare( + counting_execute, + str(src), + bare, + sha, + dep_ref=dep_ref, + ) + + assert result is True + # execute should NOT be called (SHA already present) + assert execute_calls == [], "execute_transport_plan must not be called when SHA is present" + + # Pin ref should exist + refs = subprocess.run( + [ + "git", + "--git-dir", + str(bare), + "for-each-ref", + "--format=%(refname)", + "refs/heads/apm-pin-*", + ], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + expected_ref = f"refs/heads/apm-pin-{sha[:12]}" + assert expected_ref in refs, f"Expected {expected_ref} in refs, got: {refs!r}" diff --git a/tests/unit/deps/test_shared_clone_cache.py b/tests/unit/deps/test_shared_clone_cache.py index eeb9f1b3f..8eabb7c93 100644 --- a/tests/unit/deps/test_shared_clone_cache.py +++ b/tests/unit/deps/test_shared_clone_cache.py @@ -1145,8 +1145,11 @@ def test_sha_already_present_returns_true_without_fetch(self, tmp_path: Path) -> assert result is True # execute_transport_plan should NOT be called mock_execute.assert_not_called() - # Only one rev-parse call (the initial check) - assert mock_run.call_count == 1 + # Two subprocess.run calls: rev-parse (check) + update-ref (pin ref) + assert mock_run.call_count == 2 + pin_call_argv = mock_run.call_args_list[1][0][0] + assert "update-ref" in pin_call_argv + assert f"refs/heads/apm-pin-{sha[:12]}" in pin_call_argv def test_shallow_fetch_full_sha_succeeds(self, tmp_path: Path) -> None: """Full 40-char SHA: shallow fetch via transport plan succeeds.""" @@ -1175,6 +1178,7 @@ def mock_execute( mock_run.side_effect = [ MagicMock(returncode=1), # SHA not present initially MagicMock(returncode=0), # SHA present after fetch + MagicMock(returncode=0), # update-ref pin (apm-pin-) ] result = fetch_sha_into_bare( @@ -1202,6 +1206,10 @@ def mock_execute( assert "--depth=1" in call_args assert sha in call_args assert "https://github.com/owner/repo.git" in call_args + # P8: verify pin ref uses the correct ref name + pin_argv = mock_run.call_args_list[-1][0][0] + assert "update-ref" in pin_argv + assert f"refs/heads/apm-pin-{sha[:12]}" in pin_argv def test_short_sha_skips_shallow_fetch_goes_to_broad(self, tmp_path: Path) -> None: """Short 7-char SHA: skip shallow fetch, go directly to broad fetch.""" @@ -1223,6 +1231,7 @@ def mock_execute( with patch("apm_cli.deps.bare_cache.subprocess.run") as mock_run: # First rev-parse returns 1 (SHA not present) # Second rev-parse (after broad fetch) returns 0 + # NOTE: update-ref is skipped for non-40-char SHAs (hex validation guard) mock_run.side_effect = [ MagicMock(returncode=1), # SHA not present MagicMock(returncode=0), # SHA present after broad fetch @@ -1253,6 +1262,8 @@ def mock_execute( # Should NOT have the SHA in a broad fetch (only the URL) assert sha not in call_args assert "https://github.com/owner/repo.git" in call_args + # Non-40-char SHA: pin ref is intentionally skipped (hex validation guard) + assert mock_run.call_count == 2 def test_all_steps_fail_returns_false(self, tmp_path: Path) -> None: """All fetch attempts fail: return False.""" @@ -1306,6 +1317,7 @@ def mock_execute( mock_run.side_effect = [ MagicMock(returncode=1), # SHA not present MagicMock(returncode=0), # SHA present after fetch + MagicMock(returncode=0), # update-ref pin (apm-pin-) ] fetch_sha_into_bare(