diff options
author | mstaib <mstaib@google.com> | 2017-09-19 17:06:32 +0200 |
---|---|---|
committer | László Csomor <laszlocsomor@google.com> | 2017-09-19 17:18:55 +0200 |
commit | 807a9b236963ff863573050d5aba146a9bbe23db (patch) | |
tree | 62b94e0f55a8c3f2a6f806f0f2288a507a6a5f77 /src/test/java/com/google/devtools/build/lib/analysis | |
parent | fd62e761b5109a721aeec7879c7194404a512535 (diff) |
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
Diffstat (limited to 'src/test/java/com/google/devtools/build/lib/analysis')
6 files changed, 52 insertions, 98 deletions
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 924e3ba48b..5738cb1923 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.AspectDefinition; 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 @@ public class AspectDefinitionTest { 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 6267d9658f..9dcd016a17 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 @@ package com.google.devtools.build.lib.analysis; 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.collect.nestedset.Order; 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 @@ public class AspectTest extends AnalysisTestCase { @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 @@ public class AspectTest extends AnalysisTestCase { } /** - * 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 7112ba6dfa..67b125f68d 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 static com.google.devtools.build.lib.packages.BuildType.LABEL; 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 @@ public class ConfigurationsForLateBoundTargetsTest extends AnalysisTestCase { } }; - /** - * 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 36df7a21d5..4296f10375 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.config.InvalidConfigurationExcepti 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 @@ public class LateBoundSplitUtil { } } - /** - * 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 44c4e62709..7f7049c152 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.BaseRuleClasses; 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 @@ public class ConstraintsTest extends AbstractConstraintsTest { .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 d731a956e7..b5ff067ecb 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.collect.nestedset.Order; 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 @@ import java.util.List; */ 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 @@ public class TestAspects { * 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) { |