Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.1.0] Make it possible to toggle cache key scrubbing by rule kind #21276

Merged
merged 1 commit into from
Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/main/java/com/google/devtools/build/lib/remote/Scrubber.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.remote.RemoteScrubbing.Config;
Expand Down Expand Up @@ -104,6 +105,7 @@ public static class SpawnScrubber {

private final Pattern mnemonicPattern;
private final Pattern labelPattern;
private final Pattern kindPattern;
private final boolean matchTools;

private final ImmutableList<Pattern> omittedInputPatterns;
Expand All @@ -114,6 +116,7 @@ private SpawnScrubber(Config.Rule ruleProto) {
Config.Matcher matcherProto = ruleProto.getMatcher();
this.mnemonicPattern = Pattern.compile(emptyToAll(matcherProto.getMnemonic()));
this.labelPattern = Pattern.compile(emptyToAll(matcherProto.getLabel()));
this.kindPattern = Pattern.compile(emptyToAll(matcherProto.getKind()));
this.matchTools = matcherProto.getMatchTools();

Config.Transform transformProto = ruleProto.getTransform();
Expand All @@ -134,12 +137,15 @@ private String emptyToAll(String s) {
/** Whether this scrubber applies to the given {@link Spawn}. */
private boolean matches(Spawn spawn) {
String mnemonic = spawn.getMnemonic();
String label = spawn.getResourceOwner().getOwner().getLabel().getCanonicalForm();
boolean isForTool = spawn.getResourceOwner().getOwner().isBuildConfigurationForTool();
ActionOwner actionOwner = spawn.getResourceOwner().getOwner();
String label = actionOwner.getLabel().getCanonicalForm();
String kind = actionOwner.getTargetKind();
boolean isForTool = actionOwner.isBuildConfigurationForTool();

return (!isForTool || matchTools)
&& mnemonicPattern.matcher(mnemonic).matches()
&& labelPattern.matcher(label).matches();
&& labelPattern.matcher(label).matches()
&& kindPattern.matcher(kind).matches();
}

/** Whether the given input should be omitted from the cache key. */
Expand Down
4 changes: 4 additions & 0 deletions src/main/protobuf/remote_scrubbing.proto
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ option java_package = "com.google.devtools.build.lib.remote";
message Config {
// Describes a set of actions. An action must pass all criteria to match.
message Matcher {
// A regex matching the rule kind of the action owner.
// Use .* if a partial match is desired.
// If empty, matches every kind.
string kind = 4;
// A regex matching the canonical label of the action owner.
// Use .* if a partial match is desired.
// If empty, matches every label.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@
public class FakeOwner implements ActionExecutionMetadata {
private final String mnemonic;
private final String progressMessage;
@Nullable private final String ownerLabel;
private final String ownerLabel;
private final String ownerRuleKind;
@Nullable private final Artifact primaryOutput;
@Nullable private final PlatformInfo platform;
private final ImmutableMap<String, String> execProperties;
Expand All @@ -53,13 +54,15 @@ public class FakeOwner implements ActionExecutionMetadata {
String mnemonic,
String progressMessage,
String ownerLabel,
String ownerRuleKind,
@Nullable Artifact primaryOutput,
@Nullable PlatformInfo platform,
ImmutableMap<String, String> execProperties,
boolean isBuiltForToolConfiguration) {
this.mnemonic = mnemonic;
this.progressMessage = progressMessage;
this.ownerLabel = checkNotNull(ownerLabel);
this.ownerRuleKind = checkNotNull(ownerRuleKind);
this.primaryOutput = primaryOutput;
this.platform = platform;
this.execProperties = execProperties;
Expand All @@ -72,6 +75,7 @@ private FakeOwner(
mnemonic,
progressMessage,
ownerLabel,
/* ownerRuleKind= */ "dummy-target-kind",
/* primaryOutput= */ null,
platform,
ImmutableMap.of(),
Expand All @@ -87,7 +91,7 @@ public ActionOwner getOwner() {
return ActionOwner.createDummy(
Label.parseCanonicalUnchecked(ownerLabel),
new Location("dummy-file", 0, 0),
/* targetKind= */ "dummy-target-kind",
ownerRuleKind,
mnemonic,
/* configurationChecksum= */ "configurationChecksum",
new BuildConfigurationEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public final class SpawnBuilder {
private String mnemonic = "Mnemonic";
private String progressMessage = "progress message";
private String ownerLabel = "//dummy:label";
private String ownerRuleKind = "dummy-target-kind";
@Nullable private Artifact ownerPrimaryOutput;
@Nullable private PlatformInfo platform;
private final List<String> args;
Expand Down Expand Up @@ -96,6 +97,7 @@ public Spawn build() {
mnemonic,
progressMessage,
ownerLabel,
ownerRuleKind,
ownerPrimaryOutput,
platform,
execProperties,
Expand Down Expand Up @@ -140,6 +142,12 @@ public SpawnBuilder withOwnerLabel(String ownerLabel) {
return this;
}

@CanIgnoreReturnValue
public SpawnBuilder withOwnerRuleKind(String ownerRuleKind) {
this.ownerRuleKind = checkNotNull(ownerRuleKind);
return this;
}

@CanIgnoreReturnValue
public SpawnBuilder withOwnerPrimaryOutput(Artifact output) {
ownerPrimaryOutput = checkNotNull(output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,54 @@ public void matchWildcardLabel() {
assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo"))).isNull();
}

@Test
public void matchExactKind() {
var scrubber =
new Scrubber(
Config.newBuilder()
.addRules(
Config.Rule.newBuilder()
.setMatcher(Config.Matcher.newBuilder().setKind("java_library")))
.build());

assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", false)))
.isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//foo:barbaz", "Foo", "java_test", false))).isNull();
}

@Test
public void matchUnionKind() {
var scrubber =
new Scrubber(
Config.newBuilder()
.addRules(
Config.Rule.newBuilder()
.setMatcher(Config.Matcher.newBuilder().setKind("java_library|java_test")))
.build());

assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", false)))
.isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo", "java_test", false)))
.isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//quux:xyzzy", "Foo", "go_library", false))).isNull();
}

@Test
public void matchWildcardKind() {
var scrubber =
new Scrubber(
Config.newBuilder()
.addRules(
Config.Rule.newBuilder()
.setMatcher(Config.Matcher.newBuilder().setKind("java_.*")))
.build());

assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", false)))
.isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//foo:baz", "Foo", "java_test", false))).isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//spam:eggs", "Foo", "go_library", false))).isNull();
}

@Test
public void rejectToolAction() {
var scrubber =
Expand All @@ -137,7 +185,9 @@ public void rejectToolAction() {
.build());

assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", /* forTool= */ true))).isNull();
assertThat(
scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", /* forTool= */ true)))
.isNull();
}

@Test
Expand All @@ -155,7 +205,9 @@ public void acceptToolAction() {
.build());

assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo"))).isNotNull();
assertThat(scrubber.forSpawn(createSpawn("//foo:bar", "Foo", /* forTool= */ true))).isNotNull();
assertThat(
scrubber.forSpawn(createSpawn("//foo:bar", "Foo", "java_library", /* forTool= */ true)))
.isNotNull();
}

@Test
Expand Down Expand Up @@ -420,13 +472,15 @@ private static Spawn createSpawn() {
}

private static Spawn createSpawn(String label, String mnemonic) {
return createSpawn(label, mnemonic, /* forTool= */ false);
return createSpawn(label, mnemonic, /* ruleKind= */ "dummy-target-kind", /* forTool= */ false);
}

private static Spawn createSpawn(String label, String mnemonic, boolean forTool) {
private static Spawn createSpawn(
String label, String mnemonic, String ruleKind, boolean forTool) {
return new SpawnBuilder("cmd")
.withOwnerLabel(label)
.withMnemonic(mnemonic)
.withOwnerRuleKind(ruleKind)
.setBuiltForToolConfiguration(forTool)
.build();
}
Expand Down
Loading