From 89d2cfba7b03f04f873fa22439043ed6fb36ace2 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Tue, 7 Jul 2026 16:38:59 -0400 Subject: [PATCH] fix(serve): fetch unreachable revisions by SHA so PR-head commits resolve The #411 on-demand refetch runs `git fetch --all --prune`, which only downloads objects reachable from the refs the clone's refspec covers (branches/tags). A commit named by full SHA that is reachable from no fetched ref -- a GitHub PR-head under refs/pull//head, or a commit force-pushed off its branch -- never arrives, so resolveSha keeps failing and the request 400s with "revision '' is missing from the local clone". Escalate recovery in ImpactedTargetsService.resolveBoth: after the broad fetch, attempt a targeted by-SHA `git fetch ` (new GitClient.fetchRevision) for each revision still missing. This works against servers that permit reachable-SHA fetches (uploadpack.allowReachableSHA1InWant, GitHub's default). JGitClient routes it through native git, since JGit 5.13 cannot reliably fetch by SHA. A revision still missing after both steps is genuinely unknown and still 400s. Coverage: real-git PR-head tests in GitClientTest/JGitClientTest, a DirectFetchGitClient unit case in ImpactedTargetsServiceTest, and a C5 (refs/pull/*) scenario in tools/serve_harness.py that fails without the fix (negative-control verified) across both git engines. Co-Authored-By: Claude Opus 4.8 --- .../kotlin/com/bazel_diff/server/GitClient.kt | 50 +++++++++++++- .../server/ImpactedTargetsService.kt | 65 +++++++++++++----- .../com/bazel_diff/server/JGitClient.kt | 15 +++++ .../com/bazel_diff/server/GitClientTest.kt | 66 ++++++++++++++++++- .../server/ImpactedTargetsServiceTest.kt | 60 ++++++++++++++++- .../com/bazel_diff/server/JGitClientTest.kt | 49 +++++++++++++- tools/serve_harness.py | 29 ++++++++ 7 files changed, 309 insertions(+), 25 deletions(-) diff --git a/cli/src/main/kotlin/com/bazel_diff/server/GitClient.kt b/cli/src/main/kotlin/com/bazel_diff/server/GitClient.kt index 9dd3280..541f7c3 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/GitClient.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/GitClient.kt @@ -19,8 +19,10 @@ open class GitClientException(message: String, cause: Throwable? = null) : * absent from the object database. * * The query service treats this as *retryable*: it only fetches at startup, so a commit that landed - * on the remote afterwards is simply not here yet. Callers refetch and retry once before surfacing - * the failure (see [com.bazel_diff.server.ImpactedTargetsService]). + * on the remote afterwards is simply not here yet. Callers refetch and retry before surfacing the + * failure -- first a broad [GitClient.fetch], then, for a commit not reachable from any fetched ref + * (e.g. a PR-head SHA), a targeted [GitClient.fetchRevision] (see + * [com.bazel_diff.server.ImpactedTargetsService]). */ class MissingRevisionException(val revision: String, cause: Throwable? = null) : GitClientException("revision '$revision' is missing from the local clone", cause) @@ -36,6 +38,21 @@ interface GitClient { /** Fetches the latest refs from all remotes. Throws [GitClientException] on failure. */ fun fetch() + /** + * Best-effort targeted fetch of a single [revision] (a SHA or ref name) directly from the + * remote(s), for a commit a broad [fetch] does not bring in because it is reachable from no ref + * this clone fetches -- e.g. a GitHub PR-head SHA (advertised only under `refs/pull//head`), + * or a commit force-pushed off its branch. Works against servers that permit fetching a reachable + * object by SHA (`uploadpack.allowReachableSHA1InWant`, enabled by default on GitHub). + * + * Returns true if a fetch reported success, false if no remote could supply it (server disallows + * by-SHA fetch, or the revision is genuinely unknown). Never throws for an ordinary miss: callers + * re-run [resolveSha], the authoritative check, which raises [MissingRevisionException] if the + * object is still absent. Defaults to a no-op returning false for clients that cannot target an + * individual revision. + */ + fun fetchRevision(revision: String): Boolean = false + /** * Resolves a revision (branch, tag, short or full SHA) to a full 40-char commit SHA, verifying * that the commit actually exists in the local clone. Throws [MissingRevisionException] if the @@ -63,6 +80,35 @@ class ProcessGitClient( run("fetch", "--all", "--prune") } + override fun fetchRevision(revision: String): Boolean { + // A broad `fetch --all` only downloads objects reachable from the refs the refspec covers, so a + // commit named by a SHA that no fetched ref reaches -- a GitHub PR-head commit (advertised only + // under refs/pull/*), or a commit force-pushed off its branch -- never arrives. Ask each remote + // for that exact object: servers that allow reachable-SHA fetches + // (uploadpack.allowReachableSHA1InWant, GitHub's default) return it even though no local ref + // ends up pointing at it. Best-effort per remote; a refusal or unknown SHA moves on, and the + // authoritative miss is left to the caller's resolveSha. + for (remote in remoteNames()) { + try { + logger.i { "git fetch $remote $revision in $workspacePath" } + run("fetch", remote, revision) + return true + } catch (e: GitClientException) { + logger.i { "targeted fetch of '$revision' from '$remote' failed: ${e.message}" } + } + } + return false + } + + /** Configured remote names (`git remote`); empty when none are configured or the query fails. */ + private fun remoteNames(): List = + try { + run("remote").map(String::trim).filter(String::isNotEmpty) + } catch (e: GitClientException) { + logger.i { "could not list git remotes in $workspacePath: ${e.message}" } + emptyList() + } + override fun resolveSha(revision: String): String { // `rev-parse --verify ^{commit}` both resolves the revision to a commit SHA and confirms // that commit is present in the local clone. A bare `rev-parse ` echoes any diff --git a/cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt b/cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt index e9ee43a..4814d6f 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt @@ -138,34 +138,69 @@ class ImpactedTargetsService( } /** - * Resolves both revisions to commit SHAs, refetching once if either is missing from the local - * clone. The service only fetches at startup, so a commit that landed on the remote afterwards - * (or a branch this clone has not yet seen) would otherwise fail with a stale-clone - * [MissingRevisionException]; a single on-demand `git fetch` brings it in. + * Resolves both revisions to commit SHAs, fetching on demand if either is missing from the local + * clone. The service only fetches at startup, so a revision that appeared on the remote + * afterwards would otherwise fail with a stale-clone [MissingRevisionException]. Recovery + * escalates in two steps: a broad [GitClient.fetch] (refreshes branch/tag tips -- the common + * "commit just landed" case), then, for a revision still missing because it is reachable from no + * ref this clone fetches (a GitHub PR-head SHA, or a commit force-pushed off its branch), a + * targeted [GitClient.fetchRevision] of that exact object. * * Fetches are serialized and double-checked through [fetchLock]: concurrent requests for the same * new commit wait on the lock, and whoever holds it re-resolves first, so an earlier fetch that * already made the revision resolvable saves the rest a redundant fetch. A revision still missing - * after the refetch is genuinely unknown (bad SHA, or a ref this clone's refspec doesn't fetch) - * and propagates as [MissingRevisionException] -> HTTP 400. + * after both steps is genuinely unknown (bad SHA, or an object the remote will not serve) and + * propagates as [MissingRevisionException] -> HTTP 400. */ private fun resolveBoth(fromRev: String, toRev: String): Pair { - try { - return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev) - } catch (missing: MissingRevisionException) { - logger.i { "Revision '${missing.revision}' not in local clone; refetching and retrying" } + resolveBothOrNull(fromRev, toRev)?.let { + return it } synchronized(fetchLock) { - // A concurrent request may have refetched while we waited on the lock; retry before fetching. - try { - return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev) - } catch (stillMissing: MissingRevisionException) { - gitClient.fetch() + // A concurrent request may have fetched while we waited on the lock; retry before fetching. + resolveBothOrNull(fromRev, toRev)?.let { + return it + } + // Broad fetch first: refreshes branch/tag tips, covering the common case of a commit that + // landed on the remote after startup. + logger.i { "Revision missing from local clone; fetching all refs and retrying" } + gitClient.fetch() + resolveBothOrNull(fromRev, toRev)?.let { + return it + } + // A broad fetch only downloads objects reachable from the refs this clone's refspec covers. A + // revision still missing is not among them -- a GitHub PR-head SHA (advertised under + // refs/pull/*), or a commit force-pushed off its branch. Ask the remote for each such object + // by SHA directly before giving up. + for (rev in linkedSetOf(fromRev, toRev)) { + if (!resolvable(rev)) { + logger.i { "Revision '$rev' unreachable via broad fetch; fetching it directly" } + gitClient.fetchRevision(rev) + } } } + // Authoritative: a revision still unresolved now is genuinely unknown and propagates as a + // MissingRevisionException -> HTTP 400. return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev) } + /** Resolves both revisions, or null if either is still missing from the local clone. */ + private fun resolveBothOrNull(fromRev: String, toRev: String): Pair? = + try { + gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev) + } catch (missing: MissingRevisionException) { + null + } + + /** True if [rev] resolves to a commit present in the local clone. */ + private fun resolvable(rev: String): Boolean = + try { + gitClient.resolveSha(rev) + true + } catch (missing: MissingRevisionException) { + false + } + // Synthetic //external:* labels are not buildable in bzlmod-only mode; mirror the // get-impacted-targets default of excluding them when Bzlmod is enabled (issue #326). private fun excludeExternalTargets(): Boolean = bazelModService.isBzlmodEnabled 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 b4bbd80..3ec6ee6 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt @@ -74,6 +74,21 @@ class JGitClient( } } + override fun fetchRevision(revision: String): Boolean { + // Fetching an object by SHA is a negotiation JGit 5.13 does not reliably perform (it resolves + // the ref spec against the remote's advertised refs, and a PR-head or force-pushed SHA is not + // advertised). Route it through native git -- the same reason fetch() falls back -- which + // supports `git fetch ` against servers that allow reachable-SHA fetches. + // With the fallback disabled the in-process engine cannot supply it, so report that honestly. + if (!nativeFetchFallback) { + logger.i { + "JGit cannot fetch revision '$revision' by SHA and the native fallback is disabled" + } + return false + } + return nativeGit.fetchRevision(revision) + } + /** * 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 diff --git a/cli/src/test/kotlin/com/bazel_diff/server/GitClientTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/GitClientTest.kt index 3b3954c..774488a 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/GitClientTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/GitClientTest.kt @@ -3,7 +3,9 @@ package com.bazel_diff.server import assertk.assertThat import assertk.assertions.hasLength import assertk.assertions.isEqualTo +import assertk.assertions.isFalse import assertk.assertions.isNotEqualTo +import assertk.assertions.isTrue import com.bazel_diff.SilentLogger import com.bazel_diff.log.Logger import java.io.File @@ -26,9 +28,11 @@ class GitClientTest : KoinTest { @get:Rule val temp: TemporaryFolder = TemporaryFolder() /** Runs git for test setup, returning trimmed stdout; fails the test on a non-zero exit. */ - private fun git(vararg args: String): String { - val proc = - ProcessBuilder(listOf("git") + args).directory(temp.root).redirectErrorStream(true).start() + private fun git(vararg args: String): String = runGit(temp.root, *args) + + /** Like [git] but in an explicit working directory (for multi-repo fetch tests). */ + private fun runGit(dir: File, vararg args: String): String { + val proc = ProcessBuilder(listOf("git") + args).directory(dir).redirectErrorStream(true).start() val output = proc.inputStream.readBytes().decodeToString() val code = proc.waitFor() check(code == 0) { "git ${args.joinToString(" ")} failed ($code): $output" } @@ -94,4 +98,60 @@ class GitClientTest : KoinTest { // `git fetch --all` is a no-op (exit 0) when there are no remotes, so it must not throw. ProcessGitClient(temp.root.toPath()).fetch() } + + @Test + fun fetchRevisionBringsInACommitNoBranchReaches() { + // A commit advertised only under refs/pull/* (a GitHub PR head) is reachable from no branch, so + // a broad `fetch --all` never brings it in: resolveSha stays missing until fetchRevision pulls + // that exact object. file:// forces a real pack transfer (a local-path clone would hardlink + // every object and mask the miss). + 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") + runGit(origin, "config", "uploadpack.allowReachableSHA1InWant", "true") + File(origin, "file.txt").writeText("one") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "first") + val branch = runGit(origin, "rev-parse", "--abbrev-ref", "HEAD") + + // A commit reachable only from refs/pull/7/head, not from any branch. + runGit(origin, "checkout", "-q", "--detach") + File(origin, "pr.txt").writeText("pr") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "pr head") + val prHead = runGit(origin, "rev-parse", "HEAD") + runGit(origin, "update-ref", "refs/pull/7/head", prHead) + // Re-attach HEAD to the branch so the clone checks out the branch, not the detached PR commit. + runGit(origin, "checkout", "-q", branch) + + val workspace = File(temp.root, "workspace") + runGit(temp.root, "clone", "-q", "file://${origin.absolutePath}", workspace.absolutePath) + val client = ProcessGitClient(workspace.toPath()) + + // A broad fetch does not reach the PR head; resolveSha must still report it missing. + client.fetch() + assertThrows(MissingRevisionException::class.java) { client.resolveSha(prHead) } + + // The targeted fetch pulls the exact object, after which it resolves. + assertThat(client.fetchRevision(prHead)).isTrue() + assertThat(client.resolveSha(prHead)).isEqualTo(prHead) + } + + @Test + fun fetchRevisionReturnsFalseForUnknownSha() { + initRepoWithTwoCommits() + git("remote", "add", "origin", temp.root.absolutePath) + // A SHA no remote can supply: the best-effort targeted fetch reports failure rather than + // throwing, leaving the authoritative miss to resolveSha. + val absent = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + assertThat(ProcessGitClient(temp.root.toPath()).fetchRevision(absent)).isFalse() + } + + @Test + fun fetchRevisionReturnsFalseWithoutRemotes() { + initRepoWithTwoCommits() + // No remotes to ask -> nothing to fetch from -> false, and no throw. + assertThat(ProcessGitClient(temp.root.toPath()).fetchRevision("HEAD")).isFalse() + } } diff --git a/cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt index 97b8691..3f6ead0 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt @@ -83,14 +83,20 @@ class ImpactedTargetsServiceTest : KoinTest { override fun checkout(revision: String) = Unit } - /** [GitClient] whose [missing] revisions never resolve, even after a fetch. */ + /** [GitClient] whose [missing] revisions never resolve, even after a broad or targeted fetch. */ private class AlwaysMissingGitClient(private val missing: Set) : GitClient { var fetchCount = 0 + var fetchRevisionCount = 0 override fun fetch() { fetchCount++ } + override fun fetchRevision(revision: String): Boolean { + fetchRevisionCount++ + return false + } + override fun resolveSha(revision: String): String { if (revision in missing) throw MissingRevisionException(revision) return revision @@ -99,6 +105,36 @@ class ImpactedTargetsServiceTest : KoinTest { override fun checkout(revision: String) = Unit } + /** + * [GitClient] modeling a PR-head SHA: a broad [fetch] never brings [prHead] in (it is on no + * fetched ref); only a targeted [fetchRevision] does. + */ + private class DirectFetchGitClient(private val prHead: String) : GitClient { + var fetchCount = 0 + var fetchRevisionCount = 0 + private var directlyFetched = false + + override fun fetch() { + fetchCount++ + } + + override fun fetchRevision(revision: String): Boolean { + fetchRevisionCount++ + if (revision == prHead) { + directlyFetched = true + return true + } + return false + } + + override fun resolveSha(revision: String): String { + if (revision == prHead && !directlyFetched) throw MissingRevisionException(revision) + return revision + } + + override fun checkout(revision: String) = Unit + } + /** * [HashProvider] returning canned data. By default the module-change workspace path is treated as * an error (the pure hash-diff path must not touch the workspace); set [allowWorkspaceAt] to @@ -285,8 +321,8 @@ class ImpactedTargetsServiceTest : KoinTest { @Test fun propagatesMissingRevisionAfterRefetchStillFails() { - // A genuinely unknown revision: one refetch is attempted, then the miss propagates (the HTTP - // layer maps it to a 400). + // A genuinely unknown revision: a broad refetch AND a targeted by-SHA fetch are both attempted, + // then the miss propagates (the HTTP layer maps it to a 400). val git = AlwaysMissingGitClient(missing = setOf("to-sha")) val service = ImpactedTargetsService(git, FakeHashProvider(emptyMap())) @@ -296,5 +332,23 @@ class ImpactedTargetsServiceTest : KoinTest { } assertThat(ex.revision).isEqualTo("to-sha") assertThat(git.fetchCount).isEqualTo(1) + assertThat(git.fetchRevisionCount).isEqualTo(1) + } + + @Test + fun fetchesUnreachableRevisionDirectlyWhenBroadFetchDoesNotBringItIn() { + // The `to` SHA is a PR-head commit reachable from no fetched ref, so a broad `git fetch --all` + // never brings it in. Rather than 400, the service must fall back to a targeted by-SHA fetch. + val from = HashFileData(mapOf("//:a" to TargetHash("Rule", "h1", "d1")), null) + val to = HashFileData(mapOf("//:a" to TargetHash("Rule", "h2", "d2")), null) + val git = DirectFetchGitClient(prHead = "to-sha") + val service = + ImpactedTargetsService(git, FakeHashProvider(mapOf("from-sha" to from, "to-sha" to to))) + + val result = service.getImpactedTargets("from-sha", "to-sha", null) + + assertThat(result.impactedTargets).isEqualTo(listOf("//:a")) + assertThat(git.fetchCount).isEqualTo(1) // broad fetch tried first + assertThat(git.fetchRevisionCount).isEqualTo(1) // then the targeted by-SHA fetch } } 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 ec876f6..20a3f7e 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/JGitClientTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/JGitClientTest.kt @@ -3,7 +3,9 @@ package com.bazel_diff.server import assertk.assertThat import assertk.assertions.hasLength import assertk.assertions.isEqualTo +import assertk.assertions.isFalse import assertk.assertions.isNotEqualTo +import assertk.assertions.isTrue import assertk.assertions.messageContains import com.bazel_diff.SilentLogger import com.bazel_diff.log.Logger @@ -179,12 +181,55 @@ class JGitClientTest : KoinTest { 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) + 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 fetchRevisionBringsInACommitNoBranchReachesViaNativeGit() { + // JGit routes by-SHA fetch through native git; a PR-head commit (reachable from no branch) is + // absent after a broad fetch and arrives only via the targeted fetchRevision. file:// forces a + // real pack transfer so the object is genuinely missing until then. + 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") + runGit(origin, "config", "uploadpack.allowReachableSHA1InWant", "true") + File(origin, "file.txt").writeText("one") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "first") + val branch = runGit(origin, "rev-parse", "--abbrev-ref", "HEAD") + + // A commit reachable only from refs/pull/7/head, not from any branch. + runGit(origin, "checkout", "-q", "--detach") + File(origin, "pr.txt").writeText("pr") + runGit(origin, "add", ".") + runGit(origin, "commit", "-q", "-m", "pr head") + val prHead = runGit(origin, "rev-parse", "HEAD") + runGit(origin, "update-ref", "refs/pull/7/head", prHead) + // Re-attach HEAD to the branch so the clone checks out the branch, not the detached PR commit. + runGit(origin, "checkout", "-q", branch) + + val workspace = File(temp.root, "workspace") + runGit(temp.root, "clone", "-q", "file://${origin.absolutePath}", workspace.absolutePath) + val client = JGitClient(workspace.toPath()) + + client.fetch() + assertThrows(MissingRevisionException::class.java) { client.resolveSha(prHead) } + assertThat(client.fetchRevision(prHead)).isTrue() + assertThat(client.resolveSha(prHead)).isEqualTo(prHead) + } + + @Test + fun fetchRevisionReturnsFalseWhenNativeFallbackDisabled() { + initRepoWithTwoCommits() + // With the native fallback off, the in-process engine will not attempt an unreliable JGit + // by-SHA fetch; it reports failure so the caller surfaces the revision as missing. + val client = JGitClient(temp.root.toPath(), gitPath = "git", nativeFetchFallback = false) + assertThat(client.fetchRevision("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef")).isFalse() + } + @Test fun fetchBringsInNewCommitForShallowClone() { // A shallow clone is a shape JGit 5.13 may be unable to fetch (the remote's thin pack can be diff --git a/tools/serve_harness.py b/tools/serve_harness.py index ca14d54..adeb018 100755 --- a/tools/serve_harness.py +++ b/tools/serve_harness.py @@ -9,6 +9,7 @@ * 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 targeted by-SHA path -- a commit reachable only via refs/pull/* that a broad fetch misses * 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 @@ -241,6 +242,19 @@ def commit(self, msg: str, core=None, mid=None, other=None) -> str: git(self.origin, "commit", "-q", "-m", msg) return git(self.origin, "rev-parse", "HEAD") + def pr_head_commit(self, msg: str, pr: int, base: str, core=None, mid=None, other=None) -> str: + """Create a commit advertised ONLY under refs/pull//head (like a GitHub PR head), + reachable from no branch. A clone's default refspec covers only refs/heads/*, so a broad + `git fetch --all` cannot bring it in -- resolving it requires the targeted by-SHA fetch.""" + tmp = f"_pr{pr}" + git(self.origin, "checkout", "-q", "-b", tmp, base) + sha = self.commit(msg, core=core, mid=mid, other=other) + git(self.origin, "update-ref", f"refs/pull/{pr}/head", sha) + # Drop the branch so only the pull ref reaches the commit; leave the tree on DEFAULT_BRANCH. + git(self.origin, "checkout", "-q", DEFAULT_BRANCH) + git(self.origin, "branch", "-q", "-D", tmp) + return sha + 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. @@ -546,6 +560,17 @@ def case_full(remote: Remote, clone: Path, root: Path, rep: Report, engine: str, ok_detail=str(sorted(got)), fail_detail=f"code={code} got={sorted(got)} raw={sorted(_iset(data))}\n{s.stderr_tail()}") + # On-demand targeted fetch of a commit reachable only via refs/pull/7/head (a GitHub PR + # head): a broad `git fetch --all` cannot bring it in, so this is the exact case that used to + # 400 with "revision ... is missing from the local clone". It must now resolve via the + # targeted by-SHA fetch and return the same set as the equivalent branch edit (C4). + code, body, data = _impacted(s, sh["C2"], sh["C5"]) + got = _stable(data) + rep.check(case, "targeted fetch pr-head commit C2->C5 (refs/pull/*, 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) @@ -745,6 +770,10 @@ def main() -> int: 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) + # C5: reachable only via refs/pull/7/head (a GitHub PR head), so a broad fetch cannot bring + # it in -- resolving it exercises the targeted by-SHA fetch fallback (the reported bug). + remote.shas["C5"] = remote.pr_head_commit( + "C5 edit other (pr head)", pr=7, base=remote.shas["C2"], other="other v2\n") remote.repack() # force new blobs to delta against the most similar (possibly out-of-clone) base vlog(f"shas: {remote.shas}")