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 | |
| 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')
34 files changed, 636 insertions, 810 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index ddadda214f..6bc24fa7fc 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -513,6 +513,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index f52d66f209..8a8e815ad2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -40,11 +40,9 @@ import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; -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.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.Transition; import com.google.devtools.build.lib.packages.AttributeMap; -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.packages.RuleClass.Builder.RuleClassType; @@ -73,35 +71,32 @@ public class BaseRuleClasses { } }; + // TODO(b/65746853): provide a way to do this without passing the entire configuration /** * Implementation for the :action_listener attribute. + * + * <p>action_listeners are special rules; they tell the build system to add extra_actions to + * existing rules. As such they need an edge to every ConfiguredTarget with the limitation that + * they only run on the target configuration and should not operate on action_listeners and + * extra_actions themselves (to avoid cycles). */ @VisibleForTesting - static final LateBoundLabelList<BuildConfiguration> ACTION_LISTENER = - new LateBoundLabelList<BuildConfiguration>() { - @Override - public List<Label> resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - // action_listeners are special rules; they tell the build system to add extra_actions to - // existing rules. As such they need an edge to every ConfiguredTarget with the limitation - // that they only run on the target configuration and should not operate on action_listeners - // and extra_actions themselves (to avoid cycles). - return configuration.getActionListeners(); - } - }; + static final LateBoundDefault<?, List<Label>> ACTION_LISTENER = + LateBoundDefault.fromTargetConfiguration( + BuildConfiguration.class, + ImmutableList.of(), + (rule, attributes, configuration) -> configuration.getActionListeners()); - /** - * Implementation for the :run_under attribute. - */ - public static final LateBoundLabel<BuildConfiguration> RUN_UNDER = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - RunUnder runUnder = configuration.getRunUnder(); - return runUnder == null ? null : runUnder.getLabel(); - } - }; + // TODO(b/65746853): provide a way to do this without passing the entire configuration + /** Implementation for the :run_under attribute. */ + public static final LateBoundDefault<?, Label> RUN_UNDER = + LateBoundDefault.fromTargetConfiguration( + BuildConfiguration.class, + null, + (rule, attributes, configuration) -> { + RunUnder runUnder = configuration.getRunUnder(); + return runUnder != null ? runUnder.getLabel() : null; + }); /** * A base rule for all test rules. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 4997a895f8..855c985486 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -367,9 +368,7 @@ public abstract class DependencyResolver { continue; } - @SuppressWarnings("unchecked") - LateBoundDefault<BuildConfiguration> lateBoundDefault = - (LateBoundDefault<BuildConfiguration>) attribute.getLateBoundDefault(); + LateBoundDefault<?, ?> lateBoundDefault = attribute.getLateBoundDefault(); Collection<BuildOptions> splitOptions = getSplitOptions(depResolver.rule, attribute, ruleConfig); @@ -452,20 +451,8 @@ public abstract class DependencyResolver { throws EvalException, InterruptedException { Preconditions.checkArgument(attribute.isLateBound()); - @SuppressWarnings("unchecked") - LateBoundDefault<BuildConfiguration> lateBoundDefault = - (LateBoundDefault<BuildConfiguration>) attribute.getLateBoundDefault(); - - // TODO(bazel-team): This might be too expensive - can we cache this somehow? - if (!lateBoundDefault.getRequiredConfigurationFragments().isEmpty()) { - if (!config.hasAllFragments(lateBoundDefault.getRequiredConfigurationFragments())) { - return ImmutableList.<Label>of(); - } - } - - // TODO(bazel-team): We should check if the implementation tries to access an undeclared - // fragment. - Object actualValue = lateBoundDefault.resolve(rule, attributeMap, config); + Object actualValue = + resolveLateBoundDefault(attribute.getLateBoundDefault(), rule, attributeMap, config); if (EvalUtils.isNullOrNone(actualValue)) { return ImmutableList.<Label>of(); } @@ -495,6 +482,29 @@ public abstract class DependencyResolver { } } + @VisibleForTesting(/* used to test LateBoundDefaults' default values */ ) + public static <FragmentT, ValueT> ValueT resolveLateBoundDefault( + LateBoundDefault<FragmentT, ValueT> lateBoundDefault, + Rule rule, + AttributeMap attributeMap, + BuildConfiguration config) { + Class<FragmentT> fragmentClass = lateBoundDefault.getFragmentClass(); + // TODO(b/65746853): remove this when nothing uses it anymore + if (BuildConfiguration.class.equals(fragmentClass)) { + return lateBoundDefault.resolve(rule, attributeMap, fragmentClass.cast(config)); + } + if (Void.class.equals(fragmentClass)) { + return lateBoundDefault.resolve(rule, attributeMap, null); + } + FragmentT fragment = + fragmentClass.cast( + config.getFragment((Class<? extends BuildConfiguration.Fragment>) fragmentClass)); + if (fragment == null) { + return null; + } + return lateBoundDefault.resolve(rule, attributeMap, fragment); + } + /** * Adds new dependencies to the given rule under the given attribute name * diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java b/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java index 8ddadf130f..07d8c755b6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.util.Preconditions; - import java.util.Locale; import java.util.Map; import java.util.Set; @@ -29,52 +28,41 @@ import java.util.Set; /** * A helper class to compute and inject a defaults package into the package cache. * - * <p>The <code>//tools/defaults</code> package provides a mechanism let tool locations be - * specified over the commandline, without requiring any special support in the rule code. - * As such, it can be used in genrule <code>$(location)</code> substitutions. + * <p>The <code>//tools/defaults</code> package provides a mechanism let tool locations be specified + * over the commandline, without requiring any special support in the rule code. As such, it can be + * used in genrule <code>$(location)</code> substitutions. * * <p>It works as follows: - * <ul> - * - * <li> SomeLanguage.createCompileAction will refer to a host-configured target for the - * compiler by looking for - * <code>env.getHostPrerequisiteArtifact("$somelanguage_compiler")</code>. - * - * <li> the attribute <code>$somelanguage_compiler</code> is defined in the - * {@link RuleDefinition} subclass for that language. - * - * <li> if the attribute cannot be set on the command-line, its value may be a normal label. - * - * <li> if the attribute can be set on the command-line, its value will be - * <code>//tools/defaults:somelanguage_compiler</code>. - * - * <li> in the latter case, the {@link BuildConfiguration.Fragment} subclass will define the - * option (with an existing target, eg. <code>//third_party/somelanguage:compiler</code>), and - * return the name in its implementation of {@link FragmentOptions#getDefaultsLabels}. - * - * <li> On startup, the rule is wired up with <code>//tools/defaults:somelanguage_compiler</code>. - * - * <li> On starting a build, the <code>//tools/defaults</code> package is synthesized, using - * the values as specified on the command-line. The contents of - * <code>tools/defaults/BUILD</code> is ignored. - * - * <li> Hence, changes in the command line values for tools are now handled exactly as if they - * were changes in a BUILD file. - * - * <li> The file <code>tools/defaults/BUILD</code> must exist, so we create a package in that - * location. - * - * <li> The code in {@link DefaultsPackage} can dump the synthesized package as a BUILD file, - * so external tooling does not need to understand the intricacies of handling command-line - * options. * + * <ul> + * <li>SomeLanguage.createCompileAction will refer to a host-configured target for the compiler by + * looking for <code>env.getHostPrerequisiteArtifact("$somelanguage_compiler")</code>. + * <li>the attribute <code>$somelanguage_compiler</code> is defined in the {@link RuleDefinition} + * subclass for that language. + * <li>if the attribute cannot be set on the command-line, its value may be a normal label. + * <li>if the attribute can be set on the command-line, its value will be <code> + * //tools/defaults:somelanguage_compiler</code>. + * <li>in the latter case, the {@link BuildConfiguration.Fragment} subclass will define the option + * (with an existing target, eg. <code>//third_party/somelanguage:compiler</code>), and return + * the name in its implementation of {@link FragmentOptions#getDefaultsLabels}. + * <li>On startup, the rule is wired up with <code>//tools/defaults:somelanguage_compiler</code>. + * <li>On starting a build, the <code>//tools/defaults</code> package is synthesized, using the + * values as specified on the command-line. The contents of <code>tools/defaults/BUILD</code> + * is ignored. + * <li>Hence, changes in the command line values for tools are now handled exactly as if they were + * changes in a BUILD file. + * <li>The file <code>tools/defaults/BUILD</code> must exist, so we create a package in that + * location. + * <li>The code in {@link DefaultsPackage} can dump the synthesized package as a BUILD file, so + * external tooling does not need to understand the intricacies of handling command-line + * options. * </ul> * - * <p>For built-in rules (as opposed to genrules), late-bound labels provide an alternative - * method of depending on command-line values. These work by declaring attribute default values - * to be {@link LateBoundLabel} instances, whose <code>resolve(Rule rule, AttributeMap attributes, - * T configuration)</code> method will have access to {@link BuildConfiguration}, which in turn - * may depend on command line flag values. + * <p>For built-in rules (as opposed to genrules), late-bound labels provide an alternative method + * of depending on command-line values. These work by declaring attribute default values to be + * {@link LateBoundDefault} instances, whose <code>resolve(Rule rule, AttributeMap attributes, + * FragmentT configuration)</code> method will have access to a {@link BuildConfiguration.Fragment}, + * which in turn may depend on command line flag values. */ public final class DefaultsPackage { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index 21a5a31ffd..39f1dcbf93 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -74,7 +74,7 @@ import com.google.devtools.build.lib.bazel.rules.workspace.MavenServerRule; import com.google.devtools.build.lib.bazel.rules.workspace.NewGitRepositoryRule; import com.google.devtools.build.lib.bazel.rules.workspace.NewHttpArchiveRule; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.rules.Alias.AliasRule; import com.google.devtools.build.lib.rules.android.AarImportBaseRule; import com.google.devtools.build.lib.rules.android.AndroidBinaryOnlyRule; @@ -464,8 +464,7 @@ public class BazelRuleClassProvider { new RuleSet() { @Override public void init(Builder builder) { - LateBoundLabel<BuildConfiguration> hostJdkAttribute = - JavaSemantics.hostJdkAttribute(builder); + LateBoundDefault<?, Label> hostJdkAttribute = JavaSemantics.hostJdkAttribute(builder); BazelJavaProtoAspect bazelJavaProtoAspect = new BazelJavaProtoAspect(hostJdkAttribute); BazelJavaLiteProtoAspect bazelJavaLiteProtoAspect = new BazelJavaLiteProtoAspect(hostJdkAttribute); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidRuleClasses.java index ca5a6699dd..abc40e90ef 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidRuleClasses.java @@ -16,13 +16,13 @@ package com.google.devtools.build.lib.bazel.rules.android; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; +import static com.google.devtools.build.lib.rules.android.AndroidRuleClasses.getAndroidSdkLabel; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.rules.android.AndroidRuleClasses; -import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidSdkLabel; import com.google.devtools.build.lib.rules.android.AndroidToolsDefaultsJar; /** Rule class definitions for Android rules. */ @@ -37,8 +37,9 @@ public class BazelAndroidRuleClasses { .add( attr(":android_sdk", LABEL) .allowedRuleClasses("android_sdk", "filegroup") - .value(new AndroidSdkLabel( - environment.getToolsLabel(AndroidRuleClasses.DEFAULT_SDK)))) + .value( + getAndroidSdkLabel( + environment.getToolsLabel(AndroidRuleClasses.DEFAULT_SDK)))) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java index dc79061223..b5778480ab 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java @@ -42,15 +42,12 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.PlatformConfiguration; 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.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; -import com.google.devtools.build.lib.packages.RawAttributeMapper; -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.packages.RuleClass.Builder.RuleClassType; @@ -73,36 +70,30 @@ public class BazelCppRuleClasses { static final SafeImplicitOutputsFunction CC_BINARY_IMPLICIT_OUTPUTS = fromFunctions(CppRuleClasses.CC_BINARY_STRIPPED, CppRuleClasses.CC_BINARY_DEBUG_PACKAGE); - public static final LateBoundLabel<BuildConfiguration> STL = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - return getStl(rule, configuration); - } - }; - /** * Returns the STL prerequisite of the rule. * - * <p>If rule has an implicit $stl_default attribute returns STL version set on the - * command line or if not set, the value of the $stl_default attribute. Returns - * {@code null} otherwise. + * <p>If rule has an implicit $stl_default attribute returns STL version set on the command line + * or if not set, the value of the $stl_default attribute. Returns {@code null} otherwise. */ - private static Label getStl(Rule rule, BuildConfiguration original) { - Label stl = null; - if (rule.getRuleClassObject().hasAttr("$stl_default", BuildType.LABEL)) { - Label stlConfigLabel = original.getFragment(CppConfiguration.class).getStl(); - Label stlRuleLabel = RawAttributeMapper.of(rule).get("$stl_default", BuildType.LABEL); - if (stlConfigLabel == null) { - stl = stlRuleLabel; - } else if (!stlConfigLabel.equals(rule.getLabel()) && stlRuleLabel != null) { - // prevents self-reference and a cycle through standard STL in the dependency graph - stl = stlConfigLabel; - } - } - return stl; - } + public static final LateBoundDefault<?, Label> STL = + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + null, + (rule, attributes, cppConfig) -> { + Label stl = null; + if (attributes.has("$stl_default", BuildType.LABEL)) { + Label stlConfigLabel = cppConfig.getStl(); + Label stlRuleLabel = attributes.get("$stl_default", BuildType.LABEL); + if (stlConfigLabel == null) { + stl = stlRuleLabel; + } else if (!stlConfigLabel.equals(rule.getLabel()) && stlRuleLabel != null) { + // prevents self-reference and a cycle through standard STL in the dependency graph + stl = stlConfigLabel; + } + } + return stl; + }); static final FileTypeSet ALLOWED_SRC_FILES = FileTypeSet.of( @@ -421,14 +412,14 @@ public class BazelCppRuleClasses { } /** Implementation for the :lipo_context attribute. */ - static final LateBoundLabel<BuildConfiguration> LIPO_CONTEXT = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - Label result = configuration.getFragment(CppConfiguration.class).getLipoContextLabel(); - return (rule == null || rule.getLabel().equals(result)) ? null : result; - } - }; + static final LateBoundDefault<?, Label> LIPO_CONTEXT = + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + null, + (rule, attributes, cppConfig) -> { + Label result = cppConfig.getLipoContextLabel(); + return (rule == null || rule.getLabel().equals(result)) ? null : result; + }); /** * Helper rule class. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoAspect.java index 041956afe6..43139b40c5 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoAspect.java @@ -14,9 +14,9 @@ package com.google.devtools.build.lib.bazel.rules.java.proto; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaSemantics; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.rules.java.proto.JavaLiteProtoAspect; /** An Aspect which BazelJavaLiteProtoLibrary injects to build Java Lite protos. */ @@ -25,7 +25,7 @@ public class BazelJavaLiteProtoAspect extends JavaLiteProtoAspect { public static final String DEFAULT_PROTO_TOOLCHAIN_LABEL = "@com_google_protobuf_javalite//:javalite_toolchain"; - public BazelJavaLiteProtoAspect(LateBoundLabel<BuildConfiguration> hostJdkAttribute) { + public BazelJavaLiteProtoAspect(LateBoundDefault<?, Label> hostJdkAttribute) { super( BazelJavaSemantics.INSTANCE, null /* jacocoLabel */, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java index 9aed78a240..98f37d0fa8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java @@ -20,13 +20,13 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaSemantics; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; 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.LateBoundDefault; import com.google.devtools.build.lib.rules.java.proto.JavaProtoAspect; import com.google.devtools.build.lib.rules.java.proto.RpcSupport; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder; @@ -35,7 +35,7 @@ import java.util.List; /** An Aspect which BazelJavaProtoLibrary injects to build Java SPEED protos. */ public class BazelJavaProtoAspect extends JavaProtoAspect { - public BazelJavaProtoAspect(LateBoundLabel<BuildConfiguration> hostJdkAttribute) { + public BazelJavaProtoAspect(LateBoundDefault<?, Label> hostJdkAttribute) { super( BazelJavaSemantics.INSTANCE, null, /* jacocoAttr */ diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java index 006b82c660..dee60f9ba7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java @@ -26,12 +26,9 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.BaseRuleClasses; 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.bazel.rules.cpp.BazelCppRuleClasses; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; -import com.google.devtools.build.lib.packages.AttributeMap; -import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TriState; @@ -46,13 +43,11 @@ import com.google.devtools.build.lib.util.FileType; public final class BazelPyRuleClasses { public static final FileType PYTHON_SOURCE = FileType.of(".py"); - public static final LateBoundLabel<BuildConfiguration> PY_INTERPRETER = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(BazelPythonConfiguration.class).getPythonTop(); - } - }; + public static final LateBoundDefault<?, Label> PY_INTERPRETER = + LateBoundDefault.fromTargetConfiguration( + BazelPythonConfiguration.class, + null, + (rule, attributes, bazelPythonConfig) -> bazelPythonConfig.getPythonTop()); /** * Base class for Python rule definitions. diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 5a020be02f..2fedb6a0b5 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -28,7 +28,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; @@ -676,9 +675,9 @@ public final class Attribute implements Comparable<Attribute> { /** * Sets the attribute default value to be late-bound, i.e., it is derived from the build - * configuration. + * configuration and/or the rule's configured attributes. */ - public Builder<TYPE> value(LateBoundDefault<?> defaultValue) { + public Builder<TYPE> value(LateBoundDefault<?, ? extends TYPE> defaultValue) { Preconditions.checkState(!valueSet, "the default value is already set"); Preconditions.checkState(name.isEmpty() || isLateBound(name)); value = defaultValue; @@ -1549,137 +1548,204 @@ public final class Attribute implements Comparable<Attribute> { } } + // TODO(b/65746853): Remove documentation about accepting BuildConfiguration when uses are cleaned + // up. /** - * Marker interface for late-bound values. Unfortunately, we can't refer to BuildConfiguration - * right now, since that is in a separate compilation unit. - * - * <p>Implementations of this interface must be immutable. + * Provider of values for late-bound attributes. See + * {@link Attribute#value(LateBoundDefault<?, ? extends TYPE> value)}. * * <p>Use sparingly - having different values for attributes during loading and analysis can * confuse users. + * + * @param <FragmentT> The type of value that is used to compute this value. This is usually a + * subclass of BuildConfiguration.Fragment. It may also be Void to receive null, or + * BuildConfiguration itself to receive the entire configuration. + * @param <ValueT> The type of value returned by this class. */ - public interface LateBoundDefault<T> { + @Immutable + public static final class LateBoundDefault<FragmentT, ValueT> { /** - * Whether to look up the label in the host configuration. This is only here for host - * compilation tools - we usually need to look up labels in the target configuration. + * Functional interface for computing the value of a late-bound attribute. * - * <p>This method only sets the configuration passed to {@link #resolve}. If you want the - * dependency to also be analyzed in the host configuration, use - * {@link ConfigurationTransition#HOST}. + * <p>Implementations of this interface must be immutable. */ - boolean useHostConfiguration(); + @FunctionalInterface + public interface Resolver<FragmentT, ValueT> { + ValueT resolve(Rule rule, AttributeMap attributeMap, FragmentT input); + } - /** - * Returns the set of required configuration fragments, i.e., fragments that will be accessed by - * the code. - */ - Set<Class<?>> getRequiredConfigurationFragments(); + private final boolean useHostConfiguration; + private final ValueT defaultValue; + private final Class<FragmentT> fragmentClass; + private final Resolver<FragmentT, ValueT> resolver; /** - * The default value for the attribute that is set during the loading phase. + * Creates a new LateBoundDefault which uses the rule, its configured attributes, and a fragment + * of the target configuration. + * + * <p>Note that the configuration fragment here does not take into account any transitions that + * are on the attribute with this LateBoundDefault as its value. The configuration will be the + * same as the configuration given to the target bearing the attribute. + * + * <p>Nearly all LateBoundDefaults should use this constructor. There are few situations where + * it isn't the appropriate option. + * + * <p>If you want a late-bound dependency which is configured in the host configuration, just + * use this method with {@link ConfigurationTransition#HOST}. If you also need to decide the + * label of the dependency with information gained from the host configuration - and it's very + * unlikely that you do - you can use {@link #fromHostConfiguration} as well. + * + * <p>If you want to decide an attribute's value based on the value of its other attributes, + * use a subclass of {@link ComputedDefault}. The only time you should need + * {@link #fromRuleAndAttributes} is if you need access to three or more configurable + * attributes, or if you need to match names with a late-bound attribute on another rule. + * + * <p>If you have a constant-valued attribute, but you need it to have the same name as an + * attribute on another rule which is late-bound, use {@link #fromConstant} or + * {@link #alwaysNull}. + * + * @param fragmentClass The fragment to receive from the target configuration. May also be + * BuildConfiguration.class to receive the entire configuration (deprecated) - in this case, + * you must only use methods of BuildConfiguration itself, and not use any fragments. + * @param defaultValue The default value to return at loading time, when the configuration is + * not available. + * @param resolver A function which will compute the actual value with the configuration. */ - Object getDefault(); + public static <FragmentT, ValueT> LateBoundDefault<FragmentT, ValueT> fromTargetConfiguration( + Class<FragmentT> fragmentClass, ValueT defaultValue, Resolver<FragmentT, ValueT> resolver) { + Preconditions.checkArgument( + !fragmentClass.equals(Void.class), + "Use fromRuleAndAttributesOnly to specify a LateBoundDefault which does not use " + + "configuration."); + return new LateBoundDefault<>(false, fragmentClass, defaultValue, resolver); + } /** - * The actual value for the attribute for the analysis phase, which depends on the build - * configuration. Note that configurations transitions are applied after the late-bound - * attribute was evaluated. + * Creates a new LateBoundDefault which uses the rule, its configured attributes, and a fragment + * of the host configuration. * - * @param rule the rule being evaluated - * @param attributes interface for retrieving the values of the rule's other attributes - * @param o the configuration to evaluate with + * <p>This should only be necessary in very specialized cases. In almost all cases, you don't + * need this method, just {@link #fromTargetConfiguration} and + * {@link ConfigurationTransition#HOST}. + * + * <p>This method only affects the configuration fragment passed to {@link #resolve}. You must + * also use {@link ConfigurationTransition#HOST}, so that the dependency will be analyzed in the + * host configuration. + * + * @param fragmentClass The fragment to receive from the host configuration. May also be + * BuildConfiguration.class to receive the entire configuration (deprecated) - in this case, + * you must only use methods of BuildConfiguration itself, and not use any fragments. + * It is very rare that a LateBoundDefault should need a host configuration fragment; use + * {@link #fromTargetConfiguration} in most cases. + * @param defaultValue The default value to return at loading time, when the configuration is + * not available. + * @param resolver A function which will compute the actual value with the configuration. + */ + public static <FragmentT, ValueT> LateBoundDefault<FragmentT, ValueT> fromHostConfiguration( + Class<FragmentT> fragmentClass, ValueT defaultValue, Resolver<FragmentT, ValueT> resolver) { + Preconditions.checkArgument( + !fragmentClass.equals(Void.class), + "Use fromRuleAndAttributesOnly to specify a LateBoundDefault which does not use " + + "configuration."); + return new LateBoundDefault<>(true, fragmentClass, defaultValue, resolver); + } + + /** + * Creates a new LateBoundDefault which uses only the rule and its configured attributes. + * + * <p>This should only be necessary in very specialized cases. In almost all cases, you don't + * need this method, just use {@link ComputedDefault}. + * + * <p>This is used primarily for computing values based on three or more configurable + * attributes and/or matching names with late-bound attributes on other rules. + * + * @param defaultValue The default value to return when the configuration is not available at + * loading time. + * @param resolver A function which will compute the actual value with the configuration. */ - Object resolve(Rule rule, AttributeMap attributes, T o) - throws EvalException, InterruptedException; - } - - /** - * Abstract super class for label-typed {@link LateBoundDefault} implementations that simplifies - * the client code a little and makes it a bit more type-safe. - */ - public abstract static class LateBoundLabel<T> implements LateBoundDefault<T> { - private final Label label; - private final ImmutableSet<Class<?>> requiredConfigurationFragments; - - public LateBoundLabel() { - this((Label) null); - } - - public LateBoundLabel(Class<?>... requiredConfigurationFragments) { - this((Label) null, requiredConfigurationFragments); + public static <ValueT> LateBoundDefault<Void, ValueT> fromRuleAndAttributesOnly( + ValueT defaultValue, Resolver<Void, ValueT> resolver) { + return new LateBoundDefault<>(false, Void.class, defaultValue, resolver); } - public LateBoundLabel(Label label) { - this.label = label; - this.requiredConfigurationFragments = ImmutableSet.of(); + /** + * Creates a new LateBoundDefault which always returns the given value. + * + * <p>This is used primarily for matching names with late-bound attributes on other rules and + * for testing. Use normal default values if the name does not matter. + */ + public static <ValueT> LateBoundDefault<Void, ValueT> fromConstant(final ValueT defaultValue) { + if (defaultValue == null) { + return alwaysNull(); + } + return new LateBoundDefault<>( + false, Void.class, defaultValue, (rule, attributes, unused) -> defaultValue); } - public LateBoundLabel(Label label, Class<?>... requiredConfigurationFragments) { - this.label = label; - this.requiredConfigurationFragments = ImmutableSet.copyOf(requiredConfigurationFragments); + /** + * Creates a new LateBoundDefault which always returns null. + * + * <p>This is used primarily for matching names with late-bound attributes on other rules and + * for testing. Use normal default values if the name does not matter. + */ + @SuppressWarnings("unchecked") // bivariant implementation + public static <ValueT> LateBoundDefault<Void, ValueT> alwaysNull() { + return (LateBoundDefault<Void, ValueT>) ALWAYS_NULL; } - public LateBoundLabel(String label) { - this(Label.parseAbsoluteUnchecked(label)); - } + private static final LateBoundDefault<Void, Void> ALWAYS_NULL = + new LateBoundDefault<>(false, Void.class, null, (rule, attributes, unused) -> null); - public LateBoundLabel(String label, Class<?>... requiredConfigurationFragments) { - this(Label.parseAbsoluteUnchecked(label), requiredConfigurationFragments); + private LateBoundDefault( + boolean useHostConfiguration, + Class<FragmentT> fragmentClass, + ValueT defaultValue, + Resolver<FragmentT, ValueT> resolver) { + this.useHostConfiguration = useHostConfiguration; + this.defaultValue = defaultValue; + this.fragmentClass = fragmentClass; + this.resolver = resolver; } - @Override + /** + * Whether to look up the label in the host configuration. This is only here for host + * compilation tools - we usually need to look up labels in the target configuration. + */ public boolean useHostConfiguration() { - return false; + return useHostConfiguration; } - @Override - public ImmutableSet<Class<?>> getRequiredConfigurationFragments() { - return requiredConfigurationFragments; - } - - @Override - public final Label getDefault() { - return label; - } - - @Override - public abstract Label resolve(Rule rule, AttributeMap attributes, T configuration); - } - - /** - * Abstract super class for label-list-typed {@link LateBoundDefault} implementations that - * simplifies the client code a little and makes it a bit more type-safe. - */ - public abstract static class LateBoundLabelList<T> implements LateBoundDefault<T> { - private final ImmutableList<Label> labels; - private final ImmutableSet<Class<?>> requiredConfigurationFragments; - - public LateBoundLabelList(Class<?>... requiredConfigurationFragments) { - this(ImmutableList.<Label>of(), requiredConfigurationFragments); - } - - public LateBoundLabelList(List<Label> labels, Class<?>... requiredConfigurationFragments) { - this.labels = ImmutableList.copyOf(labels); - this.requiredConfigurationFragments = ImmutableSet.copyOf(requiredConfigurationFragments); - } - - @Override - public boolean useHostConfiguration() { - return false; + /** + * Returns the input type that the attribute expects. This is almost always a configuration + * fragment to be retrieved from the target's configuration (or the host configuration). + * + * <p>It may also be {@link Void} to receive null. This is rarely necessary, but can be used, + * e.g., if the attribute is named to match an attribute in another rule which is late-bound. + * + * <p>It may also be BuildConfiguration to receive the entire configuration. This is deprecated, + * and only necessary when the default is computed from methods of BuildConfiguration itself. + */ + public Class<FragmentT> getFragmentClass() { + return fragmentClass; } - @Override - public ImmutableSet<Class<?>> getRequiredConfigurationFragments() { - return requiredConfigurationFragments; + /** The default value for the attribute that is set during the loading phase. */ + public ValueT getDefault() { + return defaultValue; } - @Override - public final List<Label> getDefault() { - return labels; + /** + * The actual value for the attribute for the analysis phase, which depends on the build + * configuration. Note that configurations transitions are applied after the late-bound + * attribute was evaluated. + * + * @param rule the rule being evaluated + * @param attributes interface for retrieving the values of the rule's other attributes + * @param input the configuration fragment to evaluate with + */ + public ValueT resolve(Rule rule, AttributeMap attributes, FragmentT input) { + return resolver.resolve(rule, attributes, input); } - - @Override - public abstract List<Label> resolve(Rule rule, AttributeMap attributes, T configuration); } private final String name; @@ -1776,7 +1842,7 @@ public final class Attribute implements Comparable<Attribute> { "late bound attributes require a default value that is late bound (and vice versa): %s", name); if (isLateBound(name)) { - LateBoundDefault<?> lateBoundDefault = (LateBoundDefault<?>) defaultValue; + LateBoundDefault<?, ?> lateBoundDefault = (LateBoundDefault<?, ?>) defaultValue; Preconditions.checkArgument(!lateBoundDefault.useHostConfiguration() || (configTransition == ConfigurationTransition.HOST), "a late bound default value using the host configuration must use the host transition"); @@ -2045,8 +2111,8 @@ public final class Attribute implements Comparable<Attribute> { public Object getDefaultValue(Rule rule) { if (!getCondition().apply(rule == null ? null : NonconfigurableAttributeMapper.of(rule))) { return null; - } else if (defaultValue instanceof LateBoundDefault<?>) { - return ((LateBoundDefault<?>) defaultValue).getDefault(); + } else if (defaultValue instanceof LateBoundDefault<?, ?>) { + return ((LateBoundDefault<?, ?>) defaultValue).getDefault(); } else { return defaultValue; } @@ -2061,9 +2127,9 @@ public final class Attribute implements Comparable<Attribute> { return defaultValue; } - public LateBoundDefault<?> getLateBoundDefault() { + public LateBoundDefault<?, ?> getLateBoundDefault() { Preconditions.checkState(isLateBound()); - return (LateBoundDefault<?>) defaultValue; + return (LateBoundDefault<?, ?>) defaultValue; } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index 2842572fd5..b2f064848e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -39,7 +39,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.SplitTransition; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; @@ -191,17 +191,12 @@ public final class AndroidRuleClasses { fromTemplates("%{name}_images/emulator-meta-data.pb"); static final FileType APK = FileType.of(".apk"); - /** - * The default label of android_sdk option - */ - public static final class AndroidSdkLabel extends LateBoundLabel<BuildConfiguration> { - public AndroidSdkLabel(Label androidSdk) { - super(androidSdk, AndroidConfiguration.class); - } - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(AndroidConfiguration.class).getSdk(); - } + /** The default label of android_sdk option */ + public static LateBoundDefault<?, Label> getAndroidSdkLabel(Label androidSdk) { + return LateBoundDefault.fromTargetConfiguration( + AndroidConfiguration.class, + androidSdk, + (rule, attributes, configuration) -> configuration.getSdk()); } public static final SplitTransition<BuildOptions> ANDROID_SPLIT_TRANSITION = @@ -555,7 +550,7 @@ public final class AndroidRuleClasses { .add( attr(":android_sdk", LABEL) .allowedRuleClasses("android_sdk", "filegroup") - .value(new AndroidSdkLabel(env.getToolsLabel(AndroidRuleClasses.DEFAULT_SDK)))) + .value(getAndroidSdkLabel(env.getToolsLabel(AndroidRuleClasses.DEFAULT_SDK)))) /* <!-- #BLAZE_RULE($android_base).ATTRIBUTE(plugins) --> Java compiler plugins to run at compile-time. Every <code>java_plugin</code> specified in diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java index b3a5ff58e7..8f4a8ff45c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java @@ -146,7 +146,7 @@ public final class DexArchiveAspect extends NativeAspectClass implements Configu attr(":dex_archive_android_sdk", LABEL) .allowedRuleClasses("android_sdk", "filegroup") .value( - new AndroidRuleClasses.AndroidSdkLabel( + AndroidRuleClasses.getAndroidSdkLabel( Label.parseAbsoluteUnchecked( toolsRepository + AndroidRuleClasses.DEFAULT_SDK)))) .requiresConfigurationFragments(AndroidConfiguration.class) diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java index a380f52713..0cc09b83ea 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java @@ -22,12 +22,9 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.RuleContext; 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.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; -import com.google.devtools.build.lib.packages.AttributeMap; -import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -151,20 +148,13 @@ public class AppleToolchain { return sdkDir() + relativePath; } - /** - * The default label of the build-wide {@code xcode_config} configuration rule. - */ - @Immutable - public static final class XcodeConfigLabel extends LateBoundLabel<BuildConfiguration> { - public XcodeConfigLabel(String toolsRepository) { - super(toolsRepository + AppleCommandLineOptions.DEFAULT_XCODE_VERSION_CONFIG_LABEL, - AppleConfiguration.class); - } - - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(AppleConfiguration.class).getXcodeConfigLabel(); - } + /** The default label of the build-wide {@code xcode_config} configuration rule. */ + public static LateBoundDefault<?, Label> getXcodeConfigLabel(String toolsRepository) { + return LateBoundDefault.fromTargetConfiguration( + AppleConfiguration.class, + Label.parseAbsoluteUnchecked( + toolsRepository + AppleCommandLineOptions.DEFAULT_XCODE_VERSION_CONFIG_LABEL), + (rule, attributes, appleConfig) -> appleConfig.getXcodeConfigLabel()); } /** @@ -180,11 +170,12 @@ public class AppleToolchain { @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { return builder - .add(attr(XcodeConfigRule.XCODE_CONFIG_ATTR_NAME, LABEL) - .allowedRuleClasses("xcode_config") - .checkConstraints() - .direct_compile_time_input() - .value(new XcodeConfigLabel(toolsRepository))) + .add( + attr(XcodeConfigRule.XCODE_CONFIG_ATTR_NAME, LABEL) + .allowedRuleClasses("xcode_config") + .checkConstraints() + .direct_compile_time_input() + .value(getXcodeConfigLabel(toolsRepository))) .build(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index 0bd4093864..11781594b5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -70,7 +70,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { public static final String CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME = ":cc_toolchain"; /** Default attribute name for the c++ toolchain type */ - public static final String CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME = ":cc_toolchain_type"; + public static final String CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME = "$cc_toolchain_type"; /** * This file (found under the sysroot) may be unconditionally included in every C/C++ compilation. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index 23cc7fc326..6672ca93bd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -25,10 +25,8 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.MakeVariableInfo; 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.cmdline.Label; -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.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; @@ -66,13 +64,11 @@ public final class CcToolchainRule implements RuleDefinition { return ruleClass.endsWith("cc_toolchain"); } - private static final LateBoundLabel<BuildConfiguration> LIBC_TOP = - new LateBoundLabel<BuildConfiguration>(CppConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(CppConfiguration.class).getSysrootLabel(); - } - }; + private static final LateBoundDefault<?, Label> LIBC_TOP = + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + null, + (rule, attributes, cppConfig) -> cppConfig.getSysrootLabel()); @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { @@ -117,19 +113,11 @@ public final class CcToolchainRule implements RuleDefinition { .cfg(HOST) .singleArtifact() .value( - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - CppConfiguration cppConfiguration = - configuration.getFragment(CppConfiguration.class); - if (cppConfiguration.isLLVMOptimizedFdo()) { - return zipper; - } else { - return null; - } - } - })) + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + null, + (rule, attributes, cppConfig) -> + cppConfig.isLLVMOptimizedFdo() ? zipper : null))) .add(attr(":libc_top", LABEL).value(LIBC_TOP)) .add( attr(":lipo_context_collector", LABEL) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 953044781b..a4ad59ec05 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -33,16 +33,13 @@ import static com.google.devtools.build.lib.rules.cpp.CppFileTypes.VERSIONED_SHA import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.LanguageDependentFragment.LibraryLanguage; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.Transition; -import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleTransitionFactory; import com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition; import com.google.devtools.build.lib.rules.cpp.transitions.EnableLipoTransition; @@ -55,17 +52,16 @@ import com.google.devtools.build.lib.util.OsUtils; public class CppRuleClasses { /** * Implementation for the :lipo_context_collector attribute. + * + * <p>This attribute connects a target to the LIPO context target configured with the lipo input + * collector configuration. */ - public static final LateBoundLabel<BuildConfiguration> LIPO_CONTEXT_COLLECTOR = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - // This attribute connects a target to the LIPO context target configured with the - // lipo input collector configuration. - CppConfiguration cppConfiguration = configuration.getFragment(CppConfiguration.class); - return cppConfiguration.isLipoOptimization() ? cppConfiguration.getLipoContextLabel() : null; - } - }; + public static final LateBoundDefault<?, Label> LIPO_CONTEXT_COLLECTOR = + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + null, + (rule, attributes, cppConfig) -> + cppConfig.isLipoOptimization() ? cppConfig.getLipoContextLabel() : null); /** * Declares the implementations for C++ transition enums. @@ -91,34 +87,20 @@ public class CppRuleClasses { */ public static final String CROSSTOOL_LABEL = "//tools/cpp:toolchain"; - public static final LateBoundLabel<BuildConfiguration> DEFAULT_MALLOC = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(CppConfiguration.class).customMalloc(); - } - }; + public static final LateBoundDefault<?, Label> DEFAULT_MALLOC = + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, null, (rule, attributes, cppConfig) -> cppConfig.customMalloc()); - public static LateBoundLabel<BuildConfiguration> ccToolchainAttribute( + public static LateBoundDefault<CppConfiguration, Label> ccToolchainAttribute( RuleDefinitionEnvironment env) { - return new LateBoundLabel<BuildConfiguration>( - env.getToolsLabel(CROSSTOOL_LABEL), CppConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(CppConfiguration.class).getCcToolchainRuleLabel(); - } - }; + return LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + env.getToolsLabel(CROSSTOOL_LABEL), + (rules, attributes, cppConfig) -> cppConfig.getCcToolchainRuleLabel()); } - public static LateBoundLabel<BuildConfiguration> ccToolchainTypeAttribute( - RuleDefinitionEnvironment env) { - return new LateBoundLabel<BuildConfiguration>( - env.getToolsLabel(CppHelper.TOOLCHAIN_TYPE_LABEL), CppConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return CppHelper.getCcToolchainType(env.getToolsRepository()); - } - }; + public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) { + return env.getToolsLabel(CppHelper.TOOLCHAIN_TYPE_LABEL); } // Artifacts of these types are discarded from the 'hdrs' attribute in cc rules diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index dd73adffd1..35538cc8c0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -31,16 +31,13 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; 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.AttributeMap; import com.google.devtools.build.lib.packages.NativeAspectClass; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.cpp.CcCommon; import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper; @@ -71,21 +68,17 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect private static final String PROTO_TOOLCHAIN_ATTR = ":aspect_cc_proto_toolchain"; - private static final Attribute.LateBoundLabel<BuildConfiguration> PROTO_TOOLCHAIN_LABEL = - new Attribute.LateBoundLabel<BuildConfiguration>( - "@com_google_protobuf_cc//:cc_toolchain", ProtoConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(ProtoConfiguration.class).protoToolchainForCc(); - } - }; + private static final Attribute.LateBoundDefault<?, Label> PROTO_TOOLCHAIN_LABEL = + Attribute.LateBoundDefault.fromTargetConfiguration( + ProtoConfiguration.class, + Label.parseAbsoluteUnchecked("@com_google_protobuf_cc//:cc_toolchain"), + (rule, attributes, protoConfig) -> protoConfig.protoToolchainForCc()); private final CppSemantics cppSemantics; - private final Attribute.LateBoundLabel<BuildConfiguration> ccToolchainAttrValue; + private final Attribute.LateBoundDefault<?, Label> ccToolchainAttrValue; public CcProtoAspect( - CppSemantics cppSemantics, - Attribute.LateBoundLabel<BuildConfiguration> ccToolchainAttrValue) { + CppSemantics cppSemantics, Attribute.LateBoundDefault<?, Label> ccToolchainAttrValue) { this.cppSemantics = cppSemantics; this.ccToolchainAttrValue = ccToolchainAttrValue; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java index 67e5a6f8b7..1bd75edd74 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBaseRule.java @@ -25,13 +25,12 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.MakeVariableInfo; 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.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.ComputedDefault; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; 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.RuleClassType; import com.google.devtools.build.lib.rules.cpp.CppConfiguration; @@ -50,29 +49,25 @@ public class GenRuleBaseRule implements RuleDefinition { * Late-bound dependency on the C++ toolchain <i>iff</i> the genrule has make variables that need * that toolchain. */ - public static Attribute.LateBoundLabel<BuildConfiguration> ccToolchainAttribute( - RuleDefinitionEnvironment env) { - return new LateBoundLabel<BuildConfiguration>( - env.getToolsLabel(CppRuleClasses.CROSSTOOL_LABEL), CppConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return attributes != null - && GenRuleBase.requiresCrosstool(attributes.get("cmd", Type.STRING)) - ? CppRuleClasses.ccToolchainAttribute(env).resolve(rule, attributes, configuration) - : null; - } - }; + public static LateBoundDefault<?, Label> ccToolchainAttribute(RuleDefinitionEnvironment env) { + return LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + env.getToolsLabel(CppRuleClasses.CROSSTOOL_LABEL), + // null guards are needed for LateBoundAttributeTest + (rule, attributes, cppConfig) -> + attributes != null + && attributes.get("cmd", Type.STRING) != null + && GenRuleBase.requiresCrosstool(attributes.get("cmd", Type.STRING)) + ? CppRuleClasses.ccToolchainAttribute(env).resolve(rule, attributes, cppConfig) + : null); } - /** Late-bound dependency on the C++ toolchain type. */ - public static Attribute.LateBoundLabel<BuildConfiguration> ccToolchainTypeAttribute( - RuleDefinitionEnvironment env) { - return new LateBoundLabel<BuildConfiguration>( - env.getToolsLabel(CppHelper.TOOLCHAIN_TYPE_LABEL), CppConfiguration.class) { + /** Computed dependency on the C++ toolchain type. */ + public static ComputedDefault ccToolchainTypeAttribute(RuleDefinitionEnvironment env) { + return new ComputedDefault("cmd") { @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return attributes != null - && GenRuleBase.requiresCrosstool(attributes.get("cmd", Type.STRING)) + public Object getDefault(AttributeMap rule) { + return GenRuleBase.requiresCrosstool(rule.get("cmd", Type.STRING)) ? CppHelper.getCcToolchainType(env.getToolsRepository()) : null; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java index f83d0fe82e..ca43c7e92e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java @@ -35,11 +35,8 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -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.ImplicitOutputsFunction.SafeImplicitOutputsFunction; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression; import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaOptimizationMode; @@ -112,14 +109,11 @@ public interface JavaSemantics { /** The java_toolchain.compatible_javacopts key for proto compilations. */ public static final String PROTO_JAVACOPTS_KEY = "proto"; - LateBoundLabel<BuildConfiguration> JAVA_TOOLCHAIN = - new LateBoundLabel<BuildConfiguration>(JAVA_TOOLCHAIN_LABEL, JavaConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - return configuration.getFragment(JavaConfiguration.class).getToolchainLabel(); - } - }; + LateBoundDefault<?, Label> JAVA_TOOLCHAIN = + LateBoundDefault.fromTargetConfiguration( + JavaConfiguration.class, + Label.parseAbsoluteUnchecked(JAVA_TOOLCHAIN_LABEL), + (rule, attributes, javaConfig) -> javaConfig.getToolchainLabel()); /** * Name of the output group used for source jars. @@ -135,111 +129,86 @@ public interface JavaSemantics { OutputGroupProvider.HIDDEN_OUTPUT_GROUP_PREFIX + "gen_jars"; /** Implementation for the :jvm attribute. */ - static LateBoundLabel<BuildConfiguration> jvmAttribute(RuleDefinitionEnvironment env) { - return new LateBoundLabel<BuildConfiguration>( - env.getToolsLabel(JavaImplicitAttributes.JDK_LABEL), Jvm.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(Jvm.class).getJvmLabel(); - } - }; + static LateBoundDefault<?, Label> jvmAttribute(RuleDefinitionEnvironment env) { + return LateBoundDefault.fromTargetConfiguration( + Jvm.class, + env.getToolsLabel(JavaImplicitAttributes.JDK_LABEL), + (rule, attributes, jvm) -> jvm.getJvmLabel()); } /** Implementation for the :host_jdk attribute. */ - static LateBoundLabel<BuildConfiguration> hostJdkAttribute(RuleDefinitionEnvironment env) { - return new LateBoundLabel<BuildConfiguration>( - env.getToolsLabel(JavaImplicitAttributes.JDK_LABEL), Jvm.class) { - @Override - public boolean useHostConfiguration() { - return true; - } - - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(Jvm.class).getJvmLabel(); - } - }; + static LateBoundDefault<?, Label> hostJdkAttribute(RuleDefinitionEnvironment env) { + return LateBoundDefault.fromHostConfiguration( + Jvm.class, + env.getToolsLabel(JavaImplicitAttributes.JDK_LABEL), + (rule, attributes, jvm) -> jvm.getJvmLabel()); } /** * Implementation for the :java_launcher attribute. Note that the Java launcher is disabled by * default, so it returns null for the configuration-independent default value. */ - LateBoundLabel<BuildConfiguration> JAVA_LAUNCHER = - new LateBoundLabel<BuildConfiguration>(JavaConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - // This nullness check is purely for the sake of a test that doesn't bother to include an - // attribute map when calling this method. - if (attributes != null) { - // Don't depend on the launcher if we don't create an executable anyway - if (attributes.has("create_executable") - && !attributes.get("create_executable", Type.BOOLEAN)) { - return null; + LateBoundDefault<?, Label> JAVA_LAUNCHER = + LateBoundDefault.fromTargetConfiguration( + JavaConfiguration.class, + null, + (rule, attributes, javaConfig) -> { + // This nullness check is purely for the sake of a test that doesn't bother to include + // an + // attribute map when calling this method. + if (attributes != null) { + // Don't depend on the launcher if we don't create an executable anyway + if (attributes.has("create_executable") + && !attributes.get("create_executable", Type.BOOLEAN)) { + return null; + } + + // don't read --java_launcher if this target overrides via a launcher attribute + if (attributes.isAttributeValueExplicitlySpecified("launcher")) { + return attributes.get("launcher", LABEL); + } } - - // don't read --java_launcher if this target overrides via a launcher attribute - if (attributes.isAttributeValueExplicitlySpecified("launcher")) { - return attributes.get("launcher", LABEL); + return javaConfig.getJavaLauncherLabel(); + }); + + // TODO(b/65746853): provide a way to do this without passing the entire configuration + LateBoundDefault<?, List<Label>> JAVA_PLUGINS = + LateBoundDefault.fromTargetConfiguration( + BuildConfiguration.class, + ImmutableList.of(), + (rule, attributes, configuration) -> ImmutableList.copyOf(configuration.getPlugins())); + + /** Implementation for the :proguard attribute. */ + LateBoundDefault<?, Label> PROGUARD = + LateBoundDefault.fromTargetConfiguration( + JavaConfiguration.class, + null, + (rule, attributes, javaConfig) -> javaConfig.getProguardBinary()); + + LateBoundDefault<?, List<Label>> EXTRA_PROGUARD_SPECS = + LateBoundDefault.fromTargetConfiguration( + JavaConfiguration.class, + ImmutableList.of(), + (rule, attributes, javaConfig) -> + ImmutableList.copyOf(javaConfig.getExtraProguardSpecs())); + + LateBoundDefault<?, List<Label>> BYTECODE_OPTIMIZERS = + LateBoundDefault.fromTargetConfiguration( + JavaConfiguration.class, + ImmutableList.of(), + (rule, attributes, javaConfig) -> { + // Use a modicum of smarts to avoid implicit dependencies where we don't need them. + JavaOptimizationMode optMode = javaConfig.getJavaOptimizationMode(); + boolean hasProguardSpecs = + attributes.has("proguard_specs") + && !attributes.get("proguard_specs", LABEL_LIST).isEmpty(); + if (optMode == JavaOptimizationMode.NOOP + || (optMode == JavaOptimizationMode.LEGACY && !hasProguardSpecs)) { + return ImmutableList.<Label>of(); } - } - return configuration.getFragment(JavaConfiguration.class).getJavaLauncherLabel(); - } - }; - - LateBoundLabelList<BuildConfiguration> JAVA_PLUGINS = - new LateBoundLabelList<BuildConfiguration>() { - @Override - public List<Label> resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - return ImmutableList.copyOf(configuration.getPlugins()); - } - }; - - /** - * Implementation for the :proguard attribute. - */ - LateBoundLabel<BuildConfiguration> PROGUARD = - new LateBoundLabel<BuildConfiguration>(JavaConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - return configuration.getFragment(JavaConfiguration.class).getProguardBinary(); - } - }; - - LateBoundLabelList<BuildConfiguration> EXTRA_PROGUARD_SPECS = - new LateBoundLabelList<BuildConfiguration>() { - @Override - public List<Label> resolve(Rule rule, AttributeMap attributes, - BuildConfiguration configuration) { - return ImmutableList.copyOf( - configuration.getFragment(JavaConfiguration.class).getExtraProguardSpecs()); - } - }; - - LateBoundLabelList<BuildConfiguration> BYTECODE_OPTIMIZERS = - new LateBoundLabelList<BuildConfiguration>(JavaConfiguration.class) { - @Override - public List<Label> resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - // Use a modicum of smarts to avoid implicit dependencies where we don't need them. - JavaOptimizationMode optMode = - configuration.getFragment(JavaConfiguration.class).getJavaOptimizationMode(); - boolean hasProguardSpecs = attributes.has("proguard_specs") - && !attributes.get("proguard_specs", LABEL_LIST).isEmpty(); - if (optMode == JavaOptimizationMode.NOOP - || (optMode == JavaOptimizationMode.LEGACY && !hasProguardSpecs)) { - return ImmutableList.<Label>of(); - } - return ImmutableList.copyOf( - Optional.presentInstances( - configuration - .getFragment(JavaConfiguration.class) - .getBytecodeOptimizers() - .values())); - } - }; + return ImmutableList.copyOf( + Optional.presentInstances(javaConfig.getBytecodeOptimizers().values())); + }); String IJAR_LABEL = "//tools/defaults:ijar"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java index fbf667e4d8..9b09be1b1a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder; import com.google.devtools.build.lib.analysis.WrappingProvider; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; import com.google.devtools.build.lib.cmdline.Label; @@ -45,10 +44,8 @@ import com.google.devtools.build.lib.collect.nestedset.Order; 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.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.NativeAspectClass; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaCompilationHelper; @@ -72,28 +69,24 @@ public class JavaLiteProtoAspect extends NativeAspectClass implements Configured public static final String PROTO_TOOLCHAIN_ATTR = ":aspect_proto_toolchain_for_javalite"; - public static Attribute.LateBoundLabel<BuildConfiguration> getProtoToolchainLabel( - String defaultValue) { - return new Attribute.LateBoundLabel<BuildConfiguration>( - defaultValue, ProtoConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(ProtoConfiguration.class).protoToolchainForJavaLite(); - } - }; + public static LateBoundDefault<?, Label> getProtoToolchainLabel(String defaultValue) { + return LateBoundDefault.fromTargetConfiguration( + ProtoConfiguration.class, + Label.parseAbsoluteUnchecked(defaultValue), + (rule, attributes, protoConfig) -> protoConfig.protoToolchainForJavaLite()); } private final JavaSemantics javaSemantics; @Nullable private final String jacocoLabel; private final String defaultProtoToolchainLabel; - private final LateBoundLabel<BuildConfiguration> hostJdkAttribute; + private final LateBoundDefault<?, Label> hostJdkAttribute; public JavaLiteProtoAspect( JavaSemantics javaSemantics, @Nullable String jacocoLabel, String defaultProtoToolchainLabel, - LateBoundLabel<BuildConfiguration> hostJdkAttribute) { + LateBoundDefault<?, Label> hostJdkAttribute) { this.javaSemantics = javaSemantics; this.jacocoLabel = jacocoLabel; this.defaultProtoToolchainLabel = defaultProtoToolchainLabel; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java index ec843a00d8..91e4b3b734 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder; import com.google.devtools.build.lib.analysis.WrappingProvider; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; import com.google.devtools.build.lib.cmdline.Label; @@ -44,10 +43,8 @@ import com.google.devtools.build.lib.collect.nestedset.Order; 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.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.NativeAspectClass; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaCompilationHelper; import com.google.devtools.build.lib.rules.java.JavaConfiguration; @@ -71,17 +68,13 @@ import javax.annotation.Nullable; public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspectFactory { private static final String SPEED_PROTO_TOOLCHAIN_ATTR = ":aspect_java_proto_toolchain"; - private final LateBoundLabel<BuildConfiguration> hostJdkAttribute; - - private static Attribute.LateBoundLabel<BuildConfiguration> getSpeedProtoToolchainLabel( - String defaultValue) { - return new Attribute.LateBoundLabel<BuildConfiguration>( - defaultValue, ProtoConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(ProtoConfiguration.class).protoToolchainForJava(); - } - }; + private final LateBoundDefault<?, Label> hostJdkAttribute; + + private static LateBoundDefault<?, Label> getSpeedProtoToolchainLabel(String defaultValue) { + return LateBoundDefault.fromTargetConfiguration( + ProtoConfiguration.class, + Label.parseAbsoluteUnchecked(defaultValue), + (rule, attributes, protoConfig) -> protoConfig.protoToolchainForJava()); } private final JavaSemantics javaSemantics; @@ -95,7 +88,7 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe @Nullable String jacocoLabel, RpcSupport rpcSupport, String defaultSpeedProtoToolchainLabel, - LateBoundLabel<BuildConfiguration> hostJdkAttribute) { + LateBoundDefault<?, Label> hostJdkAttribute) { this.javaSemantics = javaSemantics; this.jacocoLabel = jacocoLabel; this.rpcSupport = rpcSupport; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java index 78aaba6988..d5d5851072 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java @@ -27,9 +27,8 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute.ComputedDefault; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.AttributeMap; -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.RuleClassType; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; @@ -140,21 +139,16 @@ public class IosTestRule implements RuleDefinition { attr(IosTest.OBJC_GCOV_ATTR, LABEL) .cfg(HOST) .value(env.getToolsLabel("//tools/objc:gcov"))) + // TODO(b/65746853): provide a way to do this without passing the entire configuration .add( attr(IosTest.MCOV_TOOL_ATTR, LABEL) .cfg(HOST) .value( - new LateBoundLabel<BuildConfiguration>(mcov) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - if (!configuration.isCodeCoverageEnabled()) { - return null; - } - - return mcov; - } - })) + LateBoundDefault.fromTargetConfiguration( + BuildConfiguration.class, + mcov, + (rule, attributes, configuration) -> + configuration.isCodeCoverageEnabled() ? mcov : null))) .cfg(AppleCrosstoolTransition.APPLE_CROSSTOOL_TRANSITION) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java index 016ec5c091..87c24ab678 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java @@ -37,18 +37,15 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorAr import com.google.devtools.build.lib.analysis.actions.ParamFileInfo; import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; 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.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; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.apple.AppleToolchain; @@ -105,13 +102,11 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF protected static final ImmutableList<String> J2OBJC_PLUGIN_PARAMS = ImmutableList.of("file_dir_mapping", "generate_class_mappings"); - private static final LateBoundLabel<BuildConfiguration> DEAD_CODE_REPORT = - new LateBoundLabel<BuildConfiguration>(J2ObjcConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(J2ObjcConfiguration.class).deadCodeReport().orNull(); - } - }; + private static final LateBoundDefault<?, Label> DEAD_CODE_REPORT = + LateBoundDefault.fromTargetConfiguration( + J2ObjcConfiguration.class, + null, + (rule, attributes, j2objcConfig) -> j2objcConfig.deadCodeReport().orNull()); /** Adds additional attribute aspects and attributes to the given AspectDefinition.Builder. */ protected AspectDefinition.Builder addAdditionalAttributes(AspectDefinition.Builder builder) { @@ -182,14 +177,12 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF .value( Label.parseAbsoluteUnchecked( toolsRepository + "//third_party/java/j2objc:jre_emul.jar"))) + .add(attr(":dead_code_report", LABEL).cfg(HOST).value(DEAD_CODE_REPORT)) .add( - attr(":dead_code_report", LABEL) - .cfg(HOST) - .value(DEAD_CODE_REPORT)) - .add(attr("$jre_lib", LABEL) - .value( - Label.parseAbsoluteUnchecked( - toolsRepository + "//third_party/java/j2objc:jre_core_lib"))) + attr("$jre_lib", LABEL) + .value( + Label.parseAbsoluteUnchecked( + toolsRepository + "//third_party/java/j2objc:jre_core_lib"))) .add( attr("$protobuf_lib", LABEL) .value( @@ -210,7 +203,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF .allowedRuleClasses("xcode_config") .checkConstraints() .direct_compile_time_input() - .value(new AppleToolchain.XcodeConfigLabel(toolsRepository))) + .value(AppleToolchain.getXcodeConfigLabel(toolsRepository))) .add( attr("$zipper", LABEL) .cfg(HOST) @@ -224,8 +217,12 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF toolsRepository + "//tools/j2objc:j2objc_proto_blacklist")))) .add(attr(":j2objc_cc_toolchain", LABEL).value(ObjcRuleClasses.APPLE_TOOLCHAIN)) .add( + // Objc builds do not use a lipo context collector, but must specify the attribute as + // a late-bound attribute to match with the similar attribute on the cc rules. + // TODO(b/28084560): Allow :lipo_context_collector not to be set instead of having a + // null instance. attr(":lipo_context_collector", LABEL) - .value(ObjcRuleClasses.NULL_LIPO_CONTEXT_COLLECTOR) + .value(LateBoundDefault.alwaysNull()) .skipPrereqValidatorCheck()) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java index 2fee1f098f..56c7ae505b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java @@ -41,11 +41,10 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; -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.packages.RuleClass.Builder.RuleClassType; @@ -160,30 +159,13 @@ public class ObjcRuleClasses { /** * Late-bound attribute giving the CcToolchain for CROSSTOOL_LABEL. * - * TODO(cpeyser): Use AppleCcToolchain instead of CcToolchain once released. + * <p>TODO(cpeyser): Use AppleCcToolchain instead of CcToolchain once released. */ - public static final LateBoundLabel<BuildConfiguration> APPLE_TOOLCHAIN = - new LateBoundLabel<BuildConfiguration>(CROSSTOOL_LABEL, CppConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(CppConfiguration.class).getCcToolchainRuleLabel(); - } - }; - - /** - * A null value for the lipo context collector. Objc builds do not use a lipo context collector. - */ - // TODO(b/28084560): Allow :lipo_context_collector not to be set instead of having a null - // instance. - public static final LateBoundLabel<BuildConfiguration> NULL_LIPO_CONTEXT_COLLECTOR = - new LateBoundLabel<BuildConfiguration>() { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return null; - } - }; + public static final LateBoundDefault<?, Label> APPLE_TOOLCHAIN = + LateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + Label.parseAbsoluteUnchecked(CROSSTOOL_LABEL), + (rule, attributes, cppConfig) -> cppConfig.getCcToolchainRuleLabel()); /** * Creates a new spawn action builder with apple environment variables set that are typically @@ -543,8 +525,12 @@ public class ObjcRuleClasses { attr(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, LABEL) .value(CppRuleClasses.ccToolchainTypeAttribute(env))) .add( + // Objc builds do not use a lipo context collector, but must specify the attribute as + // a late-bound attribute to match with the similar attribute on the cc rules. + // TODO(b/28084560): Allow :lipo_context_collector not to be set instead of having a + // null instance. attr(":lipo_context_collector", LABEL) - .value(NULL_LIPO_CONTEXT_COLLECTOR) + .value(LateBoundDefault.alwaysNull()) .skipPrereqValidatorCheck()) .build(); } @@ -677,7 +663,7 @@ public class ObjcRuleClasses { .direct_compile_time_input() .allowedRuleClasses(ALLOWED_CC_DEPS_RULE_CLASSES) .mandatoryProviders(ObjcProvider.SKYLARK_CONSTRUCTOR.id()) - .allowedFileTypes()) + .allowedFileTypes()) /* <!-- #BLAZE_RULE($objc_compiling_rule).ATTRIBUTE(runtime_deps) --> The list of framework targets that are late loaded at runtime. They are included in the app bundle but not linked against at build time. @@ -721,9 +707,7 @@ public class ObjcRuleClasses { If specified, Bazel will not generate a module map for this target, but will pass the provided module map to the compiler. <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ - .add( - attr("module_map", LABEL) - .allowedFileTypes(FileType.of(".modulemap"))) + .add(attr("module_map", LABEL).allowedFileTypes(FileType.of(".modulemap"))) /* Provides the label for header_scanner tool that is used to scan inclusions for ObjC sources and provide a list of required headers via a .header_list file. @@ -735,33 +719,21 @@ public class ObjcRuleClasses { attr(HEADER_SCANNER_ATTRIBUTE, LABEL) .cfg(HOST) .value( - new LateBoundLabel<BuildConfiguration>( + LateBoundDefault.fromTargetConfiguration( + ObjcConfiguration.class, env.getToolsLabel("//tools/objc:header_scanner"), - ObjcConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration - .getFragment(ObjcConfiguration.class) - .getObjcHeaderScannerTool(); - } - })) + (rule, attributes, objcConfig) -> objcConfig.getObjcHeaderScannerTool()))) .add( attr(APPLE_SDK_ATTRIBUTE, LABEL) .value( - new LateBoundLabel<BuildConfiguration>(ObjcConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - ObjcConfiguration objcConfiguration = - configuration.getFragment(ObjcConfiguration.class); + LateBoundDefault.fromTargetConfiguration( + ObjcConfiguration.class, + null, // Apple SDKs are currently only used by ObjC header thinning feature - if (objcConfiguration.useExperimentalHeaderThinning()) { - return objcConfiguration.getAppleSdk(); - } - return null; - } - })) + (rule, attributes, objcConfig) -> + objcConfig.useExperimentalHeaderThinning() + ? objcConfig.getAppleSdk() + : null))) .build(); } @Override @@ -1191,15 +1163,10 @@ public class ObjcRuleClasses { .singleArtifact() .cfg(HOST) .value( - new LateBoundLabel<BuildConfiguration>(ObjcConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration - .getFragment(ObjcConfiguration.class) - .getExtraEntitlements(); - } - }) + LateBoundDefault.fromTargetConfiguration( + ObjcConfiguration.class, + null, + (rule, attributes, objcConfig) -> objcConfig.getExtraEntitlements())) .allowedFileTypes(ENTITLEMENTS_TYPE)) .add( attr(DEBUG_ENTITLEMENTS_ATTR, LABEL) @@ -1222,22 +1189,20 @@ public class ObjcRuleClasses { .singleArtifact() .allowedFileTypes(FileType.of(".mobileprovision")) .value( - new LateBoundLabel<BuildConfiguration>(ObjcConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - AppleConfiguration appleConfiguration = - configuration.getFragment(AppleConfiguration.class); - if (appleConfiguration.getMultiArchPlatform(PlatformType.IOS) - != ApplePlatform.IOS_DEVICE) { - return null; - } - if (rule.isAttributeValueExplicitlySpecified(PROVISIONING_PROFILE_ATTR)) { - return null; - } - return appleConfiguration.getDefaultProvisioningProfileLabel(); - } - })) + LateBoundDefault.fromTargetConfiguration( + AppleConfiguration.class, + null, + (rule, attributes, appleConfig) -> { + if (appleConfig.getMultiArchPlatform(PlatformType.IOS) + != ApplePlatform.IOS_DEVICE) { + return null; + } + if (attributes.isAttributeValueExplicitlySpecified( + PROVISIONING_PROFILE_ATTR)) { + return null; + } + return appleConfig.getDefaultProvisioningProfileLabel(); + }))) /* <!-- #BLAZE_RULE($objc_release_bundling_rule).ATTRIBUTE(app_icon) --> The name of the application icon. @@ -1507,23 +1472,20 @@ public class ObjcRuleClasses { .singleArtifact() .allowedFileTypes(FileType.of(".mobileprovision")) .value( - new LateBoundLabel<BuildConfiguration>(ObjcConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - AppleConfiguration appleConfiguration = - configuration.getFragment(AppleConfiguration.class); - if (appleConfiguration.getMultiArchPlatform(PlatformType.IOS) - != ApplePlatform.IOS_DEVICE) { - return null; - } - if (rule.isAttributeValueExplicitlySpecified( - WATCH_EXT_PROVISIONING_PROFILE_ATTR)) { - return null; - } - return appleConfiguration.getDefaultProvisioningProfileLabel(); - } - })) + LateBoundDefault.fromTargetConfiguration( + AppleConfiguration.class, + null, + (rule, attributes, appleConfig) -> { + if (appleConfig.getMultiArchPlatform(PlatformType.IOS) + != ApplePlatform.IOS_DEVICE) { + return null; + } + if (attributes.isAttributeValueExplicitlySpecified( + WATCH_EXT_PROVISIONING_PROFILE_ATTR)) { + return null; + } + return appleConfig.getDefaultProvisioningProfileLabel(); + }))) /* <!-- #BLAZE_RULE($watch_extension_bundle_rule).ATTRIBUTE(ext_resources) --> Files to include in the final watch extension bundle. @@ -1696,23 +1658,20 @@ public class ObjcRuleClasses { .singleArtifact() .allowedFileTypes(FileType.of(".mobileprovision")) .value( - new LateBoundLabel<BuildConfiguration>(ObjcConfiguration.class) { - @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - AppleConfiguration appleConfiguration = - configuration.getFragment(AppleConfiguration.class); - if (appleConfiguration.getMultiArchPlatform(PlatformType.IOS) - != ApplePlatform.IOS_DEVICE) { - return null; - } - if (rule.isAttributeValueExplicitlySpecified( - WATCH_APP_PROVISIONING_PROFILE_ATTR)) { - return null; - } - return appleConfiguration.getDefaultProvisioningProfileLabel(); - } - })) + LateBoundDefault.fromTargetConfiguration( + AppleConfiguration.class, + null, + (rule, attributes, appleConfig) -> { + if (appleConfig.getMultiArchPlatform(PlatformType.IOS) + != ApplePlatform.IOS_DEVICE) { + return null; + } + if (attributes.isAttributeValueExplicitlySpecified( + WATCH_APP_PROVISIONING_PROFILE_ATTR)) { + return null; + } + return appleConfig.getDefaultProvisioningProfileLabel(); + }))) /* <!-- #BLAZE_RULE($objc_resources_rule).ATTRIBUTE(app_storyboards) --> Files which are .storyboard resources for the watch application, possibly localizable. diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java index 168f82f415..e7891c4eb2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java @@ -22,11 +22,8 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import com.google.devtools.build.lib.analysis.BaseRuleClasses; 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.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.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.util.FileType; @@ -36,15 +33,16 @@ import com.google.devtools.build.lib.util.FileType; */ public final class BazelProtoLibraryRule implements RuleDefinition { - private static final Attribute.LateBoundLabel<BuildConfiguration> PROTO_COMPILER = - new Attribute.LateBoundLabel<BuildConfiguration>( - "@com_google_protobuf//:protoc", ProtoConfiguration.class) { - @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - Label label = configuration.getFragment(ProtoConfiguration.class).protoCompiler(); - return label != null ? label : getDefault(); - } - }; + private static final Label DEFAULT_PROTO_COMPILER = + Label.parseAbsoluteUnchecked("@com_google_protobuf//:protoc"); + private static final Attribute.LateBoundDefault<?, Label> PROTO_COMPILER = + Attribute.LateBoundDefault.fromTargetConfiguration( + ProtoConfiguration.class, + DEFAULT_PROTO_COMPILER, + (rule, attributes, protoConfig) -> + protoConfig.protoCompiler() != null + ? protoConfig.protoCompiler() + : DEFAULT_PROTO_COMPILER); @Override public RuleClass build(Builder builder, final RuleDefinitionEnvironment env) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java index 3f8a001961..093ec2c534 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java @@ -197,9 +197,14 @@ public class TransitiveTargetFunction // Declared by late-bound attributes: for (Attribute attr : rule.getAttributes()) { - if (attr.isLateBound()) { - addFragmentsIfNew(builder, - attr.getLateBoundDefault().getRequiredConfigurationFragments()); + if (attr.isLateBound() + && attr.getLateBoundDefault().getFragmentClass() != null + && BuildConfiguration.Fragment.class.isAssignableFrom( + attr.getLateBoundDefault().getFragmentClass())) { + addFragmentIfNew( + builder, + (Class<? extends BuildConfiguration.Fragment>) + attr.getLateBoundDefault().getFragmentClass()); } } 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) { 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 c0c0c26fc3..513c3e0a0f 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.events.Location; 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 @@ public class AttributeValueSourceTest { }); 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"); } |