LateBoundDefault: enforce access to a single fragment (or none).
Currently, there is no way to enforce that LateBoundDefaults only access
the fragments that they declare. This means that LateBoundDefaults can
fail to declare fragments at all, or declare the wrong ones, and still
have no troubles.
But when trimming, these fragments must be declared, because otherwise
they will not necessarily be available.
This change refactors LateBoundDefault to declare a single fragment type,
not a set. All existing LateBoundDefaults use sets with a single element
or no elements at all for their set of fragment classes, so this does not
limit anything being done currently.
To account for LateBoundDefaults which do not use configuration at all,
typically those which only want to access the configured attribute map,
it is possible for Void to be the fragment class which is requested.
To account for LateBoundDefaults which need to access methods of the
BuildConfiguration instance itself, it is possible for BuildConfiguration
to be the fragment class which is requested; however, this is unsafe, so
it is only a temporary state until a way to do this without also giving
access to all of the fragments can be added.
Drive-by refactoring: LateBoundDefaults' values are now typed. All actual
production LateBoundDefaults were Label or List<Label> typed, through the
LateBoundLabel and LateBoundLabelList subclasses. These subclasses have
been removed, and LateBoundDefault has two type parameters, one for the
type of its input, and one for the type of its output.
RELNOTES: None.
PiperOrigin-RevId: 169242278
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java
index 924e3ba..5738cb1 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java
@@ -25,12 +25,10 @@
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
-import com.google.devtools.build.lib.packages.AttributeMap;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.packages.NativeAspectClass;
-import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.util.FileTypeSet;
import org.junit.Test;
@@ -74,12 +72,8 @@
Attribute implicit = attr("$runtime", BuildType.LABEL)
.value(Label.parseAbsoluteUnchecked("//run:time"))
.build();
- LateBoundLabel<String> latebound = new LateBoundLabel<String>() {
- @Override
- public Label resolve(Rule rule, AttributeMap attributes, String configuration) {
- return Label.parseAbsoluteUnchecked("//run:away");
- }
- };
+ LateBoundDefault<Void, Label> latebound =
+ LateBoundDefault.fromConstant(Label.parseAbsoluteUnchecked("//run:away"));
AspectDefinition simple = new AspectDefinition.Builder(TEST_ASPECT_CLASS)
.add(implicit)
.add(attr(":latebound", BuildType.LABEL).value(latebound))
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 6267d96..9dcd016 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.analysis.BaseRuleClasses.ACTION_LISTENER;
import static com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode.TARGET;
-import static com.google.devtools.build.lib.analysis.util.TestAspects.EMPTY_LATE_BOUND_LABEL;
import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST;
import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
@@ -44,6 +43,7 @@
import com.google.devtools.build.lib.events.OutputFilter.RegexOutputFilter;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.skyframe.AspectValue;
@@ -426,7 +426,8 @@
@Override
public AspectDefinition getDefinition(AspectParameters params) {
return new AspectDefinition.Builder(this)
- .add(attr(":late", LABEL).value(EMPTY_LATE_BOUND_LABEL)).build();
+ .add(attr(":late", LABEL).value(LateBoundDefault.alwaysNull()))
+ .build();
}
@Override
@@ -448,10 +449,9 @@
}
/**
- * An Aspect has a late-bound attribute with no value (that is, a LateBoundLabel whose
- * getDefault() returns `null`).
- * Test that this attribute is available in the RuleContext which is provided to the Aspect's
- * `create()` method.
+ * An Aspect has a late-bound attribute with no value (that is, a LateBoundDefault whose
+ * getDefault() returns `null`). Test that this attribute is available in the RuleContext which is
+ * provided to the Aspect's `create()` method.
*/
@Test
public void emptyAspectAttributesAreAvailableInRuleContext() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
index 7112ba6..67b125f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
@@ -20,15 +20,12 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.PatchTransition;
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.AttributeMap;
-import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -59,30 +56,22 @@
}
};
- /**
- * Mock late-bound attribute resolver that returns a fixed label.
- */
- private static final Attribute.LateBoundLabel<BuildConfiguration> LATEBOUND_VALUE_RESOLVER =
- new Attribute.LateBoundLabel<BuildConfiguration>() {
- @Override
- public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration config) {
- return Label.parseAbsoluteUnchecked("//foo:latebound_dep");
- }
- };
-
- /**
- * Rule definition with a latebound dependency.
- */
- private static final RuleDefinition LATE_BOUND_DEP_RULE = (MockRule) () -> MockRule.define(
- "rule_with_latebound_attr",
- (builder, env) -> {
- builder
- .add(
- attr(":latebound_attr", LABEL)
- .value(LATEBOUND_VALUE_RESOLVER)
- .cfg(CHANGE_FOO_FLAG_TRANSITION))
- .requiresConfigurationFragments(LateBoundSplitUtil.TestFragment.class);
- });
+ /** Rule definition with a latebound dependency. */
+ private static final RuleDefinition LATE_BOUND_DEP_RULE =
+ (MockRule)
+ () ->
+ MockRule.define(
+ "rule_with_latebound_attr",
+ (builder, env) -> {
+ builder
+ .add(
+ attr(":latebound_attr", LABEL)
+ .value(
+ Attribute.LateBoundDefault.fromConstant(
+ Label.parseAbsoluteUnchecked("//foo:latebound_dep")))
+ .cfg(CHANGE_FOO_FLAG_TRANSITION))
+ .requiresConfigurationFragments(LateBoundSplitUtil.TestFragment.class);
+ });
@Before
public void setupCustomLateBoundRules() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
index 36df7a2..4296f10 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
@@ -27,9 +27,7 @@
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.common.options.Option;
@@ -97,32 +95,24 @@
}
}
- /**
- * The resolver that chooses the late-bound attribute's value.
- */
- private static final Attribute.LateBoundLabel<BuildConfiguration> SIMPLE_LATEBOUND_RESOLVER =
- new Attribute.LateBoundLabel<BuildConfiguration>() {
- @Override
- public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
- return Label.parseAbsoluteUnchecked("//foo:latebound_dep");
- }
- };
-
- /**
- * A custom rule that applies a late-bound split attribute.
- */
- static final RuleDefinition RULE_WITH_LATEBOUND_SPLIT_ATTR = (MockRule) () -> MockRule.define(
- "rule_with_latebound_split",
- (builder, env) -> {
- builder
- .add(
- attr(":latebound_split_attr", BuildType.LABEL)
- .allowedFileTypes(FileTypeSet.ANY_FILE)
- .allowedRuleClasses(Attribute.ANY_RULE)
- .cfg(SIMPLE_SPLIT)
- .value(SIMPLE_LATEBOUND_RESOLVER))
- .requiresConfigurationFragments(TestFragment.class);
- });
+ /** A custom rule that applies a late-bound split attribute. */
+ static final RuleDefinition RULE_WITH_LATEBOUND_SPLIT_ATTR =
+ (MockRule)
+ () ->
+ MockRule.define(
+ "rule_with_latebound_split",
+ (builder, env) -> {
+ builder
+ .add(
+ attr(":latebound_split_attr", BuildType.LABEL)
+ .allowedFileTypes(FileTypeSet.ANY_FILE)
+ .allowedRuleClasses(Attribute.ANY_RULE)
+ .cfg(SIMPLE_SPLIT)
+ .value(
+ Attribute.LateBoundDefault.fromConstant(
+ Label.parseAbsoluteUnchecked("//foo:latebound_dep"))))
+ .requiresConfigurationFragments(TestFragment.class);
+ });
/**
* A custom rule that requires {@link TestFragment}.
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
index 44c4e62..7f7049c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
@@ -19,13 +19,10 @@
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -100,13 +97,8 @@
.value(Label.parseAbsoluteUnchecked("//helpers:implicit")))
.add(Attribute.attr(":latebound", BuildType.LABEL)
.value(
- new Attribute.LateBoundLabel<BuildConfiguration>() {
- @Override
- public Label resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return Label.parseAbsoluteUnchecked("//helpers:latebound");
- }
- }))
+ Attribute.LateBoundDefault.fromConstant(
+ Label.parseAbsoluteUnchecked("//helpers:latebound"))))
.add(Attribute.attr("normal", BuildType.LABEL)
.allowedFileTypes(FileTypeSet.NO_FILE)
.value(Label.parseAbsoluteUnchecked("//helpers:default"))));
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index d731a95..b5ff067 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -51,9 +51,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
-import com.google.devtools.build.lib.packages.AttributeMap;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.Rule;
@@ -72,13 +70,6 @@
*/
public class TestAspects {
- public static final LateBoundLabel EMPTY_LATE_BOUND_LABEL = new LateBoundLabel<Object>() {
- @Override
- public Label resolve(Rule rule, AttributeMap attributes, Object configuration) {
- return null;
- }
- };
-
/**
* A transitive info provider for collecting aspects in the transitive closure. Created by
* aspects.
@@ -999,14 +990,12 @@
* Rule with a late-bound dependency.
*/
public static class LateBoundDepRule implements RuleDefinition {
- private static final LateBoundLabelList<BuildConfiguration> PLUGINS_LABEL_LIST =
- new LateBoundLabelList<BuildConfiguration>() {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.getPlugins();
- }
- };
+ // TODO(b/65746853): provide a way to do this without passing the entire configuration
+ private static final LateBoundDefault<?, List<Label>> PLUGINS_LABEL_LIST =
+ LateBoundDefault.fromTargetConfiguration(
+ BuildConfiguration.class,
+ ImmutableList.of(),
+ (rule, attributes, configuration) -> configuration.getPlugins());
@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/AttributeValueSourceTest.java b/src/test/java/com/google/devtools/build/lib/packages/AttributeValueSourceTest.java
index c0c0c26..513c3e0 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/AttributeValueSourceTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/AttributeValueSourceTest.java
@@ -22,13 +22,10 @@
import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
import com.google.devtools.build.lib.syntax.EvalException;
-
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.util.Set;
-
/**
* Test class for {@link AttributeValueSource}.
*/
@@ -126,30 +123,7 @@
});
private static final Attribute.Builder<?> LATE_BOUND_BUILDER =
- attr(":x", STRING)
- .value(
- new LateBoundDefault<String>() {
- @Override
- public boolean useHostConfiguration() {
- return false;
- }
-
- @Override
- public Set<Class<?>> getRequiredConfigurationFragments() {
- return null;
- }
-
- @Override
- public Object getDefault() {
- return null;
- }
-
- @Override
- public Object resolve(Rule rule, AttributeMap attributes, String o)
- throws EvalException, InterruptedException {
- return null;
- }
- });
+ attr(":x", STRING).value(LateBoundDefault.alwaysNull());
private static final Attribute.Builder<?> DIRECT_BUILDER = attr("x", STRING).value("value");
}
