Introduce configuration transition fragment declarations.
See https://github.com/bazelbuild/bazel/issues/11258 for context.
This essentially converts ConfigurationTransition.apply(BuildOptions...)
to ConfigurationTransition.apply(RestrictedBuildOptions...). The new
BuildOptionsView wraps BuildOptions while only allowing access
to options fragments the transitions declare.
Because there are a *lot* of uses of PatchTransition and SplitTransition,
this change only adds a parallel interface to PatchTransition.patch and
SplitTransition.split. Implementers continue to work as-is with the
BuildOptions variant, while we can incrementally switch them over to
the RestrictedBuildOptions variant at whatever pace we desire.
This helps decouple risk of failing CI if/when we accidentally forget
to migrate a few implementers or get their set of declared fragments wrong.
This implements step 1 of https://github.com/bazelbuild/bazel/issues/11258.
PiperOrigin-RevId: 312311815
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 d22f09e..0fa4dad 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1468,7 +1468,10 @@
java_library(
name = "config/build_options",
- srcs = ["config/BuildOptions.java"],
+ srcs = [
+ "config/BuildOptions.java",
+ "config/BuildOptionsView.java",
+ ],
deps = [
":config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -1625,11 +1628,13 @@
deps = [
":config/build_options",
":config/core_options",
+ ":config/fragment_options",
":config/transitions/patch_transition",
":config/transitions/transition_factory",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//third_party:auto_value",
+ "//third_party:guava",
],
)
@@ -1720,6 +1725,7 @@
srcs = ["config/transitions/ComposingTransition.java"],
deps = [
":config/build_options",
+ ":config/fragment_options",
":config/transitions/configuration_transition",
":config/transitions/no_transition",
":config/transitions/null_transition",
@@ -1745,10 +1751,15 @@
java_library(
name = "config/transitions/configuration_transition",
- srcs = ["config/transitions/ConfigurationTransition.java"],
+ srcs = [
+ "config/transitions/ConfigurationTransition.java",
+ "config/transitions/TransitionUtil.java",
+ ],
deps = [
":config/build_options",
+ ":config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/events",
+ "//third_party:guava",
],
)
@@ -2190,6 +2201,7 @@
srcs = ["test/TestTrimmingTransitionFactory.java"],
deps = [
":config/build_options",
+ ":config/fragment_options",
":config/transitions/no_transition",
":config/transitions/patch_transition",
":config/transitions/transition_factory",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsView.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsView.java
new file mode 100644
index 0000000..49551ad
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptionsView.java
@@ -0,0 +1,83 @@
+// Copyright 2020 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.analysis.config;
+
+import com.google.common.base.Preconditions;
+import java.util.Set;
+
+/**
+ * Wrapper for {@link BuildOptions} that only permits {@link BuildOptions#get} calls to {@link
+ * FragmentOptions} from a pre-declared set.
+ *
+ * <p>This lets Blaze understand what fragments transitions require, which is helpful for
+ * understanding what flags are and are not important to a given build.
+ */
+public class BuildOptionsView implements Cloneable {
+ private final BuildOptions options;
+ private final Set<Class<? extends FragmentOptions>> allowedFragments;
+
+ /** Wraps a given {@link BuildOptions} with a "permitted" set of {@link FragmentOptions}. */
+ public BuildOptionsView(
+ BuildOptions options, Set<Class<? extends FragmentOptions>> allowedFragments) {
+ this.options = options;
+ this.allowedFragments = allowedFragments;
+ }
+
+ /**
+ * Wrapper for {@link BuildOptions#get} that throws an {@link IllegalArgumentException} if the
+ * given {@link FragmentOptions} isn't in the "permitted" set.
+ */
+ public <T extends FragmentOptions> T get(Class<T> optionsClass) {
+ return options.get(checkFragment(optionsClass));
+ }
+
+ /**
+ * Wrapper for {@link BuildOptions#contains} that throws an {@link IllegalArgumentException} if
+ * the given {@link FragmentOptions} isn't in the "permitted" set.
+ */
+ public boolean contains(Class<? extends FragmentOptions> optionsClass) {
+ return options.contains(checkFragment(optionsClass));
+ }
+
+ /**
+ * Returns a new {@link BuildOptionsView} instance bound to a clone of the original's {@link
+ * BuildOptions}.
+ */
+ @Override
+ public BuildOptionsView clone() {
+ return new BuildOptionsView(options.clone(), allowedFragments);
+ }
+
+ /**
+ * Returns the underlying {@link BuildOptions}.
+ *
+ * <p>Since this sheds all extra security from {@link BuildOptionsView}, this should only be used
+ * when a transition is returning its final result.
+ *
+ * <p>!!! No transition should call any {@link BuildOptions} accessor after this! !!!
+ */
+ public BuildOptions underlying() {
+ return options;
+ }
+
+ private <T extends FragmentOptions> Class<T> checkFragment(Class<T> optionsClass) {
+ Preconditions.checkArgument(
+ allowedFragments.contains(optionsClass),
+ "Can't access %s in allowed fragments %s",
+ optionsClass,
+ allowedFragments);
+ return optionsClass;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
index c141859..8fa0701 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
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.analysis.config.transitions.TransitionUtil;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition.TransitionException;
import com.google.devtools.build.lib.cmdline.Label;
@@ -602,7 +603,8 @@
// TODO(bazel-team): Add safety-check that this never mutates fromOptions.
StoredEventHandler handlerWithErrorStatus = new StoredEventHandler();
- Map<String, BuildOptions> result = transition.apply(fromOptions, handlerWithErrorStatus);
+ Map<String, BuildOptions> result =
+ transition.apply(TransitionUtil.restrict(transition, fromOptions), handlerWithErrorStatus);
if (doesStarlarkTransition) {
// We use a temporary StoredEventHandler instead of the caller's event handler because
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 f1c159a..6bbb5eb 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
@@ -16,11 +16,15 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
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.FragmentOptions;
import com.google.devtools.build.lib.events.EventHandler;
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.
@@ -49,12 +53,23 @@
}
@Override
- public Map<String, BuildOptions> apply(BuildOptions buildOptions, EventHandler eventHandler) {
+ public Set<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ return ImmutableSet.<Class<? extends FragmentOptions>>builder()
+ .addAll(transition1.requiresOptionFragments())
+ .addAll(transition2.requiresOptionFragments())
+ .build();
+ }
+
+ @Override
+ public Map<String, BuildOptions> apply(BuildOptionsView buildOptions, EventHandler eventHandler) {
ImmutableMap.Builder<String, BuildOptions> toOptions = ImmutableMap.builder();
- for (Map.Entry<String, BuildOptions> entry1 :
- transition1.apply(buildOptions, eventHandler).entrySet()) {
- for (Map.Entry<String, BuildOptions> entry2 :
- transition2.apply(entry1.getValue(), eventHandler).entrySet()) {
+ Map<String, BuildOptions> transition1Output =
+ transition1.apply(
+ TransitionUtil.restrict(transition1, buildOptions.underlying()), eventHandler);
+ for (Map.Entry<String, BuildOptions> entry1 : transition1Output.entrySet()) {
+ Map<String, BuildOptions> transition2Output =
+ transition2.apply(TransitionUtil.restrict(transition2, entry1.getValue()), eventHandler);
+ for (Map.Entry<String, BuildOptions> entry2 : transition2Output.entrySet()) {
toOptions.put(composeKeys(entry1.getKey(), entry2.getKey()), entry2.getValue());
}
}
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 fc3b961..b9783ba 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
@@ -14,9 +14,13 @@
package com.google.devtools.build.lib.analysis.config.transitions;
+import com.google.common.collect.ImmutableSet;
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.FragmentOptions;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.Map;
+import java.util.Set;
/**
* A configuration transition.
@@ -29,13 +33,26 @@
String PATCH_TRANSITION_KEY = "";
/**
+ * Declares the {@link FragmentOptions} this transition may read.
+ *
+ * <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() {
+ return ImmutableSet.of();
+ }
+
+ /**
* Returns the map of {@code BuildOptions} after applying this transition. The returned map keys
* are only used for dealing with split transitions. Patch transitions, including internal, native
* Patch transitions, should return a single entry map with key {@code PATCH_TRANSITION_KEY}.
*
+ * <p>Blaze throws an {@link IllegalArgumentException} if this method reads any options fragment
+ * not declared in {@link #requiresOptionFragments}.
+ *
* <p>Returning an empty or null map triggers a {@link RuntimeException}.
*/
- Map<String, BuildOptions> apply(BuildOptions buildOptions, EventHandler eventHandler);
+ Map<String, BuildOptions> apply(BuildOptionsView buildOptions, EventHandler eventHandler);
/**
* We want to keep the number of transition interfaces no larger than what's necessary to maintain
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 16f95a8..b72329a 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
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis.config.transitions;
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 java.util.Collections;
import java.util.Map;
@@ -25,13 +26,12 @@
* <p>Also see {@link SplitTransition}, which maps a single input {@link BuildOptions} to possibly
* multiple {@link BuildOptions}.
*
- * <p>The concept is simple: given the input configuration's build options, the
- * transition does whatever it wants to them and returns the modified result.
+ * <p>The concept is simple: given the input configuration's build options, the transition does
+ * whatever it wants to them and returns the modified result.
*
- * <p>Implementations must be stateless: the output must exclusively depend on the
- * input build options and any immutable member fields. Implementations must also override
- * {@link Object#equals} and {@link Object#hashCode} unless exclusively accessed as
- * singletons. For example:
+ * <p>Implementations must be stateless: the output must exclusively depend on the input build
+ * options and any immutable member fields. Implementations must also override {@link Object#equals}
+ * and {@link Object#hashCode} unless exclusively accessed as singletons. For example:
*
* <pre>
* public class MyTransition implements PatchTransition {
@@ -40,7 +40,7 @@
* private MyTransition() {}
*
* {@literal @}Override
- * public BuildOptions patch(BuildOptions options) {
+ * public BuildOptions patch(RestrictedBuildOptions options) {
* BuildOptions toOptions = options.clone();
* // Change some setting on toOptions
* return toOptions;
@@ -49,26 +49,48 @@
* </pre>
*
* <p>For performance reasons, the input options are passed as a <i>reference</i>, not a
- * <i>copy</i>. Implementations should <i>always</i> treat these as immutable, and call
- * {@link com.google.devtools.build.lib.analysis.config.BuildOptions#clone}
- * before making changes. Unfortunately,
- * {@link com.google.devtools.build.lib.analysis.config.BuildOptions} doesn't currently
- * enforce immutability. So care must be taken not to modify the wrong instance.
+ * <i>copy</i>. Implementations should <i>always</i> treat these as immutable, and call {@link
+ * com.google.devtools.build.lib.analysis.config.BuildOptions#clone} before making changes.
+ * Unfortunately, {@link com.google.devtools.build.lib.analysis.config.BuildOptions} doesn't
+ * currently enforce immutability. So care must be taken not to modify the wrong instance.
*/
-@FunctionalInterface
public interface PatchTransition extends ConfigurationTransition {
/**
* Applies the transition.
*
+ * <p>This method is being deprecated (https://github.com/bazelbuild/bazel/issues/11258). Please
+ * use {@link #patch(BuildOptionsView, EventHandler)} for new uses.
+ *
* @param options the options representing the input configuration to this transition. <b>DO NOT
* MODIFY THIS VARIABLE WITHOUT CLONING IT FIRST!</b>
* @param eventHandler
* @return the options representing the desired post-transition configuration
*/
- BuildOptions patch(BuildOptions options, EventHandler eventHandler);
+ default BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
+ throw new UnsupportedOperationException(
+ "Either this or patch(RestrictedBuildOptions) must be overriden");
+ }
+
+ /**
+ * Applies the transition.
+ *
+ * <p>Blaze throws an {@link IllegalArgumentException} if this method reads any options fragment
+ * not declard 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>
+ * @param eventHandler
+ * @return the options representing the desired post-transition configuration
+ */
+ default BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
+ // Escape hatch for implementers of the BuildOptions method: provide uninhibited access. When
+ // all implementers use this variation we'll remove this default implementation.
+ return patch(options.underlying(), eventHandler);
+ }
@Override
- default Map<String, BuildOptions> apply(BuildOptions buildOptions, EventHandler eventHandler) {
+ default Map<String, BuildOptions> apply(
+ BuildOptionsView buildOptions, EventHandler eventHandler) {
return Collections.singletonMap(PATCH_TRANSITION_KEY, patch(buildOptions, eventHandler));
}
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 5e630d2..5ec519e 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
@@ -17,6 +17,7 @@
import com.google.common.base.Verify;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.Collection;
@@ -35,16 +36,38 @@
* way (e.g. for determining which CPU corresponds to which dep for a multi-arch split dependency).
*/
@ThreadSafety.Immutable
-@FunctionalInterface
public interface SplitTransition extends ConfigurationTransition {
/**
* Returns the map of {@code BuildOptions} after splitting, or the original options if this split
* is a noop. The key values are used as dict keys in ctx.split_attr, so human-readable strings
* are recommended.
*
+ * <p>This method is being deprecated (https://github.com/bazelbuild/bazel/issues/11258). Please
+ * use {@link #split(BuildOptionsView, EventHandler)} for new uses.
+ *
* <p>Returning an empty or null list triggers a {@link RuntimeException}.
*/
- Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler);
+ default Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
+ throw new UnsupportedOperationException(
+ "Either this or patch(RestrictedBuildOptions) must be overriden");
+ }
+
+ /**
+ * Returns the map of {@code BuildOptions} after splitting, or the original options if this split
+ * is a noop. The key values are used as dict keys in ctx.split_attr, so human-readable strings
+ * are recommended.
+ *
+ * <p>Blaze throws an {@link IllegalArgumentException} if this method reads any options fragment
+ * not declard in {@link ConfigurationTransition#requiresOptionFragments}.
+ *
+ * <p>Returning an empty or null list triggers a {@link RuntimeException}.
+ */
+ default Map<String, BuildOptions> split(
+ BuildOptionsView buildOptions, EventHandler eventHandler) {
+ // Escape hatch for implementers of the BuildOptions method: provide uninhibited access. When
+ // all implementers use this variation we'll remove this default implementation.
+ return split(buildOptions.underlying(), eventHandler);
+ }
/**
* Returns true iff {@code option} and {@code splitOptions} are equal.
@@ -56,7 +79,8 @@
}
@Override
- default Map<String, BuildOptions> apply(BuildOptions buildOptions, EventHandler eventHandler) {
+ default Map<String, BuildOptions> apply(
+ BuildOptionsView buildOptions, EventHandler eventHandler) {
Map<String, BuildOptions> splitOptions = split(buildOptions, eventHandler);
Verify.verifyNotNull(splitOptions, "Split transition output may not be null");
Verify.verify(!splitOptions.isEmpty(), "Split transition output may not be empty");
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionUtil.java
new file mode 100644
index 0000000..60a20ea
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionUtil.java
@@ -0,0 +1,31 @@
+// Copyright 2017 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.analysis.config.transitions;
+
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
+
+/** Utility methods for configuration transitions. */
+public class TransitionUtil {
+ private TransitionUtil() {}
+
+ /**
+ * Returns a {@link BuildOptionsView} bound to the fragment options a {@link
+ * ConfigurationTransition} reads.
+ */
+ public static BuildOptionsView restrict(
+ ConfigurationTransition transition, BuildOptions options) {
+ return new BuildOptionsView(options, transition.requiresOptionFragments());
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
index 9991a1d..fa20d77 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
+import com.google.devtools.build.lib.analysis.config.transitions.TransitionUtil;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.cmdline.Label;
@@ -146,7 +147,9 @@
// Skyframe loading to get flag default values. See ConfigurationResolver.applyTransition
// for an example of the required logic.
Collection<BuildOptions> toOptions =
- dep.getTransition().apply(fromOptions, eventHandler).values();
+ dep.getTransition()
+ .apply(TransitionUtil.restrict(dep.getTransition(), fromOptions), eventHandler)
+ .values();
String hostConfigurationChecksum = hostConfiguration.checksum();
String dependencyName;
if (attributeAndDep.getKey() == DependencyKind.TOOLCHAIN_DEPENDENCY) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
index 045500e..8e4f081 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.util.AnalysisCachingTestBase;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.rules.java.JavaInfo;
import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
import com.google.devtools.build.lib.syntax.StarlarkValue;
@@ -544,11 +545,14 @@
public static final OptionDefinition ALSO_IRRELEVANT_OPTION =
OptionsParser.getOptionDefinitionByName(DiffResetOptions.class, "also_irrelevant");
public static final PatchTransition CLEAR_IRRELEVANT =
- (options, eventHandler) -> {
- BuildOptions cloned = options.clone();
- cloned.get(DiffResetOptions.class).probablyIrrelevantOption = "(cleared)";
- cloned.get(DiffResetOptions.class).alsoIrrelevantOption = "(cleared)";
- return cloned;
+ new PatchTransition() {
+ @Override
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
+ BuildOptions cloned = options.clone();
+ cloned.get(DiffResetOptions.class).probablyIrrelevantOption = "(cleared)";
+ cloned.get(DiffResetOptions.class).alsoIrrelevantOption = "(cleared)";
+ return cloned;
+ }
};
@Option(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
index 77fdbda..b49af3d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
+import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -266,15 +267,19 @@
new TransitionFactory<AttributeTransitionData>() {
@Override
public SplitTransition create(AttributeTransitionData data) {
- return (BuildOptions options, EventHandler eventHandler) -> {
- String define = data.attributes().get("define", STRING);
- BuildOptions newOptions = options.clone();
- CoreOptions optionsFragment = newOptions.get(CoreOptions.class);
- optionsFragment.commandLineBuildVariables =
- optionsFragment.commandLineBuildVariables.stream()
- .filter((pair) -> !pair.getKey().equals(define))
- .collect(toImmutableList());
- return ImmutableMap.of("define_cleaner", newOptions);
+ return new SplitTransition() {
+ @Override
+ public Map<String, BuildOptions> split(
+ BuildOptions options, EventHandler eventHandler) {
+ String define = data.attributes().get("define", STRING);
+ BuildOptions newOptions = options.clone();
+ CoreOptions optionsFragment = newOptions.get(CoreOptions.class);
+ optionsFragment.commandLineBuildVariables =
+ optionsFragment.commandLineBuildVariables.stream()
+ .filter((pair) -> !pair.getKey().equals(define))
+ .collect(toImmutableList());
+ return ImmutableMap.of("define_cleaner", newOptions);
+ }
};
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
index e2f3cb6..150bdae 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
@@ -26,6 +26,7 @@
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.Suite;
@@ -48,10 +49,13 @@
@RunWith(JUnit4.class)
public class ConfigurationsForLateBoundTargetsTest extends AnalysisTestCase {
private static final PatchTransition CHANGE_FOO_FLAG_TRANSITION =
- (options, eventHandler) -> {
- BuildOptions toOptions = options.clone();
- toOptions.get(LateBoundSplitUtil.TestOptions.class).fooFlag = "PATCHED!";
- return toOptions;
+ new PatchTransition() {
+ @Override
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
+ BuildOptions toOptions = options.clone();
+ toOptions.get(LateBoundSplitUtil.TestOptions.class).fooFlag = "PATCHED!";
+ return toOptions;
+ }
};
/** Rule definition with a latebound dependency. */
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsViewTest.java
new file mode 100644
index 0000000..6f9a723
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsViewTest.java
@@ -0,0 +1,86 @@
+// Copyright 2020 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.analysis.config;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.rules.cpp.CppOptions;
+import com.google.devtools.common.options.OptionsParser;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link BuildOptionsView}. */
+@RunWith(JUnit4.class)
+public final class BuildOptionsViewTest {
+
+ private static final ImmutableList<Class<? extends FragmentOptions>> BUILD_CONFIG_OPTIONS =
+ ImmutableList.of(CoreOptions.class, CppOptions.class);
+
+ private BuildOptions options;
+
+ @Before
+ public void constructBuildOptions() {
+ options =
+ BuildOptions.of(
+ BUILD_CONFIG_OPTIONS,
+ OptionsParser.builder().optionsClasses(BUILD_CONFIG_OPTIONS).build());
+ }
+
+ @Test
+ public void allowedGet() throws Exception {
+ BuildOptionsView restrictedOptions =
+ new BuildOptionsView(options, ImmutableSet.of(CoreOptions.class));
+ assertThat(restrictedOptions.get(CoreOptions.class))
+ .isSameInstanceAs(options.get(CoreOptions.class));
+ ;
+ }
+
+ @Test
+ public void prohibitedGet() throws Exception {
+ BuildOptionsView restrictedOptions =
+ new BuildOptionsView(options, ImmutableSet.of(CoreOptions.class));
+ assertThrows(IllegalArgumentException.class, () -> restrictedOptions.get(CppOptions.class));
+ }
+
+ @Test
+ public void allowedContains() throws Exception {
+ BuildOptionsView restrictedOptions =
+ new BuildOptionsView(options, ImmutableSet.of(CoreOptions.class));
+ assertThat(restrictedOptions.contains(CoreOptions.class)).isTrue();
+ }
+
+ @Test
+ public void prohibitedContains() throws Exception {
+ BuildOptionsView restrictedOptions =
+ new BuildOptionsView(options, ImmutableSet.of(CoreOptions.class));
+ assertThrows(
+ IllegalArgumentException.class, () -> restrictedOptions.contains(CppOptions.class));
+ }
+
+ @Test
+ public void cloneTest() throws Exception {
+ BuildOptionsView restrictedOptions =
+ new BuildOptionsView(options, ImmutableSet.of(CoreOptions.class));
+ BuildOptionsView clone = restrictedOptions.clone();
+ assertThat(clone).isNotSameInstanceAs(restrictedOptions);
+ assertThat(restrictedOptions.underlying()).isSameInstanceAs(options);
+ assertThat(clone.underlying()).isNotSameInstanceAs(options);
+ assertThat(clone.underlying()).isEqualTo(options);
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/TransitionFactoriesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/TransitionFactoriesTest.java
index aee0d37..67dcf08 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/TransitionFactoriesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/TransitionFactoriesTest.java
@@ -20,6 +20,8 @@
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
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;
+import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -59,8 +61,13 @@
public void splitTransition() {
TransitionFactory<Object> factory =
TransitionFactories.of(
- (SplitTransition)
- (buildOptions, eventHandler) -> ImmutableMap.of("test0", buildOptions.clone()));
+ new SplitTransition() {
+ @Override
+ public Map<String, BuildOptions> split(
+ BuildOptions buildOptions, EventHandler eventHandler) {
+ return ImmutableMap.of("test0", buildOptions.clone());
+ }
+ });
assertThat(factory).isNotNull();
assertThat(factory.isHost()).isFalse();
assertThat(factory.isSplit()).isTrue();
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactoryTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactoryTest.java
index 85855b5..fb31d78 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactoryTest.java
@@ -58,7 +58,10 @@
assertThat(composed.isSplit()).isFalse();
ConfigurationTransition transition = composed.create(new StubData());
Collection<BuildOptions> results =
- transition.apply(BuildOptions.builder().build(), eventHandler).values();
+ transition
+ .apply(
+ TransitionUtil.restrict(transition, BuildOptions.builder().build()), eventHandler)
+ .values();
assertThat(results).isNotNull();
assertThat(results).hasSize(1);
BuildOptions result = Iterables.getOnlyElement(results);
@@ -78,7 +81,8 @@
assertThat(composed.isSplit()).isTrue();
ConfigurationTransition transition = composed.create(new StubData());
Map<String, BuildOptions> results =
- transition.apply(BuildOptions.builder().build(), eventHandler);
+ transition.apply(
+ TransitionUtil.restrict(transition, BuildOptions.builder().build()), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -105,7 +109,8 @@
assertThat(composed.isSplit()).isTrue();
ConfigurationTransition transition = composed.create(new StubData());
Map<String, BuildOptions> results =
- transition.apply(BuildOptions.builder().build(), eventHandler);
+ transition.apply(
+ TransitionUtil.restrict(transition, BuildOptions.builder().build()), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
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 acc758e..a3375b3 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
@@ -18,13 +18,19 @@
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
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.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
+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;
@@ -52,7 +58,8 @@
assertThat(composed).isNotNull();
Map<String, BuildOptions> results =
- composed.apply(BuildOptions.builder().build(), eventHandler);
+ composed.apply(
+ TransitionUtil.restrict(composed, BuildOptions.builder().build()), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(1);
BuildOptions result = Iterables.getOnlyElement(results.values());
@@ -69,7 +76,8 @@
assertThat(composed).isNotNull();
Map<String, BuildOptions> results =
- composed.apply(BuildOptions.builder().build(), eventHandler);
+ composed.apply(
+ TransitionUtil.restrict(composed, BuildOptions.builder().build()), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -93,7 +101,8 @@
assertThat(composed).isNotNull();
Map<String, BuildOptions> results =
- composed.apply(BuildOptions.builder().build(), eventHandler);
+ composed.apply(
+ TransitionUtil.restrict(composed, BuildOptions.builder().build()), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -119,7 +128,9 @@
assertThat(composed).isNotNull();
assertThrows(
IllegalStateException.class,
- () -> composed.apply(BuildOptions.builder().build(), eventHandler));
+ () ->
+ composed.apply(
+ TransitionUtil.restrict(composed, BuildOptions.builder().build()), eventHandler));
}
@Test
@@ -197,4 +208,32 @@
i -> updateOptions(options, flagLabel, flagValues.get(i))));
}
}
+
+ private static final class TransitionWithCustomFragments implements PatchTransition {
+ private final Set<Class<? extends FragmentOptions>> fragments;
+
+ TransitionWithCustomFragments(Set<Class<? extends FragmentOptions>> fragments) {
+ this.fragments = fragments;
+ }
+
+ @Override
+ public Set<Class<? extends FragmentOptions>> requiresOptionFragments() {
+ return fragments;
+ }
+
+ @Override
+ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
+ return options.underlying();
+ }
+ }
+
+ @Test
+ public void composed_required_fragments() throws Exception {
+ ConfigurationTransition composed =
+ ComposingTransition.of(
+ new TransitionWithCustomFragments(ImmutableSet.of(CppOptions.class)),
+ new TransitionWithCustomFragments(ImmutableSet.of(JavaOptions.class)));
+ assertThat(composed.requiresOptionFragments())
+ .containsExactly(CppOptions.class, JavaOptions.class);
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java b/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java
index 0e13c65..b4454e3 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
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.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.events.EventHandler;
@@ -79,8 +80,8 @@
new ConfigurationTransition() {
@Override
public ImmutableMap<String, BuildOptions> apply(
- BuildOptions buildOptions, EventHandler eventHandler) {
- return ImmutableMap.of("", buildOptions);
+ BuildOptionsView buildOptions, EventHandler eventHandler) {
+ return ImmutableMap.of("", buildOptions.underlying());
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java
index ae02981..cf2227b 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java
@@ -567,17 +567,20 @@
* prefix + "2"}.
*/
private static SplitTransition newSplitTransition(final String prefix) {
- return (buildOptions, eventHandler) -> {
- ImmutableMap.Builder<String, BuildOptions> result = ImmutableMap.builder();
- for (int index = 1; index <= 2; index++) {
- BuildOptions toOptions = buildOptions.clone();
- TestConfiguration.TestOptions baseOptions =
- toOptions.get(TestConfiguration.TestOptions.class);
- baseOptions.testFilter =
- (baseOptions.testFilter == null ? "" : baseOptions.testFilter) + prefix + index;
- result.put(prefix + index, toOptions);
+ return new SplitTransition() {
+ @Override
+ public Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
+ ImmutableMap.Builder<String, BuildOptions> result = ImmutableMap.builder();
+ for (int index = 1; index <= 2; index++) {
+ BuildOptions toOptions = buildOptions.clone();
+ TestConfiguration.TestOptions baseOptions =
+ toOptions.get(TestConfiguration.TestOptions.class);
+ baseOptions.testFilter =
+ (baseOptions.testFilter == null ? "" : baseOptions.testFilter) + prefix + index;
+ result.put(prefix + index, toOptions);
+ }
+ return result.build();
}
- return result.build();
};
}
