diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index e0fc26f51cc5e3..aca6bafcf396a5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -59,6 +59,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/actions:action_output_directory_helper", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 519905904227f9..4519ef2dfc40ff 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -65,6 +65,7 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; @@ -512,7 +513,8 @@ public MerkleTree uncachedBuildMerkleTreeVisitor( } @Nullable - private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnScrubber, ImmutableMap mnemonicCacheSalts) { + private static ByteString buildSalt( + Spawn spawn, @Nullable SpawnScrubber spawnScrubber, @Nullable String contentKey, ImmutableMap mnemonicCacheSalts) { CacheSalt.Builder saltBuilder = CacheSalt.newBuilder().setMayBeExecutedRemotely(Spawns.mayBeExecutedRemotely(spawn)); @@ -522,9 +524,14 @@ private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnSc saltBuilder.setWorkspace(workspace); } - if (spawnScrubber != null) { - saltBuilder.setScrubSalt( - CacheSalt.ScrubSalt.newBuilder().setSalt(spawnScrubber.getSalt()).build()); + String scrubSalt = spawnScrubber != null ? spawnScrubber.getSalt() : ""; + + if (contentKey != null) { + scrubSalt = scrubSalt + ":ck=" + contentKey; + } + + if (!scrubSalt.isEmpty()) { + saltBuilder.setScrubSalt(CacheSalt.ScrubSalt.newBuilder().setSalt(scrubSalt).build()); } // Add mnemonic-specific salt for targeted cache invalidation. @@ -536,6 +543,45 @@ private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnSc return saltBuilder.build().toByteString(); } + /** + * Reads the content key file for a TestRunner spawn. Derives the path from the spawn's output + * paths: a test log at {@code bazel-out//testlogs///test.log} implies the + * content key at {@code bazel-out//bin//.content_key}. + * + *

Returns null if the file cannot be found or read (e.g. aspect not enabled). + */ + @Nullable + private String readContentKey(Spawn spawn) { + Label label = spawn.getTargetLabel(); + if (label == null) { + return null; + } + String relativeContentKeyPath = + label.getPackageName() + "/" + label.getName() + ".content_key"; + + for (ActionInput output : spawn.getOutputFiles()) { + String path = output.getExecPathString(); + int idx = path.indexOf("/testlogs/"); + if (idx >= 0) { + // e.g. bazel-out/k8-fastbuild/testlogs/foo/bar/baz/test.log + // → bazel-out/k8-fastbuild/bin/foo/bar/baz.content_key + String configPrefix = path.substring(0, idx); // "bazel-out/k8-fastbuild" + Path keyFile = + execRoot.getRelative(configPrefix + "/bin/" + relativeContentKeyPath); + if (keyFile.exists()) { + try { + return new String(keyFile.getInputStream().readAllBytes(), + java.nio.charset.StandardCharsets.UTF_8).trim(); + } catch (java.io.IOException e) { + // Non-fatal: fall back to normal cache key. + } + } + break; + } + } + return null; + } + /** * Semaphore for limiting the concurrent number of Merkle tree input roots we compute and keep in * memory. @@ -653,6 +699,20 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context RemotePathResolver.createMapped(baseRemotePathResolver, execRoot, spawn.getPathMapper()); ToolSignature toolSignature = getToolSignature(spawn, context); SpawnScrubber spawnScrubber = scrubber != null ? scrubber.forSpawn(spawn) : null; + + // For TestRunner actions with --experimental_test_content_key: inject jar omission + // programmatically when the content key is present. This keeps jar-omission in sync with + // key injection without requiring an xplat.cfg entry — when the key is absent (aspect not + // run, file not downloaded, stale files from a disabled flag, etc.), jars remain in the + // Merkle tree and serve as the cache discriminator (safe fallback, no false hits). + String contentKeyForSalt = null; + if (remoteOptions.testContentKey && "TestRunner".equals(spawn.getMnemonic())) { + contentKeyForSalt = readContentKey(spawn); + if (contentKeyForSalt != null && spawnScrubber != null) { + spawnScrubber = spawnScrubber.withJarOmission(); + } + } + final MerkleTree merkleTree = buildInputMerkleTree(spawn, context, toolSignature, spawnScrubber, remotePathResolver); @@ -684,7 +744,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context platform, context.getTimeout(), Spawns.mayBeCachedRemotely(spawn), - buildSalt(spawn, spawnScrubber, mnemonicCacheSalts)); + buildSalt(spawn, spawnScrubber, contentKeyForSalt, mnemonicCacheSalts)); ActionKey actionKey = digestUtil.computeActionKey(action); diff --git a/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java b/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java index 7fd399624a8b1f..e06ee7be8865cb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java +++ b/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java @@ -138,6 +138,35 @@ private SpawnScrubber(Config.Rule ruleProto) { this.salt = ruleProto.getTransform().getSalt(); } + private SpawnScrubber(SpawnScrubber base, ImmutableList extraOmittedInputPatterns) { + this.mnemonicPattern = base.mnemonicPattern; + this.labelPattern = base.labelPattern; + this.kindPattern = base.kindPattern; + this.matchTools = base.matchTools; + this.omittedInputPatterns = + ImmutableList.builder() + .addAll(base.omittedInputPatterns) + .addAll(extraOmittedInputPatterns) + .build(); + this.omittedInputPatternsExternal = base.omittedInputPatternsExternal; + this.argReplacements = base.argReplacements; + this.omittedEnvVars = base.omittedEnvVars; + this.salt = base.salt; + } + + /** Returns a new SpawnScrubber identical to this one but with first-party .jar inputs omitted. + * + *

External/maven jars (under .../bin/external/) are intentionally kept in the Merkle tree + * so that version bumps are detected naturally without needing the content key to cover them. + * First-party jars are omitted because the content key already captures exactly which + * first-party class bytecodes the test uses. + */ + public SpawnScrubber withJarOmission() { + return new SpawnScrubber( + this, + ImmutableList.of(Pattern.compile(".*/bin/(?!external/).*[.]jar$"))); + } + private String emptyToAll(String s) { return s.isEmpty() ? ".*" : s; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 8ebee903f8608b..32bb75d357c952 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -86,6 +86,19 @@ public final class RemoteOptions extends CommonRemoteOptions { + "disable TLS.") public String remoteExecutor; + @Option( + name = "experimental_test_content_key", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "If enabled, TestRunner actions include a per-test content key in the remote cache" + + " salt. The content key (produced by //rules/test:compute_test_key_aspect) hashes" + + " only the class bytecodes transitively used by the test. Combined with jar-omission" + + " scrubbing rules, this makes remote cache hits insensitive to unused dependency" + + " changes, avoiding unnecessary test re-executions.") + public boolean testContentKey; + @Option( name = "experimental_remote_execution_keepalive", defaultValue = "false",