diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index b096dcb50d0893..eb84111920299d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -1526,6 +1526,7 @@ java_library( ":config/run_under", ":config/transitive_option_details", ":platform_options", + ":starlark/function_transition_util", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/buildeventstream", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 578af01bb76de3..af8bc0183e9091 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.config.OutputDirectories.InvalidMnemonicException; +import com.google.devtools.build.lib.analysis.starlark.FunctionTransitionUtil; import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId; @@ -67,7 +68,7 @@ * *

Instances of BuildConfiguration are canonical: * - * 

c1.equals(c2) <=> c1==c2.
+ * 
{@code c1.equals(c2) <=> c1==c2.}
*/ // TODO(janakr): If overhead of fragments class names is too high, add constructor that just takes // fragments and gets names from them. @@ -107,6 +108,14 @@ public interface ActionEnvironmentProvider { private final BuildOptions buildOptions; private final CoreOptions options; + /** + * If non-empty, this is appended to output directories as ST-[transitionDirectoryNameFragment]. + * The value is a hash of BuildOptions that have been affected by a Starlark transition. + * + *

See b/203470434 or #14023 for more information and planned behavior changes. + */ + private final String transitionDirectoryNameFragment; + private final ImmutableMap commandLineBuildVariables; private final int hashCode; // We can precompute the hash code as all its inputs are immutable. @@ -189,6 +198,8 @@ public BuildConfiguration( if (buildOptions.contains(PlatformOptions.class)) { platformOptions = buildOptions.get(PlatformOptions.class); } + this.transitionDirectoryNameFragment = + FunctionTransitionUtil.computeOutputDirectoryNameFragment(buildOptions); this.outputDirectories = new OutputDirectories( directories, @@ -196,7 +207,8 @@ public BuildConfiguration( platformOptions, this.fragments, mainRepositoryName, - siblingRepositoryLayout); + siblingRepositoryLayout, + transitionDirectoryNameFragment); this.mainRepositoryName = mainRepositoryName; this.siblingRepositoryLayout = siblingRepositoryLayout; @@ -403,6 +415,11 @@ public String getMnemonic() { return outputDirectories.getMnemonic(); } + @VisibleForTesting + public String getTransitionDirectoryNameFragment() { + return transitionDirectoryNameFragment; + } + @Override public String toString() { return checksum(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java index 744acfa7cb7890..5203a883b811f6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java @@ -240,22 +240,6 @@ public class CoreOptions extends FragmentOptions implements Cloneable { + "'fastbuild', 'dbg', 'opt'.") public CompilationMode hostCompilationMode; - /** - * This option is used by starlark transitions to add a distinguishing element to the output - * directory name, in order to avoid name clashing. - */ - @Option( - name = "transition directory name fragment", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = { - OptionEffectTag.LOSES_INCREMENTAL_STATE, - OptionEffectTag.AFFECTS_OUTPUTS, - OptionEffectTag.LOADING_AND_ANALYSIS - }, - metadataTags = {OptionMetadataTag.INTERNAL}) - public String transitionDirectoryNameFragment; - @Option( name = "experimental_enable_aspect_hints", defaultValue = "false", @@ -839,7 +823,6 @@ public IncludeConfigFragmentsEnumConverter() { public FragmentOptions getHost() { CoreOptions host = (CoreOptions) getDefault(); - host.transitionDirectoryNameFragment = transitionDirectoryNameFragment; host.affectedByStarlarkTransition = affectedByStarlarkTransition; host.compilationMode = hostCompilationMode; host.isHost = true; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java index ddd7fec1f56228..413251d5f63b99 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java @@ -147,10 +147,12 @@ public ArtifactRoot getRoot( @Nullable PlatformOptions platformOptions, ImmutableSortedMap, Fragment> fragments, RepositoryName mainRepositoryName, - boolean siblingRepositoryLayout) + boolean siblingRepositoryLayout, + String transitionDirectoryNameFragment) throws InvalidMnemonicException { this.directories = directories; - this.mnemonic = buildMnemonic(options, platformOptions, fragments); + this.mnemonic = + buildMnemonic(options, platformOptions, fragments, transitionDirectoryNameFragment); this.outputDirName = options.isHost ? "host" : mnemonic; this.outputDirectory = @@ -207,7 +209,8 @@ private static void validateMnemonicPart(String part, String errorTemplate, Obje private static String buildMnemonic( CoreOptions options, @Nullable PlatformOptions platformOptions, - ImmutableSortedMap, Fragment> fragments) + ImmutableSortedMap, Fragment> fragments, + String transitionDirectoryNameFragment) throws InvalidMnemonicException { // See explanation at declaration for outputRoots. List nameParts = new ArrayList<>(); @@ -230,9 +233,7 @@ private static String buildMnemonic( // Add the transition suffix. addMnemonicPart( - nameParts, - options.transitionDirectoryNameFragment, - "Transition directory name fragment '%s'"); + nameParts, transitionDirectoryNameFragment, "Transition directory name fragment '%s'"); // Join all the parts. String mnemonic = nameParts.stream().filter(not(Strings::isNullOrEmpty)).collect(joining("-")); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java index 4e4d2a1182b228..2c95da7fe32ac0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.analysis.starlark; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition.COMMAND_LINE_OPTION_PREFIX; import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition.PATCH_TRANSITION_KEY; import static java.util.stream.Collectors.joining; @@ -57,7 +58,7 @@ * Utility class for common work done across {@link StarlarkAttributeTransitionProvider} and {@link * StarlarkRuleTransitionProvider}. */ -public class FunctionTransitionUtil { +public final class FunctionTransitionUtil { // The length of the hash of the config tacked onto the end of the output path. // Limited for ergonomics and MAX_PATH reasons. @@ -165,7 +166,7 @@ static ImmutableMap buildOptionInfo(BuildOptions buildOption ImmutableSet> optionClasses = buildOptions.getNativeOptions().stream() .map(FragmentOptions::getClass) - .collect(ImmutableSet.toImmutableSet()); + .collect(toImmutableSet()); for (Class optionClass : optionClasses) { ImmutableList optionDefinitions = @@ -394,7 +395,8 @@ private static BuildOptions applyTransition( convertedNewValues.add("//command_line_option:evaluating for analysis test"); toOptions.get(CoreOptions.class).evaluatingForAnalysisTest = true; } - updateOutputDirectoryNameFragment(convertedNewValues, optionInfoMap, toOptions); + + updateAffectedByStarlarkTransition(toOptions.get(CoreOptions.class), convertedNewValues); return toOptions; } @@ -404,27 +406,20 @@ private static BuildOptions applyTransition( * the build by Starlark transitions. Options only set on command line are not affecting the * computation. * - * @param changedOptions the names of all options changed by this transition in label form e.g. - * "//command_line_option:cpu" for native options and "//myapp:foo" for starlark options. - * @param optionInfoMap a map of all native options (name -> OptionInfo) present in {@code - * toOptions}. - * @param toOptions the newly transitioned {@link BuildOptions} for which we need to updated - * {@code transitionDirectoryNameFragment} and {@code affectedByStarlarkTransition}. + * @param toOptions the {@link BuildOptions} to use to calculate which we need to compute {@code + * transitionDirectoryNameFragment}. */ // TODO(bazel-team): This hashes different forms of equivalent values differently though they // should be the same configuration. Starlark transitions are flexible about the values they // take (e.g. bool-typed options can take 0/1, True/False, "0"/"1", or "True"/"False") which // makes it so that two configurations that are the same in value may hash differently. - private static void updateOutputDirectoryNameFragment( - Set changedOptions, Map optionInfoMap, BuildOptions toOptions) { - // Return without doing anything if this transition hasn't changed any option values. - if (changedOptions.isEmpty()) { - return; - } - + public static String computeOutputDirectoryNameFragment(BuildOptions toOptions) { CoreOptions buildConfigOptions = toOptions.get(CoreOptions.class); - - updateAffectedByStarlarkTransition(buildConfigOptions, changedOptions); + if (buildConfigOptions.affectedByStarlarkTransition.isEmpty()) { + return ""; + } + // TODO(blaze-configurability-team): A mild performance optimization would have this be global. + Map optionInfoMap = buildOptionInfo(toOptions); TreeMap toHash = new TreeMap<>(); for (String optionName : buildConfigOptions.affectedByStarlarkTransition) { @@ -445,19 +440,21 @@ private static void updateOutputDirectoryNameFragment( toHash.put(optionName, value); } else { Object value = toOptions.getStarlarkOptions().get(Label.parseAbsoluteUnchecked(optionName)); - if (value != null) { - toHash.put(optionName, value); - } + toHash.put(optionName, value); } } ImmutableList.Builder hashStrs = ImmutableList.builderWithExpectedSize(toHash.size()); for (Map.Entry singleOptionAndValue : toHash.entrySet()) { - String toAdd = singleOptionAndValue.getKey() + "=" + singleOptionAndValue.getValue(); - hashStrs.add(toAdd); + Object value = singleOptionAndValue.getValue(); + if (value != null) { + hashStrs.add(singleOptionAndValue.getKey() + "=" + value); + } else { + // Avoid using =null to different from value being the non-null String "null" + hashStrs.add(singleOptionAndValue.getKey() + "@null"); + } } - buildConfigOptions.transitionDirectoryNameFragment = - transitionDirectoryNameFragment(hashStrs.build()); + return transitionDirectoryNameFragment(hashStrs.build()); } /** @@ -466,6 +463,9 @@ private static void updateOutputDirectoryNameFragment( */ private static void updateAffectedByStarlarkTransition( CoreOptions buildConfigOptions, Set changedOptions) { + if (changedOptions.isEmpty()) { + return; + } Set mutableCopyToUpdate = new TreeSet<>(buildConfigOptions.affectedByStarlarkTransition); mutableCopyToUpdate.addAll(changedOptions); @@ -503,4 +503,6 @@ OptionDefinition getDefinition() { return definition; } } + + private FunctionTransitionUtil() {} } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java index 39a8afe67c3f97..bb7211909ccc71 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java @@ -1181,6 +1181,81 @@ public void testTransitionOnBuildSetting_fromCommandLine() throws Exception { .isEqualTo(StarlarkInt.of(42)); } + @Test + public void testTransitionBackToStarlarkDefaultOK() throws Exception { + writeAllowlistFile(); + writeBuildSettingsBzl(); + scratch.file( + "test/starlark/rules.bzl", + "load('//myinfo:myinfo.bzl', 'MyInfo')", + "def _transition_impl(settings, attr):", + " return {", + " '//test/starlark:the-answer': attr.answer_for_dep,", + " '//test/starlark:did-transition': 1,", + "}", + "my_transition = transition(", + " implementation = _transition_impl,", + " inputs = [],", + " outputs = ['//test/starlark:the-answer', '//test/starlark:did-transition']", + ")", + "def _rule_impl(ctx):", + " return MyInfo(dep = ctx.attr.dep)", + "my_rule = rule(", + " implementation = _rule_impl,", + " attrs = {", + " 'dep': attr.label(cfg = my_transition),", + " 'answer_for_dep': attr.int(),", + " '_allowlist_function_transition': attr.label(", + " default = '//tools/allowlists/function_transition_allowlist'),", + " }", + ")"); + scratch.file( + "test/starlark/BUILD", + "load('//test/starlark:rules.bzl', 'my_rule')", + "load('//test/starlark:build_settings.bzl', 'int_flag')", + "my_rule(name = 'test', dep = ':dep1', answer_for_dep=0)", + "my_rule(name = 'dep1', dep = ':dep2', answer_for_dep=42)", + "my_rule(name = 'dep2', dep = ':dep3', answer_for_dep=0)", + "my_rule(name = 'dep3')", + "int_flag(name = 'the-answer', build_setting_default=0)", + "int_flag(name = 'did-transition', build_setting_default=0)"); + useConfiguration(ImmutableMap.of(), "--cpu=FOO"); + + ConfiguredTarget test = getConfiguredTarget("//test/starlark:test"); + + // '//test/starlark:did-transition ensures ST-hash is 'turned on' since :test has no ST-hash + // and thus will trivially have a unique getTransitionDirectoryNameFragment + + @SuppressWarnings("unchecked") + ConfiguredTarget dep1 = + Iterables.getOnlyElement( + (List) getMyInfoFromTarget(test).getValue("dep")); + + @SuppressWarnings("unchecked") + ConfiguredTarget dep2 = + Iterables.getOnlyElement( + (List) getMyInfoFromTarget(dep1).getValue("dep")); + + @SuppressWarnings("unchecked") + ConfiguredTarget dep3 = + Iterables.getOnlyElement( + (List) getMyInfoFromTarget(dep2).getValue("dep")); + + // These must be true + assertThat(getTransitionDirectoryNameFragment(dep1)) + .isNotEqualTo(getTransitionDirectoryNameFragment(dep2)); + + assertThat(getTransitionDirectoryNameFragment(dep2)) + .isNotEqualTo(getTransitionDirectoryNameFragment(dep3)); + + // TODO(blaze-configurability-team): When "affected by starlark transition" is gone, + // will be equal and thus getTransitionDirectoryNameFragment can be equal. + if (!getConfiguration(dep1).equals(getConfiguration(dep3))) { + assertThat(getTransitionDirectoryNameFragment(dep1)) + .isNotEqualTo(getTransitionDirectoryNameFragment(dep3)); + } + } + @Test public void testTransitionOnBuildSetting_onlyTransitionsAffectsDirectory() throws Exception { writeAllowlistFile(); @@ -1216,7 +1291,7 @@ public void testTransitionOnBuildSetting_onlyTransitionsAffectsDirectory() throw // Assert that transitionDirectoryNameFragment is only affected by options // set via transitions. Not by native or starlark options set via command line, // never touched by any transition. - assertThat(getCoreOptions(dep).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(dep)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//test/starlark:the-answer=42"))); @@ -1234,6 +1309,10 @@ private Object getStarlarkOption(ConfiguredTarget target, String absName) { return getStarlarkOptions(target).get(Label.parseAbsoluteUnchecked(absName)); } + private String getTransitionDirectoryNameFragment(ConfiguredTarget target) { + return getConfiguration(target).getTransitionDirectoryNameFragment(); + } + @Test public void testOutputDirHash_multipleNativeOptionTransitions() throws Exception { writeAllowlistFile(); @@ -1287,12 +1366,12 @@ public void testOutputDirHash_multipleNativeOptionTransitions() throws Exception assertThat(affectedOptions) .containsExactly("//command_line_option:foo", "//command_line_option:bar"); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(test)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//command_line_option:foo=foosball"))); - assertThat(getCoreOptions(dep).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(dep)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of( @@ -1346,8 +1425,8 @@ public void testOutputDirHash_noop_changeToSameState() throws Exception { Iterables.getOnlyElement( (List) getMyInfoFromTarget(test).getValue("dep")); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) - .isEqualTo(getCoreOptions(dep).transitionDirectoryNameFragment); + assertThat(getTransitionDirectoryNameFragment(test)) + .isEqualTo(getTransitionDirectoryNameFragment(dep)); } @Test @@ -1390,8 +1469,8 @@ public void testOutputDirHash_noop_emptyReturns() throws Exception { Iterables.getOnlyElement( (List) getMyInfoFromTarget(test).getValue("dep")); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) - .isEqualTo(getCoreOptions(dep).transitionDirectoryNameFragment); + assertThat(getTransitionDirectoryNameFragment(test)) + .isEqualTo(getTransitionDirectoryNameFragment(dep)); } // Test that setting all starlark options back to default != null hash of top level. @@ -1452,7 +1531,7 @@ public void testOutputDirHash_multipleStarlarkOptionTransitions_backToDefaultCom (List) getMyInfoFromTarget(getConfiguredTarget("//test")).getValue("dep")); - assertThat(getCoreOptions(dep).transitionDirectoryNameFragment).isNotNull(); + assertThat(getTransitionDirectoryNameFragment(dep)).isNotEmpty(); } /** See comment above {@link FunctionTransitionUtil#updateOutputDirectoryNameFragment} */ @@ -1507,11 +1586,11 @@ public void testOutputDirHash_starlarkOption_differentBoolRepresentationsNotEqua Iterables.getOnlyElement( (List) getMyInfoFromTarget(test).getValue("dep")); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(test)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//test:foo=1"))); - assertThat(getCoreOptions(dep).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(dep)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//test:foo=true"))); @@ -1561,8 +1640,8 @@ public void testOutputDirHash_nativeOption_differentBoolRepresentationsEquals() Iterables.getOnlyElement( (List) getMyInfoFromTarget(test).getValue("dep")); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) - .isEqualTo(getCoreOptions(dep).transitionDirectoryNameFragment); + assertThat(getTransitionDirectoryNameFragment(test)) + .isEqualTo(getTransitionDirectoryNameFragment(dep)); } @Test @@ -1619,11 +1698,11 @@ public void testOutputDirHash_multipleStarlarkTransitions() throws Exception { getConfiguration(dep).getOptions().get(CoreOptions.class).affectedByStarlarkTransition; assertThat(affectedOptions).containsExactly("//test:bar", "//test:foo"); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(test)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//test:foo=foosball"))); - assertThat(getCoreOptions(dep).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(dep)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//test:bar=barsball", "//test:foo=foosball"))); @@ -1707,7 +1786,7 @@ public void testOutputDirHash_multipleMixedTransitions() throws Exception { assertThat(affectedOptionsTop).containsExactly("//command_line_option:foo"); assertThat(getConfiguration(top).getOptions().getStarlarkOptions()).isEmpty(); - assertThat(getCoreOptions(top).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(top)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of("//command_line_option:foo=foosball"))); @@ -1727,7 +1806,7 @@ public void testOutputDirHash_multipleMixedTransitions() throws Exception { .containsExactly( Maps.immutableEntry(Label.parseAbsoluteUnchecked("//test:zee"), "zeesball")); - assertThat(getCoreOptions(middle).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(middle)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of( @@ -1752,7 +1831,7 @@ public void testOutputDirHash_multipleMixedTransitions() throws Exception { .containsExactly( Maps.immutableEntry(Label.parseAbsoluteUnchecked("//test:zee"), "zeesball"), Maps.immutableEntry(Label.parseAbsoluteUnchecked("//test:xan"), "xansball")); - assertThat(getCoreOptions(bottom).transitionDirectoryNameFragment) + assertThat(getTransitionDirectoryNameFragment(bottom)) .isEqualTo( FunctionTransitionUtil.transitionDirectoryNameFragment( ImmutableList.of( @@ -2022,8 +2101,8 @@ private void testNoOpTransitionLeavesSameConfig_native(boolean directRead) throw ConfiguredTarget dep = Iterables.getOnlyElement( (List) getMyInfoFromTarget(test).getValue("dep")); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) - .isEqualTo(getCoreOptions(dep).transitionDirectoryNameFragment); + assertThat(getTransitionDirectoryNameFragment(test)) + .isEqualTo(getTransitionDirectoryNameFragment(dep)); } @Test @@ -2090,8 +2169,8 @@ private void testNoOpTransitionLeavesSameConfig_starlark(boolean directRead, boo ConfiguredTarget dep = Iterables.getOnlyElement( (List) getMyInfoFromTarget(test).getValue("dep")); - assertThat(getCoreOptions(test).transitionDirectoryNameFragment) - .isEqualTo(getCoreOptions(dep).transitionDirectoryNameFragment); + assertThat(getTransitionDirectoryNameFragment(test)) + .isEqualTo(getTransitionDirectoryNameFragment(dep)); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/DummyTestFragment.java b/src/test/java/com/google/devtools/build/lib/analysis/util/DummyTestFragment.java index c07afa21817329..f9d5dda1bde143 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/DummyTestFragment.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/DummyTestFragment.java @@ -22,6 +22,7 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; import java.util.List; /** @@ -58,6 +59,15 @@ public static class DummyTestOptions extends FragmentOptions { help = "A regular string-typed option") public String foo; + @Option( + name = "internal foo", + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.NO_OP}, + metadataTags = {OptionMetadataTag.INTERNAL}, + help = "A string-typed option that cannot be set on the commandline") + public String internalFoo; + @Option( name = "bar", defaultValue = "", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java index 3c19e2048da518..1e0785f4f04655 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java @@ -20,15 +20,18 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.MissingInputFileException; +import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.PlatformConfiguration; import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.CoreOptions; import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.analysis.util.DummyTestFragment; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; +import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.EvaluationResult; import java.util.Optional; @@ -58,6 +61,14 @@ public final class PlatformMappingFunctionTest extends BuildViewTestCase { private BuildOptions defaultBuildOptions; + @Override + protected ConfiguredRuleClassProvider createRuleClassProvider() { + ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); + TestRuleClassProvider.addStandardRules(builder); + builder.addConfigurationFragment(DummyTestFragment.class); + return builder.build(); + } + @Before public void setDefaultBuildOptions() { defaultBuildOptions = @@ -218,7 +229,7 @@ public void ableToChangeInternalOption() throws Exception { "my_mapping_file", "platforms:", // Force line break " //platforms:one", // Force line break - " --transition directory name fragment=updated_output_dir"); + " --internal foo=something_new"); PlatformMappingValue platformMappingValue = executeFunction(PlatformMappingValue.Key.create(PathFragment.create("my_mapping_file"))); @@ -228,8 +239,8 @@ public void ableToChangeInternalOption() throws Exception { BuildConfigurationValue.Key mapped = platformMappingValue.map(keyForOptions(modifiedOptions)); - assertThat(mapped.getOptions().get(CoreOptions.class).transitionDirectoryNameFragment) - .isEqualTo("updated_output_dir"); + assertThat(mapped.getOptions().get(DummyTestFragment.DummyTestOptions.class).internalFoo) + .isEqualTo("something_new"); } private PlatformMappingValue executeFunction(PlatformMappingValue.Key key) throws Exception {