Move some native transitions to BuildOptionsView.
In service of https://github.com/bazelbuild/bazel/issues/11258.
PiperOrigin-RevId: 315577744
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 f373029..da5080b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1591,6 +1591,7 @@
":config/build_options",
":config/build_options_cache",
":config/core_options",
+ ":config/fragment_options",
":config/transitions/patch_transition",
":config/transitions/transition_factory",
":platform_options",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCache.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCache.java
index e74552c..dd1dee0 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCache.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsCache.java
@@ -60,7 +60,7 @@
* @param transitionFunc the transition to apply if {@code (fromOptions, context)} isn't cached
*/
public BuildOptions applyTransition(
- BuildOptions fromOptions, T context, Supplier<BuildOptions> transitionFunc) {
+ BuildOptionsView fromOptions, T context, Supplier<BuildOptions> transitionFunc) {
try {
return cache.get(new ReferenceCacheKey(fromOptions, context), () -> transitionFunc.get());
} catch (ExecutionException e) {
@@ -77,17 +77,17 @@
* BuildOptions#equals} calls.
*/
private static final class ReferenceCacheKey {
- private final BuildOptions options;
+ private final BuildOptionsView options;
private final Object context;
- ReferenceCacheKey(BuildOptions options, Object context) {
+ ReferenceCacheKey(BuildOptionsView options, Object context) {
this.options = options;
this.context = context;
}
@Override
public String toString() {
- return String.format("ReferenceCacheKey(%s, %s)", options, context);
+ return String.format("ReferenceCacheKey(%s, %s)", options.underlying(), context);
}
@Override
@@ -101,7 +101,9 @@
// because the memory problem we're trying to solve is the same transition applying to the
// same BuildOptions instance over and over again.
@SuppressWarnings("ReferenceEquality")
- boolean match = this.options == casted.options && this.context.equals(casted.context);
+ boolean match =
+ this.options.underlying() == casted.options.underlying()
+ && this.context.equals(casted.context);
return match;
}
@@ -109,7 +111,7 @@
public int hashCode() {
// Computing BuildOptions.hashCode is potentially expensive and its reference is sufficient,
// so we just hash its reference.
- return Objects.hash(System.identityHashCode(options), context);
+ return Objects.hash(System.identityHashCode(options.underlying()), context);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java
index 28a5bf5..9a1dcd3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java
@@ -18,6 +18,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
@@ -99,10 +100,15 @@
private static final BuildOptionsCache<Label> cache = new BuildOptionsCache<>();
@Override
- public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
+ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ return ImmutableSet.of(CoreOptions.class, PlatformOptions.class);
+ }
+
+ @Override
+ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
if (executionPlatform == null) {
// No execution platform is known, so don't change anything.
- return options;
+ return options.underlying();
}
return cache.applyTransition(
options,
@@ -110,11 +116,13 @@
executionPlatform,
() -> {
// Start by converting to host options.
- BuildOptions execConfiguration = options.createHostOptions();
+ BuildOptionsView execOptions =
+ new BuildOptionsView(
+ options.underlying().createHostOptions(), requiresOptionFragments());
// Then unset isHost, if CoreOptions is available.
CoreOptions coreOptions =
- Preconditions.checkNotNull(execConfiguration.get(CoreOptions.class));
+ Preconditions.checkNotNull(execOptions.get(CoreOptions.class));
coreOptions.isHost = false;
coreOptions.isExec = true;
coreOptions.outputDirectoryName = null;
@@ -122,12 +130,12 @@
String.format("-exec-%X", executionPlatform.getCanonicalForm().hashCode());
// Then set the target to the saved execution platform if there is one.
- if (execConfiguration.get(PlatformOptions.class) != null) {
- execConfiguration.get(PlatformOptions.class).platforms =
+ if (execOptions.get(PlatformOptions.class) != null) {
+ execOptions.get(PlatformOptions.class).platforms =
ImmutableList.of(executionPlatform);
}
- return execConfiguration;
+ return execOptions.underlying();
});
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransition.java
index e2c1436..33ed134 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransition.java
@@ -24,7 +24,6 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
/**
* A configuration transition that composes two other transitions in an ordered sequence.
@@ -53,7 +52,7 @@
}
@Override
- public Set<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
return ImmutableSet.<Class<? extends FragmentOptions>>builder()
.addAll(transition1.requiresOptionFragments())
.addAll(transition2.requiresOptionFragments())
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java
index b9783ba..6232b13 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.Map;
-import java.util.Set;
/**
* A configuration transition.
@@ -38,7 +37,7 @@
* <p>Blaze throws an {@link IllegalArgumentException} if {@link #apply} is called on an options
* fragment that isn't declared here.
*/
- default Set<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ default ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
return ImmutableSet.of();
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java
index b8fcaa3..dca8509 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java
@@ -15,6 +15,7 @@
import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -26,8 +27,8 @@
private NoTransition() {}
@Override
- public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
- return options;
+ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
+ return options.underlying();
}
/** Returns a {@link TransitionFactory} instance that generates the no transition. */
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java
index b287107..fb7d5cf 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java
@@ -15,6 +15,7 @@
import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -27,7 +28,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
+ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
throw new UnsupportedOperationException(
"This is only referenced in a few places, so it's easier and more efficient to optimize "
+ "Blaze's transition logic in the presence of null transitions vs. actually call this "
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/PatchTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/PatchTransition.java
index b72329a..99bff12 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/PatchTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/PatchTransition.java
@@ -68,14 +68,14 @@
*/
default BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
throw new UnsupportedOperationException(
- "Either this or patch(RestrictedBuildOptions) must be overriden");
+ "Either this or patch(BuildOptionsView) must be overridden");
}
/**
* Applies the transition.
*
* <p>Blaze throws an {@link IllegalArgumentException} if this method reads any options fragment
- * not declard in {@link ConfigurationTransition#requiresOptionFragments}.
+ * not declared in {@link ConfigurationTransition#requiresOptionFragments}.
*
* @param options the options representing the input configuration to this transition. <b>DO NOT
* MODIFY THIS VARIABLE WITHOUT CLONING IT FIRST!</b>
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java
index 5ec519e..d3b7d0b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java
@@ -49,7 +49,7 @@
*/
default Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
throw new UnsupportedOperationException(
- "Either this or patch(RestrictedBuildOptions) must be overriden");
+ "Either this or patch(BuildOptionsView) must be overridden");
}
/**
@@ -58,7 +58,7 @@
* are recommended.
*
* <p>Blaze throws an {@link IllegalArgumentException} if this method reads any options fragment
- * not declard in {@link ConfigurationTransition#requiresOptionFragments}.
+ * not declared in {@link ConfigurationTransition#requiresOptionFragments}.
*
* <p>Returning an empty or null list triggers a {@link RuntimeException}.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
index 8084e56..d0a0262 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
@@ -16,9 +16,12 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
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.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
+import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.EventHandler;
@@ -40,8 +43,14 @@
public static final PatchTransition APPLE_CROSSTOOL_TRANSITION = new AppleCrosstoolTransition();
@Override
- public BuildOptions patch(BuildOptions buildOptions, EventHandler eventHandler) {
- BuildOptions result = buildOptions.clone();
+ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ return ImmutableSet.of(
+ AppleCommandLineOptions.class, CoreOptions.class, CppOptions.class, PlatformOptions.class);
+ }
+
+ @Override
+ public BuildOptions patch(BuildOptionsView buildOptions, EventHandler eventHandler) {
+ BuildOptionsView result = buildOptions.clone();
AppleCommandLineOptions appleOptions = buildOptions.get(AppleCommandLineOptions.class);
CoreOptions configOptions = buildOptions.get(CoreOptions.class);
@@ -50,7 +59,7 @@
// The configuration distinguisher is only set by AppleCrosstoolTransition and
// AppleBinaryTransition, both of which also set the Crosstool and the CPU to Apple ones.
// So we are fine not doing anything.
- return buildOptions;
+ return buildOptions.underlying();
}
String cpu =
@@ -58,20 +67,20 @@
appleOptions.applePlatformType,
determineSingleArchitecture(appleOptions, configOptions));
setAppleCrosstoolTransitionConfiguration(buildOptions, result, cpu);
- return result;
+ return result.underlying();
}
-
+
/**
* Sets configuration fields required for a transition that uses apple_crosstool_top in place of
* the default CROSSTOOL.
*
* @param from options from the originating configuration
- * @param to options for the destination configuration. This instance will be modified
- * to so the destination configuration uses the apple crosstool
+ * @param to options for the destination configuration. This instance will be modified to so the
+ * destination configuration uses the apple crosstool
* @param cpu {@code --cpu} value for toolchain selection in the destination configuration
*/
- public static void setAppleCrosstoolTransitionConfiguration(BuildOptions from,
- BuildOptions to, String cpu) {
+ public static void setAppleCrosstoolTransitionConfiguration(
+ BuildOptionsView from, BuildOptionsView to, String cpu) {
Label crosstoolTop = from.get(AppleCommandLineOptions.class).appleCrosstoolTop;
Label libcTop = from.get(AppleCommandLineOptions.class).appleLibcTop;
String cppCompiler = from.get(AppleCommandLineOptions.class).cppCompiler;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
index 8f6af96..443667d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
@@ -24,9 +24,12 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
+import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
+import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.events.EventHandler;
@@ -38,6 +41,7 @@
import com.google.devtools.build.lib.rules.apple.ApplePlatform;
import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
+import com.google.devtools.build.lib.rules.cpp.CppOptions;
import com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.PlatformRule;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skylarkbuildapi.SplitTransitionProviderApi;
@@ -198,8 +202,18 @@
}
@Override
+ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ return ImmutableSet.of(
+ AppleCommandLineOptions.class,
+ CoreOptions.class,
+ CppOptions.class,
+ ObjcCommandLineOptions.class,
+ PlatformOptions.class);
+ }
+
+ @Override
public final Map<String, BuildOptions> split(
- BuildOptions buildOptions, EventHandler eventHandler) {
+ BuildOptionsView buildOptions, EventHandler eventHandler) {
List<String> cpus;
DottedVersion actualMinimumOsVersion;
ConfigurationDistinguisher configurationDistinguisher;
@@ -269,7 +283,7 @@
cpus = ImmutableSortedSet.copyOf(cpus).asList();
ImmutableMap.Builder<String, BuildOptions> splitBuildOptions = ImmutableMap.builder();
for (String cpu : cpus) {
- BuildOptions splitOptions = buildOptions.clone();
+ BuildOptionsView splitOptions = buildOptions.clone();
AppleCommandLineOptions appleCommandLineOptions =
splitOptions.get(AppleCommandLineOptions.class);
@@ -306,7 +320,7 @@
}
appleCommandLineOptions.configurationDistinguisher = configurationDistinguisher;
- splitBuildOptions.put(IOS_CPU_PREFIX + cpu, splitOptions);
+ splitBuildOptions.put(IOS_CPU_PREFIX + cpu, splitOptions.underlying());
}
return splitBuildOptions.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java
index f6b094b..b8fcb07 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet;
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.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.ConvenienceSymlinks.SymlinkDefinition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
@@ -145,7 +146,13 @@
}
};
return targetConfigs.stream()
- .map(config -> configGetter.apply(transition.patch(config.getOptions(), e)))
+ .map(
+ config ->
+ configGetter.apply(
+ transition.patch(
+ new BuildOptionsView(
+ config.getOptions(), transition.requiresOptionFragments()),
+ e)))
.map(config -> config.getOutputDirectory(repositoryName).getRoot().asPath())
.distinct()
.collect(toImmutableSet());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersionTransition.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersionTransition.java
index d505c10..6a91e1e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersionTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersionTransition.java
@@ -15,8 +15,11 @@
package com.google.devtools.build.lib.rules.python;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsCache;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
+import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.errorprone.annotations.Immutable;
@@ -55,7 +58,7 @@
* <p>Caution: This method must not modify {@code options}. See the class javadoc for {@link
* PatchTransition}.
*/
- protected abstract PythonVersion determineNewVersion(BuildOptions options);
+ protected abstract PythonVersion determineNewVersion(BuildOptionsView options);
@Override
public abstract boolean equals(Object other);
@@ -69,22 +72,27 @@
private static final BuildOptionsCache<PythonVersion> cache = new BuildOptionsCache<>();
@Override
- public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
+ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ return ImmutableSet.of(PythonOptions.class);
+ }
+
+ @Override
+ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
PythonVersion newVersion = determineNewVersion(options);
Preconditions.checkArgument(newVersion.isTargetValue());
PythonOptions opts = options.get(PythonOptions.class);
if (!opts.canTransitionPythonVersion(newVersion)) {
- return options;
+ return options.underlying();
}
return cache.applyTransition(
options,
newVersion,
() -> {
- BuildOptions newOptions = options.clone();
+ BuildOptionsView newOptions = options.clone();
PythonOptions newOpts = newOptions.get(PythonOptions.class);
newOpts.setPythonVersion(newVersion);
- return newOptions;
+ return newOptions.underlying();
});
}
@@ -98,7 +106,7 @@
}
@Override
- protected PythonVersion determineNewVersion(BuildOptions options) {
+ protected PythonVersion determineNewVersion(BuildOptionsView options) {
return newVersion;
}
@@ -128,7 +136,7 @@
private ToDefault() {}
@Override
- protected PythonVersion determineNewVersion(BuildOptions options) {
+ protected PythonVersion determineNewVersion(BuildOptionsView options) {
return options.get(PythonOptions.class).getDefaultPythonVersion();
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java
index f70e716..c80d2a1 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java
@@ -51,7 +51,10 @@
ImmutableList.of(CoreOptions.class, PlatformOptions.class),
"--platforms=//platform:target");
- BuildOptions result = transition.patch(options, new StoredEventHandler());
+ BuildOptions result =
+ transition.patch(
+ new BuildOptionsView(options, transition.requiresOptionFragments()),
+ new StoredEventHandler());
assertThat(result).isNotNull();
assertThat(result).isNotSameInstanceAs(options);
@@ -81,7 +84,10 @@
ImmutableList.of(CoreOptions.class, PlatformOptions.class),
"--platforms=//platform:target");
- BuildOptions result = transition.patch(options, new StoredEventHandler());
+ BuildOptions result =
+ transition.patch(
+ new BuildOptionsView(options, transition.requiresOptionFragments()),
+ new StoredEventHandler());
assertThat(result).isNotNull();
assertThat(result).isEqualTo(options);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionTest.java
index a3375b3..b7798d7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionTest.java
@@ -30,7 +30,6 @@
import com.google.devtools.build.lib.rules.cpp.CppOptions;
import com.google.devtools.build.lib.rules.java.JavaOptions;
import java.util.Map;
-import java.util.Set;
import java.util.stream.IntStream;
import org.junit.Before;
import org.junit.Test;
@@ -210,14 +209,14 @@
}
private static final class TransitionWithCustomFragments implements PatchTransition {
- private final Set<Class<? extends FragmentOptions>> fragments;
+ private final ImmutableSet<Class<? extends FragmentOptions>> fragments;
- TransitionWithCustomFragments(Set<Class<? extends FragmentOptions>> fragments) {
+ TransitionWithCustomFragments(ImmutableSet<Class<? extends FragmentOptions>> fragments) {
this.fragments = fragments;
}
@Override
- public Set<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
return fragments;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java
index 0338df6..5c900d7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
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.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
@@ -44,7 +45,10 @@
BuildOptions.of(
ImmutableList.of(CoreOptions.class, TestOptions.class), "--trim_test_configuration");
- BuildOptions result = TRIM_TRANSITION.patch(options, new StoredEventHandler());
+ BuildOptions result =
+ TRIM_TRANSITION.patch(
+ new BuildOptionsView(options, TRIM_TRANSITION.requiresOptionFragments()),
+ new StoredEventHandler());
// Verify the transitions actually applied.
assertThat(result).isNotNull();
@@ -58,7 +62,10 @@
BuildOptions.of(
ImmutableList.of(CoreOptions.class, TestOptions.class), "--notrim_test_configuration");
- BuildOptions result = TRIM_TRANSITION.patch(options, new StoredEventHandler());
+ BuildOptions result =
+ TRIM_TRANSITION.patch(
+ new BuildOptionsView(options, TRIM_TRANSITION.requiresOptionFragments()),
+ new StoredEventHandler());
// Verify the transitions actually applied.
assertThat(result).isNotNull();
@@ -77,7 +84,10 @@
.addStarlarkOption(starlarkOptionKey, starlarkOptionValue)
.build();
- BuildOptions result = TRIM_TRANSITION.patch(options, new StoredEventHandler());
+ BuildOptions result =
+ TRIM_TRANSITION.patch(
+ new BuildOptionsView(options, TRIM_TRANSITION.requiresOptionFragments()),
+ new StoredEventHandler());
// Verify the transitions actually applied.
assertThat(result).isNotNull();
@@ -106,10 +116,22 @@
"--trim_test_configuration");
EventHandler handler = new StoredEventHandler();
+
+ BuildOptions execTransitionOptions =
+ execTransition.patch(
+ new BuildOptionsView(options, execTransition.requiresOptionFragments()), handler);
BuildOptions execThenTrim =
- TRIM_TRANSITION.patch(execTransition.patch(options, handler), handler);
+ TRIM_TRANSITION.patch(
+ new BuildOptionsView(execTransitionOptions, TRIM_TRANSITION.requiresOptionFragments()),
+ handler);
+
+ BuildOptions trimTransitionOptions =
+ TRIM_TRANSITION.patch(
+ new BuildOptionsView(options, TRIM_TRANSITION.requiresOptionFragments()), handler);
BuildOptions trimThenExec =
- execTransition.patch(TRIM_TRANSITION.patch(options, handler), handler);
+ execTransition.patch(
+ new BuildOptionsView(trimTransitionOptions, execTransition.requiresOptionFragments()),
+ handler);
assertThat(execThenTrim).isEqualTo(trimThenExec);
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 8800810..68d1dc1 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
@@ -85,6 +85,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CoreOptions.ConfigsMode;
import com.google.devtools.build.lib.analysis.config.Fragment;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -1845,7 +1846,9 @@
return skyframeExecutor.getConfigurationForTesting(
reporter,
fromConfig.fragmentClasses(),
- transition.patch(fromConfig.getOptions(), eventCollector));
+ transition.patch(
+ new BuildOptionsView(fromConfig.getOptions(), transition.requiresOptionFragments()),
+ eventCollector));
} catch (OptionsParsingException | InvalidConfigurationException e) {
throw new AssertionError(e);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
index ce11174..81c9356 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
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.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CompilationMode;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
@@ -314,8 +315,10 @@
throws InterruptedException, OptionsParsingException, InvalidConfigurationException {
ImmutableList.Builder<BuildConfiguration> splitConfigs = ImmutableList.builder();
+ BuildOptionsView fragmentRestrictedOptions =
+ new BuildOptionsView(configuration.getOptions(), splitTransition.requiresOptionFragments());
for (BuildOptions splitOptions :
- splitTransition.split(configuration.getOptions(), eventCollector).values()) {
+ splitTransition.split(fragmentRestrictedOptions, eventCollector).values()) {
splitConfigs.add(getSkyframeExecutor().getConfigurationForTesting(
reporter, configuration.fragmentClasses(), splitOptions));
}
