From 20a44ee6d0da21780c2acf9700ceee7564a215f4 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Tue, 30 Jun 2026 19:47:28 -0400 Subject: [PATCH] feat: compute target distances in `bazel-diff serve` Add a `/impacted_targets_with_distances` endpoint to the serve query service that returns each impacted target annotated with its build-graph distance metrics (targetDistance and packageDistance), reusing the same distance math as `get-impacted-targets --depEdgesFile`. Dependency-edge tracking is opt-in via a new `serve --trackDeps` flag. When enabled, the per-SHA hash cache persists the dep-edge adjacency list under `metadata.depEdges` and round-trips it on a cache hit. The flag is folded into the cache fingerprint so a deps-tracking server never reuses a deps-less cache entry. Hitting the distances endpoint without `--trackDeps` returns a clear 400. The distance computation in CalculateImpactedTargetsInteractor is extracted into a structured `computeImpactedTargetsWithDistances` reused by both the CLI writer path and the service, keeping CLI JSON output byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 36 ++++++- .../kotlin/com/bazel_diff/cli/ServeCommand.kt | 21 +++- .../CalculateImpactedTargetsInteractor.kt | 49 ++++++--- .../interactor/DeserialiseHashesInteractor.kt | 20 +++- .../com/bazel_diff/server/BazelDiffServer.kt | 28 ++++- .../com/bazel_diff/server/HashService.kt | 40 +++++-- .../server/ImpactedTargetsService.kt | 101 +++++++++++++++--- .../com/bazel_diff/cli/ServeCommandTest.kt | 9 ++ .../test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 28 ++++- .../CalculateImpactedTargetsInteractorTest.kt | 41 +++++++ .../DeserialiseHashesInteractorTest.kt | 27 +++++ .../bazel_diff/server/BazelDiffServerTest.kt | 69 ++++++++++++ .../com/bazel_diff/server/HashServiceTest.kt | 42 +++++++- .../server/ImpactedTargetsServiceTest.kt | 70 ++++++++++++ tools/readme_template.md | 28 +++++ 15 files changed, 557 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 6e21f99d..483b7b4d 100644 --- a/README.md +++ b/README.md @@ -149,8 +149,36 @@ curl 'http://localhost:8080/impacted_targets?from=main&to=my-feature-branch' } ``` +* `GET /impacted_targets_with_distances?from=&to=` — like `/impacted_targets`, but each + impacted target is annotated with its build-graph distance metrics: `targetDistance` (the number of + dependency hops to the nearest directly-changed target) and `packageDistance` (how many of those + hops cross a package boundary). Directly-changed targets sit at distance `0`. Requires the server + to have been started with `--trackDeps` (see below); otherwise this endpoint returns `400`. The + same optional `targetType` filter applies. + +```bash +curl 'http://localhost:8080/impacted_targets_with_distances?from=main&to=my-feature-branch' +``` + +```json +{ + "from": "9a1c0e2…", + "to": "3f7b8d4…", + "impactedTargets": [ + {"label": "//foo:bar", "targetDistance": 0, "packageDistance": 0}, + {"label": "//foo:baz", "targetDistance": 1, "packageDistance": 1} + ] +} +``` + Notes and current limitations: +* Distance metrics (`/impacted_targets_with_distances`) require the dependency-edge graph, which is + only tracked when the server is started with `--trackDeps`. Tracking deps grows each cached hash + entry, so it is opt-in. The flag is folded into the cache key, so enabling or disabling it never + reuses a previously cached entry of the other kind. This mirrors the `generate-hashes --depEdgesFile` + / `get-impacted-targets --depEdgesFile` flow used by the CLI. + * The service checks out revisions inside `--workspacePath`, so point it at a dedicated clone, not a working tree you edit. All workspace-mutating work (git checkout + `bazel query`) is serialized, so a single instance answers one cold query at a time; the per-SHA cache absorbs the rest. @@ -394,8 +422,8 @@ Command-line utility to analyze the state of the bazel build graph ```terminal Usage: bazel-diff serve [-hkvV] [--[no-]excludeExternalTargets] - [--no-initial-fetch] [--[no-]useCquery] - [-b=] --cacheDir= + [--no-initial-fetch] [--[no-]trackDeps] [--[no-] + useCquery] [-b=] --cacheDir= [--cqueryExpression=] [--fineGrainedHashExternalReposFile=] [--gitEngine=] @@ -456,6 +484,10 @@ targets between two git revisions, caching generated hashes per commit SHA. -so, --bazelStartupOptions= Additional space separated Bazel client startup options used when invoking Bazel + --[no-]trackDeps Track dependency edges and persist them per commit + SHA so build-graph distance metrics can be served + via /impacted_targets_with_distances. Increases + cache size and memory. Defaults to false. --[no-]useCquery If true, use cquery instead of query when generating dependency graphs. -v, --verbose Display query string, missing files and elapsed time 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 9b4ca152..841c24f4 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt @@ -177,6 +177,16 @@ class ServeCommand : Callable { description = ["If true, exclude external targets (do not query //external:all-targets)."]) var excludeExternalTargets = false + @CommandLine.Option( + names = ["--trackDeps"], + negatable = true, + description = + [ + "Track dependency edges and persist them per commit SHA so build-graph distance " + + "metrics can be served via /impacted_targets_with_distances. Increases cache " + + "size and memory. Defaults to false."]) + var trackDeps = false + override fun call(): Int { org.koin.core.context.GlobalContext.stopKoin() startKoin { @@ -191,7 +201,7 @@ class ServeCommand : Callable { useCquery, cqueryExpression, keepGoing, - false, + trackDeps, fineGrainedHashExternalRepos, fineGrainedHashExternalReposFile, excludeExternalTargets, @@ -238,8 +248,10 @@ class ServeCommand : Callable { storage, computeConfigFingerprint(), loadSeedFilepaths(), - ignoredRuleHashingAttributes) - val impactedTargetsService = ImpactedTargetsService(gitClient, hashService) + ignoredRuleHashingAttributes, + trackDeps) + val impactedTargetsService = + ImpactedTargetsService(gitClient, hashService, depsTracked = trackDeps) val ready = AtomicBoolean(false) val server = BazelDiffServer(port, impactedTargetsService) { ready.get() } @@ -316,6 +328,9 @@ class ServeCommand : Callable { append("ignoredRuleHashingAttributes=") .append(ignoredRuleHashingAttributes.sorted().joinToString(",")) .append('\n') + // Distinct cache keys for deps-tracked vs deps-less runs: a deps-tracking server must never + // serve a deps-less cache entry (its distance queries would have no edges to traverse). + append("trackDeps=").append(trackDeps).append('\n') append("version=").append(VersionProvider().version.firstOrNull() ?: "unknown").append('\n') } return sha256 { putBytes(canonical.toByteArray()) }.toHexString().take(12) 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 db29fea2..a3d84012 100644 --- a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt +++ b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt @@ -17,6 +17,13 @@ import org.koin.core.component.inject data class TargetDistanceMetrics(val targetDistance: Int, val packageDistance: Int) {} +/** An impacted target paired with its build-graph distance metrics. */ +data class ImpactedTargetWithDistance( + val label: String, + val targetDistance: Int, + val packageDistance: Int +) + class CalculateImpactedTargetsInteractor : KoinComponent { private val gson: Gson by inject() private val logger: Logger by inject() @@ -94,6 +101,33 @@ class CalculateImpactedTargetsInteractor : KoinComponent { toModuleGraphJson: String? = null, excludeExternalTargets: Boolean = false, ) { + val impactedTargets = + computeImpactedTargetsWithDistances( + from, + to, + depEdges, + targetTypes, + fromModuleGraphJson, + toModuleGraphJson, + excludeExternalTargets) + outputWriter.use { writer -> writer.write(gson.toJson(impactedTargets)) } + } + + /** + * Computes the impacted targets between [from] and [to] together with their build-graph distance + * metrics, filtered by [targetTypes]/[excludeExternalTargets] and ordered the same way the + * non-distance path orders its output. Returned in-memory so both the CLI writer path + * ([executeWithDistances]) and the query service consume identical data without reparsing JSON. + */ + fun computeImpactedTargetsWithDistances( + from: Map, + to: Map, + depEdges: Map>, + targetTypes: Set?, + fromModuleGraphJson: String? = null, + toModuleGraphJson: String? = null, + excludeExternalTargets: Boolean = false, + ): List { val typeFilter = TargetTypeFilter(targetTypes, to) // Quick check: if module graph JSON is identical, skip module change detection entirely @@ -121,21 +155,12 @@ class CalculateImpactedTargetsInteractor : KoinComponent { } val ordering = impactedTargetOrdering(to, from) - impactedTargets + return impactedTargets .filterKeys { typeFilter.accepts(it) } .filterKeys { !excludeExternalTargets || !it.startsWith("//external:") } .toSortedMap(ordering) - .let { filtered -> - outputWriter.use { writer -> - writer.write( - gson.toJson( - filtered.map { - mapOf( - "label" to it.key, - "targetDistance" to it.value.targetDistance, - "packageDistance" to it.value.packageDistance) - })) - } + .map { + ImpactedTargetWithDistance(it.key, it.value.targetDistance, it.value.packageDistance) } } 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 d370ef02..8146366f 100644 --- a/cli/src/main/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractor.kt +++ b/cli/src/main/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractor.kt @@ -9,7 +9,11 @@ import java.io.FileReader import org.koin.core.component.KoinComponent import org.koin.core.component.inject -data class HashFileData(val hashes: Map, val moduleGraphJson: String?) +data class HashFileData( + val hashes: Map, + val moduleGraphJson: String?, + val depEdges: Map> = emptyMap() +) class DeserialiseHashesInteractor : KoinComponent { private val gson: Gson by inject() @@ -40,9 +44,19 @@ class DeserialiseHashesInteractor : KoinComponent { val hashesMap: Map = gson.fromJson(jsonObject.get("hashes"), hashesShape) val hashes = hashesMap.mapValues { TargetHash.fromJson(it.value) } - val moduleGraphJson = jsonObject.getAsJsonObject("metadata")?.get("moduleGraphJson")?.asString + val metadata = jsonObject.getAsJsonObject("metadata") + val moduleGraphJson = metadata?.get("moduleGraphJson")?.asString - return HashFileData(hashes, moduleGraphJson) + // 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 + // be computed on a cache hit without re-tracking deps. Absent for CLI-produced hashes. + val depEdges: Map> = + metadata?.get("depEdges")?.let { + val depShape = object : TypeToken>>() {}.type + gson.fromJson>>(it, depShape) + } ?: emptyMap() + + return HashFileData(hashes, moduleGraphJson, depEdges) } 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/BazelDiffServer.kt b/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt index 06f372be..685c1c25 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt @@ -21,6 +21,9 @@ import org.koin.core.component.inject * lame-ducked by a fatal git error. * - `GET /impacted_targets?from=&to=[&targetType=Rule,SourceFile]` -- returns `{"from": * , "to": , "impactedTargets": [...]}`. + * - `GET /impacted_targets_with_distances?from=&to=[&targetType=...]` -- like above but + * each impacted target is `{"label", "targetDistance", "packageDistance"}`. Requires the server + * to have been started with `--trackDeps`; returns `400` otherwise. * * Built on the JDK's [HttpServer] so the service needs no new third-party dependency. The handler * pool is unbounded (cached) so that health checks are always served even while a long hash @@ -47,6 +50,8 @@ class BazelDiffServer( } httpServer.createContext("/health", ::handleHealth) httpServer.createContext("/impacted_targets", ::handleImpactedTargets) + httpServer.createContext( + "/impacted_targets_with_distances", ::handleImpactedTargetsWithDistances) httpServer.start() server = httpServer logger.i { "bazel-diff query service listening on port ${boundPort()} " } @@ -80,6 +85,24 @@ class BazelDiffServer( } private fun handleImpactedTargets(exchange: HttpExchange) = + handleQuery(exchange) { from, to, targetTypes -> + impactedTargetsProvider.getImpactedTargets(from, to, targetTypes) + } + + private fun handleImpactedTargetsWithDistances(exchange: HttpExchange) = + handleQuery(exchange) { from, to, targetTypes -> + impactedTargetsProvider.getImpactedTargetsWithDistances(from, to, targetTypes) + } + + /** + * Shared handling for the impacted-targets endpoints: enforces GET + readiness, parses and + * validates `from`/`to`/`targetType`, then serializes the result of [compute] as JSON, mapping + * the known failure modes to the appropriate status codes. + */ + private fun handleQuery( + exchange: HttpExchange, + compute: (from: String, to: String, targetTypes: Set?) -> Any + ) = withExchange(exchange) { if (!exchange.requestMethod.equals("GET", ignoreCase = true)) { respondJson(exchange, 405, mapOf("error" to "method not allowed, use GET")) @@ -102,8 +125,9 @@ class BazelDiffServer( params["targetType"]?.split(",")?.map { it.trim() }?.filter { it.isNotEmpty() }?.toSet() try { - val result = impactedTargetsProvider.getImpactedTargets(from, to, targetTypes) - respondJson(exchange, 200, result) + respondJson(exchange, 200, compute(from, to, targetTypes)) + } catch (e: DistancesUnavailableException) { + respondJson(exchange, 400, mapOf("error" to (e.message ?: "distances unavailable"))) } catch (e: GitClientException) { logger.e(e) { "git error computing impacted targets" } respondJson(exchange, 400, mapOf("error" to "git error: ${e.message}")) 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 2d6b716e..a89f1776 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/HashService.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/HashService.kt @@ -40,6 +40,9 @@ interface HashProvider { * server started with different flags never reads another configuration's cached hashes. * @param seedFilepaths seed files passed through to [BuildGraphHasher]. * @param ignoredRuleHashingAttributes rule attributes ignored when hashing. + * @param trackDeps when true, persist each revision's dependency-edge adjacency list in the cache + * so build-graph distance metrics can be served. Requires the hasher to have been built with + * dep-tracking on (so [TargetHash.deps] is populated). */ class HashService( private val gitClient: GitClient, @@ -47,6 +50,7 @@ class HashService( private val configFingerprint: String, private val seedFilepaths: Set, private val ignoredRuleHashingAttributes: Set, + private val trackDeps: Boolean = false, ) : HashProvider, KoinComponent { private val buildGraphHasher: BuildGraphHasher by inject() private val bazelModService: BazelModService by inject() @@ -89,24 +93,42 @@ class HashService( buildGraphHasher.hashAllBazelTargetsAndSourcefiles( seedFilepaths, ignoredRuleHashingAttributes) val moduleGraphJson = runBlocking { bazelModService.getModuleGraphJson() } + val depEdges = depEdgesOf(hashes) storage.put( - cacheKey(sha), serialize(hashes, moduleGraphJson).toByteArray(StandardCharsets.UTF_8)) - HashFileData(hashes, moduleGraphJson) + cacheKey(sha), + serialize(hashes, moduleGraphJson, depEdges).toByteArray(StandardCharsets.UTF_8)) + HashFileData(hashes, moduleGraphJson, depEdges) } + /** + * The dependency-edge adjacency list (label -> direct dep labels) when [trackDeps] is on, else + * empty. Derived from [TargetHash.deps], the same way `generate-hashes --depEdgesFile` derives + * it. + */ + private fun depEdgesOf(hashes: Map): Map> = + if (trackDeps) hashes.mapValues { it.value.deps ?: emptyList() } else emptyMap() + /** * Serializes hashes into the same JSON shape `generate-hashes` writes (see * [com.bazel_diff.interactor.GenerateHashesInteractor]), so cached entries are interchangeable - * with hashes produced by the CLI. Target type is always included for the richest data. + * with hashes produced by the CLI. Target type is always included for the richest data. When + * [depEdges] is non-empty (i.e. --trackDeps), it is persisted under `metadata.depEdges` so + * distance metrics can be served on a cache hit. */ - private fun serialize(hashes: Map, moduleGraphJson: String?): String { + private fun serialize( + hashes: Map, + moduleGraphJson: String?, + depEdges: Map> + ): String { + val serializedHashes = hashes.mapValues { it.value.toJson(true) } val output = - if (moduleGraphJson != null) { - mapOf( - "hashes" to hashes.mapValues { it.value.toJson(true) }, - "metadata" to mapOf("moduleGraphJson" to moduleGraphJson)) + if (moduleGraphJson != null || depEdges.isNotEmpty()) { + val metadata = mutableMapOf() + if (moduleGraphJson != null) metadata["moduleGraphJson"] = moduleGraphJson + if (depEdges.isNotEmpty()) metadata["depEdges"] = depEdges + mapOf("hashes" to serializedHashes, "metadata" to metadata) } else { - hashes.mapValues { it.value.toJson(true) } + serializedHashes } return gson.toJson(output) } 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 7fc57650..0e79895b 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt @@ -2,6 +2,7 @@ package com.bazel_diff.server import com.bazel_diff.bazel.BazelModService import com.bazel_diff.interactor.CalculateImpactedTargetsInteractor +import com.bazel_diff.interactor.ImpactedTargetWithDistance import com.bazel_diff.log.Logger import java.io.StringWriter import org.koin.core.component.KoinComponent @@ -14,6 +15,22 @@ data class ImpactedTargetsResult( val impactedTargets: List, ) +/** + * Result of an impacted-targets-with-distances request: the resolved SHAs and the impacted targets + * each paired with their build-graph distance metrics. + */ +data class ImpactedTargetsWithDistancesResult( + val from: String, + val to: String, + val impactedTargets: List, +) + +/** + * Thrown when a distance query arrives but the server was started without `--trackDeps`, so no + * dependency edges were tracked or cached. The HTTP layer maps this to a `400`. + */ +class DistancesUnavailableException(message: String) : Exception(message) + /** * Computes the impacted targets between two revisions. Extracted behind an interface so the HTTP * layer ([BazelDiffServer]) can be tested with a fake. @@ -29,6 +46,17 @@ interface ImpactedTargetsProvider { toRev: String, targetTypes: Set? ): ImpactedTargetsResult + + /** + * Like [getImpactedTargets] but additionally returns each impacted target's build-graph distance + * metrics. Throws [DistancesUnavailableException] if the server was not started with + * `--trackDeps`. + */ + fun getImpactedTargetsWithDistances( + fromRev: String, + toRev: String, + targetTypes: Set? + ): ImpactedTargetsWithDistancesResult } /** @@ -39,6 +67,7 @@ interface ImpactedTargetsProvider { class ImpactedTargetsService( private val gitClient: GitClient, private val hashProvider: HashProvider, + private val depsTracked: Boolean = false, ) : ImpactedTargetsProvider, KoinComponent { private val bazelModService: BazelModService by inject() private val logger: Logger by inject() @@ -55,13 +84,9 @@ class ImpactedTargetsService( val fromData = hashProvider.getHashes(fromSha) val toData = hashProvider.getHashes(toSha) - // 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). - val excludeExternalTargets = bazelModService.isBzlmodEnabled val writer = StringWriter() - val interactor = CalculateImpactedTargetsInteractor() - val runCalc = { + runOnGraph(fromData.moduleGraphJson, toData.moduleGraphJson, toSha) { interactor.execute( fromData.hashes, toData.hashes, @@ -69,19 +94,65 @@ class ImpactedTargetsService( targetTypes, fromData.moduleGraphJson, toData.moduleGraphJson, - excludeExternalTargets) - } - - // When the module graph changed, the interactor issues a live `rdeps` query against the - // working tree, so it must be checked out at `to` and held there while the query runs. The - // common (pure hash-diff) path touches no workspace state, so it runs without the lock. - if (fromData.moduleGraphJson != toData.moduleGraphJson) { - hashProvider.withWorkspaceAt(toSha) { runCalc() } - } else { - runCalc() + excludeExternalTargets()) } val impacted = writer.toString().split("\n").filter { it.isNotBlank() } return ImpactedTargetsResult(fromSha, toSha, impacted) } + + override fun getImpactedTargetsWithDistances( + fromRev: String, + toRev: String, + targetTypes: Set? + ): ImpactedTargetsWithDistancesResult { + if (!depsTracked) { + throw DistancesUnavailableException( + "distances unavailable: server started without --trackDeps") + } + val fromSha = gitClient.resolveSha(fromRev) + val toSha = gitClient.resolveSha(toRev) + logger.i { "Computing impacted targets with distances $fromSha -> $toSha" } + + val fromData = hashProvider.getHashes(fromSha) + val toData = hashProvider.getHashes(toSha) + + val interactor = CalculateImpactedTargetsInteractor() + val impacted = + runOnGraph(fromData.moduleGraphJson, toData.moduleGraphJson, toSha) { + // Use the `to` revision's dependency edges: distances are measured by traversing the + // impacted labels through the final build graph (matches the CLI's depEdgesFile usage). + interactor.computeImpactedTargetsWithDistances( + fromData.hashes, + toData.hashes, + toData.depEdges, + targetTypes, + fromData.moduleGraphJson, + toData.moduleGraphJson, + excludeExternalTargets()) + } + return ImpactedTargetsWithDistancesResult(fromSha, toSha, impacted) + } + + // 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 + + /** + * Runs [block], holding the workspace at [toSha] only when the module graph changed. In that case + * the interactor issues a live `rdeps` query against the working tree, so it must be checked out + * at `to` and held there while the query runs. The common (pure hash-diff) path touches no + * workspace state, so it runs without the lock. + */ + private fun runOnGraph( + fromModuleGraphJson: String?, + toModuleGraphJson: String?, + toSha: String, + block: () -> T + ): T = + if (fromModuleGraphJson != toModuleGraphJson) { + hashProvider.withWorkspaceAt(toSha) { block() } + } else { + block() + } } diff --git a/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt b/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt index b9e4e353..01d42092 100644 --- a/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt @@ -106,6 +106,15 @@ class ServeCommandTest : KoinTest { assertThat(withRepos).isNotEqualTo(base) } + @Test + fun configFingerprintChangesWithTrackDeps() { + // A deps-tracking server must never reuse deps-less cache entries, so the flag must affect the + // cache key. + val base = ServeCommand().computeConfigFingerprint() + val withTrackDeps = ServeCommand().apply { trackDeps = true }.computeConfigFingerprint() + assertThat(withTrackDeps).isNotEqualTo(base) + } + @Test fun configFingerprintIsOrderIndependentForSets() { val first = diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index 445ef65f..438655dc 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -156,10 +156,12 @@ class E2ETest { /** * End-to-end coverage for the `serve` query service: builds a two-commit git repo from the - * shell-only `distance_metrics` workspace, runs `bazel-diff serve` in a background thread, then - * hits `/health` and `/impacted_targets` over real HTTP. Exercises the full + * shell-only `distance_metrics` workspace, runs `bazel-diff serve` (with `--trackDeps`) in a + * background thread, then hits `/health`, `/impacted_targets`, and + * `/impacted_targets_with_distances` over real HTTP. Exercises the full * [com.bazel_diff.cli.ServeCommand] path (Koin + hasher wiring, git checkout, real `bazel query` - * for both revisions, and the cache) the same way the other E2E tests cover the CLI commands. + * for both revisions, dep-edge tracking, distance computation, and the cache) the same way the + * other E2E tests cover the CLI commands. */ @Test fun testServeEndToEnd() { @@ -201,7 +203,8 @@ class E2ETest { cacheDir.absolutePath, "--port", port.toString(), - "--no-initial-fetch") + "--no-initial-fetch", + "--trackDeps") } .apply { isDaemon = true @@ -221,6 +224,23 @@ class E2ETest { @Suppress("UNCHECKED_CAST") val impacted = parsed["impactedTargets"] as List // Editing lib.sh must impact at least its own sh_library target. assertThat(impacted.contains("//:lib")).isEqualTo(true) + + // The distances endpoint returns the same impacted targets, each annotated with build-graph + // distance metrics. //:lib is directly edited, so it sits at distance 0. + val (distCode, distBody) = + httpGetServe( + "http://localhost:$port/impacted_targets_with_distances?from=$fromSha&to=$toSha") + assertThat(distCode).isEqualTo(200) + val distParsed: Map = + Gson().fromJson(distBody, object : TypeToken>() {}.type) + assertThat(distParsed["from"]).isEqualTo(fromSha) + assertThat(distParsed["to"]).isEqualTo(toSha) + @Suppress("UNCHECKED_CAST") + val withDistances = distParsed["impactedTargets"] as List> + val libEntry = withDistances.single { it["label"] == "//:lib" } + // Gson decodes JSON numbers as Double. + assertThat(libEntry["targetDistance"]).isEqualTo(0.0) + assertThat(libEntry["packageDistance"]).isEqualTo(0.0) } finally { serveThread.interrupt() serveThread.join(10_000) 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 a06289a1..946a18cf 100644 --- a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt @@ -138,6 +138,47 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { ) } + @Test + fun testComputeImpactedTargetsWithDistancesReturnsOrderedList() { + val (depEdges, startHashes) = createTargetHashes("//:1 <- //:2 <- //:3") + val endHashes = startHashes.toMutableMap() + makeDirectlyChanged(endHashes, "//:1") + makeIndirectlyChanged(endHashes, "//:2", "//:3") + + val interactor = CalculateImpactedTargetsInteractor() + val impacted = + interactor.computeImpactedTargetsWithDistances(startHashes, endHashes, depEdges, null) + + assertThat(impacted) + .containsExactly( + ImpactedTargetWithDistance("//:1", 0, 0), + ImpactedTargetWithDistance("//:2", 1, 0), + ImpactedTargetWithDistance("//:3", 2, 0), + ) + } + + @Test + fun testExecuteWithDistancesWritesSameDataAsCompute() { + // The writer path must serialize exactly the structured compute result, so the CLI JSON output + // is unchanged by the refactor and matches what the query service returns. + val (depEdges, startHashes) = createTargetHashes("//A:1 <- //A:2 <- //B:3") + val endHashes = startHashes.toMutableMap() + makeDirectlyChanged(endHashes, "//A:1") + makeIndirectlyChanged(endHashes, "//A:2", "//B:3") + + val interactor = CalculateImpactedTargetsInteractor() + val expected = + interactor.computeImpactedTargetsWithDistances(startHashes, endHashes, depEdges, null) + + val writer = StringWriter() + interactor.executeWithDistances(startHashes, endHashes, depEdges, writer, null) + + val gson = com.google.gson.Gson() + val parsed = + gson.fromJson(writer.toString(), Array::class.java).toList() + assertThat(parsed).isEqualTo(expected) + } + @Test fun testPackageDistance() { var (depEdges, startHashes) = createTargetHashes("//A:1 <- //A:2 <- //B:3 <- //B:4 <- //C:5") diff --git a/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt b/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt index 13f5437f..75319b88 100644 --- a/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt @@ -48,4 +48,31 @@ class DeserialiseHashesInteractorTest : KoinTest { "target-2" to TargetHash("Rule", "hash2", "direct2"), "target-3" to TargetHash("SourceFile", "hash3", "direct3"))) } + + @Test + fun parsesDepEdgesFromMetadata() { + val json = + """{ + | "hashes": {"//a:lib":"Rule#h1~d1", "//b:lib":"Rule#h2~d2"}, + | "metadata": { + | "moduleGraphJson": "{}", + | "depEdges": {"//a:lib":["//b:lib"], "//b:lib":[]} + | } + |}""" + .trimMargin() + + val data = interactor.executeTargetHashWithMetadataFromString(json) + + assertThat(data.depEdges) + .isEqualTo(mapOf("//a:lib" to listOf("//b:lib"), "//b:lib" to emptyList())) + } + + @Test + fun depEdgesDefaultToEmptyWhenAbsent() { + val json = """{"hashes": {"//a:lib":"Rule#h1~d1"}, "metadata": {"moduleGraphJson": "{}"}}""" + + val data = interactor.executeTargetHashWithMetadataFromString(json) + + assertThat(data.depEdges).isEqualTo(emptyMap()) + } } diff --git a/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt index 60458fec..b02ea4b8 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt @@ -5,6 +5,7 @@ import assertk.assertions.contains import assertk.assertions.containsExactly import assertk.assertions.isEqualTo import com.bazel_diff.SilentLogger +import com.bazel_diff.interactor.ImpactedTargetWithDistance import com.bazel_diff.log.Logger import com.google.gson.Gson import com.google.gson.GsonBuilder @@ -41,7 +42,15 @@ class BazelDiffServerTest : KoinTest { private class FixedProvider( var result: ImpactedTargetsResult = ImpactedTargetsResult("from-sha", "to-sha", listOf("//:a", "//:b")), + var distancesResult: ImpactedTargetsWithDistancesResult = + ImpactedTargetsWithDistancesResult( + "from-sha", + "to-sha", + listOf( + ImpactedTargetWithDistance("//:a", 0, 0), + ImpactedTargetWithDistance("//:b", 1, 1))), var error: Exception? = null, + var distancesError: Exception? = null, var lastTargetTypes: Set? = null, ) : ImpactedTargetsProvider { override fun getImpactedTargets( @@ -53,6 +62,16 @@ class BazelDiffServerTest : KoinTest { error?.let { throw it } return result } + + override fun getImpactedTargetsWithDistances( + fromRev: String, + toRev: String, + targetTypes: Set? + ): ImpactedTargetsWithDistancesResult { + lastTargetTypes = targetTypes + distancesError?.let { throw it } + return distancesResult + } } @Before @@ -72,6 +91,12 @@ class BazelDiffServerTest : KoinTest { object : ImpactedTargetsProvider { override fun getImpactedTargets(fromRev: String, toRev: String, targetTypes: Set?) = provider.getImpactedTargets(fromRev, toRev, targetTypes) + + override fun getImpactedTargetsWithDistances( + fromRev: String, + toRev: String, + targetTypes: Set? + ) = provider.getImpactedTargetsWithDistances(fromRev, toRev, targetTypes) } private data class Response(val code: Int, val body: String) @@ -162,4 +187,48 @@ class BazelDiffServerTest : KoinTest { assertThat(response.code).isEqualTo(500) assertThat(response.body).contains("boom") } + + @Test + fun impactedTargetsWithDistancesReturnsJson() { + provider = FixedProvider() + val response = get("/impacted_targets_with_distances?from=main&to=feature") + assertThat(response.code).isEqualTo(200) + val parsed = gson.fromJson(response.body, ImpactedTargetsWithDistancesResult::class.java) + assertThat(parsed.from).isEqualTo("from-sha") + assertThat(parsed.to).isEqualTo("to-sha") + assertThat(parsed.impactedTargets) + .containsExactly( + ImpactedTargetWithDistance("//:a", 0, 0), ImpactedTargetWithDistance("//:b", 1, 1)) + } + + @Test + fun impactedTargetsWithDistancesPassesTargetTypeFilter() { + val fixed = FixedProvider() + provider = fixed + get("/impacted_targets_with_distances?from=a&to=b&targetType=Rule") + assertThat(fixed.lastTargetTypes).isEqualTo(setOf("Rule")) + } + + @Test + fun impactedTargetsWithDistancesMissingParamsReturns400() { + assertThat(get("/impacted_targets_with_distances?from=main").code).isEqualTo(400) + } + + @Test + fun impactedTargetsWithDistancesReturns503WhenNotReady() { + ready.set(false) + assertThat(get("/impacted_targets_with_distances?from=a&to=b").code).isEqualTo(503) + } + + @Test + fun distancesUnavailableReturns400() { + provider = + FixedProvider( + distancesError = + DistancesUnavailableException( + "distances unavailable: server started without --trackDeps")) + val response = get("/impacted_targets_with_distances?from=a&to=b") + assertThat(response.code).isEqualTo(400) + assertThat(response.body).contains("--trackDeps") + } } 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 c7f353e2..e840aae7 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/HashServiceTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/HashServiceTest.kt @@ -1,6 +1,8 @@ package com.bazel_diff.server import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.doesNotContain import assertk.assertions.hasSize import assertk.assertions.isEqualTo import assertk.assertions.isNull @@ -66,8 +68,13 @@ class HashServiceTest : KoinTest { private val sampleHashes = mapOf("//:a" to TargetHash("Rule", "h", "d")) - private fun newService(git: GitClient, storage: HashCacheStorage) = - HashService(git, storage, "fp", emptySet(), emptySet()) + private val sampleHashesWithDeps = + mapOf( + "//a:lib" to TargetHash("Rule", "ha", "da", deps = listOf("//b:lib")), + "//b:lib" to TargetHash("Rule", "hb", "db", deps = emptyList())) + + private fun newService(git: GitClient, storage: HashCacheStorage, trackDeps: Boolean = false) = + HashService(git, storage, "fp", emptySet(), emptySet(), trackDeps) @Test fun cacheMissGeneratesAndStores() { @@ -130,4 +137,35 @@ class HashServiceTest : KoinTest { assertThat(fromCache.hashes).isEqualTo(sampleHashes) assertThat(fromCache.moduleGraphJson).isEqualTo("""{"graph":1}""") } + + @Test + fun trackDepsPersistsAndRoundTripsDepEdges() { + whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any())) + .thenReturn(sampleHashesWithDeps) + runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn(null) } + val storage = InMemoryStorage() + + val generated = newService(RecordingGitClient(), storage, trackDeps = true).getHashes("sha1") + // Cache JSON carries the dependency-edge adjacency list under metadata.depEdges. + assertThat(String(storage.entries.values.single())).contains("depEdges") + // Read back through a fresh service over the same storage (cache-hit path). + val fromCache = newService(RecordingGitClient(), storage, trackDeps = true).getHashes("sha1") + + val expectedDepEdges = mapOf("//a:lib" to listOf("//b:lib"), "//b:lib" to emptyList()) + assertThat(generated.depEdges).isEqualTo(expectedDepEdges) + assertThat(fromCache.depEdges).isEqualTo(expectedDepEdges) + } + + @Test + fun withoutTrackDepsCacheHasNoDepEdges() { + whenever(buildGraphHasher.hashAllBazelTargetsAndSourcefiles(any(), any(), any())) + .thenReturn(sampleHashesWithDeps) + runBlocking { whenever(bazelModService.getModuleGraphJson()).thenReturn(null) } + val storage = InMemoryStorage() + + val generated = newService(RecordingGitClient(), storage, trackDeps = false).getHashes("sha1") + + assertThat(generated.depEdges).isEqualTo(emptyMap()) + assertThat(String(storage.entries.values.single())).doesNotContain("depEdges") + } } 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 5079a25a..dbeb329d 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt @@ -1,12 +1,14 @@ package com.bazel_diff.server import assertk.assertThat +import assertk.assertions.containsExactly import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.isEqualTo import com.bazel_diff.SilentLogger import com.bazel_diff.bazel.BazelModService import com.bazel_diff.hash.TargetHash import com.bazel_diff.interactor.HashFileData +import com.bazel_diff.interactor.ImpactedTargetWithDistance import com.bazel_diff.log.Logger import com.google.gson.GsonBuilder import org.junit.Rule @@ -126,4 +128,72 @@ class ImpactedTargetsServiceTest : KoinTest { assertThat(result.impactedTargets).isEqualTo(listOf("//:a")) assertThat(provider.workspaceAtCalls).isEqualTo(listOf("to-sha")) } + + @Test + fun computesDistancesFromToRevisionDepEdges() { + // //a:lib changed its own content (directHash) -> DIRECT, distance 0. + // //b:lib only changed transitively (hash differs, directHash same) and depends on //a:lib -> + // INDIRECT, one hop across a package boundary -> targetDistance 1, packageDistance 1. + val from = + HashFileData( + mapOf( + "//a:lib" to TargetHash("Rule", "h1", "d1"), + "//b:lib" to TargetHash("Rule", "hb1", "db1")), + null) + val to = + HashFileData( + mapOf( + "//a:lib" to TargetHash("Rule", "h2", "d2"), + "//b:lib" to TargetHash("Rule", "hb2", "db1")), + null, + depEdges = mapOf("//b:lib" to listOf("//a:lib"), "//a:lib" to emptyList())) + val service = + ImpactedTargetsService( + IdentityGitClient(), + FakeHashProvider(mapOf("from-sha" to from, "to-sha" to to)), + depsTracked = true) + + val result = service.getImpactedTargetsWithDistances("from-sha", "to-sha", null) + + assertThat(result.from).isEqualTo("from-sha") + assertThat(result.to).isEqualTo("to-sha") + assertThat(result.impactedTargets) + .containsExactly( + ImpactedTargetWithDistance("//a:lib", 0, 0), + ImpactedTargetWithDistance("//b:lib", 1, 1)) + } + + @Test + fun distancesThrowWhenDepsNotTracked() { + val from = HashFileData(mapOf("//:a" to TargetHash("Rule", "h1", "d1")), null) + val to = HashFileData(mapOf("//:a" to TargetHash("Rule", "h2", "d2")), null) + val service = + ImpactedTargetsService( + IdentityGitClient(), + FakeHashProvider(mapOf("from-sha" to from, "to-sha" to to)), + depsTracked = false) + + org.junit.Assert.assertThrows(DistancesUnavailableException::class.java) { + service.getImpactedTargetsWithDistances("from-sha", "to-sha", null) + } + } + + @Test + fun distancesModuleGraphChangePinsWorkspaceToTo() { + // Same forced live-query path as the non-distance test, but through the distances method. + val from = HashFileData(mapOf("//:a" to TargetHash("Rule", "h1", "d1")), "graph-v1") + val to = + HashFileData( + mapOf("//:a" to TargetHash("Rule", "h2", "d2")), + "graph-v2", + depEdges = mapOf("//:a" to emptyList())) + val provider = + FakeHashProvider(mapOf("from-sha" to from, "to-sha" to to), allowWorkspaceAt = true) + val service = ImpactedTargetsService(IdentityGitClient(), provider, depsTracked = true) + + val result = service.getImpactedTargetsWithDistances("from-sha", "to-sha", null) + + assertThat(result.impactedTargets).containsExactly(ImpactedTargetWithDistance("//:a", 0, 0)) + assertThat(provider.workspaceAtCalls).isEqualTo(listOf("to-sha")) + } } diff --git a/tools/readme_template.md b/tools/readme_template.md index 579ace5c..1b187d29 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -149,8 +149,36 @@ curl 'http://localhost:8080/impacted_targets?from=main&to=my-feature-branch' } ``` +* `GET /impacted_targets_with_distances?from=&to=` — like `/impacted_targets`, but each + impacted target is annotated with its build-graph distance metrics: `targetDistance` (the number of + dependency hops to the nearest directly-changed target) and `packageDistance` (how many of those + hops cross a package boundary). Directly-changed targets sit at distance `0`. Requires the server + to have been started with `--trackDeps` (see below); otherwise this endpoint returns `400`. The + same optional `targetType` filter applies. + +```bash +curl 'http://localhost:8080/impacted_targets_with_distances?from=main&to=my-feature-branch' +``` + +```json +{ + "from": "9a1c0e2…", + "to": "3f7b8d4…", + "impactedTargets": [ + {"label": "//foo:bar", "targetDistance": 0, "packageDistance": 0}, + {"label": "//foo:baz", "targetDistance": 1, "packageDistance": 1} + ] +} +``` + Notes and current limitations: +* Distance metrics (`/impacted_targets_with_distances`) require the dependency-edge graph, which is + only tracked when the server is started with `--trackDeps`. Tracking deps grows each cached hash + entry, so it is opt-in. The flag is folded into the cache key, so enabling or disabling it never + reuses a previously cached entry of the other kind. This mirrors the `generate-hashes --depEdgesFile` + / `get-impacted-targets --depEdgesFile` flow used by the CLI. + * The service checks out revisions inside `--workspacePath`, so point it at a dedicated clone, not a working tree you edit. All workspace-mutating work (git checkout + `bazel query`) is serialized, so a single instance answers one cold query at a time; the per-SHA cache absorbs the rest.