Allow starlark rules to be build settings. For skylark rules that are declared build settings, we auto-add a nonconfigurable build_setting_default attribute which must be set on a per target basis. We also add access to the build setting value via the rule context object. All of this functionality is hidden behind the --experimental_build_setting_api flag which by default is flipped off. This includes: - Add a build_setting parameter to starlark rule construction. - If a starlark rule sets the build_setting parameter, auto-add a mandatory 'build_setting_default' attribute of the same type. - Allow the value of a build setting skylark rules to be accessed via ctx.build_setting_value. - Get rid of BuildSettingDescriptor since we don't need both it and BuildSetting. - Add the --experimental_build_setting_api flag to SkylarkSemantics flags. Work towards issue #5577 PiperOrigin-RevId: 219537101
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkConfig.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkConfig.java index bc4d6e4..061e0b5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkConfig.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkConfig.java
@@ -21,69 +21,48 @@ import static com.google.devtools.build.lib.syntax.Type.STRING; import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; +import com.google.devtools.build.lib.packages.BuildSetting; import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; -import com.google.devtools.build.lib.syntax.Type; /** - * Skylark namespace for creating build setting descriptors. + * Skylark namespace for creating build settings. + * TODO(juliexxia): Consider adding more types of build settings, specifically other label types. */ public class SkylarkConfig implements SkylarkConfigApi { @Override - public BuildSettingDescriptor intSetting(Boolean flag) { - return new BuildSettingDescriptor(flag, INTEGER); + public BuildSetting intSetting(Boolean flag) { + return new BuildSetting(flag, INTEGER); } @Override - public BuildSettingDescriptor boolSetting(Boolean flag) { - return new BuildSettingDescriptor(flag, BOOLEAN); + public BuildSetting boolSetting(Boolean flag) { + return new BuildSetting(flag, BOOLEAN); } @Override - public BuildSettingDescriptor stringSetting(Boolean flag) { - return new BuildSettingDescriptor(flag, STRING); + public BuildSetting stringSetting(Boolean flag) { + return new BuildSetting(flag, STRING); } @Override - public BuildSettingDescriptor stringListSetting(Boolean flag) { - return new BuildSettingDescriptor(flag, STRING_LIST); + public BuildSetting stringListSetting(Boolean flag) { + return new BuildSetting(flag, STRING_LIST); } @Override - public BuildSettingDescriptor labelSetting(Boolean flag) { - return new BuildSettingDescriptor(flag, LABEL); + public BuildSetting labelSetting(Boolean flag) { + return new BuildSetting(flag, LABEL); } @Override - public BuildSettingDescriptor labelListSetting(Boolean flag) { - return new BuildSettingDescriptor(flag, LABEL_LIST); + public BuildSetting labelListSetting(Boolean flag) { + return new BuildSetting(flag, LABEL_LIST); } @Override public void repr(SkylarkPrinter printer) { printer.append("<config>"); } - - /** - * An object that describes what time of build setting a skylark rule is (if any type). - */ - private static final class BuildSettingDescriptor implements SkylarkConfigApi.BuildSettingApi { - private final boolean isFlag; - private final Type<?> type; - - BuildSettingDescriptor(boolean isFlag, Type<?> type) { - this.isFlag = isFlag; - this.type = type; - } - - public Type<?> getType() { - return type; - } - - @Override - public void repr(SkylarkPrinter printer) { - printer.append("<build_setting." + type.toString() + ">"); - } - } }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index 264ae49..814cf5a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -46,6 +46,7 @@ import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.AttributeValueSource; +import com.google.devtools.build.lib.packages.BuildSetting; import com.google.devtools.build.lib.packages.FunctionSplitTransitionWhitelist; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SkylarkImplicitOutputsFunctionWithCallback; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SkylarkImplicitOutputsFunctionWithMap; @@ -272,6 +273,7 @@ Boolean executionPlatformConstraintsAllowed, SkylarkList<?> execCompatibleWith, Object analysisTest, + Object buildSetting, FuncallExpression ast, Environment funcallEnv) throws EvalException, ConversionException { @@ -352,6 +354,17 @@ builder.addRequiredToolchains( collectToolchainLabels( toolchains.getContents(String.class, "toolchains"), ast.getLocation())); + if (!buildSetting.equals(Runtime.NONE)) { + if (funcallEnv.getSemantics().experimentalBuildSettingApi()) { + builder.setBuildSetting((BuildSetting) buildSetting); + } else { + throw new EvalException( + ast.getLocation(), + "build_setting parameter is experimental and not available for " + + "general use. It is subject to change at any time. It may be enabled by " + + "specifying --experimental_build_setting_api"); + } + } for (Object o : providesArg) { if (!SkylarkAttr.isProvider(o)) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java index b311dfb..136ab55 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
@@ -14,6 +14,8 @@ package com.google.devtools.build.lib.analysis.skylark; +import static com.google.devtools.build.lib.packages.RuleClass.Builder.SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME; + import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -575,6 +577,30 @@ } @Override + @Nullable + public Object getBuildSettingValue() throws EvalException { + if (ruleContext.getRule().getRuleClassObject().getBuildSetting() == null) { + throw new EvalException( + Location.BUILTIN, + String.format( + "attempting to access 'build_setting_value' of non-build setting %s", + ruleLabelCanonicalName)); + } + ImmutableMap<String, Object> skylarkFlagSettings = + ruleContext.getConfiguration().getOptions().getSkylarkOptions(); + + Type<?> buildSettingType = + ruleContext.getRule().getRuleClassObject().getBuildSetting().getType(); + if (skylarkFlagSettings.containsKey(ruleLabelCanonicalName)) { + return skylarkFlagSettings.get(ruleLabelCanonicalName); + } else { + return ruleContext + .attributes() + .get(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, buildSettingType); + } + } + + @Override public boolean instrumentCoverage(Object targetUnchecked) throws EvalException { checkMutable("coverage_instrumented"); BuildConfiguration config = ruleContext.getConfiguration();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java b/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java new file mode 100644 index 0000000..506aaab --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java
@@ -0,0 +1,47 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.packages; + +import com.google.common.annotations.VisibleForTesting; +import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi.BuildSettingApi; +import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; +import com.google.devtools.build.lib.syntax.Type; + +/** + * Metadata of a build setting rule's properties. This describes the build setting's type (for + * example, 'int' or 'string'), and whether the build setting corresponds to a command line flag. + */ +public class BuildSetting implements BuildSettingApi { + private final boolean isFlag; + private final Type<?> type; + + public BuildSetting(boolean isFlag, Type<?> type) { + this.isFlag = isFlag; + this.type = type; + } + + public Type<?> getType() { + return type; + } + + @VisibleForTesting + public boolean isFlag() { + return isFlag; + } + + @Override + public void repr(SkylarkPrinter printer) { + printer.append("<build_setting." + type + ">"); + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index fba2b88..56ceb24 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -57,6 +57,7 @@ import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.syntax.Type.ConversionException; +import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.StringUtil; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -111,6 +112,8 @@ * tree. All targets appearing in these attributes appears beneath the ".runfiles" tree; in * addition, "deps" may have rule-specific semantics. * </ul> + * + * TODO(bazel-team): Consider breaking up this class in more manageable subclasses. */ // Non-final only for mocking in tests. Do not subclass! @Immutable @@ -607,6 +610,12 @@ } } + /** + * Name of default attribute implicitly added to all Skylark RuleClasses that are {@code + * build_setting}s. + */ + public static final String SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME = "build_setting_default"; + /** List of required attributes for normal rules, name and type. */ public static final ImmutableList<Attribute> REQUIRED_ATTRIBUTES_FOR_NORMAL_RULES = ImmutableList.of(attr("tags", Type.STRING_LIST).build()); @@ -641,6 +650,7 @@ private Predicate<String> preferredDependencyPredicate = Predicates.alwaysFalse(); private AdvertisedProviderSet.Builder advertisedProviders = AdvertisedProviderSet.builder(); private BaseFunction configuredTargetFunction = null; + private BuildSetting buildSetting = null; private Function<? super Rule, Map<String, Label>> externalBindingsFunction = NO_EXTERNAL_BINDINGS; private Function<? super Rule, ? extends Set<String>> optionReferenceFunction = @@ -770,6 +780,17 @@ if (skylark) { assertRuleClassProperStarlarkDefinedTransitionUsage(); } + if (buildSetting != null) { + Type<?> type = buildSetting.getType(); + Attribute.Builder<?> attrBuilder = + attr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, type) + .nonconfigurable("Build setting defaults are referenced during analysis.") + .mandatory(); + if (BuildType.isLabelType(type)) { + attrBuilder.allowedFileTypes(FileTypeSet.ANY_FILE); + } + this.add(attrBuilder); + } return new RuleClass( name, @@ -803,7 +824,8 @@ executionPlatformConstraintsAllowed, executionPlatformConstraints, outputFileKind, - attributes.values()); + attributes.values(), + buildSetting); } private void assertSkylarkRuleClassHasImplementationFunction() { @@ -1142,6 +1164,11 @@ return this; } + public Builder setBuildSetting(BuildSetting buildSetting) { + this.buildSetting = buildSetting; + return this; + } + public Builder setExternalBindingsFunction(Function<? super Rule, Map<String, Label>> func) { this.externalBindingsFunction = func; return this; @@ -1418,6 +1445,12 @@ @Nullable private final BaseFunction configuredTargetFunction; /** + * The BuildSetting associated with this rule. Null for all RuleClasses except Skylark-defined + * rules that pass {@code build_setting} to their {@code rule()} declaration. + */ + @Nullable private final BuildSetting buildSetting; + + /** * Returns the extra bindings a workspace function adds to the WORKSPACE file. */ private final Function<? super Rule, Map<String, Label>> externalBindingsFunction; @@ -1506,7 +1539,8 @@ ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed, Set<Label> executionPlatformConstraints, OutputFile.Kind outputFileKind, - Collection<Attribute> attributes) { + Collection<Attribute> attributes, + @Nullable BuildSetting buildSetting) { this.name = name; this.key = key; this.type = type; @@ -1541,6 +1575,7 @@ this.supportsPlatforms = supportsPlatforms; this.executionPlatformConstraintsAllowed = executionPlatformConstraintsAllowed; this.executionPlatformConstraints = ImmutableSet.copyOf(executionPlatformConstraints); + this.buildSetting = buildSetting; // Create the index and collect non-configurable attributes. int index = 0; @@ -2307,6 +2342,11 @@ return configuredTargetFunction; } + @Nullable + public BuildSetting getBuildSetting() { + return buildSetting; + } + /** * Returns a function that computes the external bindings a repository function contributes to * the WORKSPACE file.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java index c57e823..55ae637 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -72,6 +72,16 @@ public boolean experimentalAnalysisTestingImprovements; @Option( + name = "experimental_build_setting_api", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS, + help = + "If set to true, allows access to value of build setting rules via " + + "ctx.build_setting_value.") + public boolean experimentalBuildSettingApi; + + @Option( name = "experimental_cc_skylark_api_enabled_packages", converter = CommaSeparatedOptionListConverter.class, defaultValue = "", @@ -520,6 +530,7 @@ return SkylarkSemantics.builder() // <== Add new options here in alphabetic order ==> .experimentalAnalysisTestingImprovements(experimentalAnalysisTestingImprovements) + .experimentalBuildSettingApi(experimentalBuildSettingApi) .experimentalCcSkylarkApiEnabledPackages(experimentalCcSkylarkApiEnabledPackages) .experimentalEnableAndroidMigrationApis(experimentalEnableAndroidMigrationApis) .experimentalEnableRepoMapping(experimentalEnableRepoMapping)
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java index 979a447..20e04fa 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleContextApi.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.syntax.SkylarkSemantics.FlagIdentifier; import java.util.Map; import javax.annotation.Nullable; @@ -229,6 +230,17 @@ public BuildConfigurationApi getHostConfiguration() throws EvalException; @SkylarkCallable( + name = "build_setting_value", + structField = true, + enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_BUILD_SETTING_API, + doc = + "<b>Experimental. This field is experimental and subject to change at any time. Do not " + + "depend on it.</b> <p>Returns the value of the build setting that is represented " + + "by the current target. It is an error to access this field for rules that do not " + + "set the <code>build_setting</code> attribute in their rule definition.") + public Object getBuildSettingValue() throws EvalException; + + @SkylarkCallable( name = "coverage_instrumented", doc = "Returns whether code coverage instrumentation should be generated when performing "
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java index 247f01d..32d73f1 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
@@ -16,6 +16,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.skylarkbuildapi.SkylarkConfigApi.BuildSettingApi; import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; @@ -307,6 +308,20 @@ doc = "<b>Experimental: This parameter is experimental and subject to change at any " + "time.</b><p> If true, then this rule is treated as an analysis test."), + @Param( + name = "build_setting", + type = BuildSettingApi.class, + noneable = true, + defaultValue = "None", + named = true, + positional = false, + // TODO(juliexxia): Link to in-build testing documentation when it is available. + doc = + "<b>Experimental: This parameter is experimental and subject to change at any " + + "time.</b><p> If set, describes what kind of build setting this rule is. " + + "See the <a href='config.html'><code>config</code></a> module. If this is " + + "set, a mandatory attribute named \"build_setting_default\" is automatically" + + "added to this rule, with a type corresponding to the value passed in here."), }, useAst = true, useEnvironment = true) @@ -326,6 +341,7 @@ Boolean executionPlatformConstraintsAllowed, SkylarkList<?> execCompatibleWith, Object analysisTest, + Object buildSetting, FuncallExpression ast, Environment funcallEnv) throws EvalException;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java index db78fb1..4580e2e 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -43,6 +43,7 @@ SkylarkSemantics::experimentalAnalysisTestingImprovements), EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS( SkylarkSemantics::experimentalEnableAndroidMigrationApis), + EXPERIMENTAL_BUILD_SETTING_API(SkylarkSemantics::experimentalBuildSettingApi), EXPERIMENTAL_PLATFORM_API(SkylarkSemantics::experimentalPlatformsApi), INCOMPATIBLE_DISABLE_OBJC_PROVIDER_RESOURCES( SkylarkSemantics::incompatibleDisableObjcProviderResources), @@ -109,6 +110,8 @@ // <== Add new options here in alphabetic order ==> public abstract boolean experimentalAnalysisTestingImprovements(); + public abstract boolean experimentalBuildSettingApi(); + public abstract List<String> experimentalCcSkylarkApiEnabledPackages(); public abstract boolean experimentalEnableAndroidMigrationApis(); @@ -193,6 +196,7 @@ builder() // <== Add new options here in alphabetic order ==> .experimentalAnalysisTestingImprovements(false) + .experimentalBuildSettingApi(false) .experimentalCcSkylarkApiEnabledPackages(ImmutableList.of()) .experimentalEnableAndroidMigrationApis(false) .experimentalEnableRepoMapping(false) @@ -236,6 +240,8 @@ // <== Add new options here in alphabetic order ==> public abstract Builder experimentalAnalysisTestingImprovements(boolean value); + public abstract Builder experimentalBuildSettingApi(boolean value); + public abstract Builder experimentalCcSkylarkApiEnabledPackages(List<String> value); public abstract Builder experimentalEnableAndroidMigrationApis(boolean value);
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java index 29a7cb2..8b62d92 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
@@ -117,6 +117,7 @@ Boolean executionPlatformConstraintsAllowed, SkylarkList<?> execCompatibleWith, Object analysisTest, + Object buildSetting, FuncallExpression ast, Environment funcallEnv) throws EvalException {
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java index 9810988..331f2df 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParser.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -14,6 +14,7 @@ package com.google.devtools.common.options; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; @@ -197,7 +198,8 @@ return skylarkOptions; } - void setSkylarkOptions(Map<String, Object> skylarkOptions) { + @VisibleForTesting + public void setSkylarkOptionsForTesting(Map<String, Object> skylarkOptions) { this.skylarkOptions = skylarkOptions; }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index ef657dd..1e87ef3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -18,6 +18,7 @@ import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith; import static org.junit.Assert.fail; +import com.google.common.base.Ascii; import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -341,8 +342,8 @@ return ResourceSet.createWithRamCpu(Double.MAX_VALUE, Double.MAX_VALUE); } - protected final BuildConfigurationCollection createConfigurations(String... args) - throws Exception { + private BuildConfigurationCollection createConfigurations( + ImmutableMap<String, Object> skylarkOptions, String... args) throws Exception { optionsParser = OptionsParser.newOptionsParser( Iterables.concat( @@ -358,6 +359,9 @@ optionsParser.parse(allArgs); optionsParser.parse(args); + // TODO(juliexxia): when the skylark options parsing work goes in, add type verification here. + optionsParser.setSkylarkOptionsForTesting(skylarkOptions); + InvocationPolicyEnforcer optionsPolicyEnforcer = getAnalysisMock().getInvocationPolicyEnforcer(); optionsPolicyEnforcer.enforce(optionsParser); @@ -504,20 +508,32 @@ * Sets host and target configuration using the specified options, falling back to the default * options for unspecified ones, and recreates the build view. * + * TODO(juliexxia): when skylark option parsing exists, find a way to combine these parameters + * into a single parameter so skylark/native options don't have to be specified separately. + * + * @param skylarkOptions map of skylark-defined options where the keys are option names (in the + * form of label-like strings) and the values are option values + * @param args native option name/pair descriptions in command line form (e.g. "--cpu=k8") + * * @throws IllegalArgumentException */ - protected void useConfiguration(String... args) throws Exception { + protected void useConfiguration(ImmutableMap<String, Object> skylarkOptions, String... args) + throws Exception { String[] actualArgs; actualArgs = Arrays.copyOf(args, args.length + 1); - actualArgs[args.length] = "--experimental_dynamic_configs=" - + configsMode.toString().toLowerCase(); - masterConfig = createConfigurations(actualArgs); + actualArgs[args.length] = + "--experimental_dynamic_configs=" + Ascii.toLowerCase(configsMode.toString()); + masterConfig = createConfigurations(skylarkOptions, actualArgs); targetConfig = getTargetConfiguration(); targetConfigKey = BuildConfigurationValue.key(targetConfig); configurationArgs = Arrays.asList(actualArgs); createBuildView(); } + protected void useConfiguration(String... args) throws Exception { + useConfiguration(ImmutableMap.of(), args); + } + /** * Makes subsequent {@link #useConfiguration} calls automatically use the specified style for * configurations.
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index a83f34a..eb3544b 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.packages.BuildType.OUTPUT_LIST; import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.substitutePlaceholderIntoTemplate; +import static com.google.devtools.build.lib.packages.RuleClass.Builder.SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME; import static com.google.devtools.build.lib.packages.RuleClass.NO_EXTERNAL_BINDINGS; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; import static com.google.devtools.build.lib.syntax.Type.INTEGER; @@ -911,7 +912,8 @@ ExecutionPlatformConstraintsAllowed.PER_RULE, /* executionPlatformConstraints= */ ImmutableSet.of(), OutputFile.Kind.FILE, - ImmutableList.copyOf(attributes)); + ImmutableList.copyOf(attributes), + /* buildSetting= */ null); } private static RuleClass createParentRuleClass() { @@ -1233,4 +1235,34 @@ .isEqualTo(ExecutionPlatformConstraintsAllowed.PER_TARGET); assertThat(childRuleClass.hasAttr("exec_compatible_with", LABEL_LIST)).isTrue(); } + + @Test + public void testBuildSetting_createsDefaultAttribute() { + RuleClass labelFlag = + new RuleClass.Builder("label_flag", RuleClassType.NORMAL, false) + .factory(DUMMY_CONFIGURED_TARGET_FACTORY) + .add(attr("tags", STRING_LIST)) + .setBuildSetting(new BuildSetting(true, LABEL)) + .build(); + RuleClass stringSetting = + new RuleClass.Builder("string_setting", RuleClassType.NORMAL, false) + .factory(DUMMY_CONFIGURED_TARGET_FACTORY) + .add(attr("tags", STRING_LIST)) + .setBuildSetting(new BuildSetting(false, STRING)) + .build(); + + assertThat(labelFlag.hasAttr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL)).isTrue(); + assertThat(stringSetting.hasAttr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, STRING)).isTrue(); + } + + @Test + public void testBuildSetting_doesNotCreateDefaultAttributeIfNotBuildSetting() { + RuleClass stringSetting = + new RuleClass.Builder("non_build_setting", RuleClassType.NORMAL, false) + .factory(DUMMY_CONFIGURED_TARGET_FACTORY) + .add(attr("tags", STRING_LIST)) + .build(); + + assertThat(stringSetting.hasAttr(SKYLARK_BUILD_SETTING_DEFAULT_ATTR_NAME, LABEL)).isFalse(); + } }
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 7fcda27..8996b5a 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -119,6 +119,7 @@ return parseOptions( // <== Add new options here in alphabetic order ==> "--experimental_analysis_testing_improvements=" + rand.nextBoolean(), + "--experimental_build_setting_api=" + rand.nextBoolean(), "--experimental_cc_skylark_api_enabled_packages=" + rand.nextDouble() + "," @@ -166,6 +167,7 @@ return SkylarkSemantics.builder() // <== Add new options here in alphabetic order ==> .experimentalAnalysisTestingImprovements(rand.nextBoolean()) + .experimentalBuildSettingApi(rand.nextBoolean()) .experimentalCcSkylarkApiEnabledPackages( ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble()))) .experimentalEnableAndroidMigrationApis(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index abc9c75..56c118e 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.AttributeContainer; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; +import com.google.devtools.build.lib.packages.BuildSetting; import com.google.devtools.build.lib.packages.Provider; import com.google.devtools.build.lib.packages.SkylarkProvider; import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey; @@ -50,6 +51,7 @@ import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator; import com.google.devtools.build.skyframe.SkyFunction; @@ -2280,6 +2282,110 @@ + "with for_analysis_testing=True"); } + + @Test + public void testBuildSettingRule_flag() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=true"); + + scratch.file("test/rules.bzl", + "def _impl(ctx): return None", + "build_setting_rule = rule(_impl, build_setting = config.string(flag=True))"); + scratch.file("test/BUILD", + "load('//test:rules.bzl', 'build_setting_rule')", + "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')"); + + BuildSetting buildSetting = + getTarget("//test:my_build_setting") + .getAssociatedRule() + .getRuleClassObject() + .getBuildSetting(); + + assertThat(buildSetting.getType()).isEqualTo(Type.STRING); + assertThat(buildSetting.isFlag()).isTrue(); + } + + @Test + public void testBuildSettingRule_settingByDefault() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=true"); + + scratch.file("test/rules.bzl", + "def _impl(ctx): return None", + "build_setting_rule = rule(_impl, build_setting = config.string())"); + scratch.file("test/BUILD", + "load('//test:rules.bzl', 'build_setting_rule')", + "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')"); + + BuildSetting buildSetting = + getTarget("//test:my_build_setting") + .getAssociatedRule() + .getRuleClassObject() + .getBuildSetting(); + + assertThat(buildSetting.getType()).isEqualTo(Type.STRING); + assertThat(buildSetting.isFlag()).isFalse(); + } + + @Test + public void testBuildSettingRule_settingByFlagParameter() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=true"); + + scratch.file("test/rules.bzl", + "def _impl(ctx): return None", + "build_setting_rule = rule(_impl, build_setting = config.string(flag=False))"); + scratch.file("test/BUILD", + "load('//test:rules.bzl', 'build_setting_rule')", + "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')"); + + BuildSetting buildSetting = + getTarget("//test:my_build_setting") + .getAssociatedRule() + .getRuleClassObject() + .getBuildSetting(); + + assertThat(buildSetting.getType()).isEqualTo(Type.STRING); + assertThat(buildSetting.isFlag()).isFalse(); + } + + + @Test + public void testBuildSettingRule_noDefault() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=true"); + + scratch.file("test/rules.bzl", + "def _impl(ctx): return None", + "build_setting_rule = rule(_impl, build_setting = config.string())"); + scratch.file("test/BUILD", + "load('//test:rules.bzl', 'build_setting_rule')", + "build_setting_rule(name = 'my_build_setting')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//test:my_build_setting"); + assertContainsEvent("missing value for mandatory attribute " + + "'build_setting_default' in 'build_setting_rule' rule"); + + } + + @Test + public void testBuildSettingRule_errorsWithoutExperimentalFlag() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=false"); + + scratch.file( + "test/rules.bzl", + "def _impl(ctx): return None", + "build_setting_rule = rule(_impl, build_setting = config.string())"); + scratch.file( + "test/BUILD", + "load('//test:rules.bzl', 'build_setting_rule')", + "build_setting_rule(name = 'my_build_setting', build_setting_default = 'default')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//test:my_build_setting"); + assertContainsEvent( + "build_setting parameter is experimental and not " + + "available for general use. It is subject to change at any time. It may be enabled " + + "by specifying --experimental_build_setting_api"); + } + /** * Skylark integration test that forces inlining. */
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index 5521b9c..3e8c104 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -36,7 +36,9 @@ import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext; import com.google.devtools.build.lib.analysis.util.MockRule; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Provider; import com.google.devtools.build.lib.packages.SkylarkInfo; +import com.google.devtools.build.lib.packages.SkylarkProvider; import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey; import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.packages.StructImpl; @@ -2206,4 +2208,71 @@ SkylarkList<String> keys = (SkylarkList<String>) keyInfo.getValue("keys"); assertThat(keys).containsExactly("c", "b", "a", "f", "e", "d").inOrder(); } + + private void writeIntFlagBuildSettingFiles() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=True"); + scratch.file( + "test/build_setting.bzl", + "BuildSettingInfo = provider(fields = ['name', 'value'])", + "def _impl(ctx):", + " return [BuildSettingInfo(name = ctx.attr.name, value = ctx.build_setting_value)]", + "", + "int_flag = rule(", + " implementation = _impl,", + " build_setting = config.int(flag = True),", + ")"); + scratch.file( + "test/BUILD", + "load('//test:build_setting.bzl', 'int_flag')", + "int_flag(name = 'int_flag', build_setting_default = 42)"); + } + + @Test + public void testBuildSettingValue_explicitlySet() throws Exception { + writeIntFlagBuildSettingFiles(); + useConfiguration(ImmutableMap.of("//test:int_flag", 24)); + + ConfiguredTarget buildSetting = getConfiguredTarget("//test:int_flag"); + Provider.Key key = + new SkylarkProvider.SkylarkKey( + Label.create(buildSetting.getLabel().getPackageIdentifier(), "build_setting.bzl"), + "BuildSettingInfo"); + StructImpl buildSettingInfo = (StructImpl) buildSetting.get(key); + + assertThat(buildSettingInfo.getValue("value")).isEqualTo(24); + } + + @Test + public void testBuildSettingValue_defaultFallback() throws Exception { + writeIntFlagBuildSettingFiles(); + + ConfiguredTarget buildSetting = getConfiguredTarget("//test:int_flag"); + Provider.Key key = + new SkylarkProvider.SkylarkKey( + Label.create(buildSetting.getLabel().getPackageIdentifier(), "build_setting.bzl"), + "BuildSettingInfo"); + StructImpl buildSettingInfo = (StructImpl) buildSetting.get(key); + + assertThat(buildSettingInfo.getValue("value")).isEqualTo(42); + } + + @Test + public void testBuildSettingValue_nonBuildSettingRule() throws Exception { + setSkylarkSemanticsOptions("--experimental_build_setting_api=True"); + scratch.file( + "test/rule.bzl", + "def _impl(ctx):", + " foo = ctx.build_setting_value", + " return []", + "non_build_setting = rule(implementation = _impl)"); + scratch.file( + "test/BUILD", + "load('//test:rule.bzl', 'non_build_setting')", + "non_build_setting(name = 'my_non_build_setting')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//test:my_non_build_setting"); + assertContainsEvent("attempting to access 'build_setting_value' of non-build setting " + + "//test:my_non_build_setting"); + } }