diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelModService.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelModService.kt index 22198653..f831a50a 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelModService.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelModService.kt @@ -1,8 +1,12 @@ package com.bazel_diff.bazel +import com.bazel_diff.extensions.toHexString +import com.bazel_diff.hash.sha256 import com.bazel_diff.log.Logger import com.bazel_diff.process.Redirect import com.bazel_diff.process.process +import java.io.File +import java.nio.charset.StandardCharsets import java.nio.file.Path import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking @@ -109,6 +113,133 @@ class BazelModService( } } + /** + * Computes a stable fingerprint of the currently resolved external dependency state. + * + * The hash includes: + * - bzlmod mode marker + `bazel mod graph --output=json` + * - repository-definition bytes from `bazel mod show_repo` (streamed proto when available) + */ + suspend fun getDependencyFingerprint(): String? { + if (!isBzlmodEnabled) { + return sha256 { putBytes("mode:legacy".toByteArray(StandardCharsets.UTF_8)) }.toHexString() + } + + val moduleGraphJson = getModuleGraphJson() ?: "" + val canonicalRepos = discoverCanonicalBzlmodRepos() + + val streamedShowRepo = showRepoStreamedProto(canonicalRepos) + if (streamedShowRepo != null && streamedShowRepo.exitCode == 0) { + return sha256 { + putBytes("mode:bzlmod\n".toByteArray(StandardCharsets.UTF_8)) + putBytes("moduleGraphJson:".toByteArray(StandardCharsets.UTF_8)) + putBytes(moduleGraphJson.toByteArray(StandardCharsets.UTF_8)) + putBytes("\nshowRepo:\n".toByteArray(StandardCharsets.UTF_8)) + putBytes(streamedShowRepo.stdout) + } + .toHexString() + } + + val showRepoText = resolveShowRepoTextFallback(canonicalRepos) ?: return null + return sha256 { + putBytes("mode:bzlmod\n".toByteArray(StandardCharsets.UTF_8)) + putBytes("moduleGraphJson:".toByteArray(StandardCharsets.UTF_8)) + putBytes(moduleGraphJson.toByteArray(StandardCharsets.UTF_8)) + putBytes("\nshowRepoText:\n".toByteArray(StandardCharsets.UTF_8)) + putBytes(showRepoText.toByteArray(StandardCharsets.UTF_8)) + } + .toHexString() + } + + /** + * Returns canonical bzlmod repo names in @@ form, discovered from + * `bazel mod dump_repo_mapping ""`. + */ + private fun discoverCanonicalBzlmodRepos(): List { + val output = runBazelRaw(listOf("mod", "dump_repo_mapping", "")) ?: return emptyList() + if (output.exitCode != 0) { + return emptyList() + } + return String(output.stdout, StandardCharsets.UTF_8) + .lineSequence() + .mapNotNull { line -> parseCanonicalRepoNames(line) } + .flatten() + .filter { it.contains('+') || it.contains('~') } + .map { "@@$it" } + .toSet() + .sorted() + } + + private fun parseCanonicalRepoNames(line: String): List? { + val parsed = runCatching { + @Suppress("UNCHECKED_CAST") + com.google.gson.Gson().fromJson(line.trim(), Map::class.java) as Map + } + if (parsed.isFailure) return null + return parsed.getOrNull()?.values?.mapNotNull { it as? String } + } + + private fun showRepoStreamedProto(canonicalRepos: List): RawCommandResult? { + val args = mutableListOf("mod", "show_repo") + if (canonicalRepos.isNotEmpty()) { + args.addAll(canonicalRepos) + } + args.add("--output=streamed_proto") + return runBazelRaw(args) + } + + private fun resolveShowRepoTextFallback(canonicalRepos: List): String? { + val allVisible = runBazelRaw(listOf("mod", "show_repo", "--all_visible_repos", "--output=text")) + if (allVisible != null && allVisible.exitCode == 0) { + return String(allVisible.stdout, StandardCharsets.UTF_8) + } + + val args = mutableListOf("mod", "show_repo") + if (canonicalRepos.isNotEmpty()) { + args.addAll(canonicalRepos) + } + args.add("--output=text") + val fallback = runBazelRaw(args) ?: return null + if (fallback.exitCode != 0) { + return null + } + return String(fallback.stdout, StandardCharsets.UTF_8) + } + + private data class RawCommandResult( + val exitCode: Int, + val stdout: ByteArray, + ) + + private fun runBazelRaw(args: List): RawCommandResult? { + val command = + mutableListOf().apply { + add(bazelPath.toString()) + if (noBazelrc) { + add("--bazelrc=/dev/null") + } + addAll(startupOptions) + addAll(args) + } + return try { + val nullDevice = if (System.getProperty("os.name").startsWith("Windows")) "NUL" else "/dev/null" + val process = + ProcessBuilder(command) + .directory(workingDirectory.toFile()) + .redirectError(ProcessBuilder.Redirect.to(File(nullDevice))) + .start() + val stdout = process.inputStream.readBytes() + val exitCode = process.waitFor() + if (exitCode != 0) { + logger.w { "Command failed (exit=$exitCode): ${command.joinToString(" ")}" } + } + RawCommandResult(exitCode = exitCode, stdout = stdout) + } catch (e: Exception) { + logger.w { "Failed to execute ${command.joinToString(" ")}: ${e.message}" } + null + } + } + @OptIn(ExperimentalCoroutinesApi::class) private suspend fun checkBzlmodEnabled(): Boolean { val cmd = diff --git a/cli/src/main/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractor.kt b/cli/src/main/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractor.kt index 8146366f..d9742f6a 100644 --- a/cli/src/main/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractor.kt +++ b/cli/src/main/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractor.kt @@ -12,7 +12,8 @@ import org.koin.core.component.inject data class HashFileData( val hashes: Map, val moduleGraphJson: String?, - val depEdges: Map> = emptyMap() + val depEdges: Map> = emptyMap(), + val dependencyFingerprint: String? = null, ) class DeserialiseHashesInteractor : KoinComponent { @@ -46,6 +47,7 @@ class DeserialiseHashesInteractor : KoinComponent { val metadata = jsonObject.getAsJsonObject("metadata") val moduleGraphJson = metadata?.get("moduleGraphJson")?.asString + val dependencyFingerprint = metadata?.get("dependencyFingerprint")?.asString // The query service persists the dependency-edge adjacency list (label -> direct dep labels) // under metadata.depEdges when started with --trackDeps, so build-graph distance metrics can @@ -56,7 +58,7 @@ class DeserialiseHashesInteractor : KoinComponent { gson.fromJson>>(it, depShape) } ?: emptyMap() - return HashFileData(hashes, moduleGraphJson, depEdges) + return HashFileData(hashes, moduleGraphJson, depEdges, dependencyFingerprint) } else { // Legacy format - just a flat map of hashes val shape = object : TypeToken>() {}.type diff --git a/cli/src/main/kotlin/com/bazel_diff/server/HashService.kt b/cli/src/main/kotlin/com/bazel_diff/server/HashService.kt index 9c39cada..10283f40 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/HashService.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/HashService.kt @@ -115,6 +115,11 @@ class HashService( val generation: HashGenerationBreakdown? = null, ) + private data class GuardState( + var currentDependencyFingerprint: String? = null, + var computed: Boolean = false, + ) + override fun getHashes( sha: String, modifiedFilepaths: Set, @@ -135,20 +140,11 @@ class HashService( /** * Returns the hash data plus whether it was served from the cache. "Hit" includes the - * waited-behind-another-generation case (the after-lock re-check): this request itself ran no - * checkout/query, though its duration then includes the lock wait. + * waited-behind-another-generation case (the after-lock re-check). Cache hits may still do a + * checkout to validate the dependency fingerprint, but they do not rerun the hasher. */ private fun retrieve(sha: String, modifiedFilepaths: Set): Retrieval { val key = cacheKey(sha, modifiedFilepaths) - val readStartNanos = System.nanoTime() - storage.get(key)?.let { bytes -> - val data = - deserialiser.executeTargetHashWithMetadataFromString( - String(bytes, StandardCharsets.UTF_8)) - val readMillis = elapsedMillis(readStartNanos) - logger.i { "Hash cache hit for $sha (read+deserialize ${readMillis}ms)" } - return Retrieval(data, cacheHit = true, cacheReadMillis = readMillis) - } return generate(sha, modifiedFilepaths, key) } @@ -160,6 +156,7 @@ class HashService( private fun generate(sha: String, modifiedFilepaths: Set, key: String): Retrieval { val lockStartNanos = System.nanoTime() + val guard = GuardState() synchronized(generationLock) { val lockWaitMillis = elapsedMillis(lockStartNanos) // Re-check under the lock: another thread may have generated this revision while we waited. @@ -168,17 +165,19 @@ class HashService( val data = deserialiser.executeTargetHashWithMetadataFromString(String(it, StandardCharsets.UTF_8)) val readMillis = elapsedMillis(readStartNanos) - logger.i { - "Hash cache hit for $sha (after ${lockWaitMillis}ms lock wait, " + - "read+deserialize ${readMillis}ms)" + if (cacheEntryMatchesDependencyFingerprint(sha, data, guard)) { + logger.i { + "Hash cache hit for $sha (after ${lockWaitMillis}ms lock wait, " + + "read+deserialize ${readMillis}ms)" + } + return Retrieval( + data, cacheHit = true, lockWaitMillis = lockWaitMillis, cacheReadMillis = readMillis) } - return Retrieval( - data, cacheHit = true, lockWaitMillis = lockWaitMillis, cacheReadMillis = readMillis) } logger.i { "Hash cache miss for $sha - generating hashes" } val checkoutStartNanos = System.nanoTime() - gitClient.checkout(sha) + ensureWorkspaceAndFingerprintForSha(sha, guard) val checkoutMillis = elapsedMillis(checkoutStartNanos) val hasherTimings = HasherPhaseTimings() @@ -192,8 +191,12 @@ class HashService( val writeStartNanos = System.nanoTime() val depEdges = depEdgesOf(hashes) + val dependencyFingerprint = + guard.currentDependencyFingerprint ?: runBlocking { bazelModService.getDependencyFingerprint() } storage.put( - key, serialize(hashes, moduleGraphJson, depEdges).toByteArray(StandardCharsets.UTF_8)) + key, + serialize(hashes, moduleGraphJson, depEdges, dependencyFingerprint) + .toByteArray(StandardCharsets.UTF_8)) val cacheWriteMillis = elapsedMillis(writeStartNanos) val breakdown = @@ -218,13 +221,35 @@ class HashService( "targets=${hashes.size}" } return Retrieval( - HashFileData(hashes, moduleGraphJson, depEdges), + HashFileData(hashes, moduleGraphJson, depEdges, dependencyFingerprint), cacheHit = false, lockWaitMillis = lockWaitMillis, generation = breakdown) } } + private fun cacheEntryMatchesDependencyFingerprint( + sha: String, + data: HashFileData, + guard: GuardState, + ): Boolean { + val cachedFingerprint = data.dependencyFingerprint + if (cachedFingerprint == null) { + return false + } + ensureWorkspaceAndFingerprintForSha(sha, guard) + val current = guard.currentDependencyFingerprint + return current != null && current == cachedFingerprint + } + + private fun ensureWorkspaceAndFingerprintForSha(sha: String, guard: GuardState) { + if (guard.computed) return + gitClient.checkout(sha) + guard.currentDependencyFingerprint = runBlocking { bazelModService.getDependencyFingerprint() } + guard.computed = true + } + + private fun elapsedMillis(startNanos: Long): Long = (System.nanoTime() - startNanos) / 1_000_000 /** @@ -245,14 +270,16 @@ class HashService( private fun serialize( hashes: Map, moduleGraphJson: String?, - depEdges: Map> + depEdges: Map>, + dependencyFingerprint: String?, ): String { val serializedHashes = hashes.mapValues { it.value.toJson(true) } val output = - if (moduleGraphJson != null || depEdges.isNotEmpty()) { + if (moduleGraphJson != null || depEdges.isNotEmpty() || dependencyFingerprint != null) { val metadata = mutableMapOf() if (moduleGraphJson != null) metadata["moduleGraphJson"] = moduleGraphJson if (depEdges.isNotEmpty()) metadata["depEdges"] = depEdges + if (dependencyFingerprint != null) metadata["dependencyFingerprint"] = dependencyFingerprint mapOf("hashes" to serializedHashes, "metadata" to metadata) } else { serializedHashes diff --git a/cli/src/test/kotlin/com/bazel_diff/server/HashServiceTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/HashServiceTest.kt index e4eaeaa6..dfbf69e3 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/HashServiceTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/HashServiceTest.kt @@ -87,6 +87,10 @@ class HashServiceTest : KoinTest { private fun newService(git: GitClient, storage: HashCacheStorage, trackDeps: Boolean = false) = HashService(git, storage, "fp", emptySet(), emptySet(), trackDeps) + private fun stubDependencyFingerprint(value: String = "dep-fp") { + runBlocking { whenever(bazelModService.getDependencyFingerprint()).thenReturn(value) } + } + @Test fun cacheMissGeneratesAndStores() { whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull())) @@ -144,6 +148,7 @@ class HashServiceTest : KoinTest { whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull())) .thenReturn(sampleHashes) runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn(null) } + stubDependencyFingerprint() val git = RecordingGitClient() val storage = InMemoryStorage() val service = newService(git, storage) @@ -152,8 +157,9 @@ class HashServiceTest : KoinTest { val second = service.getHashes("sha1") assertThat(second.hashes).isEqualTo(sampleHashes) - // Only the first call touches the workspace / runs the hasher. - assertThat(git.checkouts).isEqualTo(listOf("sha1")) + // Cache hits still checkout to validate the dependency fingerprint, but they do not rerun the + // hasher. + assertThat(git.checkouts).isEqualTo(listOf("sha1", "sha1")) verify(buildGraphHasher, times(1)) .hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull()) } @@ -163,6 +169,7 @@ class HashServiceTest : KoinTest { whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull())) .thenReturn(sampleHashes) runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn(null) } + stubDependencyFingerprint() val service = newService(RecordingGitClient(), InMemoryStorage()) val missProfiler = QueryProfiler() @@ -204,6 +211,7 @@ class HashServiceTest : KoinTest { whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull())) .thenReturn(sampleHashes) runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn("""{"graph":1}""") } + stubDependencyFingerprint() val storage = InMemoryStorage() // Generate once, then read back through a fresh service over the same storage (cache hit path). @@ -219,6 +227,7 @@ class HashServiceTest : KoinTest { whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull())) .thenReturn(sampleHashesWithDeps) runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn(null) } + stubDependencyFingerprint() val storage = InMemoryStorage() val generated = newService(RecordingGitClient(), storage, trackDeps = true).getHashes("sha1") @@ -281,6 +290,8 @@ class HashServiceTest : KoinTest { @Test fun deserializeLegacyFlatCacheEntry() { + whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull())) + .thenReturn(sampleHashes) val storage = InMemoryStorage() storage.entries["sha1.fp"] = """{"//:a":"Rule#h~d"}""".toByteArray(StandardCharsets.UTF_8) @@ -289,8 +300,8 @@ class HashServiceTest : KoinTest { assertThat(data.hashes).isEqualTo(sampleHashes) assertThat(data.moduleGraphJson).isNull() assertThat(data.depEdges).isEqualTo(emptyMap()) - // No generation on a pure cache hit. - verify(buildGraphHasher, times(0)) + // Legacy cache entries without dependencyFingerprint are now recomputed. + verify(buildGraphHasher, times(1)) .hashAllBazelTargetsAndSourcefiles(any(), any(), any(), anyOrNull()) } @@ -307,6 +318,7 @@ class HashServiceTest : KoinTest { sampleHashes } runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn(null) } + stubDependencyFingerprint() val service = newService(RecordingGitClient(), InMemoryStorage()) val missDone = CountDownLatch(1) diff --git a/src/bazel.rs b/src/bazel.rs index 8aab1b5a..9dd18337 100644 --- a/src/bazel.rs +++ b/src/bazel.rs @@ -11,6 +11,7 @@ use std::fs::{self, File}; use std::io::{BufRead, BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::time::Instant; use tempfile::NamedTempFile; #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] @@ -73,6 +74,54 @@ impl BazelOptions { .then(|| String::from_utf8_lossy(&output.stdout).trim().to_owned()) } + pub fn dependency_fingerprint(&self) -> Result { + const STREAMED_VERSION: &str = "bzlmod-streamed-proto-v2"; + const TEXT_FALLBACK_VERSION: &str = "bzlmod-show-repo-text-v1"; + let total_start = Instant::now(); + let mut hasher = Sha256::new(); + if !self.is_bzlmod_enabled() { + hasher.update(b"mode:legacy"); + eprintln!( + "[BD-DBG][fingerprint-ms] mode=legacy version={TEXT_FALLBACK_VERSION} mapping=0 show_repo=0 total={}", + total_start.elapsed().as_millis() + ); + return Ok(hex::encode(hasher.finalize())); + } + + hasher.update(b"mode:bzlmod\0"); + let module_graph = self.module_graph_json().unwrap_or_default(); + hasher.update(module_graph.as_bytes()); + hasher.update(b"\0"); + + let mapping_start = Instant::now(); + let canonical_names = self.canonical_bzlmod_repo_names()?; + let mapping_millis = mapping_start.elapsed().as_millis(); + + let show_repo_start = Instant::now(); + let streamed = self.show_repo_streamed_proto(&canonical_names); + let show_repo_millis = show_repo_start.elapsed().as_millis(); + if let Ok(repo_state) = streamed { + hasher.update(&repo_state); + eprintln!( + "[BD-DBG][fingerprint-ms] mode=bzlmod version={STREAMED_VERSION} mapping={mapping_millis} show_repo={show_repo_millis} total={} repoCount={} fallback=false", + total_start.elapsed().as_millis(), + canonical_names.len(), + ); + return Ok(hex::encode(hasher.finalize())); + } + + let fallback_show_repo_start = Instant::now(); + let repo_state = self.show_repo_fingerprint_text_fallback(&canonical_names)?; + let fallback_show_repo_millis = fallback_show_repo_start.elapsed().as_millis(); + hasher.update(repo_state.as_bytes()); + eprintln!( + "[BD-DBG][fingerprint-ms] mode=bzlmod version={TEXT_FALLBACK_VERSION} mapping={mapping_millis} show_repo={fallback_show_repo_millis} total={} repoCount={} fallback=true", + total_start.elapsed().as_millis(), + canonical_names.len(), + ); + Ok(hex::encode(hasher.finalize())) + } + fn version(&self) -> Result { let output = self.run_capture(&["version"])?; if !output.status.success() { @@ -351,6 +400,87 @@ impl BazelOptions { .map(str::to_owned) .collect()) } + + fn canonical_bzlmod_repo_names(&self) -> Result> { + let mapping_output = self.run_capture(&["mod", "dump_repo_mapping", ""])?; + if !mapping_output.status.success() { + bail!("bazel mod dump_repo_mapping failed"); + } + Ok( + parse_repo_mapping(&String::from_utf8_lossy(&mapping_output.stdout)) + .keys() + .filter(|name| name.contains('+') || name.contains('~')) + .map(|name| format!("@@{name}")) + .collect::>(), + ) + } + + fn show_repo_streamed_proto(&self, canonical_names: &[String]) -> Result> { + if canonical_names.is_empty() { + return Ok(Vec::new()); + } + let mut command = self.command(); + command + .arg("mod") + .arg("show_repo") + .args(canonical_names) + .arg("--output=streamed_proto") + .stdin(Stdio::null()) + .stderr(Stdio::piped()); + eprintln!("[BD-DBG][fingerprint-command] path=streamed_proto_selected cmd={command:?}"); + if self.verbose { + eprintln!("[Info] Command: {command:?}"); + } + let output = command + .output() + .context("execute bazel mod show_repo streamed_proto")?; + if !output.status.success() { + bail!( + "bazel mod show_repo --output=streamed_proto failed with {}", + output.status + ); + } + Ok(output.stdout) + } + + fn show_repo_fingerprint_text_fallback(&self, canonical_names: &[String]) -> Result { + let mut all_visible_command = self.command(); + all_visible_command + .arg("mod") + .arg("show_repo") + .arg("--all_visible_repos") + .arg("--output=text"); + eprintln!( + "[BD-DBG][fingerprint-command] path=text_all_visible cmd={all_visible_command:?}" + ); + let output = all_visible_command + .stdin(Stdio::null()) + .stderr(Stdio::piped()) + .output(); + if let Ok(output) = output { + if output.status.success() { + return Ok(String::from_utf8_lossy(&output.stdout).into_owned()); + } + } + + let mut command = self.command(); + command.arg("mod").arg("show_repo"); + if canonical_names.is_empty() { + command.arg("--output=text"); + } else { + command.args(canonical_names).arg("--output=text"); + } + eprintln!("[BD-DBG][fingerprint-command] path=text_selected_repos cmd={command:?}"); + command.stdin(Stdio::null()).stderr(Stdio::piped()); + let output = command.output().context("execute bazel mod show_repo")?; + if !output.stderr.is_empty() && self.verbose { + eprint!("{}", String::from_utf8_lossy(&output.stderr)); + } + if !output.status.success() { + bail!("bazel mod show_repo failed with {}", output.status); + } + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + } } fn parse_repo_mapping(text: &str) -> BTreeMap> { diff --git a/src/hash.rs b/src/hash.rs index c615786d..ca244e92 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -336,6 +336,7 @@ fn hash_targets_with_environment( Ok(HashFileData { hashes, module_graph_json, + dependency_fingerprint: None, dep_edges, }) } diff --git a/src/model.rs b/src/model.rs index d98e9171..b3fcb588 100644 --- a/src/model.rs +++ b/src/model.rs @@ -107,11 +107,16 @@ impl Serialize for SerializedMetadata<'_> { let include_dep_edges = self.include_deps && !self.data.dep_edges.is_empty(); let mut state = serializer.serialize_struct( "HashMetadata", - usize::from(self.data.module_graph_json.is_some()) + usize::from(include_dep_edges), + usize::from(self.data.module_graph_json.is_some()) + + usize::from(include_dep_edges) + + usize::from(self.data.dependency_fingerprint.is_some()), )?; if let Some(module_graph_json) = &self.data.module_graph_json { state.serialize_field("moduleGraphJson", module_graph_json)?; } + if let Some(dependency_fingerprint) = &self.data.dependency_fingerprint { + state.serialize_field("dependencyFingerprint", dependency_fingerprint)?; + } if include_dep_edges { state.serialize_field("depEdges", &self.data.dep_edges)?; } @@ -135,6 +140,7 @@ impl Serialize for SerializedHashFile<'_> { include_target_type: self.include_target_type, }; if self.data.module_graph_json.is_none() + && self.data.dependency_fingerprint.is_none() && (!self.include_deps || self.data.dep_edges.is_empty()) { return hashes.serialize(serializer); @@ -156,6 +162,7 @@ impl Serialize for SerializedHashFile<'_> { pub struct HashFileData { pub hashes: BTreeMap, pub module_graph_json: Option, + pub dependency_fingerprint: Option, pub dep_edges: BTreeMap>, } @@ -205,6 +212,10 @@ impl HashFileData { .and_then(|value| value.get("moduleGraphJson")) .and_then(Value::as_str) .map(str::to_owned); + let dependency_fingerprint = metadata + .and_then(|value| value.get("dependencyFingerprint")) + .and_then(Value::as_str) + .map(str::to_owned); let dep_edges = metadata .and_then(|value| value.get("depEdges")) .map(|value| serde_json::from_value(value.clone())) @@ -214,6 +225,7 @@ impl HashFileData { Ok(Self { hashes, module_graph_json, + dependency_fingerprint, dep_edges, }) } @@ -229,7 +241,10 @@ impl HashFileData { ) }) .collect::>(); - if self.module_graph_json.is_none() && (!include_deps || self.dep_edges.is_empty()) { + if self.module_graph_json.is_none() + && self.dependency_fingerprint.is_none() + && (!include_deps || self.dep_edges.is_empty()) + { return Value::Object(hashes); } let mut metadata = Map::new(); @@ -239,6 +254,12 @@ impl HashFileData { Value::String(module_graph_json.clone()), ); } + if let Some(dependency_fingerprint) = &self.dependency_fingerprint { + metadata.insert( + "dependencyFingerprint".to_owned(), + Value::String(dependency_fingerprint.clone()), + ); + } if include_deps && !self.dep_edges.is_empty() { metadata.insert( "depEdges".to_owned(), @@ -538,6 +559,13 @@ mod tests { data.dep_edges = BTreeMap::from([("//a:a".into(), vec!["//b:b".into()])]); let streamed = serde_json::to_value(data.serialized(true, true)).unwrap(); assert_eq!(streamed, data.to_value(true, true)); + + data.module_graph_json = None; + data.dep_edges.clear(); + data.dependency_fingerprint = Some("dep-fp".into()); + let streamed = serde_json::to_value(data.serialized(true, false)).unwrap(); + assert_eq!(streamed, data.to_value(true, false)); + assert!(streamed.get("metadata").is_some()); } #[test] @@ -545,16 +573,19 @@ mod tests { let legacy = HashFileData::from_slice(br#"{"//a:a":"Rule#overall~direct"}"#).unwrap(); assert_eq!(legacy.hashes["//a:a"], target("Rule", "overall", "direct")); assert!(legacy.module_graph_json.is_none()); + assert!(legacy.dependency_fingerprint.is_none()); let bytes = br#"{ "hashes": {"//a:a": "Rule#overall~direct"}, "metadata": { "moduleGraphJson": "{\"root\":true}", + "dependencyFingerprint": "dep-fp", "depEdges": {"//a:a": ["//b:b"]} } }"#; let parsed = HashFileData::from_slice(bytes).unwrap(); assert_eq!(parsed.module_graph_json.as_deref(), Some("{\"root\":true}")); + assert_eq!(parsed.dependency_fingerprint.as_deref(), Some("dep-fp")); assert_eq!(parsed.dep_edges["//a:a"], ["//b:b"]); let mut file = tempfile::NamedTempFile::new().unwrap(); diff --git a/src/server.rs b/src/server.rs index 49e311cb..c0f298dc 100644 --- a/src/server.rs +++ b/src/server.rs @@ -5,7 +5,7 @@ use crate::model::{ use crate::module_graph::impacted_with_module_changes; use anyhow::{anyhow, bail, Context, Result}; use s3::creds::Credentials; -use s3::request::ResponseData; +use s3::error::S3Error; use s3::{Bucket, Region}; use serde::Deserialize; use serde_json::{json, Value}; @@ -90,53 +90,47 @@ impl RemoteCache { format!("{}{key}.json", self.prefix) } - /// Reads an entry, degrading to a miss on any failure. - /// - /// Deliberately a bare `GetObject` with no `HeadObject` probe in front of it: besides costing a - /// second round trip on every hit, `head_object` cannot report a miss here. S3 answers a HEAD - /// miss with `404`, `Transfer-Encoding: chunked` and no body, and every HTTP client on this - /// stack fails reading that absent body -- `attohttpc: Io Error: unexpected end of file`. A GET - /// miss carries a real `NoSuchKey` body, so it reads back cleanly. fn get(&self, key: &str) -> Option> { - let failure = match self.bucket.get_object(self.object_key(key)) { - Ok(response) if response.status_code() == 200 => return Some(response.to_vec()), - Ok(response) if response.status_code() == 404 => return None, - Ok(response) => http_failure(&response), - Err(error) => error.to_string(), - }; - eprintln!( - "[Warn] S3 cache read of {} failed (treating as a miss): {failure}", - self.object_key(key) - ); - None + match self.bucket.get_object(self.object_key(key)) { + Ok(response) if response.status_code() == 200 => Some(response.to_vec()), + Ok(_) => None, + Err(error) if is_s3_not_found(&error) => None, + Err(error) => { + eprintln!( + "[Warn] S3 cache read of {} failed (treating as a miss): {error}", + self.object_key(key) + ); + None + } + } } fn put(&self, key: &str, data: &[u8]) { - let failure = match self.bucket.put_object(self.object_key(key), data) { - Ok(response) if (200..300).contains(&response.status_code()) => return, - Ok(response) => http_failure(&response), - Err(error) => error.to_string(), - }; - eprintln!( - "[Warn] S3 cache write of {} failed (entry not shared): {failure}", - self.object_key(key) - ); + if let Err(error) = self.bucket.put_object(self.object_key(key), data) { + eprintln!( + "[Warn] S3 cache write of {} failed (entry not shared): {error}", + self.object_key(key) + ); + } + } + + fn contains(&self, key: &str) -> bool { + match self.bucket.head_object(self.object_key(key)) { + Ok((_, status)) => status == 200, + Err(error) if is_s3_not_found(&error) => false, + Err(error) => { + eprintln!( + "[Warn] S3 cache check of {} failed (treating as a miss): {error}", + self.object_key(key) + ); + false + } + } } } -/// Renders a non-2xx response for a warning, matching how `S3Error::HttpFailWithBody` reads. -/// -/// Statuses are classified here rather than by `rust-s3`'s `fail-on-err` feature, which is -/// deliberately off: it turns every non-2xx into an error *after* reading the body, and the read -/// happens inside `rust-s3`'s `retry!`, so a plain 404 miss cost a duplicate request and a -/// one-second backoff sleep before being recognized as a miss. `retry!` still covers transport -/// errors, which are the ones worth retrying. -fn http_failure(response: &ResponseData) -> String { - format!( - "Got HTTP {} with content '{}'", - response.status_code(), - response.as_str().unwrap_or("").trim() - ) +fn is_s3_not_found(error: &S3Error) -> bool { + matches!(error, S3Error::HttpFailWithBody(404, _)) } fn normalize_s3_prefix(prefix: &str) -> String { @@ -573,23 +567,64 @@ fn get_hashes_locked( ) -> Result<(HashFileData, bool)> { let key = cache_key(state, sha, modified); let path = state.config.cache_dir.join(format!("{key}.json")); + let mut current_dependency_fingerprint = None; if path.is_file() { let data = HashFileData::read(&path)?; - touch(&path); - return Ok((data, true)); + if cache_entry_matches_dependency_fingerprint( + state, + sha, + "local", + &data, + &mut current_dependency_fingerprint, + )? { + eprintln!("[BD-DBG][cache-hit] scope=local sha={sha} key={key}"); + touch(&path); + return Ok((data, true)); + } + eprintln!("[BD-DBG][cache-guard-miss] scope=local sha={sha} key={key}"); } if let Some(remote) = &state.remote { - if let Some(bytes) = remote.get(&key) { - let data = HashFileData::from_slice(&bytes)?; - fs::write(&path, &bytes)?; - return Ok((data, true)); + if remote.contains(&key) { + if let Some(bytes) = remote.get(&key) { + let data = HashFileData::from_slice(&bytes)?; + if cache_entry_matches_dependency_fingerprint( + state, + sha, + "remote", + &data, + &mut current_dependency_fingerprint, + )? { + eprintln!("[BD-DBG][cache-hit] scope=remote sha={sha} key={key}"); + fs::write(&path, &bytes)?; + return Ok((data, true)); + } + eprintln!("[BD-DBG][cache-guard-miss] scope=remote sha={sha} key={key}"); + } } } + eprintln!("[BD-DBG][cache-recompute] sha={sha} key={key}"); checkout(state, sha)?; let mut options = state.config.hash_options.clone(); options.modified_filepaths = modified.clone(); options.track_deps = state.config.track_deps; - let data = generate_hashes(&options)?; + let mut data = generate_hashes(&options)?; + data.dependency_fingerprint = + current_dependency_fingerprint.take().or_else( + || match dependency_fingerprint_for_workspace(state) { + Ok(value) => Some(value), + Err(error) => { + eprintln!("[Warn] {error:#}"); + None + } + }, + ); + match data.dependency_fingerprint.as_deref() { + Some(fingerprint) => eprintln!( + "[BD-DBG][cache-write-fingerprint] sha={sha} key={key} fp={}", + short_fingerprint(fingerprint) + ), + None => eprintln!("[BD-DBG][cache-write-fingerprint] sha={sha} key={key} fp=NONE"), + } let bytes = serde_json::to_vec(&data.serialized(true, state.config.track_deps))?; let temporary = state.config.cache_dir.join(format!("{key}.tmp")); fs::write(&temporary, bytes)?; @@ -601,6 +636,62 @@ fn get_hashes_locked( Ok((data, false)) } +fn cache_entry_matches_dependency_fingerprint( + state: &Arc, + sha: &str, + scope: &str, + data: &HashFileData, + current_dependency_fingerprint: &mut Option, +) -> Result { + let Some(cached_fingerprint) = data.dependency_fingerprint.as_deref() else { + eprintln!("[BD-DBG][cache-guard-no-fingerprint] scope={scope} sha={sha} policy=recompute"); + return Ok(false); + }; + if current_dependency_fingerprint.is_none() { + checkout(state, sha)?; + *current_dependency_fingerprint = match dependency_fingerprint_for_workspace(state) { + Ok(value) => Some(value), + Err(error) => { + eprintln!("[Warn] {error:#}"); + None + } + }; + } + Ok(match current_dependency_fingerprint.as_deref() { + Some(current) => { + let matched = current == cached_fingerprint; + eprintln!( + "[BD-DBG][cache-guard-check] scope={scope} sha={sha} cachedFp={} currentFp={} matched={matched}", + short_fingerprint(cached_fingerprint), + short_fingerprint(current) + ); + matched + } + None => { + eprintln!( + "[BD-DBG][cache-guard-check] scope={scope} sha={sha} cachedFp={} currentFp=NONE matched=false", + short_fingerprint(cached_fingerprint) + ); + false + } + }) +} + +fn short_fingerprint(value: &str) -> &str { + &value[..value.len().min(12)] +} + +fn dependency_fingerprint_for_workspace(state: &State) -> Result { + state + .config + .hash_options + .bazel + .dependency_fingerprint() + .map_err(|error| { + anyhow!("failed to compute dependency fingerprint for cache guard: {error:#}") + }) +} + fn configuration_fingerprint(config: &ServerConfig) -> String { let options = &config.hash_options; let mut hasher = Sha256::new(); @@ -733,6 +824,11 @@ fn resolve_sha(state: &State, revision: &str) -> Result { } fn checkout(state: &State, sha: &str) -> Result<()> { + if let Ok(output) = git_output(state, &[String::from("rev-parse"), String::from("HEAD")]) { + if output.status.success() && String::from_utf8_lossy(&output.stdout).trim() == sha { + return Ok(()); + } + } let args = [ String::from("-c"), String::from("advice.detachedHead=false"), @@ -1052,7 +1148,6 @@ mod tests { } struct CapturedS3Request { - method: String, url: String, } @@ -1072,7 +1167,6 @@ mod tests { let request = server.recv().unwrap(); sender .send(CapturedS3Request { - method: request.method().as_str().to_owned(), url: request.url().to_owned(), }) .unwrap(); @@ -1152,6 +1246,14 @@ mod tests { assert_eq!(normalize_s3_prefix(""), ""); assert_eq!(normalize_s3_prefix("/"), ""); assert_eq!(normalize_s3_prefix("/team/cache/"), "team/cache/"); + assert!(is_s3_not_found(&S3Error::HttpFailWithBody( + 404, + "missing".into() + ))); + assert!(!is_s3_not_found(&S3Error::HttpFailWithBody( + 500, + "failure".into() + ))); assert!(RemoteCache::new(&test_config()).unwrap().is_none()); } @@ -1159,28 +1261,23 @@ mod tests { fn remote_cache_operations_use_normalized_key_and_succeed() { let (remote, requests, handle) = remote_cache_with_responses( "/team/repo/", - vec![(200, b"hello".to_vec()), (200, Vec::new())], + vec![ + (200, b"hello".to_vec()), + (200, Vec::new()), + (200, Vec::new()), + ], ); assert_eq!(remote.object_key("sha.fp"), "team/repo/sha.fp.json"); assert_eq!(remote.get("sha.fp"), Some(b"hello".to_vec())); remote.put("sha.fp", b"data"); - let captured = (0..2) + assert!(remote.contains("sha.fp")); + let captured = (0..3) .map(|_| requests.recv_timeout(Duration::from_secs(5)).unwrap()) .collect::>(); handle.join().unwrap(); assert!(captured .iter() .all(|request| request.url.contains("/bucket/team/repo/sha.fp.json"))); - // A read is a single GET: a HeadObject probe in front of it would double the round trips - // and, worse, cannot report a miss -- S3 frames a HEAD 404 as chunked with no body, which - // the client surfaces as `attohttpc: Io Error: unexpected end of file`. - assert_eq!( - captured - .iter() - .map(|request| request.method.as_str()) - .collect::>(), - vec!["GET", "PUT"] - ); } #[test] @@ -1188,33 +1285,24 @@ mod tests { let (remote, requests, handle) = remote_cache_with_responses( "", vec![ - (404, b"NoSuchKey".to_vec()), + (404, b"missing".to_vec()), (500, b"failure".to_vec()), + (404, Vec::new()), + (500, Vec::new()), (500, Vec::new()), ], ); assert_eq!(remote.get("missing"), None); assert_eq!(remote.get("failure"), None); + assert!(!remote.contains("missing")); + assert!(!remote.contains("failure")); remote.put("failure", b"data"); - // Exactly one request per call. `rust-s3`'s `fail-on-err` feature is off precisely so a - // non-2xx stays a response: as an error it would enter the crate's `retry!`, and every - // miss would cost a second request and a one-second backoff sleep. - for _ in 0..3 { + for _ in 0..5 { requests.recv_timeout(Duration::from_secs(5)).unwrap(); } - assert!(requests.recv_timeout(Duration::from_millis(200)).is_err()); handle.join().unwrap(); } - #[test] - fn remote_cache_reports_unexpected_statuses_with_their_body() { - let response = ResponseData::new("denied".into(), 403, std::collections::HashMap::new()); - assert_eq!( - http_failure(&response), - "Got HTTP 403 with content 'denied'" - ); - } - #[test] fn normalizes_target_types() { assert_eq!( @@ -1364,6 +1452,12 @@ mod tests { let (repo, _, sha) = initialize_git_repo(); let cache = tempfile::tempdir().unwrap(); let state = state_for_repo(repo.path(), cache.path(), true); + let dependency_fingerprint = state + .config + .hash_options + .bazel + .dependency_fingerprint() + .unwrap(); let data = HashFileData { hashes: std::collections::BTreeMap::from([( "//app:lib".into(), @@ -1373,6 +1467,7 @@ mod tests { "//app:lib".into(), vec!["//dep:lib".into()], )]), + dependency_fingerprint: Some(dependency_fingerprint), ..Default::default() }; let key = cache_key(&state, &sha, &BTreeSet::new()); @@ -1390,20 +1485,22 @@ mod tests { fn remote_cache_hit_is_backfilled_and_next_read_is_local() { let (repo, _, sha) = initialize_git_repo(); let cache = tempfile::tempdir().unwrap(); + let mut config = test_config(); + config.hash_options.bazel.workspace = repo.path().to_path_buf(); + config.git_path = PathBuf::from("git"); + config.cache_dir = cache.path().to_path_buf(); + let dependency_fingerprint = config.hash_options.bazel.dependency_fingerprint().unwrap(); let data = HashFileData { hashes: std::collections::BTreeMap::from([( "//app:lib".into(), hash("Rule", "overall", "direct"), )]), + dependency_fingerprint: Some(dependency_fingerprint), ..Default::default() }; let bytes = serde_json::to_vec(&data.serialized(true, false)).unwrap(); - // One GET serves the whole lookup; the second read is answered by the local backfill. - let (remote, requests, handle) = remote_cache_with_responses("", vec![(200, bytes)]); - let mut config = test_config(); - config.hash_options.bazel.workspace = repo.path().to_path_buf(); - config.git_path = PathBuf::from("git"); - config.cache_dir = cache.path().to_path_buf(); + let (remote, requests, handle) = + remote_cache_with_responses("", vec![(200, Vec::new()), (200, bytes)]); let fingerprint = configuration_fingerprint(&config); let state = Arc::new(State { config, @@ -1423,8 +1520,9 @@ mod tests { assert!(second_hit); assert_eq!(second.hashes["//app:lib"].hash, "overall"); - requests.recv_timeout(Duration::from_secs(5)).unwrap(); - assert!(requests.recv_timeout(Duration::from_millis(200)).is_err()); + for _ in 0..2 { + requests.recv_timeout(Duration::from_secs(5)).unwrap(); + } handle.join().unwrap(); } @@ -1433,11 +1531,18 @@ mod tests { let (repo, first, second) = initialize_git_repo(); let cache = tempfile::tempdir().unwrap(); let state = state_for_repo(repo.path(), cache.path(), true); + let dependency_fingerprint = state + .config + .hash_options + .bazel + .dependency_fingerprint() + .unwrap(); let from = HashFileData { hashes: std::collections::BTreeMap::from([ ("//app:source".into(), hash("SourceFile", "old", "old")), ("//app:rule".into(), hash("Rule", "old-rule", "same-direct")), ]), + dependency_fingerprint: Some(dependency_fingerprint.clone()), ..Default::default() }; let to = HashFileData { @@ -1449,6 +1554,7 @@ mod tests { "//app:rule".into(), vec!["//app:source".into()], )]), + dependency_fingerprint: Some(dependency_fingerprint), ..Default::default() }; for (sha, data) in [(&first, &from), (&second, &to)] { @@ -1506,6 +1612,30 @@ mod tests { assert!(cache.path().join("ignored.tmp").exists()); } + #[test] + fn cache_entries_without_dependency_fingerprint_are_rejected() { + let repo = tempfile::tempdir().unwrap(); + let cache = tempfile::tempdir().unwrap(); + let state = state_for_repo(repo.path(), cache.path(), false); + let data = HashFileData { + hashes: std::collections::BTreeMap::from([( + "//app:lib".into(), + hash("Rule", "overall", "direct"), + )]), + ..Default::default() + }; + let mut current_dependency_fingerprint = None; + + assert!(!cache_entry_matches_dependency_fingerprint( + &state, + "abc", + "local", + &data, + &mut current_dependency_fingerprint, + ) + .unwrap()); + } + #[test] fn metrics_and_human_sizes_report_cache_state() { let repo = tempfile::tempdir().unwrap(); @@ -1591,11 +1721,18 @@ mod tests { let (repo, first, second) = initialize_git_repo(); let cache = tempfile::tempdir().unwrap(); let state = state_for_repo(repo.path(), cache.path(), true); + let dependency_fingerprint = state + .config + .hash_options + .bazel + .dependency_fingerprint() + .unwrap(); let from = HashFileData { hashes: std::collections::BTreeMap::from([ ("//app:source".into(), hash("SourceFile", "old", "old")), ("//app:rule".into(), hash("Rule", "old-rule", "same-direct")), ]), + dependency_fingerprint: Some(dependency_fingerprint.clone()), ..Default::default() }; let to = HashFileData { @@ -1607,6 +1744,7 @@ mod tests { "//app:rule".into(), vec!["//app:source".into()], )]), + dependency_fingerprint: Some(dependency_fingerprint), ..Default::default() }; for (sha, data) in [(&first, &from), (&second, &to)] {