diff --git a/.apm/agents/performance-expert.agent.md b/.apm/agents/performance-expert.agent.md new file mode 100644 index 000000000..423f9cf6a --- /dev/null +++ b/.apm/agents/performance-expert.agent.md @@ -0,0 +1,199 @@ +--- +name: performance-expert +description: >- + Performance engineering specialist for package-manager workloads. Activate + when reviewing or designing dependency resolution, lockfile schema, cache + layout, parallel download phases, git transport, partial clones, + filesystem materialization, or any perf regression in install/update/run + paths in the APM CLI. Encodes the modern best practices for high-throughput + multi-source package managers (git protocol, HTTP archive, content stores) + applied to APM's git-first dependency model. +model: claude-opus-4.6 +--- + +# Performance Expert + +You are a performance engineer specializing in package-manager workloads +that fetch dependencies from heterogeneous sources -- git remotes, HTTP +archives, registry APIs, OCI registries -- and materialize them into a +consumer directory. You hold APM's perf invariants and the modern +package-manager performance playbook in head and apply both with +technical rigor. You do NOT hedge; you cite line numbers and quantify +costs in milliseconds, bytes, and round-trips. + +## Mental model + +A package manager's wall-time is the sum of four phases. Optimize the +dominant one; everything else is noise. + +1. **Resolve** -- ref/version -> immutable identifier (SHA, content hash). + Bounded by network RTTs to the registry/forge. Optimal: 1 round-trip + per unique (url, ref) per run; cached forever once a lockfile pins. +2. **Fetch** -- pull bytes from the network into a local content store. + Bounded by bandwidth and protocol overhead. Optimal: download exactly + the bytes the consumer needs, no more, in one TCP stream when possible. +3. **Materialize** -- copy/link/extract content from the store into the + consumer directory. Bounded by filesystem syscalls. Optimal: hardlink + or reflink, never `cp`. +4. **Verify** -- integrity check the consumer dir matches its lockfile + pin. Bounded by hash throughput. Optimal: streaming hash on fetch; + never re-hash on warm-cache hits. + +When a single phase dominates wall-time by >70%, optimizing the others +is procrastination. Identify the dominant phase first, then attack it. + +## The package-manager performance playbook + +The techniques below are the modern best practices for any package +manager that pulls deps from multiple sources. Each one has an APM +analog (or an APM gap). When asked to evaluate a perf change, walk +this list and call out which techniques are applied, missed, or +inapplicable. + +### Resolve phase + +- **In-memory dedup of (url, ref) within a run**: resolve each unique + dep exactly once per CLI invocation. APM's equivalent is + `PerRunRefCache` + `TieredRefResolver` (see + `src/apm_cli/deps/tiered_ref_resolver.py`). Verify any new code + path that hits the network calls `TieredRefResolver.resolve()` not + a raw `git ls-remote` -- the latter bypasses the L0 cache. +- **Tiered ref resolution: API before clone**: the forge's REST API + (e.g. `GET /repos/.../commits/{ref}`) costs one HTTP round-trip and + returns the SHA; a `git ls-remote` costs one round-trip plus pack + protocol handshake. Prefer the API tier when available. APM does + this at L1 (commits API) and L2 (bare rev-parse). The footgun: any + call site that does `subprocess.run(["git", "ls-remote", ...])` + directly is one extra network RTT that should have been an L1 hit. +- **Lockfile is the SHA, end of story**: once the lockfile pins an + immutable identifier, every subsequent operation skips resolution + entirely. APM's `apm.lock.yaml` is the same -- but only if the SHA + is **threaded** through to the cache lookup. If a downstream call + passes the branch name instead of the locked SHA, the cache does + an unnecessary ls-remote. Always pass `locked_sha=...` to + `GitCache.get_checkout`. + +### Fetch phase + +- **Partial clones (`--filter=blob:none`, `--filter=tree:0`)**: ask + the git server for commits and trees only, ~5% of the full repo. + Blobs are fetched lazily via the promisor remote on first access. + For a 1.7 GB monorepo with a small subdir consumer, partial clone + + sparse-cone collapses 1.7 GB to ~50 MB of trees + ~2 MB of + blobs. The single biggest possible win when the server supports + filter v2 (github.com does; older Gerrit/GHE may not). Caveat: + must configure the consumer's promisor remote correctly or + checkout will issue per-file blob fetches. +- **Archive fast-path for forge-hosted repos**: most forges expose + pre-computed tarballs (e.g. `tar.gz/`). One HTTP/2 GET, no + git protocol overhead. Combined with streaming extraction filtered + to a subdir, often beats partial clone on cold runs when only one + SHA is needed. Trade-off: tarballs lose the git object graph, so + you cannot do incremental fetches against them. +- **Connection reuse and pipelining**: reuse the same authenticated + HTTPS session across resolve + fetch when possible. Don't open one + TCP connection for ls-remote and another for clone if a single + HTTP/2 channel can carry both. +- **Concurrent downloads with bounded parallelism**: parallelize + per-dep with a worker pool sized to `min(cpu_count, ~50)`. APM's + install pipeline already does this; verify any new path does not + serialize behind a single-threaded download loop. +- **Content-addressable global cache**: store fetched objects keyed + by their immutable hash, shared across projects. Two projects + depending on the same SHA share storage and skip re-download. + APM's `GitCache` is the analog (keyed by url-shard + SHA + sparse + variant). Verify cache hits skip the network entirely; verify + cache key invariants do not cause unnecessary forks. + +### Materialize phase + +- **Hardlinks by default**: link content from the global cache into + the consumer directory. Near-zero syscall cost vs `copytree`. APM + today does `copytree` from `checkouts_v1////` + into `apm_modules/`. For a 2 MB sparse checkout this is fast + (~50ms); for a 78 MB full checkout this was ~1s. Flag any + materialization that does full-tree copies when the destination + could hardlink. +- **Reflinks on copy-on-write filesystems**: use `clonefile()` on + APFS and `FICLONE` on btrfs/XFS when hardlinks are not viable + (cross-volume installs). Same cost as hardlink but each link is + independently mutable. +- **Sparse working trees**: configure the consumer to materialize + only the directories the dep actually needs. APM uses git + sparse-cone for this. Verify the cache key variant taxonomy + separates full from sparse so the bare object store can still be + shared across all consumers. + +### Verify phase + +- **Hash on fetch, never on warm hit**: compute the content hash as + bytes stream off the network. Warm-cache hits trust the hash + already pinned in the lockfile. APM's `verify_checkout_sha` runs + on every hit (`git_cache.py:126`); this is correct for git but + adds ~5-10ms overhead per dep on warm hits. Acceptable for now; + flag if it shows up in a profile. + +## Diagnostic playbook + +When asked to assess a perf change: + +1. **Quantify the dominant phase before any opinion.** Cite measured + numbers, not guesses. "Cold takes 62s, of which X seconds is + bare clone, Y is ls-remote, Z is materialize" -- with provenance + for each number. +2. **Apply the playbook above.** For each technique, state: applied + / missed / inapplicable, with a one-line reason. +3. **Identify the next-highest-leverage follow-up.** Order by + (impact * confidence) / effort. Be honest about ceilings: for + forge-hosted multi-GB monorepos, the wire-protocol floor without + filter or archive is bounded by network bandwidth; the only way + under that is to switch transports. +4. **Call out tier-bypass footguns.** Any new cache or transport + path that opens its own `git ls-remote` instead of consulting + `TieredRefResolver` is a regression in disguise. +5. **Distinguish noise from signal.** Wall-time deltas with sample + size 1-2 are usually noise. Byte counts and round-trip counts + are deterministic; cite those when wall-time variance is high. + +## Architectural invariants for pervasive impact + +A performance optimization is only valuable if it applies wherever +the hot path runs. For APM specifically: + +- **Centralize at the cache layer, not the command.** `install`, + `update`, `run`, and any future command share `GitCache` and + `bare_cache`. A change at the cache layer benefits all of them + automatically. A change inside a command handler benefits only + that command. Always push perf logic down to the cache. +- **Preserve the bare-shared invariant.** Different consumers with + different subdirs MUST share the same bare clone. Sparse cones + and partial filters are consumer-side; the bare stays + url-keyed, not (url, subdir)-keyed. +- **Variant the consumer cache key honestly.** When the consumer's + on-disk shape depends on a parameter (sparse paths, filter + spec), include that parameter in the cache key. Otherwise two + different requests will collide on the same directory. + +## Hard constraints + +- ASCII only in any output (matches + `.apm/instructions/encoding-rules.instructions.md`). +- Never recommend changes that break the bare-clone-is-shared + invariant. +- Never recommend lockfile schema changes without considering + backward compat with existing `apm.lock.yaml` files in the wild. +- Never recommend disabling integrity verification on warm hits to + shave milliseconds -- correctness over speed. + +## What this agent is NOT + +- Not a code reviewer for style or readability (that's + `python-architect`). +- Not a security reviewer for dependency confusion or supply-chain + attacks (that's `supply-chain-security-expert`). +- Not a CLI UX reviewer (that's `devx-ux-expert` and + `cli-logging-expert`). +- Not a release-decision maker (that's `apm-ceo`). + +Stay in your lane: measurable wall-time, bytes, round-trips, and +follow-up issues that move the needle. diff --git a/.apm/skills/apm-review-panel/SKILL.md b/.apm/skills/apm-review-panel/SKILL.md index c44babfc9..4a82ca224 100644 --- a/.apm/skills/apm-review-panel/SKILL.md +++ b/.apm/skills/apm-review-panel/SKILL.md @@ -4,16 +4,16 @@ description: >- Use this skill to run a multi-persona expert advisory review on a labelled pull request in microsoft/apm. The panel fans out to five mandatory specialists plus a test-coverage specialist (active on every PR that - touches src/) plus two conditional specialists (auth, doc-writer), - all running in their own agent threads, and a CEO + touches src/) plus three conditional specialists (auth, doc-writer, + performance-expert), all running in their own agent threads, and a CEO synthesizer. The orchestrator is the sole writer to the PR: ONE recommendation comment, no verdict labels, no merge gating. The panel is advisory -- it surfaces findings, prioritizes follow-ups, and renders a ship-recommendation that the maintainer and author weigh. Activate when a non-trivial PR needs a cross-cutting recommendation (architecture, CLI logging, DevX UX, supply-chain security, - growth/positioning, optionally auth, docs, and test coverage, with CEO - arbitration). + growth/positioning, optionally auth, docs, perf, and test coverage, + with CEO arbitration). --- # APM Review Panel - Fan-Out Advisory Review @@ -71,6 +71,7 @@ surfaces findings; the maintainer and the PR author decide ship. | [Auth Expert](../../agents/auth-expert.agent.md) | Auth / Token Reviewer | Conditional (see below) | | [Doc Writer](../../agents/doc-writer.agent.md) | Documentation Reviewer | Conditional (see below) | | [Test Coverage Expert](../../agents/test-coverage-expert.agent.md) | Test-Presence Reviewer (paired with DevX UX) | Yes (skipped only on docs-only PRs -- see below) | +| [Performance Expert](../../agents/performance-expert.agent.md) | Package-Manager Performance Reviewer | Conditional (see below) | | [APM CEO](../../agents/apm-ceo.agent.md) | Strategic Arbiter / Synthesizer | Yes | ## Topology @@ -113,10 +114,10 @@ surfaces findings; the maintainer and the PR author decide ship. ## Conditional panelists -Two personas are conditional (auth, doc-writer). A third -(test-coverage) is mandatory on every PR that touches `src/` and only -skipped on documentation-only PRs -- see its section below for why. -The orchestrator ALWAYS spawns ALL three tasks to keep the schema +Three personas are conditional (auth, doc-writer, performance-expert). A +fourth (test-coverage) is mandatory on every PR that touches `src/` and +only skipped on documentation-only PRs -- see its section below for why. +The orchestrator ALWAYS spawns ALL four tasks to keep the schema return shape uniform; the prompt instructs the subagent to set `active: false` with an `inactive_reason` if the condition does not hold. @@ -168,6 +169,35 @@ Starlight content). When the doc-writer is active because of code changes that SHOULD have updated docs but did not, the persona surfaces that gap as a finding. +### Performance Expert + +Activate when the PR changes any of: +- `src/apm_cli/cache/**` +- `src/apm_cli/deps/**` +- `src/apm_cli/install/phases/**` +- `src/apm_cli/install/pipeline.py` +- `src/apm_cli/install/resolve.py` +- `scripts/perf/**` +- `src/apm_cli/core/command_logger.py` (when the diff adds perf-instrumentation logs) + +Also activate when the PR description claims a performance win +(speedup ratio, latency reduction, bytes-on-disk reduction, throughput +improvement) or attaches a perf-harness measurement table. + +Fallback self-check (when no fast-path file matched): "Does this PR +change the hot path for dependency download, materialization, cache +layout, transport (git protocol, partial clone, sparse checkout), +parallelism, or any user-visible install/update wall-time? If unsure, +answer YES." + +When active, the performance-expert reviews against the package-manager +performance playbook: transport minimization (depth, filter, sparse +scope), cache layering and dedup keys, parallelism and lock contention, +working-tree materialization cost, perf-harness methodology (cache +wipe, warm/cold separation, statistical noise), and pervasive +application of the chosen technique across install / update / run +surfaces (not just the one path the PR exercises). + ### Test Coverage Expert **Active by default on every PR that touches `src/**/*.py`.** The only @@ -249,6 +279,7 @@ output to the PR before step 6. - `auth-expert` (always - active per step 2) - `doc-writer` (always - active per step 2) - `test-coverage-expert` (always - active per step 2) + - `performance-expert` (always - active per step 2) Each task prompt MUST: - Reference its persona file by relative path so the subagent loads diff --git a/.apm/skills/apm-review-panel/assets/panelist-return-schema.json b/.apm/skills/apm-review-panel/assets/panelist-return-schema.json index cffb3cece..0bf67df88 100644 --- a/.apm/skills/apm-review-panel/assets/panelist-return-schema.json +++ b/.apm/skills/apm-review-panel/assets/panelist-return-schema.json @@ -18,12 +18,13 @@ "oss-growth-hacker", "auth-expert", "doc-writer", - "test-coverage-expert" + "test-coverage-expert", + "performance-expert" ] }, "active": { "type": "boolean", - "description": "Set to false ONLY for conditional personas (auth-expert, doc-writer, test-coverage-expert) when their fast-path file triggers and fallback self-check both miss. All mandatory personas MUST set active=true. When false, findings MUST be empty and inactive_reason MUST be a one-sentence explanation citing the touched files." + "description": "Set to false ONLY for conditional personas (auth-expert, doc-writer, test-coverage-expert, performance-expert) when their fast-path file triggers and fallback self-check both miss. All mandatory personas MUST set active=true. When false, findings MUST be empty and inactive_reason MUST be a one-sentence explanation citing the touched files." }, "inactive_reason": { "type": "string", diff --git a/CHANGELOG.md b/CHANGELOG.md index 85a3cb95f..b50c6cc4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Performance + +- Cold `apm install` for subdirectory git dependencies is dramatically faster on large monorepos (validated ~30x to ~75x range on `dotnet/skills`, network-variant). `GitCache` now performs partial bare clones (`--filter=blob:none`) with promisor remotes plus sparse-cone consumer materialization, and threads the tiered resolver's resolved SHA through to skip a redundant `ls-remote`. Bare-cache disk usage drops by orders of magnitude on the validated workload. No lockfile schema, CLI surface, or auth flow changes. Servers that reject `--filter=blob:none` (older Gerrit / pre-2.20 GHE) transparently fall back to a full bare clone. (#1436, closes #1433) + ### Added - **Experimental:** `copilot-app` target now scopes workflow rows to a real `projects` row instead of orphaning them at the App's root. When the App is running, project registration goes through the loopback WebSocket IPC surface (`~/.copilot/run/ws.{port,token}`, 0o600) so the project goes through the App's own owner/repo discovery and is immediately known to the webview; when the App is closed, registration falls through to a direct-SQLite `BEGIN IMMEDIATE` resolver against `~/.copilot/data.db`. Workflow rows are always written via SQLite (namespaced ids preserve lockfile stability). `--global` installs that carry workflow-shape prompts now emit a one-time warn-and-proceed diagnostic explaining the CWD-pivot risk and the per-row "attach to project" remediation. A one-time `Restart the Copilot App once` info hint fires on first project registration in a repo (see github/github-app#5483). (#1431) diff --git a/docs/src/content/docs/reference/cli/cache.md b/docs/src/content/docs/reference/cli/cache.md index 82b377dc8..d571e160c 100644 --- a/docs/src/content/docs/reference/cli/cache.md +++ b/docs/src/content/docs/reference/cli/cache.md @@ -115,10 +115,25 @@ Inside the cache root: / git/ db_v1/ # bare repository databases - checkouts_v1/ # per-SHA worktree checkouts + # / -- full bare clone (default) + # __p/ -- partial bare clone + # (--filter=blob:none) used + # for sparse-checkout consumers + checkouts_v1/ # per-SHA worktree checkouts, variant-keyed + # //full/ -- full tree + # //sparse-/ -- sparse cone + # ( = first + # 16 hex of + # sha256(paths)) http_v1/ # conditional-GET response cache ``` +The `full/` and `sparse-/` subdirs let two consumers of the +same commit share storage when they want the same subdirs, and keep +distinct shards when they do not -- without the variant suffix the +sparse checkout would clobber the full tree for any other consumer +of that SHA. + The cache root is created with mode `0700` and validated to be absolute with no NUL bytes before use. diff --git a/scripts/perf/measure_dotnet_skills.py b/scripts/perf/measure_dotnet_skills.py new file mode 100644 index 000000000..726f6ea8b --- /dev/null +++ b/scripts/perf/measure_dotnet_skills.py @@ -0,0 +1,235 @@ +"""Perf harness for #1433 sparse-cone consumer materialization. + +Measures cold + warm cache `apm install` against a deep-monorepo +subdir dep (default: dotnet/skills/plugins/dotnet-diag/skills/ +analyzing-dotnet-performance#main). + +Usage: + uv run python scripts/perf/measure_dotnet_skills.py + +Prints a table comparing: + - wall_time (seconds) + - bare cache bytes (db_v1//) + - checkouts_v1 shard bytes (...////) + - delivered apm_modules dep bytes + +ASCII-only output (matches encoding-rules.instructions.md). +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +import tempfile +import time +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(ROOT / "src")) + +from apm_cli.cache.paths import ( # noqa: E402 + get_cache_root, + get_git_checkouts_path, + get_git_db_path, +) + +DEFAULT_DEP = ( + "github.com/dotnet/skills/plugins/dotnet-diag/" + "skills/analyzing-dotnet-performance#main" +) +APM_YML_TEMPLATE = """\ +name: perf-harness +version: 0.0.0 +targets: + - copilot +dependencies: + apm: + - {dep} +""" + + +def _dir_size_bytes(path: Path | None) -> int: + if path is None or not path.exists(): + return 0 + total = 0 + for root, _dirs, files in os.walk(path): + for f in files: + try: + total += (Path(root) / f).stat().st_size + except OSError: + pass + return total + + +def _fmt_bytes(n: int) -> str: + if n == 0: + return "0 B" + units = ["B", "KB", "MB", "GB"] + v = float(n) + i = 0 + while v >= 1024 and i < len(units) - 1: + v /= 1024 + i += 1 + return f"{v:.1f} {units[i]}" + + +def _wipe_cache_root() -> None: + root = get_cache_root() + if not root.exists(): + return + + def _chmod_writable(_func, path, _exc): + try: + os.chmod(path, 0o700) + except OSError: + pass + + # First pass: re-grant write to any dirs the cache chmod'd to 0o700 + # (or chmod 0o500 on parent dirs after some failure mode). + for r, dirs, files in os.walk(root): + for d in dirs: + try: + os.chmod(Path(r) / d, 0o700) + except OSError: + pass + for f in files: + try: + os.chmod(Path(r) / f, 0o600) + except OSError: + pass + shutil.rmtree(root, onerror=_chmod_writable) + if root.exists(): + shutil.rmtree(root, ignore_errors=True) + + +def _largest_subdir(parent: Path | None) -> Path | None: + if parent is None or not parent.exists(): + return None + children = [c for c in parent.iterdir() if c.is_dir()] + if not children: + return None + return max(children, key=lambda c: _dir_size_bytes(c)) + + +def _run_install(project_dir: Path) -> float: + env = os.environ.copy() + # Always use `uv run apm` from this repo's root so we exercise the + # current worktree's apm, NOT a stale system-installed apm. + cmd = ["uv", "run", "--project", str(ROOT), "apm", "install", "--verbose"] + + start = time.monotonic() + result = subprocess.run( + cmd, + cwd=str(project_dir), + env=env, + capture_output=True, + text=True, + timeout=600, + ) + elapsed = time.monotonic() - start + + if result.returncode != 0: + sys.stderr.write("[x] apm install failed:\n") + sys.stderr.write(result.stdout) + sys.stderr.write(result.stderr) + raise SystemExit(2) + sys.stdout.write(result.stdout) + sys.stdout.write(result.stderr) + return elapsed + + +def _measure(project_dir: Path) -> dict: + elapsed = _run_install(project_dir) + cache_root = get_cache_root() + db_root = get_git_db_path(cache_root) + co_root = get_git_checkouts_path(cache_root) + + db_shard = _largest_subdir(db_root) + co_shard = _largest_subdir(co_root) + co_sha = _largest_subdir(co_shard) + co_variant = _largest_subdir(co_sha) + + # On baseline layout, / IS the checkout (no variant subdir). + # On the new layout, // holds the working tree. + # Report the deepest non-empty directory whose total size is the + # actual on-disk consumer materialization cost. + sha_total = _dir_size_bytes(co_sha) + variant_total = _dir_size_bytes(co_variant) + + return { + "wall_time": elapsed, + "bare_bytes": _dir_size_bytes(db_shard), + "checkout_sha_bytes": sha_total, + "checkout_variant_bytes": variant_total, + "checkouts_variant": co_variant.name if co_variant else "(none)", + "apm_modules_bytes": _dir_size_bytes(project_dir / "apm_modules"), + } + + +def main() -> int: + dep = os.environ.get("PERF_DEP", DEFAULT_DEP) + with tempfile.TemporaryDirectory(prefix="apm-perf-") as td: + project = Path(td) / "project" + project.mkdir() + (project / "apm.yml").write_text( + APM_YML_TEMPLATE.format(dep=dep), encoding="utf-8" + ) + + print(f"[i] Harness project: {project}") + print(f"[i] Dep: {dep}") + print(f"[i] Cache root: {get_cache_root()}") + print() + + print("[>] Cold run (wiping caches)...") + _wipe_cache_root() + cold = _measure(project) + + shutil.rmtree(project / "apm_modules", ignore_errors=True) + lock = project / "apm.lock.yaml" + if lock.exists(): + lock.unlink() + + print() + print("[>] Warm run (caches retained)...") + warm = _measure(project) + + print() + print("=" * 64) + print(f"{'metric':<28} {'cold':>15} {'warm':>15}") + print("-" * 64) + print( + f"{'wall_time (s)':<28} " + f"{cold['wall_time']:>15.2f} {warm['wall_time']:>15.2f}" + ) + print( + f"{'bare cache':<28} " + f"{_fmt_bytes(cold['bare_bytes']):>15} " + f"{_fmt_bytes(warm['bare_bytes']):>15}" + ) + print( + f"{'checkouts sha-dir total':<28} " + f"{_fmt_bytes(cold['checkout_sha_bytes']):>15} " + f"{_fmt_bytes(warm['checkout_sha_bytes']):>15}" + ) + print( + f"{' variant subdir (if any)':<28} " + f"{_fmt_bytes(cold['checkout_variant_bytes']):>15} " + f"{_fmt_bytes(warm['checkout_variant_bytes']):>15}" + ) + print(f" variant name (cold): {cold['checkouts_variant']}") + print(f" variant name (warm): {warm['checkouts_variant']}") + print( + f"{'apm_modules delivered':<28} " + f"{_fmt_bytes(cold['apm_modules_bytes']):>15} " + f"{_fmt_bytes(warm['apm_modules_bytes']):>15}" + ) + print("=" * 64) + speedup = cold["wall_time"] / max(warm["wall_time"], 0.001) + print(f"[i] cold/warm speedup: {speedup:.2f}x") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/apm_cli/cache/git_cache.py b/src/apm_cli/cache/git_cache.py index 3a46547cf..334e9626b 100644 --- a/src/apm_cli/cache/git_cache.py +++ b/src/apm_cli/cache/git_cache.py @@ -25,12 +25,15 @@ from __future__ import annotations import contextlib +import hashlib +import json import logging import os import re import subprocess from pathlib import Path +from ..utils.git_sparse import apply_sparse_cone from ..utils.path_security import ensure_path_within from .integrity import verify_checkout_sha from .locking import atomic_land, cleanup_incomplete, shard_lock, stage_path @@ -42,6 +45,38 @@ # Full SHA pattern: 40 hex characters _SHA_RE = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE) +# Partial bare-cache flavor suffix (perf #1433 follow-up). +# When a caller requests sparse_paths, we use a separate bare keyed at +# ``__p`` cloned with ``--filter=blob:none``. The partial bare +# downloads commits + trees only (~5% of repo size) and acts as a +# promisor remote; blobs are lazy-fetched at consumer checkout time +# scoped to the sparse cone. Full and partial bares coexist per URL +# so legacy full-tree callers keep today's behavior unchanged. +_PARTIAL_BARE_SUFFIX = "__p" + + +def _variant_key(sparse_paths: list[str] | None) -> str: + """Return the on-disk variant segment for a checkout shard. + + Layout (perf #1433): + - ``full`` -- full-tree checkout (sparse_paths is None / empty). + - ``sparse-`` -- sparse-cone checkout where ```` is + the first 16 hex chars of sha256(json.dumps(sorted(paths))). + Two consumers requesting the same set of paths share a shard; + different sets get separate shards. We do NOT promote a full + checkout to also satisfy a sparse subset -- that complicates + eviction for negligible benefit (each sparse shard is ~subdir + size, so duplication cost is small). + """ + if not sparse_paths: + return "full" + # Deduplicate AND sort so callers passing [a,a] or [a,b]+[b,a] + # all collapse to the same variant key (the "set of paths" + # semantics the docstring promises). + payload = json.dumps(sorted(set(sparse_paths)), separators=(",", ":")) + digest = hashlib.sha256(payload.encode("utf-8")).hexdigest()[:16] + return f"sparse-{digest}" + class GitCache: """Content-addressable git cache with integrity verification. @@ -74,6 +109,7 @@ def get_checkout( *, locked_sha: str | None = None, env: dict[str, str] | None = None, + sparse_paths: list[str] | None = None, ) -> Path: """Return path to a cached checkout for the given repo+ref. @@ -83,6 +119,10 @@ def get_checkout( locked_sha: If provided (from lockfile), skip resolution and use this SHA directly. env: Environment dict for git subprocesses. + sparse_paths: If non-empty, materialize only these top-level + directories using ``git sparse-checkout --cone``. The + shard is keyed by ``(sha, sparse_paths_variant)`` so + full and sparse variants of the same SHA coexist. Returns: Path to the checkout directory (guaranteed to contain valid @@ -90,26 +130,39 @@ def get_checkout( """ shard_key = cache_shard_key(url) sha = self._resolve_sha(url, ref, locked_sha=locked_sha, env=env) + variant = _variant_key(sparse_paths) - checkout_dir = self._checkouts_root / shard_key / sha + checkout_dir = self._checkouts_root / shard_key / sha / variant # Cache hit path (skip if refresh requested) if not self._refresh and checkout_dir.is_dir(): if verify_checkout_sha(checkout_dir, sha): - _log.debug("Cache HIT: %s @ %s", url, sha[:12]) + _log.debug("Cache HIT: %s @ %s [%s]", url, sha[:12], variant) return checkout_dir else: # Integrity failure -- evict _log.warning( - "[!] Evicting corrupt cache entry: %s @ %s", + "[!] Evicting corrupt cache entry: %s @ %s [%s]", _sanitize_url(url), sha[:12], + variant, ) self._evict_checkout(checkout_dir) - # Cache miss: ensure we have the bare repo, then create checkout - self._ensure_bare_repo(url, shard_key, sha, env=env) - return self._create_checkout(url, shard_key, sha, env=env) + # Cache miss: ensure we have the bare repo, then create checkout. + # Sparse callers use a partial bare (blob:none) + promisor consumer + # so only the trees + the blobs reachable from the sparse cone are + # downloaded. Full-tree callers keep the legacy non-partial bare. + use_partial = bool(sparse_paths) + self._ensure_bare_repo(url, shard_key, sha, env=env, partial=use_partial) + return self._create_checkout( + url, + shard_key, + sha, + env=env, + sparse_paths=sparse_paths, + promisor_url=url if use_partial else None, + ) def _resolve_sha( self, @@ -216,14 +269,28 @@ def _ensure_bare_repo( sha: str, *, env: dict[str, str] | None = None, + partial: bool = False, ) -> Path: """Ensure a bare repo clone exists for the given shard, fetching if needed. + Args: + partial: If True, clone with ``--filter=blob:none`` into a + separate ``__p`` directory so the bare downloads + commits + trees only (~5% of full repo size) and acts + as a promisor remote for consumer lazy-fetch. Falls + back to a full clone in the same directory if the + server rejects the filter (older Gerrit / pre-2.20 + GHE). Falling back leaves the partial-flavor dir with + full content; future sparse consumers will simply not + trigger any lazy fetch (all blobs already present), so + behavior degrades to today's baseline. + Returns the path to the bare repo directory. """ from ..utils.git_env import get_git_executable, git_subprocess_env - bare_dir = self._db_root / shard_key + bare_shard = shard_key + (_PARTIAL_BARE_SUFFIX if partial else "") + bare_dir = self._db_root / bare_shard # Containment guard: defends against pathological shard_key # values bypassing the cache root. ensure_path_within(bare_dir, self._db_root) @@ -251,14 +318,25 @@ def _ensure_bare_repo( os.chmod(str(staged), 0o700) subprocess_env = env if env is not None else git_subprocess_env() + clone_args = [git_exe, "clone", "--bare", "--no-tags"] + if partial: + # Promisor partial clone: trees + commits only. Blobs + # arrive lazily via the remote when the consumer needs + # them. Github / modern GHES / ADO support this; older + # servers reject it and we retry without --filter. + # --no-tags above skips fetching tag objects (release + # tags can sum to MBs on monorepos); the cache is + # SHA-keyed and never resolves via tags. + clone_args += ["--filter=blob:none"] + clone_args += [url, str(staged)] try: - # Full bare clone (no --filter): we extract file contents at - # checkout time, so all blobs must be present locally. A - # partial clone would leave the working tree empty after - # `git clone --local --shared` + `git checkout`, because the - # alternates pointer would resolve trees but not blobs. + # Full bare clone (or partial when requested above). The + # full path extracts file contents at checkout time, so + # all blobs must be present locally. The partial path + # relies on the consumer being configured as a promisor + # so missing blobs trigger an on-demand fetch. subprocess.run( - [git_exe, "clone", "--bare", url, str(staged)], + clone_args, capture_output=True, text=True, timeout=300, @@ -266,11 +344,54 @@ def _ensure_bare_repo( check=True, ) except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as exc: - # Clean up staged on failure - from ..utils.file_ops import robust_rmtree - - robust_rmtree(staged, ignore_errors=True) - raise RuntimeError(f"Failed to clone {_sanitize_url(url)}: {exc}") from exc + # Partial clone fallback: some servers reject --filter + # (old Gerrit / pre-2.20 GHE). Retry once without it so + # we never block on this optimization. The resulting + # bare is full; future sparse consumers find all blobs + # locally and skip lazy fetch (degrades to baseline, + # no behavior change for the user). + fallback_done = False + if partial and isinstance(exc, subprocess.CalledProcessError): + from ..utils.console import _rich_warning + + _rich_warning( + f"Partial clone (--filter=blob:none) failed for " + f"{_sanitize_url(url)}; retrying with full bare clone. " + f"Server may not support filter v2." + ) + from ..utils.file_ops import robust_rmtree + + robust_rmtree(staged, ignore_errors=True) + staged.mkdir(parents=True, exist_ok=True) + os.chmod(str(staged), 0o700) + try: + subprocess.run( + [git_exe, "clone", "--bare", "--no-tags", url, str(staged)], + capture_output=True, + text=True, + timeout=300, + env=subprocess_env, + check=True, + ) + fallback_done = True + except ( + subprocess.CalledProcessError, + subprocess.TimeoutExpired, + OSError, + ) as exc2: + from ..utils.file_ops import robust_rmtree + + robust_rmtree(staged, ignore_errors=True) + raise RuntimeError( + f"Failed to clone {_sanitize_url(url)} " + f"(partial fallback also failed): {exc2}" + ) from exc2 + if not fallback_done: + # Clean up staged on failure + from ..utils.file_ops import robust_rmtree + + robust_rmtree(staged, ignore_errors=True) + raise RuntimeError(f"Failed to clone {_sanitize_url(url)}: {exc}") from exc # Atomic land (lock is already held; pass it through so the # rename completes under the same critical section). @@ -290,12 +411,32 @@ def _create_checkout( sha: str, *, env: dict[str, str] | None = None, + sparse_paths: list[str] | None = None, + promisor_url: str | None = None, ) -> Path: """Create a checkout at the specified SHA from the bare repo. Uses ``git clone --local --shared`` from the bare repo for efficiency (no network, hardlinks objects). + Sparse-cone (perf #1433): + When ``sparse_paths`` is non-empty, ``git sparse-checkout + init --cone`` + ``set `` runs BEFORE the SHA + checkout, so the working tree contains only the requested + top-level directories. The shard lives at + ``checkouts_v1///sparse-/`` so it + coexists with a possible full-tree shard at + ``...//full/`` for the same SHA. + + Partial-clone promisor (perf #1433 follow-up): + When ``promisor_url`` is set, the bare lives at + ``__p`` (cloned with ``--filter=blob:none``) and + we configure the consumer's ``remote.origin`` to point + at the real upstream URL with ``promisor=true`` and + ``partialclonefilter=blob:none``. Sparse checkout then + lazy-fetches only the blobs reachable from the cone + (typically <2 MB instead of the full repo's blob set). + Concurrency / write-deduplication --------------------------------- Acquires the shard lock BEFORE staging any work. On lock entry @@ -309,15 +450,19 @@ def _create_checkout( """ from ..utils.git_env import get_git_executable, git_subprocess_env - bare_dir = self._db_root / shard_key - checkout_parent = self._checkouts_root / shard_key - # Containment guards: the shard_key + sha components are - # derived from sha256 / hex but defend at the boundary anyway. - ensure_path_within(checkout_parent, self._checkouts_root) - checkout_parent.mkdir(parents=True, exist_ok=True) - os.chmod(str(checkout_parent), 0o700) - - final_dir = checkout_parent / sha + bare_shard = shard_key + (_PARTIAL_BARE_SUFFIX if promisor_url else "") + bare_dir = self._db_root / bare_shard + variant = _variant_key(sparse_paths) + # New layout: ///. The level is the + # SHA dir (parent to the variant). The level is what + # the lock + atomic_land target so different variants of the + # same SHA do not race each other. + sha_parent = self._checkouts_root / shard_key / sha + ensure_path_within(sha_parent, self._checkouts_root) + sha_parent.mkdir(parents=True, exist_ok=True) + os.chmod(str(sha_parent), 0o700) + + final_dir = sha_parent / variant ensure_path_within(final_dir, self._checkouts_root) lock = shard_lock(final_dir) @@ -331,7 +476,12 @@ def _create_checkout( # rule out a poisoned half-write (atomic_land guards # against that, but we re-check defensively). if final_dir.is_dir() and verify_checkout_sha(final_dir, sha): - _log.debug("Write-dedup HIT under lock: %s @ %s", url, sha[:12]) + _log.debug( + "Write-dedup HIT under lock: %s @ %s [%s]", + url, + sha[:12], + variant, + ) return final_dir staged = stage_path(final_dir) @@ -360,6 +510,34 @@ def _create_checkout( env=subprocess_env, check=True, ) + if promisor_url: + # Configure consumer as a promisor pointing at the + # real upstream URL so missing blobs (the partial + # bare only carries trees) are lazy-fetched during + # checkout. Without this, ``git checkout`` would + # fail with "fatal: unable to read tree/blob" for + # any object missing from the local alternates. + # The fetch goes to ``promisor_url`` directly; auth + # comes from the inherited subprocess_env. + for cfg_args in ( + ["remote.origin.url", promisor_url], + ["remote.origin.promisor", "true"], + ["remote.origin.partialclonefilter", "blob:none"], + ): + subprocess.run( + [git_exe, "-C", str(staged), "config", *cfg_args], + capture_output=True, + text=True, + timeout=10, + env=subprocess_env, + check=True, + ) + if sparse_paths: + # Sparse-cone setup BEFORE checkout. Failures raise + # (not silently fallen back to full checkout) because + # a silent fallback would re-introduce the disk + # bloat this code path exists to avoid (#1433). + apply_sparse_cone(git_exe, staged, list(sparse_paths), env=subprocess_env) # Checkout the specific SHA subprocess.run( [git_exe, "-C", str(staged), "checkout", sha], @@ -437,9 +615,17 @@ def _fetch_into_bare_locked( git_exe = get_git_executable() subprocess_env = env if env is not None else git_subprocess_env() + # If this is a partial-flavor bare, preserve the filter on fetch + # so we don't pull all blobs reachable from the new SHA. Detected + # via shard-suffix naming convention (cheap, no git config probe). + is_partial = bare_dir.name.endswith(_PARTIAL_BARE_SUFFIX) + fetch_args = [git_exe, "-C", str(bare_dir), "fetch"] + if is_partial: + fetch_args += ["--filter=blob:none"] + fetch_args += [url, sha] try: subprocess.run( - [git_exe, "-C", str(bare_dir), "fetch", url, sha], + fetch_args, capture_output=True, text=True, timeout=120, diff --git a/src/apm_cli/core/command_logger.py b/src/apm_cli/core/command_logger.py index 55f3826e5..12f67fe83 100644 --- a/src/apm_cli/core/command_logger.py +++ b/src/apm_cli/core/command_logger.py @@ -379,6 +379,81 @@ def package_type_info(self, type_label: str): return _rich_echo(f" Package type: {type_label}", color="dim") + # --- Performance diagnostics (perf #1433) --- + + def subdir_download_start( + self, + dep_name: str, + cache_state: str, + sha_short: str = "", + sparse_paths: list[str] | None = None, + ): + """Log the start of a subdirectory dep download (verbose only). + + Names the dep, the bare-cache state (e.g. ``cold`` / ``warm`` / + ``persistent`` / ``shared-bare``), the resolved SHA (short), + and the sparse paths being requested. Surfaces enough state to + diagnose a perf regression from one log line. + """ + if not self.verbose: + return + sha_part = f" @{sha_short}" if sha_short else "" + paths_part = f" sparse={','.join(sparse_paths)}" if sparse_paths else " sparse=" + _rich_echo( + f" [i] perf: subdir {dep_name}{sha_part} cache={cache_state}{paths_part}", + color="dim", + ) + + def bare_clone_strategy(self, strategy: str, elapsed_ms: int): + """Log the bare-clone strategy and wall time (verbose only). + + ``strategy`` is the human-readable command shape, e.g. + ``--depth=1 --branch main`` or ``init+fetch --depth=1 ``. + ``elapsed_ms`` lets readers spot a network-bound regression + without re-running with a profiler. + """ + if not self.verbose: + return + _rich_echo( + f" [i] perf: bare clone strategy={strategy} took={elapsed_ms}ms", + color="dim", + ) + + def materialize_result(self, sparse_applied: bool, consumer_size_bytes: int): + """Log materialization outcome and consumer dir size (verbose only). + + ``sparse_applied`` tells the reader whether sparse-cone fired + on this consumer dir (sparse_paths were passed and accepted by + git). ``consumer_size_bytes`` is the on-disk size of the + working tree handed off to the integrator; a regression here + is the leading indicator that sparse-cone silently fell back. + """ + if not self.verbose: + return + size_mb = consumer_size_bytes / (1024 * 1024) + applied = "yes" if sparse_applied else "no" + _rich_echo( + f" [i] perf: materialize sparse={applied} size={size_mb:.2f} MB", + color="dim", + ) + + def tier_summary(self, stats: dict[str, int]): + """Log the tiered ref resolver hit counts (verbose only). + + Emitted at the end of the resolve phase so the reader can see + how many ref->SHA lookups hit each tier (L0 per-run cache, + L1 commits API, L2 bare rev-parse, L3 legacy clone) without + wiring a debugger. A run dominated by L3 is the canonical + signal that ref-resolution is paying full clone cost. + """ + if not self.verbose or not stats: + return + non_zero = {k: v for k, v in stats.items() if v} + if not non_zero: + return + parts = " ".join(f"{k}={v}" for k, v in non_zero.items()) + _rich_echo(f" [i] perf: ref-resolver tiers: {parts}", color="dim") + # --- Cleanup phase (stale and orphan file removal) --- def stale_cleanup(self, dep_key: str, count: int): diff --git a/src/apm_cli/deps/bare_cache.py b/src/apm_cli/deps/bare_cache.py index 4cc9d42bd..f13347ec0 100644 --- a/src/apm_cli/deps/bare_cache.py +++ b/src/apm_cli/deps/bare_cache.py @@ -33,6 +33,8 @@ from git import Repo +from ..utils.git_sparse import apply_sparse_cone + if TYPE_CHECKING: from ..models.apm_package import DependencyReference @@ -543,6 +545,7 @@ def materialize_from_bare( ref: str | None, env: dict[str, str], known_sha: str | None = None, + sparse_paths: list[str] | None = None, ) -> str: """Create a working-tree checkout from a bare repo via local-shared clone. @@ -567,6 +570,16 @@ def materialize_from_bare( disables LFS smudge cross-platform (the empty string trick works everywhere; ``cat`` is not on Windows PATH). + Sparse-cone (perf #1433): + - When ``sparse_paths`` is set, ``git sparse-checkout init --cone`` + followed by ``git sparse-checkout set `` runs BEFORE + ``git checkout``, so only the requested top-level directories + are materialized in the working tree. The bare's object DB is + unchanged; only the consumer working tree shrinks. + - Sparse-checkout failures are RAISED (not silently fallen back) + because a silent fallback would re-introduce the 78 MB bloat + this parameter exists to avoid. + Returns: The resolved commit SHA. Caller threads this into ``resolved_commit`` for the lockfile. @@ -634,6 +647,8 @@ def materialize_from_bare( check=False, ) checkout_target = known_sha or "HEAD" + if sparse_paths: + apply_sparse_cone(git_exe, consumer_dir, list(sparse_paths), env=env) subprocess.run( [git_exe, "-C", str(consumer_dir), "checkout", checkout_target], capture_output=True, diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py index 865ea541c..f586b2227 100644 --- a/src/apm_cli/deps/github_downloader.py +++ b/src/apm_cli/deps/github_downloader.py @@ -8,7 +8,7 @@ import sys import tempfile import threading -import time # noqa: F401 +import time from collections.abc import Callable from datetime import datetime from pathlib import Path @@ -93,6 +93,31 @@ def _rmtree(path) -> None: robust_rmtree(path, ignore_errors=True) +def _dir_size_bytes(path: Path) -> int: + """Return the total on-disk size of a directory tree in bytes. + + Best-effort: silently skips files that disappear or cannot be + stat-ed mid-walk (e.g. transient .git lock files). Uses ``lstat`` + so symlinks contribute the size of the link itself, never the + target -- this keeps the measurement bounded to the directory + tree and matches :func:`apm_cli.cache.git_cache._dir_size`. Used + only for verbose-mode perf diagnostics (#1433) -- never gates + behavior. + """ + total = 0 + try: + for dirpath, _dirnames, filenames in os.walk(str(path)): + for fname in filenames: + fpath = os.path.join(dirpath, fname) + try: + total += os.lstat(fpath).st_size + except OSError: + continue + except OSError: + return 0 + return total + + class GitProgressReporter(RemoteProgress): """Report git clone progress to Rich Progress.""" @@ -226,6 +251,14 @@ def __init__( # than a monkey-patched field. self._tiered_resolver = None + # Perf #1433: optional InstallLogger attached by the install + # pipeline. When set, the subdir download path emits structured + # verbose-only [perf] lines (subdir_download_start / + # bare_clone_strategy / materialize_result). None means the + # downloader is being driven outside the install pipeline (e.g. + # tests, marketplace) -- the [perf] channel stays silent. + self.install_logger = None + def _git_env_dict(self) -> dict[str, str]: """Return a sanitized git env dict for cache-layer subprocess calls. @@ -585,9 +618,17 @@ def _materialize_from_bare( ref: str | None, env: dict[str, str], known_sha: str | None = None, + sparse_paths: list[str] | None = None, ) -> str: """Thin delegate to :func:`bare_cache.materialize_from_bare` (kept on the class so test patches still work).""" - return materialize_from_bare(bare_path, consumer_dir, ref=ref, env=env, known_sha=known_sha) + return materialize_from_bare( + bare_path, + consumer_dir, + ref=ref, + env=env, + known_sha=known_sha, + sparse_paths=sparse_paths, + ) def _fetch_sha_into_bare( self, @@ -1060,6 +1101,8 @@ def download_subdirectory_package( # Use user-specified ref, or None to use repo's default branch ref = dep_ref.reference # None if not specified subdir_path = dep_ref.virtual_path + _perf_logger = getattr(self, "install_logger", None) + _dep_display = str(dep_ref) # Update progress - starting if progress_obj and progress_task_id is not None: @@ -1079,11 +1122,32 @@ def download_subdirectory_package( # Build a canonical URL for cache key derivation. _persistent_cache = self.persistent_git_cache _persistent_checkout: Path | None = None + _resolved_sha_for_cache: str | None = None if _persistent_cache is not None: _canonical_url = f"https://{cache_host}/{cache_owner}/{cache_repo}" try: + # Tiered ref resolution (perf #1433 follow-up): resolve + # the ref through the attached TieredRefResolver BEFORE + # calling get_checkout so the cache skips its internal + # ls-remote. Same pattern as the non-subdir path at + # line ~1604 which passes locked_sha=resolved. + try: + _resolved = self.resolve_git_reference(dep_ref) + _resolved_sha_for_cache = _resolved.resolved_commit + except Exception: + _resolved_sha_for_cache = None + # Sparse-cone (#1433): keying the persistent shard by + # (sha, subdir) ensures the cached working tree is the + # subdir only (<2 MB) instead of the full repo + # (~78 MB for dotnet/skills). Different subdirs of the + # same SHA land in separate variant shards; bare cache + # is unchanged so they still share object data. _persistent_checkout = _persistent_cache.get_checkout( - _canonical_url, ref, env=self._git_env_dict() + _canonical_url, + _resolved_sha_for_cache or ref, + locked_sha=_resolved_sha_for_cache, + env=self._git_env_dict(), + sparse_paths=[subdir_path], ) except Exception: # Cache miss or failure -- fall through to normal clone path. @@ -1106,6 +1170,20 @@ def download_subdirectory_package( if _persistent_checkout is not None: # WS3: persistent cache hit -- use the cached checkout directly. temp_clone_path = _persistent_checkout + if _perf_logger is not None: + _sha_short = ( + (ref or "")[:12] if ref and re.match(r"^[a-f0-9]{7,40}$", ref) else "" + ) + _perf_logger.subdir_download_start( + _dep_display, + cache_state="persistent-hit", + sha_short=_sha_short, + sparse_paths=[subdir_path], + ) + _perf_logger.materialize_result( + sparse_applied=True, + consumer_size_bytes=_dir_size_bytes(_persistent_checkout), + ) elif use_shared: # WS2 (#1126): shared cache holds BARE clones keyed by # (host, owner, repo, ref). Each consumer materializes its @@ -1114,6 +1192,7 @@ def download_subdirectory_package( # subdirectories of the same repo+ref can share one bare # without racing on sparse-checkout. See design.md sec 5.5. is_commit_sha = ref and re.match(r"^[a-f0-9]{7,40}$", ref) is not None + _perf_t0_bare = time.monotonic() def _shared_bare_clone_fn(bare_target: Path) -> None: self._bare_clone_with_fallback( @@ -1143,6 +1222,20 @@ def _shared_bare_fetch_fn(existing_bare: Path, ref_or_sha: str) -> bool: ) except Exception as e: raise RuntimeError(f"Failed to clone repository: {e}") from e + _perf_bare_elapsed_ms = int((time.monotonic() - _perf_t0_bare) * 1000) + if _perf_logger is not None: + _strategy = ( + f"init+fetch --depth=1 origin {ref[:12]}" + if is_commit_sha + else f"--depth=1 --branch {ref or ''}" + ) + _perf_logger.subdir_download_start( + _dep_display, + cache_state="shared-bare", + sha_short=ref[:12] if is_commit_sha and ref else "", + sparse_paths=[subdir_path], + ) + _perf_logger.bare_clone_strategy(_strategy, _perf_bare_elapsed_ms) # Per-consumer materialization. mkdtemp gives a unique # path so concurrent consumers do not collide. The bare @@ -1166,11 +1259,25 @@ def _shared_bare_fetch_fn(existing_bare: Path, ref_or_sha: str) -> bool: # _bare_action, so rev-parse HEAD returns 40 chars. # Copilot review finding (#1135). known_sha=ref if (is_commit_sha and len(ref) == 40) else None, + # Sparse-cone (#1433): materialize ONLY the + # subdirectory we need. Cuts the consumer + # working tree from full-repo to subdir-size + # on a typical monorepo (78 MB -> <2 MB for + # dotnet/skills). Bare cache is unchanged + # (subdir-agnostic) so multiple consumers + # requesting different subdirs of the same + # repo+SHA still share the object DB. + sparse_paths=[subdir_path], ) except Exception as e: raise RuntimeError( f"Failed to prepare dependency from cached clone: {e}" ) from e + if _perf_logger is not None: + _perf_logger.materialize_result( + sparse_applied=True, + consumer_size_bytes=_dir_size_bytes(temp_clone_path), + ) else: # Legacy per-dep clone path (no shared cache). temp_dir = tempfile.mkdtemp(dir=get_apm_temp_dir()) diff --git a/src/apm_cli/install/phases/resolve.py b/src/apm_cli/install/phases/resolve.py index 29dfb36d6..969f2ce40 100644 --- a/src/apm_cli/install/phases/resolve.py +++ b/src/apm_cli/install/phases/resolve.py @@ -125,6 +125,13 @@ def run(ctx: InstallContext) -> None: except (OSError, ValueError): pass # Cache unavailable (permissions, missing dir) -- degrade gracefully + # Perf #1433: attach the InstallLogger so the subdir download path + # can emit verbose-only [perf] lines (subdir cache state, bare + # clone strategy + elapsed, materialize sparse-applied + size). + # Optional; tests / non-install drivers leave this None. + if ctx.logger is not None: + downloader.install_logger = ctx.logger + # #1369: tiered ref resolver. Collapses N redundant shallow clones # for ref->SHA resolution into a per-run cache + cheap commits API # + bare-rev-parse waterfall, falling back to the legacy clone path. @@ -512,3 +519,13 @@ def _collect_descendants(node, visited=None): # deps have extracted their subpaths. Safe to call even if no # subdir deps were processed (no-op in that case). shared_cache.cleanup() + + # Perf #1433: emit ref-resolver tier hit counts at the end of the + # resolve phase. Verbose only; one line; lets reviewers see which + # waterfall tier carried the run without attaching a debugger. + if ctx.logger is not None and ctx.ref_resolver is not None: + _tier_stats = getattr(ctx.ref_resolver, "stats", None) + if _tier_stats: + # tier_summary is install-only; other loggers degrade silently. + if hasattr(ctx.logger, "tier_summary"): + ctx.logger.tier_summary(_tier_stats) diff --git a/src/apm_cli/utils/git_sparse.py b/src/apm_cli/utils/git_sparse.py new file mode 100644 index 000000000..120bb413f --- /dev/null +++ b/src/apm_cli/utils/git_sparse.py @@ -0,0 +1,56 @@ +"""Shared helper for sparse-checkout cone setup (perf #1433). + +Extracted so the persistent git cache (``cache.git_cache``) and the +shared-bare materialization path (``deps.bare_cache``) configure +sparse-cone with identical subprocess semantics. Single place to evolve +sparse-checkout behavior (timeouts, additional flags, future +``--no-sparse-index``) without drift between the two call sites. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + + +def apply_sparse_cone( + git_exe: str, + repo_dir: Path, + paths: list[str], + *, + env: dict[str, str] | None, + timeout: int = 30, +) -> None: + """Initialize cone-mode sparse checkout and set the requested paths. + + Issues ``git sparse-checkout init --cone`` followed by + ``git sparse-checkout set `` inside ``repo_dir``. Both + subprocesses run with ``check=True``; failures propagate to the + caller so silent fallback to a full checkout (which would defeat + the perf invariant from #1433) is impossible. + + Args: + git_exe: Absolute path to the git executable. + repo_dir: Repository working tree to configure. + paths: Top-level cone paths to materialize. Must be non-empty. + env: Subprocess environment (auth / safe.bareRepository etc.). + timeout: Per-subprocess timeout in seconds. + """ + if not paths: + return + subprocess.run( + [git_exe, "-C", str(repo_dir), "sparse-checkout", "init", "--cone"], + capture_output=True, + text=True, + timeout=timeout, + env=env, + check=True, + ) + subprocess.run( + [git_exe, "-C", str(repo_dir), "sparse-checkout", "set", *paths], + capture_output=True, + text=True, + timeout=timeout, + env=env, + check=True, + ) diff --git a/tests/integration/test_github_downloader_phase3w4.py b/tests/integration/test_github_downloader_phase3w4.py index 95eab2d1e..74a4be210 100644 --- a/tests/integration/test_github_downloader_phase3w4.py +++ b/tests/integration/test_github_downloader_phase3w4.py @@ -403,6 +403,17 @@ def test_persistent_cache_hit_used(self, tmp_path): # persistent_cache.get_checkout was called persistent_cache.get_checkout.assert_called() + # Regression: the subdir-aware sparse_paths and locked_sha (when + # available) MUST propagate to the persistent cache so the + # variant-keyed shard ((sha, sparse_paths)) is actually hit and + # the cold-path bloat fix from #1433 is not silently bypassed. + call_kwargs = persistent_cache.get_checkout.call_args.kwargs + assert "sparse_paths" in call_kwargs + assert call_kwargs["sparse_paths"] == ["packages/my-pkg"] + # locked_sha is plumbed via the kwarg; presence (not value) is the + # contract this test defends. + assert "locked_sha" in call_kwargs + def test_persistent_cache_miss_falls_through(self, tmp_path): """Lines 1088-1090: cache.get_checkout raises -> falls through to clone.""" dep_ref = self._make_dep_ref() diff --git a/tests/unit/cache/test_git_cache.py b/tests/unit/cache/test_git_cache.py index 509d6ca0d..18f7b917a 100644 --- a/tests/unit/cache/test_git_cache.py +++ b/tests/unit/cache/test_git_cache.py @@ -58,7 +58,7 @@ def test_cache_hit_with_integrity_pass(self, mock_run: MagicMock, tmp_path: Path url = "https://github.com/owner/repo" real_shard = cache_shard_key(url) - checkout_dir = tmp_path / "git" / "checkouts_v1" / real_shard / sha + checkout_dir = tmp_path / "git" / "checkouts_v1" / real_shard / sha / "full" checkout_dir.mkdir(parents=True) (checkout_dir / ".git").mkdir() @@ -82,7 +82,7 @@ def test_cache_hit_integrity_failure_evicts(self, mock_run: MagicMock, tmp_path: from apm_cli.cache.url_normalize import cache_shard_key real_shard = cache_shard_key(url) - checkout_dir = tmp_path / "git" / "checkouts_v1" / real_shard / sha + checkout_dir = tmp_path / "git" / "checkouts_v1" / real_shard / sha / "full" checkout_dir.mkdir(parents=True) # First call: rev-parse returns wrong SHA (integrity failure) @@ -282,7 +282,7 @@ def test_short_circuits_when_final_exists_under_lock(self, tmp_path: Path) -> No # Simulate "another process already landed this shard": create # the final_dir BEFORE _create_checkout runs. - final_dir = tmp_path / "git" / "checkouts_v1" / shard / sha + final_dir = tmp_path / "git" / "checkouts_v1" / shard / sha / "full" final_dir.mkdir(parents=True) (final_dir / ".git").mkdir() @@ -343,7 +343,7 @@ def test_short_circuits_on_integrity_pass_only(self, tmp_path: Path) -> None: shard = cache_shard_key(url) # Populate final_dir BUT integrity will report failure. - final_dir = tmp_path / "git" / "checkouts_v1" / shard / sha + final_dir = tmp_path / "git" / "checkouts_v1" / shard / sha / "full" final_dir.mkdir(parents=True) (tmp_path / "git" / "db_v1" / shard).mkdir(parents=True) diff --git a/tests/unit/cache/test_git_cache_sparse.py b/tests/unit/cache/test_git_cache_sparse.py new file mode 100644 index 000000000..74dfd8076 --- /dev/null +++ b/tests/unit/cache/test_git_cache_sparse.py @@ -0,0 +1,274 @@ +"""Tests for sparse-cone checkout support in GitCache (perf #1433).""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from apm_cli.cache.git_cache import GitCache, _variant_key + + +@pytest.fixture(autouse=True) +def _allow_bare_repos(monkeypatch): + """Override safe.bareRepository so `git -C ` works in test env.""" + monkeypatch.setenv("GIT_CONFIG_COUNT", "1") + monkeypatch.setenv("GIT_CONFIG_KEY_0", "safe.bareRepository") + monkeypatch.setenv("GIT_CONFIG_VALUE_0", "all") + + +class TestVariantKey: + """Variant key derivation must be deterministic and order-independent.""" + + def test_empty_or_none_is_full(self): + assert _variant_key(None) == "full" + assert _variant_key([]) == "full" + + def test_sparse_paths_produce_sparse_prefix(self): + v = _variant_key(["plugins/x"]) + assert v.startswith("sparse-") + # 16 hex chars after the prefix + assert len(v) == len("sparse-") + 16 + + def test_order_independent(self): + assert _variant_key(["a", "b"]) == _variant_key(["b", "a"]) + + def test_distinct_sets_distinct_keys(self): + assert _variant_key(["a"]) != _variant_key(["b"]) + assert _variant_key(["a", "b"]) != _variant_key(["a"]) + + def test_deterministic_across_calls(self): + v1 = _variant_key(["plugins/x", "tools/y"]) + v2 = _variant_key(["tools/y", "plugins/x"]) + assert v1 == v2 + + +def _build_local_bare_repo(tmp_path: Path) -> tuple[Path, str]: + """Create a local git repo with multiple top-level subdirs and a bare clone. + + Returns (bare_path, head_sha). + """ + work = tmp_path / "work" + work.mkdir() + subprocess.run(["git", "init", "-q", "-b", "main", str(work)], check=True) + subprocess.run(["git", "-C", str(work), "config", "user.email", "t@e"], check=True) + subprocess.run(["git", "-C", str(work), "config", "user.name", "t"], check=True) + + for sub in ("alpha", "beta", "gamma"): + d = work / sub + d.mkdir() + (d / "file.txt").write_text(f"{sub}\n", encoding="utf-8") + subprocess.run(["git", "-C", str(work), "add", "."], check=True) + subprocess.run(["git", "-C", str(work), "commit", "-q", "-m", "init"], check=True) + sha = subprocess.run( + ["git", "-C", str(work), "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + bare = tmp_path / "bare.git" + subprocess.run(["git", "clone", "-q", "--bare", str(work), str(bare)], check=True) + return bare, sha + + +class TestGetCheckoutLayout: + """get_checkout must land checkouts at ///.""" + + def test_full_variant_layout(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + result = cache.get_checkout(url, "main", locked_sha=sha) + assert result.name == "full" + assert result.parent.name == sha + assert (result / "alpha" / "file.txt").is_file() + assert (result / "beta" / "file.txt").is_file() + assert (result / "gamma" / "file.txt").is_file() + + def test_sparse_variant_layout_only_requested_subdir(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + result = cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + assert result.name.startswith("sparse-") + assert result.parent.name == sha + assert (result / "alpha" / "file.txt").is_file() + # Sparse-cone excludes other top-level dirs: + assert not (result / "beta").exists() + assert not (result / "gamma").exists() + + def test_full_and_sparse_coexist(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + full = cache.get_checkout(url, "main", locked_sha=sha) + sparse = cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + # Both live under same SHA parent, different variant subdirs. + assert full.parent == sparse.parent + assert full != sparse + assert full.is_dir() + assert sparse.is_dir() + + def test_two_distinct_sparse_sets_separate_shards(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + a = cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + b = cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["beta"]) + assert a != b + assert a.parent == b.parent + assert (a / "alpha").is_dir() + assert not (a / "beta").exists() + assert (b / "beta").is_dir() + assert not (b / "alpha").exists() + + +class TestPartialBareFlavor: + """Partial-clone (perf #1433 follow-up): sparse callers should + use the ``__p`` bare flavor and the consumer should be configured + as a promisor.""" + + def test_sparse_caller_uses_partial_bare_dir(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + + # The partial-flavor bare lives at __p. + bare_root = cache_root / "git" / "db_v1" + partial_bares = [p for p in bare_root.iterdir() if p.is_dir() and p.name.endswith("__p")] + assert len(partial_bares) == 1 + full_bares = [p for p in bare_root.iterdir() if p.is_dir() and not p.name.endswith("__p")] + assert len(full_bares) == 0 + + def test_full_caller_uses_non_partial_bare_dir(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + cache.get_checkout(url, "main", locked_sha=sha) + + bare_root = cache_root / "git" / "db_v1" + partial_bares = [p for p in bare_root.iterdir() if p.is_dir() and p.name.endswith("__p")] + assert len(partial_bares) == 0 + full_bares = [p for p in bare_root.iterdir() if p.is_dir() and not p.name.endswith("__p")] + assert len(full_bares) == 1 + + def test_full_and_sparse_callers_coexist_as_separate_bare_flavors(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + cache.get_checkout(url, "main", locked_sha=sha) + cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + + bare_root = cache_root / "git" / "db_v1" + names = sorted(p.name for p in bare_root.iterdir() if p.is_dir()) + assert len(names) == 2 + assert sum(1 for n in names if n.endswith("__p")) == 1 + assert sum(1 for n in names if not n.endswith("__p")) == 1 + + def test_promisor_config_set_on_sparse_consumer(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + result = cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + + # Consumer's remote.origin.url must point at the promisor URL, + # not the local bare path, so lazy blob fetch can reach upstream. + cfg = subprocess.run( + ["git", "-C", str(result), "config", "remote.origin.url"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + assert cfg == url + promisor = subprocess.run( + ["git", "-C", str(result), "config", "remote.origin.promisor"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + assert promisor == "true" + pfilter = subprocess.run( + ["git", "-C", str(result), "config", "remote.origin.partialclonefilter"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + assert pfilter == "blob:none" + + def test_full_consumer_has_no_promisor_config(self, tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + url = f"file://{bare}" + result = cache.get_checkout(url, "main", locked_sha=sha) + + # Full path: no promisor config; remote.origin.url points at + # local bare (default `clone --local` behavior). + rc = subprocess.run( + ["git", "-C", str(result), "config", "remote.origin.promisor"], + capture_output=True, + text=True, + ) + assert rc.returncode != 0 # config key not set + + def test_partial_clone_fallback_to_full_on_server_rejection(self, tmp_path: Path, monkeypatch): + """Server rejects --filter=blob:none -> retry without filter succeeds. + + Older Gerrit / pre-2.20 GHE do not support filter v2. The cache + must transparently degrade to a full bare clone (baseline + behavior) rather than fail the install. + """ + bare, sha = _build_local_bare_repo(tmp_path) + cache_root = tmp_path / "cache" + cache = GitCache(cache_root) + + import apm_cli.cache.git_cache as git_cache_mod + + real_run = subprocess.run + rejected: list[list[str]] = [] + retried: list[list[str]] = [] + + def fake_run(cmd, *args, **kwargs): + if isinstance(cmd, list) and "--filter=blob:none" in cmd: + rejected.append(list(cmd)) + raise subprocess.CalledProcessError( + 128, cmd, output=b"", stderr=b"fatal: server does not support filter" + ) + if ( + isinstance(cmd, list) + and "clone" in cmd + and "--bare" in cmd + and "--filter=blob:none" not in cmd + ): + retried.append(list(cmd)) + return real_run(cmd, *args, **kwargs) + + monkeypatch.setattr(git_cache_mod.subprocess, "run", fake_run) + + url = f"file://{bare}" + result = cache.get_checkout(url, "main", locked_sha=sha, sparse_paths=["alpha"]) + + assert rejected, "partial clone (with --filter) should have been attempted" + assert retried, "fallback retry (without --filter) should have been issued" + assert all("--filter=blob:none" not in c for c in retried) + assert (result / "alpha" / "file.txt").is_file() diff --git a/tests/unit/cache/test_proxy_compat.py b/tests/unit/cache/test_proxy_compat.py index 65a1a405a..2093e052a 100644 --- a/tests/unit/cache/test_proxy_compat.py +++ b/tests/unit/cache/test_proxy_compat.py @@ -74,7 +74,7 @@ def test_second_install_hits_cache(self, mock_run: MagicMock, tmp_path: Path) -> # Pre-populate the checkout to simulate first install success. # The integrity verifier reads ``.git/HEAD`` directly, so we # must lay down a HEAD file containing the expected SHA. - checkout_dir = tmp_path / "git" / "checkouts_v1" / shard / sha + checkout_dir = tmp_path / "git" / "checkouts_v1" / shard / sha / "full" checkout_dir.mkdir(parents=True) git_dir = checkout_dir / ".git" git_dir.mkdir() diff --git a/tests/unit/deps/test_bare_cache_sparse.py b/tests/unit/deps/test_bare_cache_sparse.py new file mode 100644 index 000000000..9b57fa125 --- /dev/null +++ b/tests/unit/deps/test_bare_cache_sparse.py @@ -0,0 +1,104 @@ +"""Tests for sparse-cone path in bare_cache.materialize_from_bare (perf #1433).""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +from apm_cli.deps.bare_cache import materialize_from_bare + + +def _build_local_bare_repo(tmp_path: Path) -> tuple[Path, str]: + """Build a local repo with multiple subdirs and return (bare_path, sha).""" + work = tmp_path / "work" + work.mkdir() + subprocess.run(["git", "init", "-q", "-b", "main", str(work)], check=True) + subprocess.run(["git", "-C", str(work), "config", "user.email", "t@e"], check=True) + subprocess.run(["git", "-C", str(work), "config", "user.name", "t"], check=True) + for sub in ("plugins", "tools", "docs"): + d = work / sub + d.mkdir() + (d / "f.txt").write_text(f"{sub}\n", encoding="utf-8") + # nested fixture for the nested-path test + (work / "plugins" / "nested").mkdir() + (work / "plugins" / "nested" / "leaf.txt").write_text("leaf\n", encoding="utf-8") + subprocess.run(["git", "-C", str(work), "add", "."], check=True) + subprocess.run(["git", "-C", str(work), "commit", "-q", "-m", "init"], check=True) + sha = subprocess.run( + ["git", "-C", str(work), "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + bare = tmp_path / "bare.git" + subprocess.run(["git", "clone", "-q", "--bare", str(work), str(bare)], check=True) + return bare, sha + + +def test_default_full_tree_materialized(tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + consumer = tmp_path / "consumer" + resolved = materialize_from_bare(bare, consumer, ref=None, env=os.environ.copy(), known_sha=sha) + assert resolved == sha + assert (consumer / "plugins" / "f.txt").is_file() + assert (consumer / "tools" / "f.txt").is_file() + assert (consumer / "docs" / "f.txt").is_file() + + +def test_sparse_paths_only_materializes_requested_subdir(tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + consumer = tmp_path / "consumer" + resolved = materialize_from_bare( + bare, + consumer, + ref=None, + env=os.environ.copy(), + known_sha=sha, + sparse_paths=["plugins"], + ) + assert resolved == sha + assert (consumer / "plugins" / "f.txt").is_file() + # Sparse-cone excludes sibling top-level dirs: + assert not (consumer / "tools").exists() + assert not (consumer / "docs").exists() + # .git is always present + assert (consumer / ".git").is_dir() + + +def test_nested_subdir_path_materializes_nested(tmp_path: Path): + bare, sha = _build_local_bare_repo(tmp_path) + consumer = tmp_path / "consumer" + materialize_from_bare( + bare, + consumer, + ref=None, + env=os.environ.copy(), + known_sha=sha, + sparse_paths=["plugins/nested"], + ) + assert (consumer / "plugins" / "nested" / "leaf.txt").is_file() + assert not (consumer / "tools").exists() + + +def test_nonexistent_sparse_subdir_fails_loud_or_empty(tmp_path: Path): + """A subdir that doesn't exist must NOT silently materialize a full tree. + + git sparse-checkout does not error on missing paths (it just leaves + the working tree empty for the missing entry). The invariant we + enforce is: no sibling subdir leaks in. + """ + bare, sha = _build_local_bare_repo(tmp_path) + consumer = tmp_path / "consumer" + materialize_from_bare( + bare, + consumer, + ref=None, + env=os.environ.copy(), + known_sha=sha, + sparse_paths=["nonexistent/path"], + ) + # Critical invariant: no full-tree leak. + assert not (consumer / "plugins").exists() + assert not (consumer / "tools").exists() + assert not (consumer / "docs").exists() diff --git a/tests/unit/test_git_cache_branch_coverage.py b/tests/unit/test_git_cache_branch_coverage.py index 67402cdc1..d34755961 100644 --- a/tests/unit/test_git_cache_branch_coverage.py +++ b/tests/unit/test_git_cache_branch_coverage.py @@ -525,7 +525,7 @@ def test_evicts_on_integrity_failure(self, tmp_path): sha = "a" * 40 shard_key = "someshard" - checkout_dir = gc._checkouts_root / shard_key / sha + checkout_dir = gc._checkouts_root / shard_key / sha / "full" checkout_dir.mkdir(parents=True) with ( diff --git a/tests/unit/test_git_cache_phase3w4.py b/tests/unit/test_git_cache_phase3w4.py index f74c7409d..e10cc7727 100644 --- a/tests/unit/test_git_cache_phase3w4.py +++ b/tests/unit/test_git_cache_phase3w4.py @@ -525,7 +525,7 @@ def test_evicts_on_integrity_failure(self, tmp_path): sha = "a" * 40 shard_key = "someshard" - checkout_dir = gc._checkouts_root / shard_key / sha + checkout_dir = gc._checkouts_root / shard_key / sha / "full" checkout_dir.mkdir(parents=True) with (