diff --git a/.github/workflows/serve-harness.yml b/.github/workflows/serve-harness.yml new file mode 100644 index 00000000..bc3125bc --- /dev/null +++ b/.github/workflows/serve-harness.yml @@ -0,0 +1,45 @@ +name: Serve Harness + +# Integration validation for the `bazel-diff serve` query service. Runs tools/serve_harness.py, +# which builds //cli:bazel-diff and drives the real binary over HTTP against a live git:// remote +# (git daemon) across both git engines x full/shallow/partial clones -- coverage the in-process +# E2ETest cannot reach. See the harness module docstring for the scenario matrix. + +# Cron-only for now while we confirm the harness is stable on CI runners. Once it has a healthy +# track record, add `pull_request` / `push` triggers (and/or make it a required status check) to +# gate merges. `workflow_dispatch` allows on-demand runs from the Actions tab in the meantime. +# Note: scheduled + dispatched workflows run from the default branch, so this must land on master +# before the cron fires. +on: + workflow_dispatch: + schedule: + - cron: "0 */12 * * *" + +jobs: + ServeHarness: + runs-on: ubuntu-latest + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + bazel: [ '8.x', '9.x' ] + steps: + - name: Setup Java JDK + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: '21' + - name: Setup Go environment + uses: actions/setup-go@v5 + with: + go-version: ^1.17 + id: go + - name: Setup Bazelisk + run: go install github.com/bazelbuild/bazelisk@latest + - uses: actions/checkout@v4 + - name: Run serve harness + env: + USE_BAZEL_VERSION: ${{ matrix.bazel }} + # git daemon ships with git (preinstalled on the runner); python3 is preinstalled. + # --bazel points the harness (and the serve subprocesses it spawns) at bazelisk. + run: python3 tools/serve_harness.py --bazel "$HOME/go/bin/bazelisk" diff --git a/README.md b/README.md index 3a8d74b8..11233c39 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,13 @@ Notes and current limitations: `--gitEngine=subprocess` to shell out to the `git` binary at `--gitPath` instead -- useful for workspaces that depend on checkout filters or hooks that JGit does not run. Note that JGit only moves the git plumbing in-process; the working tree is still materialized on disk for `bazel query`. + JGit cannot fetch some clone shapes native git handles fine -- notably shallow (`--depth`) and + partial (`--filter=blob:none`) clones, whose thin packs are delta-compressed against objects the + clone does not have ("Missing delta base"). When an in-process JGit fetch fails, the service + automatically falls back to the native `git` binary (at `--gitPath`) for that fetch and logs a + warning; point `--gitEngine=subprocess` at such workspaces to skip the in-process attempt (a + partial clone in particular needs the subprocess engine to *serve* queries, since JGit cannot + lazily fetch the missing blobs a checkout needs). * Hashes are cached on local disk via `--cacheDir` and survive restarts. The cache layer is pluggable behind a byte-oriented interface so a remote backend (e.g. S3) can be added without touching callers. diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt index 5989a0a8..582c8622 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt @@ -237,7 +237,7 @@ class ServeCommand : Callable { */ fun createGitClient(): GitClient = when (gitEngine.lowercase()) { - "jgit" -> JGitClient(workspacePath) + "jgit" -> JGitClient(workspacePath, gitPath) "subprocess" -> ProcessGitClient(workspacePath, gitPath) else -> throw CommandLine.ParameterException( diff --git a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt index a3d84012..1a097459 100644 --- a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt +++ b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt @@ -80,7 +80,7 @@ class CalculateImpactedTargetsInteractor : KoinComponent { from: Map, to: Map ): Set { - val difference = Maps.difference(to, from) + val difference = Maps.difference(hashIdentities(to), hashIdentities(from)) val onlyInEnd: Set = difference.entriesOnlyOnLeft().keys val changed: Set = difference.entriesDiffering().keys val impactedTargets = @@ -91,6 +91,21 @@ class CalculateImpactedTargetsInteractor : KoinComponent { return impactedTargets } + /** + * Projects each target to its hash identity (`type#hash~directHash`) for impactedness comparison. + * + * Impactedness is defined purely by a target's hash, so the comparison must not include + * [TargetHash.deps]: that field is auxiliary data used only to derive distance metrics. It is + * populated on freshly generated hashes (under `--trackDeps`) but is always null on hashes + * deserialised from cache/JSON ([TargetHash.fromJson] does not restore it). Diffing the full + * [TargetHash] data class -- whose generated `equals` includes `deps` -- would therefore report + * every dep-carrying target as changed whenever one side was generated and the other read from + * cache. That is precisely a query-service cache hit under `--trackDeps`, which spuriously + * inflated the impacted set for the cached `from` revision. + */ + private fun hashIdentities(hashes: Map): Map = + hashes.mapValues { it.value.hashWithType } + fun executeWithDistances( from: Map, to: Map, @@ -188,7 +203,9 @@ class CalculateImpactedTargetsInteractor : KoinComponent { to: Map, depEdges: Map> ): Map { - val difference = Maps.difference(to, from) + // Diff by hash identity, not the full TargetHash -- see [hashIdentities]. The keys still index + // the original `from`/`to` maps below for the directHash (DIRECT vs INDIRECT) check. + val difference = Maps.difference(hashIdentities(to), hashIdentities(from)) val newLabels = difference.entriesOnlyOnLeft().keys val existingImpactedLabels = difference.entriesDiffering().keys diff --git a/cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt b/cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt index 095e8f5e..b4bbd802 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt @@ -1,10 +1,12 @@ package com.bazel_diff.server import com.bazel_diff.log.Logger +import java.io.File import java.nio.file.Path import org.eclipse.jgit.api.Git import org.eclipse.jgit.errors.IncorrectObjectTypeException import org.eclipse.jgit.errors.MissingObjectException +import org.eclipse.jgit.lib.Repository import org.eclipse.jgit.revwalk.RevWalk import org.eclipse.jgit.storage.file.FileRepositoryBuilder import org.eclipse.jgit.transport.RemoteConfig @@ -23,9 +25,20 @@ import org.koin.core.component.inject * these are cheap relative to the `bazel query` that follows, and it keeps the client stateless and * thread-safe (the workspace itself is still serialized by [HashService]'s lock). */ -class JGitClient(private val workspacePath: Path) : GitClient, KoinComponent { +class JGitClient( + private val workspacePath: Path, + private val gitPath: String = "git", + private val nativeFetchFallback: Boolean = true, +) : GitClient, KoinComponent { private val logger: Logger by inject() + // Built lazily and only when a JGit fetch actually fails. Native git negotiates clone shapes + // JGit 5.13 cannot fetch (shallow, partial/promisor, thin packs whose delta base is absent), + // scoped to the same workspace and configured git binary. + private val nativeGit: ProcessGitClient by lazy { ProcessGitClient(workspacePath, gitPath) } + + @Volatile private var warnedCloneShape = false + private fun open(): Git { val repository = FileRepositoryBuilder() @@ -38,6 +51,7 @@ class JGitClient(private val workspacePath: Path) : GitClient, KoinComponent { override fun fetch() { open().use { git -> + warnOnUnsupportedCloneShapeOnce(git.repository) val remotes = RemoteConfig.getAllRemoteConfigs(git.repository.config) if (remotes.isEmpty()) { logger.i { "No git remotes configured; skipping fetch" } @@ -48,12 +62,68 @@ class JGitClient(private val workspacePath: Path) : GitClient, KoinComponent { try { git.fetch().setRemote(remote.name).setRemoveDeletedRefs(true).call() } catch (e: Exception) { - throw GitClientException("JGit fetch ${remote.name} failed: ${e.message}", e) + // JGit cannot fetch some clone shapes it otherwise reads fine -- notably shallow and + // partial (blob:none) clones, where the remote's thin pack is delta-compressed against a + // base object absent from this clone ("Missing delta base "). Native git negotiates + // these correctly, so fall back to it rather than lame-ducking the whole service on a + // fetch the machine is perfectly capable of. + fetchViaNativeGitOrThrow(remote.name, e) + return // native `git fetch --all` already covered every remote; don't re-attempt them. } } } } + /** + * Retries a failed JGit fetch with the native `git` binary (unless [nativeFetchFallback] is off, + * in which case the original JGit failure is surfaced). If native git also fails the two failures + * are combined so neither cause is hidden. + */ + private fun fetchViaNativeGitOrThrow(remoteName: String, jgitError: Exception) { + if (!nativeFetchFallback) { + throw GitClientException("JGit fetch $remoteName failed: ${jgitError.message}", jgitError) + } + logger.w { + "JGit fetch $remoteName failed (${jgitError.message}); retrying with native git ('$gitPath'). " + + "This usually means the workspace is a shallow or partial clone, which JGit cannot fetch; " + + "pass --gitEngine=subprocess to skip the in-process attempt entirely." + } + try { + nativeGit.fetch() + } catch (nativeError: Exception) { + throw GitClientException( + "JGit fetch $remoteName failed (${jgitError.message}) and the native git fallback " + + "('$gitPath') also failed: ${nativeError.message}", + jgitError) + .apply { addSuppressed(nativeError) } + } + } + + /** + * Logs a one-time warning if the workspace is a shallow or partial (promisor) clone -- shapes + * JGit 5.13 cannot fetch, so every fetch will take the native-git fallback path. Surfacing it up + * front (rather than only on the first failed fetch) makes the misconfiguration obvious. + */ + private fun warnOnUnsupportedCloneShapeOnce(repo: Repository) { + if (warnedCloneShape) return + warnedCloneShape = true + val gitDir = repo.directory ?: return + val shallow = File(gitDir, "shallow").exists() + val partial = + repo.config.getString("extensions", null, "partialClone") != null || + repo.config.getSubsections("remote").any { + repo.config.getBoolean("remote", it, "promisor", false) + } + if (shallow || partial) { + val kinds = listOfNotNull("shallow".takeIf { shallow }, "partial".takeIf { partial }) + logger.w { + "workspace clone is ${kinds.joinToString("+")}; JGit cannot reliably fetch these " + + "(thin-pack delta bases may be absent). Fetches will fall back to native git " + + "('$gitPath'); consider --gitEngine=subprocess for this workspace." + } + } + } + override fun resolveSha(revision: String): String { open().use { git -> val objectId = git.repository.resolve(revision) ?: throw MissingRevisionException(revision) diff --git a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt index 946a18cf..ee9d573a 100644 --- a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt @@ -41,6 +41,50 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { assertThat(impacted).containsExactlyInAnyOrder("1", "3") } + @Test + fun computeSimpleImpactedTargetsIgnoresDepsAsymmetry() { + // Impactedness is defined by a target's hash, not its `deps` metadata. A query-service cache + // hit deserialises the `from` hashes (TargetHash.deps == null, fromJson never restores it), + // while a freshly generated `to` under --trackDeps has deps populated. A target whose hash is + // unchanged must therefore NOT be reported impacted just because its deps field differs + // (null vs a list). Regression test for the --trackDeps query-service over-reporting bug. + val from = + mapOf( + "//:unchanged" to TargetHash("Rule", "h", "h", deps = null), + "//:changed" to TargetHash("Rule", "old", "old", deps = null), + ) + val to = + mapOf( + "//:unchanged" to TargetHash("Rule", "h", "h", deps = listOf("//:dep")), + "//:changed" to TargetHash("Rule", "new", "new", deps = listOf("//:dep")), + ) + + val impacted = CalculateImpactedTargetsInteractor().computeSimpleImpactedTargets(from, to) + + assertThat(impacted).containsExactlyInAnyOrder("//:changed") + } + + @Test + fun computeAllDistancesIgnoresDepsAsymmetry() { + // Same deps-asymmetry guard on the distance path (/impacted_targets_with_distances): an + // unchanged target must not surface merely because its deps field differs between a + // deserialised `from` and a freshly generated `to`. + val from = + mapOf( + "//:unchanged" to TargetHash("Rule", "h", "h", deps = null), + "//:changed" to TargetHash("Rule", "old", "old", deps = null), + ) + val to = + mapOf( + "//:unchanged" to TargetHash("Rule", "h", "h", deps = listOf("//:changed")), + "//:changed" to TargetHash("Rule", "new", "new", deps = emptyList()), + ) + + val distances = CalculateImpactedTargetsInteractor().computeAllDistances(from, to, emptyMap()) + + assertThat(distances.keys).containsExactlyInAnyOrder("//:changed") + } + @Test fun testExecuteSortsByKindThenLabel() { val startHashes = diff --git a/cli/src/test/kotlin/com/bazel_diff/server/JGitClientTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/JGitClientTest.kt index 538fa74b..ec876f69 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/JGitClientTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/JGitClientTest.kt @@ -159,4 +159,67 @@ class JGitClientTest : KoinTest { client.fetch() assertThat(client.resolveSha(sha2)).isEqualTo(sha2) } + + @Test + fun fetchWithoutFallbackSurfacesJGitFailure() { + initRepoWithTwoCommits() + // origin points at a directory that is not a git repository, so the in-process JGit fetch + // fails. With the native fallback disabled the failure surfaces directly (legacy behaviour). + val bogus = File(temp.root, "not-a-repo").apply { mkdirs() } + git("remote", "add", "origin", bogus.absolutePath) + val client = JGitClient(temp.root.toPath(), gitPath = "git", nativeFetchFallback = false) + assertThrows(GitClientException::class.java) { client.fetch() } + } + + @Test + fun fetchFallbackErrorNamesNativeGitWhenBothFail() { + initRepoWithTwoCommits() + // JGit fetch fails (origin is not a repo) AND the native fallback fails (bogus git binary). + // The surfaced error must name the native fallback so neither cause is hidden. + val bogus = File(temp.root, "not-a-repo").apply { mkdirs() } + git("remote", "add", "origin", bogus.absolutePath) + val client = + JGitClient( + temp.root.toPath(), gitPath = "/nonexistent/git", nativeFetchFallback = true) + val ex = assertThrows(GitClientException::class.java) { client.fetch() } + assertThat(ex).messageContains("native git fallback") + } + + @Test + fun fetchBringsInNewCommitForShallowClone() { + // A shallow clone is a shape JGit 5.13 may be unable to fetch (the remote's thin pack can be + // delta-compressed against a base object below the shallow boundary). The client must fall + // back to native git so a just-landed commit still arrives; whichever path succeeds, the end + // state must be the new commit present and resolvable. + val origin = File(temp.root, "origin").apply { mkdirs() } + runGit(origin, "init", "-q") + runGit(origin, "config", "user.email", "test@example.com") + runGit(origin, "config", "user.name", "test") + File(origin, "file.txt").writeText("one") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "first") + File(origin, "file.txt").writeText("two") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "second") + + val workspace = File(temp.root, "workspace") + runGit( + temp.root, + "clone", + "-q", + "--depth=1", + "file://${origin.absolutePath}", + workspace.absolutePath) + + // A new commit lands after the shallow clone; the workspace has not fetched it yet. + File(origin, "file.txt").writeText("three") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "third") + val sha3 = runGit(origin, "rev-parse", "HEAD") + + val client = JGitClient(workspace.toPath()) + assertThrows(MissingRevisionException::class.java) { client.resolveSha(sha3) } + client.fetch() + assertThat(client.resolveSha(sha3)).isEqualTo(sha3) + } } diff --git a/tools/BUILD b/tools/BUILD index 3a4603e1..c1eca91c 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -54,3 +54,8 @@ py_test( python_version = "PY3", deps = [":coverage_check_lib"], ) + +# Note: the serve integration harness (serve_harness.py) is intentionally NOT wrapped in a +# py_binary. It shells out to `bazel build //cli:bazel-diff`, which would deadlock on the output-base +# lock if it were itself launched via `bazel run` (nested bazel, same output base). Run it directly: +# `python3 tools/serve_harness.py` (see its module docstring). It uses only the Python stdlib. diff --git a/tools/readme_template.md b/tools/readme_template.md index 1b187d29..c5a6d186 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -186,6 +186,13 @@ Notes and current limitations: `--gitEngine=subprocess` to shell out to the `git` binary at `--gitPath` instead -- useful for workspaces that depend on checkout filters or hooks that JGit does not run. Note that JGit only moves the git plumbing in-process; the working tree is still materialized on disk for `bazel query`. + JGit cannot fetch some clone shapes native git handles fine -- notably shallow (`--depth`) and + partial (`--filter=blob:none`) clones, whose thin packs are delta-compressed against objects the + clone does not have ("Missing delta base"). When an in-process JGit fetch fails, the service + automatically falls back to the native `git` binary (at `--gitPath`) for that fetch and logs a + warning; point `--gitEngine=subprocess` at such workspaces to skip the in-process attempt (a + partial clone in particular needs the subprocess engine to *serve* queries, since JGit cannot + lazily fetch the missing blobs a checkout needs). * Hashes are cached on local disk via `--cacheDir` and survive restarts. The cache layer is pluggable behind a byte-oriented interface so a remote backend (e.g. S3) can be added without touching callers. diff --git a/tools/serve_harness.py b/tools/serve_harness.py new file mode 100755 index 00000000..ca14d544 --- /dev/null +++ b/tools/serve_harness.py @@ -0,0 +1,772 @@ +#!/usr/bin/env python3 +"""Integration harness that fully exercises the `bazel-diff serve` query service. + +Unlike the in-process `E2ETest#testServeEndToEnd` unit test (which only ever uses a +single full, local, `--no-initial-fetch` repo), this harness drives the *real* built +`bazel-diff` binary as a subprocess against a *live git remote* served over `git://`, +across the combinations that actually break in production: + + * both git engines -- `--gitEngine=jgit` (default) and `--gitEngine=subprocess` + * three clone shapes -- full, shallow (`--depth=1`), partial (`--filter=blob:none`) + * the on-demand refetch path -- commits/branches that land on the remote *after* startup + * the startup fetch path -- lame-duck vs. ready health semantics + +It then asserts a *parity* invariant: for every scenario, the in-process JGit engine must +succeed wherever the native-git subprocess engine succeeds. A violation is exactly the class +of bug behind reports like: + + 400 git error: JGit fetch origin failed: Missing delta base + +which happens when JGit fetches a thin pack whose delta base is not present in a shallow or +partial clone -- a case native git handles but JGit 5.13 does not. + +Usage: + tools/serve_harness.py # build, then run the full matrix + tools/serve_harness.py --skip-build # reuse an existing bazel-bin/cli/bazel-diff + tools/serve_harness.py --only shallow # run only cases whose name contains "shallow" + tools/serve_harness.py --keep-artifacts# leave the temp workdir on disk for inspection + tools/serve_harness.py -v # stream serve/git output as it happens + tools/serve_harness.py --bazel ~/go/bin/bazelisk # use a specific bazel binary (CI) + +Exit code is non-zero if any gating scenario fails (parity violations included). +Requires: git (with `git daemon`) and a bazel binary (`--bazel`, default `bazel`); `git daemon` ships +with git. Pure Python stdlib, no third-party deps. +""" + +from __future__ import annotations + +import argparse +import contextlib +import json +import shutil +import socket +import subprocess +import sys +import tempfile +import threading +import time +import traceback +import urllib.error +import urllib.request +from dataclasses import dataclass, field +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +LAUNCHER = REPO_ROOT / "bazel-bin" / "cli" / "bazel-diff" +DEFAULT_BRANCH = "master" + +# ---------------------------------------------------------------------------------------------- +# Small terminal + logging helpers +# ---------------------------------------------------------------------------------------------- + + +class C: + """ANSI colors, disabled when stdout is not a tty.""" + + _on = sys.stdout.isatty() + GREEN = "\033[32m" if _on else "" + RED = "\033[31m" if _on else "" + YELLOW = "\033[33m" if _on else "" + BLUE = "\033[34m" if _on else "" + DIM = "\033[2m" if _on else "" + BOLD = "\033[1m" if _on else "" + RESET = "\033[0m" if _on else "" + + +VERBOSE = False +BAZEL = "bazel" # overridable via --bazel (e.g. a bazelisk path on CI) + + +def log(msg: str) -> None: + print(msg, flush=True) + + +def vlog(msg: str) -> None: + if VERBOSE: + print(f"{C.DIM}{msg}{C.RESET}", flush=True) + + +# ---------------------------------------------------------------------------------------------- +# Subprocess / git helpers +# ---------------------------------------------------------------------------------------------- + + +def run(cmd, cwd=None, check=True, env=None) -> subprocess.CompletedProcess: + vlog(f"$ {' '.join(str(c) for c in cmd)}" + (f" (cwd={cwd})" if cwd else "")) + proc = subprocess.run( + [str(c) for c in cmd], + cwd=str(cwd) if cwd else None, + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + ) + if check and proc.returncode != 0: + raise RuntimeError( + f"command failed ({proc.returncode}): {' '.join(str(c) for c in cmd)}\n{proc.stdout}" + ) + return proc + + +def git(repo: Path, *args, check=True) -> str: + # -c protocol.ext... not needed; keep identity deterministic for commits. + base = [ + "git", + "-c", + "user.name=serve-harness", + "-c", + "user.email=serve-harness@example.com", + "-c", + "commit.gpgsign=false", + "-c", + "init.defaultBranch=" + DEFAULT_BRANCH, + ] + return run(base + list(args), cwd=repo, check=check).stdout.strip() + + +def free_port() -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +def wait_port(port: int, timeout: float) -> bool: + deadline = time.time() + timeout + while time.time() < deadline: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.settimeout(0.5) + try: + s.connect(("127.0.0.1", port)) + return True + except OSError: + time.sleep(0.1) + return False + + +# ---------------------------------------------------------------------------------------------- +# HTTP client for the serve endpoints +# ---------------------------------------------------------------------------------------------- + + +def http(port: int, path: str, method: str = "GET", timeout: float = 300.0): + """Returns (status_code_or_None, body_text). status is None on a transport error.""" + url = f"http://127.0.0.1:{port}{path}" + req = urllib.request.Request(url, method=method) + try: + with urllib.request.urlopen(req, timeout=timeout) as resp: + return resp.status, resp.read().decode("utf-8", "replace") + except urllib.error.HTTPError as e: + return e.code, e.read().decode("utf-8", "replace") + except (urllib.error.URLError, ConnectionError, OSError) as e: + return None, str(e) + + +# ---------------------------------------------------------------------------------------------- +# Generated hermetic Bazel workspace (genrule-only: no external deps, no network) +# ---------------------------------------------------------------------------------------------- + +MODULE_BAZEL = 'module(name = "serve_harness_ws", version = "0.0.0")\n' + +# //:mid depends on //:core's output, //:other is independent. Editing core.txt therefore +# impacts //:core AND //:mid (distance 0 and 1); editing other.txt impacts only //:other. +BUILD_BAZEL = """\ +genrule(name = "core", srcs = ["core.txt"], outs = ["core.out"], cmd = "cp $< $@") + +genrule( + name = "mid", + srcs = ["mid.txt", ":core"], + outs = ["mid.out"], + cmd = "cat $(SRCS) > $@", +) + +genrule(name = "other", srcs = ["other.txt"], outs = ["other.out"], cmd = "cp $< $@") +""" + + +def _block(tag: str, changed: set[int] = frozenset(), n: int = 400) -> str: + """A large deterministic text block so git forms real deltas between revisions.""" + lines = [] + for i in range(n): + if i in changed: + lines.append(f"{tag} line {i:03d} CHANGED") + else: + lines.append(f"{tag} line {i:03d}") + return "\n".join(lines) + "\n" + + +# core.txt content across revisions. The A-family blocks are mutually similar (small deltas); +# block B is entirely dissimilar. This is what lets a later A'' commit delta against a +# below-shallow-boundary A blob after an aggressive repack. +CORE_A = _block("A") +CORE_A1 = _block("A", {10, 20, 30}) # A' -- a few lines changed +CORE_A2 = _block("A", {10, 20, 30, 40, 50, 60}) # A'' -- still A-family +CORE_B = _block("B") # entirely different content + + +def write_workspace(ws: Path, core: str, mid: str, other: str) -> None: + ws.mkdir(parents=True, exist_ok=True) + (ws / "MODULE.bazel").write_text(MODULE_BAZEL) + (ws / "BUILD.bazel").write_text(BUILD_BAZEL) + (ws / "core.txt").write_text(core) + (ws / "mid.txt").write_text(mid) + (ws / "other.txt").write_text(other) + bv = REPO_ROOT / ".bazelversion" + if bv.exists(): + shutil.copy(bv, ws / ".bazelversion") + + +# ---------------------------------------------------------------------------------------------- +# The remote: a git repo with controlled history, served over git:// by `git daemon` +# ---------------------------------------------------------------------------------------------- + + +@dataclass +class Remote: + root: Path # temp root + srv_base: Path # git-daemon base-path + origin: Path # the served repo (srv_base/origin) + port: int + url: str + daemon: subprocess.Popen + shas: dict = field(default_factory=dict) + + def commit(self, msg: str, core=None, mid=None, other=None) -> str: + if core is not None: + (self.origin / "core.txt").write_text(core) + if mid is not None: + (self.origin / "mid.txt").write_text(mid) + if other is not None: + (self.origin / "other.txt").write_text(other) + git(self.origin, "add", "-A") + git(self.origin, "commit", "-q", "-m", msg) + return git(self.origin, "rev-parse", "HEAD") + + def repack(self) -> None: + # Aggressive repack so newer blobs are delta-compressed against the most similar base + # anywhere in history -- including objects below a shallow clone's boundary. + git(self.origin, "repack", "-a", "-d", "-f", "--window=250", "--depth=250") + + def clone(self, dest: Path, *extra) -> Path: + run(["git", "clone", "-q", *extra, self.url, str(dest)]) + # Give the clone a deterministic identity for any commits (none expected) + disable gc. + git(dest, "config", "user.name", "serve-harness") + git(dest, "config", "user.email", "serve-harness@example.com") + git(dest, "config", "gc.auto", "0") + return dest + + def stop(self) -> None: + with contextlib.suppress(Exception): + self.daemon.terminate() + self.daemon.wait(timeout=5) + + +def build_remote(root: Path) -> Remote: + srv_base = root / "srv" + origin = srv_base / "origin" + origin.mkdir(parents=True) + git(origin, "init", "-q") + git(origin, "config", "user.name", "serve-harness") + git(origin, "config", "user.email", "serve-harness@example.com") + # Server-side capabilities needed for fetch-by-sha and partial (blob:none) clones. + git(origin, "config", "uploadpack.allowFilter", "true") + git(origin, "config", "uploadpack.allowAnySHA1InWant", "true") + git(origin, "config", "uploadpack.allowReachableSHA1InWant", "true") + + shas: dict = {} + write_workspace(origin, core=CORE_A, mid="mid v0\n", other="other v0\n") + git(origin, "add", "-A") + git(origin, "commit", "-q", "-m", "C0 base") + shas["C0"] = git(origin, "rev-parse", "HEAD") + + port = free_port() + daemon = subprocess.Popen( + [ + "git", + "daemon", + "--reuseaddr", + "--listen=127.0.0.1", + f"--port={port}", + f"--base-path={srv_base}", + "--export-all", + "--enable=upload-pack", + "--informative-errors", + ], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + if not wait_port(port, timeout=10): + daemon.terminate() + raise RuntimeError(f"git daemon did not come up on port {port}") + + remote = Remote( + root=root, + srv_base=srv_base, + origin=origin, + port=port, + url=f"git://127.0.0.1:{port}/origin", + daemon=daemon, + shas=shas, + ) + + # Build the rest of the pre-clone history (C1: edit core A->A', C2: replace core A'->B). + remote.shas["C1"] = remote.commit("C1 tweak core", core=CORE_A1) + remote.shas["C2"] = remote.commit("C2 replace core", core=CORE_B) + remote.repack() + return remote + + +# ---------------------------------------------------------------------------------------------- +# A running `bazel-diff serve` process +# ---------------------------------------------------------------------------------------------- + + +@dataclass +class Serve: + proc: subprocess.Popen + port: int + workspace: Path + stderr_path: Path + engine: str + + def stderr_tail(self, n: int = 25) -> str: + try: + lines = self.stderr_path.read_text("utf-8", "replace").splitlines() + except OSError: + return "" + return "\n".join(lines[-n:]) + + def stop(self) -> None: + with contextlib.suppress(Exception): + self.proc.terminate() + try: + self.proc.wait(timeout=8) + except subprocess.TimeoutExpired: + self.proc.kill() + + +@contextlib.contextmanager +def serve( + workspace: Path, + cache: Path, + engine: str, + *, + initial_fetch: bool, + track_deps: bool, + ready_timeout: float, +): + """Launches serve, waits for a health verdict, yields (Serve, health) then tears down. + + health is one of: "ready" (200), "lameduck" (up but stuck 503), "down" (never bound). + """ + port = free_port() + stderr_path = workspace.parent / f"serve.{engine}.stderr.log" + args = [ + str(LAUNCHER), + "serve", + "-w", + str(workspace), + "-b", + BAZEL, + "--cacheDir", + str(cache), + "--port", + str(port), + "--gitEngine", + engine, + ] + if not initial_fetch: + args.append("--no-initial-fetch") + if track_deps: + args.append("--trackDeps") + + vlog(f"launch serve[{engine}] on :{port} ws={workspace.name} initial_fetch={initial_fetch}") + with open(stderr_path, "w") as errf: + proc = subprocess.Popen( + args, + cwd=str(workspace), + stdout=(None if VERBOSE else subprocess.DEVNULL), + stderr=(errf if not VERBOSE else None), + ) + s = Serve(proc=proc, port=port, workspace=workspace, stderr_path=stderr_path, engine=engine) + try: + health = _await_health(s, ready_timeout) + yield s, health + finally: + s.stop() + + +def _await_health(s: Serve, timeout: float) -> str: + """Polls /health. 200 => ready. Trailing 503 after the process is up => lameduck.""" + deadline = time.time() + timeout + last = None + bound = False + while time.time() < deadline: + if s.proc.poll() is not None: + return "down" # process exited + code, _ = http(s.port, "/health", timeout=3) + if code == 200: + return "ready" + if code == 503: + bound = True + last = 503 + time.sleep(0.4) + # Timed out. If we ever saw a 503, the server is up but not becoming ready => lame-duck. + return "lameduck" if (bound or last == 503) else "down" + + +# ---------------------------------------------------------------------------------------------- +# Reporting +# ---------------------------------------------------------------------------------------------- + +PASS, FAIL, XFAIL, INFO, SKIP = "PASS", "FAIL", "XFAIL", "INFO", "SKIP" + + +@dataclass +class Report: + rows: list = field(default_factory=list) # (case, name, status, detail) + + def add(self, case: str, name: str, status: str, detail: str = "") -> None: + self.rows.append((case, name, status, detail)) + color = { + PASS: C.GREEN, + FAIL: C.RED, + XFAIL: C.YELLOW, + INFO: C.BLUE, + SKIP: C.DIM, + }[status] + tail = f" {C.DIM}{detail}{C.RESET}" if detail else "" + log(f" {color}{status:<5}{C.RESET} {name}{tail}") + + def check(self, case, name, cond, ok_detail="", fail_detail="") -> bool: + self.add(case, name, PASS if cond else FAIL, ok_detail if cond else fail_detail) + return bool(cond) + + def summary(self) -> int: + n = {PASS: 0, FAIL: 0, XFAIL: 0, INFO: 0, SKIP: 0} + for _, _, status, _ in self.rows: + n[status] += 1 + log("") + log(f"{C.BOLD}==== summary ===={C.RESET}") + log( + f" {C.GREEN}{n[PASS]} pass{C.RESET} " + f"{C.RED}{n[FAIL]} fail{C.RESET} " + f"{C.YELLOW}{n[XFAIL]} xfail{C.RESET} " + f"{C.BLUE}{n[INFO]} info{C.RESET} " + f"{C.DIM}{n[SKIP]} skip{C.RESET}" + ) + if n[FAIL]: + log("") + log(f"{C.RED}Failing scenarios:{C.RESET}") + for case, name, status, detail in self.rows: + if status == FAIL: + log(f" - [{case}] {name}: {detail}") + return n[FAIL] + + +# ---------------------------------------------------------------------------------------------- +# Scenario primitives +# ---------------------------------------------------------------------------------------------- + + +def _impacted(s: Serve, frm: str, to: str, target_type: str | None = None, timeout: float = 300.0): + q = f"/impacted_targets?from={frm}&to={to}" + if target_type: + q += f"&targetType={target_type}" + code, body = http(s.port, q, timeout=timeout) + try: + data = json.loads(body) + except (ValueError, TypeError): + data = None + return code, body, data + + +def _iset(data) -> set: + if not data or not isinstance(data.get("impactedTargets"), list): + return set() + return set(data["impactedTargets"]) + + +def _stable(data) -> set: + """Impacted labels with generated-file (`*.out`) targets dropped. + + A generated file's hash equals its generating rule's hash, yet whether it surfaces as a + distinct impacted target fluctuates with Bazel's incremental analysis state across the + service's sequential checkouts (observed identically under both git engines). Rules and + source files are stable, so scenario assertions compare only that deterministic subset. + """ + return {label for label in _iset(data) if not label.endswith(".out")} + + +# ---------------------------------------------------------------------------------------------- +# Cases +# ---------------------------------------------------------------------------------------------- + + +def case_full(remote: Remote, clone: Path, root: Path, rep: Report, engine: str, track_deps: bool) -> None: + """The broad functional suite on a full clone, run identically for both git engines so any + divergence is attributable to the engine alone.""" + case = f"full/{engine}" + ("+deps" if track_deps else "") + log(f"\n{C.BOLD}== case {case} =={C.RESET}") + sh = remote.shas + cache = root / f"cache-full-{engine}-{int(track_deps)}" + track = track_deps + with serve(clone, cache, engine, initial_fetch=False, track_deps=track, ready_timeout=30) as (s, health): + if not rep.check(case, "health becomes ready", health == "ready", + fail_detail=f"health={health}\n{s.stderr_tail()}"): + return + + # Editing core.txt impacts //:core (directly) and //:mid (depends on //:core), plus the + # source //:core.txt. Nothing in the //:other subtree. + core_edit = {"//:core", "//:core.txt", "//:mid"} + code, body, data = _impacted(s, sh["C1"], sh["C2"]) + got = _stable(data) + rep.check(case, "impacted_targets C1->C2 (edit core.txt)", + code == 200 and got == core_edit, + ok_detail=str(sorted(got)), fail_detail=f"code={code} got={sorted(got)} raw={sorted(_iset(data))}") + + # Caching: an identical follow-up request returns the identical stable set (served from cache). + c2, _, d2 = _impacted(s, sh["C1"], sh["C2"]) + rep.check(case, "impacted_targets cached repeat is stable", + c2 == 200 and _stable(d2) == got, fail_detail=f"code={c2} got={sorted(_stable(d2))}") + + # On-demand refetch: C3 landed on the remote after this clone was made (#411 path). + code, body, data = _impacted(s, sh["C2"], sh["C3"]) + got = _stable(data) + rep.check(case, "refetch new commit C2->C3 (edit core.txt)", + code == 200 and got == core_edit, + ok_detail=str(sorted(got)), + fail_detail=f"code={code} got={sorted(got)} raw={sorted(_iset(data))}\n{s.stderr_tail()}") + + # On-demand refetch of a commit reachable only via a brand-new branch ref (feature@C4, + # which edits only other.txt). + code, body, data = _impacted(s, sh["C2"], sh["C4"]) + got = _stable(data) + rep.check(case, "refetch new branch commit C2->C4 (edit other.txt)", + code == 200 and got == {"//:other", "//:other.txt"}, + ok_detail=str(sorted(got)), + fail_detail=f"code={code} got={sorted(got)} raw={sorted(_iset(data))}\n{s.stderr_tail()}") + + _full_extras(rep, case, s, remote, track_deps) + + _bazel_shutdown(clone) + + +def _full_extras(rep: Report, case: str, s: Serve, remote: Remote, track_deps: bool) -> None: + sh = remote.shas + + # targetType filter -> only the changed source file comes back (no rules, no generated files). + code, body, data = _impacted(s, sh["C1"], sh["C2"], target_type="SourceFile") + iset = _iset(data) + rep.check(case, "targetType=SourceFile filters to sources", + code == 200 and iset == {"//:core.txt"}, + ok_detail=str(sorted(iset)), fail_detail=f"code={code} got={sorted(iset)}") + + # Genuinely-unknown revision -> 400 after one on-demand refetch that still can't find it. + dead = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + code, body, _ = _impacted(s, sh["C2"], dead) + rep.check(case, "unknown revision -> 400", + code == 400 and "missing from the local clone" in body, + fail_detail=f"code={code} body={body[:200]}") + + # Missing required params. + code, body = http(s.port, "/impacted_targets") + rep.check(case, "missing from/to -> 400", + code == 400 and "missing required query parameters" in body, + fail_detail=f"code={code} body={body[:160]}") + + # Wrong method. + code, body = http(s.port, f"/impacted_targets?from={sh['C1']}&to={sh['C2']}", method="POST") + rep.check(case, "POST -> 405", code == 405, fail_detail=f"code={code}") + + # Distances endpoint: with --trackDeps, core is directly changed (d0), mid depends on core + # (d>=1); without it, the endpoint 400s. + code, body = http(s.port, f"/impacted_targets_with_distances?from={sh['C1']}&to={sh['C2']}") + if track_deps: + try: + dd = json.loads(body) + bylabel = {e["label"]: e for e in dd.get("impactedTargets", [])} + except (ValueError, TypeError, KeyError): + bylabel = {} + rep.check(case, "distances: core d0, mid d>=1", + code == 200 + and bylabel.get("//:core", {}).get("targetDistance") == 0 + and bylabel.get("//:mid", {}).get("targetDistance", 0) >= 1, + ok_detail=f"core={bylabel.get('//:core',{}).get('targetDistance')} " + f"mid={bylabel.get('//:mid',{}).get('targetDistance')}", + fail_detail=f"code={code} body={body[:200]}") + else: + rep.check(case, "distances 400 without --trackDeps", + code == 400 and "trackDeps" in body, fail_detail=f"code={code} body={body[:160]}") + + # Concurrency: a burst of identical requests all succeed with the identical set (one shared + # workspace lock + per-SHA cache serialize the cold generation). + results: list = [] + lock = threading.Lock() + + def worker(): + c, _, d = _impacted(s, sh["C1"], sh["C2"]) + with lock: + results.append((c, frozenset(_stable(d)))) + + threads = [threading.Thread(target=worker) for _ in range(8)] + for t in threads: + t.start() + for t in threads: + t.join() + codes = {c for c, _ in results} + sets = {fs for _, fs in results} + rep.check(case, "8 concurrent requests consistent", + codes == {200} and len(sets) == 1, + ok_detail=f"codes={codes}", fail_detail=f"codes={codes} distinct_sets={len(sets)}") + + +def case_shallow(remote: Remote, clone: Path, root: Path, rep: Report, engine: str) -> None: + """Fetch-parity gate on a shallow (--depth=1) clone: a just-landed commit must be servable via + on-demand refetch under BOTH engines. This is the clone shape behind production + 'JGit fetch ... Missing delta base' failures (the remote's thin pack is delta-compressed against + an object below the shallow boundary). A cooperative local git-daemon negotiates it cleanly, so + in-process JGit usually succeeds here; the native-git fetch fallback is what keeps the default + engine at parity on the nastier real-world remotes where JGit's in-process fetch cannot.""" + case = f"shallow/{engine}" + log(f"\n{C.BOLD}== case {case} =={C.RESET}") + sh = remote.shas + cache = root / f"cache-shallow-{engine}" + with serve(clone, cache, engine, initial_fetch=False, track_deps=False, ready_timeout=30) as (s, health): + if not rep.check(case, "health becomes ready", health == "ready", + fail_detail=f"health={health}\n{s.stderr_tail()}"): + return + # from = shallow tip (present); to = C3 (landed after clone, must be fetched on demand). + code, body, data = _impacted(s, sh["C2"], sh["C3"]) + got = _stable(data) + rep.check(case, "refetch new commit into shallow clone C2->C3", + code == 200 and got == {"//:core", "//:core.txt", "//:mid"}, + ok_detail=str(sorted(got)), + fail_detail=f"code={code} got={sorted(got)} raw={sorted(_iset(data))} body={body[:200]}\n" + f"--- serve stderr ---\n{s.stderr_tail()}") + _bazel_shutdown(clone) + + +def case_partial(remote: Remote, clone: Path, root: Path, rep: Report, engine: str) -> None: + """Fetch-parity gate on a partial (--filter=blob:none) clone, exercised through the *startup* + fetch: with an initial fetch enabled the instance must reach ready under both engines. A partial + clone's thin pack can be delta-compressed against an absent promised blob, which JGit cannot + fetch in-process; the native-git fallback keeps it ready. Health-only (no requests) so JGit's + separate lack of lazy-blob checkout support is not conflated with the fetch path under test -- + a partial clone still needs --gitEngine=subprocess to actually *serve* queries.""" + case = f"partial/{engine}" + log(f"\n{C.BOLD}== case {case} =={C.RESET}") + cache = root / f"cache-partial-{engine}" + with serve(clone, cache, engine, initial_fetch=True, track_deps=False, ready_timeout=25) as (s, health): + rep.check(case, "startup fetch on partial clone -> ready", + health == "ready", + ok_detail="ready", + fail_detail=f"health={health}\n--- serve stderr ---\n{s.stderr_tail()}") + _bazel_shutdown(clone) + + +def _bazel_shutdown(clone: Path) -> None: + with contextlib.suppress(Exception): + run([BAZEL, "shutdown"], cwd=clone, check=False) + + +# ---------------------------------------------------------------------------------------------- +# Orchestration +# ---------------------------------------------------------------------------------------------- + +VARIANTS = { + "full": [], + "shallow": ["--depth=1"], + "partial": ["--filter=blob:none"], +} + +# (name, variant, engine, track_deps). The full clone runs the 2x2 of engine x trackDeps so any +# divergence is attributable to a single variable; shallow/partial exercise the fetch paths. +# Native (subprocess) rows run before their jgit twins so the correct baseline prints first. +CASES = [ + ("full/subprocess", "full", "subprocess", False), + ("full/jgit", "full", "jgit", False), + ("full/subprocess+deps", "full", "subprocess", True), + ("full/jgit+deps", "full", "jgit", True), + ("shallow/subprocess", "shallow", "subprocess", None), + ("shallow/jgit", "shallow", "jgit", None), + ("partial/subprocess", "partial", "subprocess", None), + ("partial/jgit", "partial", "jgit", None), +] + + +def _run_case(name, variant, engine, td, remote, clone, root, rep) -> None: + if variant == "full": + case_full(remote, clone, root, rep, engine, td) + elif variant == "shallow": + case_shallow(remote, clone, root, rep, engine) + else: + case_partial(remote, clone, root, rep, engine) + + +def main() -> int: + global VERBOSE, BAZEL + ap = argparse.ArgumentParser(description="Exercise `bazel-diff serve` end to end.") + ap.add_argument("--skip-build", action="store_true", help="reuse existing bazel-bin launcher") + ap.add_argument("--only", default="", help="run only cases whose name contains this substring") + ap.add_argument("--keep-artifacts", action="store_true", help="do not delete the temp workdir") + ap.add_argument( + "--bazel", default="bazel", help="bazel binary to use (e.g. a bazelisk path on CI)") + ap.add_argument("-v", "--verbose", action="store_true", help="stream serve/git output") + args = ap.parse_args() + VERBOSE = args.verbose + BAZEL = args.bazel + + if not args.skip_build: + log(f"{C.BOLD}Building //cli:bazel-diff ...{C.RESET}") + run([BAZEL, "build", "//cli:bazel-diff"], cwd=REPO_ROOT) + if not LAUNCHER.exists(): + log(f"{C.RED}launcher not found at {LAUNCHER}; run without --skip-build{C.RESET}") + return 2 + + root = Path(tempfile.mkdtemp(prefix="serve-harness-")) + log(f"{C.DIM}workdir: {root}{C.RESET}") + rep = Report() + remote = None + clones: dict = {} + try: + log(f"{C.BOLD}Setting up git remote (git daemon) + clones ...{C.RESET}") + remote = build_remote(root) + + # Pre-make each case's own clone while the remote is at C2, BEFORE the post-startup commits + # land, so those commits genuinely require an on-demand fetch. Each case gets its own clone + # + cache so sequential checkouts never cross-contaminate. + selected = [c for c in CASES if not args.only or args.only in c[0]] + for name, variant, engine, td in selected: + slug = name.replace("/", "-").replace("+", "-") + clones[name] = remote.clone(root / f"clone-{slug}", *VARIANTS[variant]) + + # Land the post-clone history on the remote. + remote.shas["C3"] = remote.commit("C3 restore core (A'')", core=CORE_A2) # master advances + git(remote.origin, "checkout", "-q", "-b", "feature", remote.shas["C2"]) + remote.shas["C4"] = remote.commit("C4 edit other (feature)", other="other v1\n") + git(remote.origin, "checkout", "-q", DEFAULT_BRANCH) + remote.repack() # force new blobs to delta against the most similar (possibly out-of-clone) base + vlog(f"shas: {remote.shas}") + + for name, variant, engine, td in selected: + try: + _run_case(name, variant, engine, td, remote, clones[name], root, rep) + except Exception: + rep.add(name, "case crashed", FAIL, traceback.format_exc().splitlines()[-1]) + vlog(traceback.format_exc()) + finally: + for clone in clones.values(): + _bazel_shutdown(clone) + if remote: + remote.stop() + if args.keep_artifacts: + log(f"{C.DIM}artifacts kept at {root}{C.RESET}") + else: + shutil.rmtree(root, ignore_errors=True) + + fails = rep.summary() + return 1 if fails else 0 + + +if __name__ == "__main__": + sys.exit(main())