Make StarlarkDefinedConfigTransition immutable.
Generally speaking, it's not safe for config transitions to keep state because
a transition instance may be shared across multiple rule instances. Rules should
not be able to affect each other through shared transitions.
This particular case removes an event handler that stores errors (like trying
to load invalid build settings) which the configuration machinery replays later
to communicate to the user. The risk trajectory is that a transition that fails
on a badly formed rule might repeat the same error for a well-formed rule
becuase of the shared state.
The most likely user-visible output of this problem is non-deterministic build
errors on rules using Starlark build settings and transitions. i.e. repeat the
same build twice and get different results.
This is a huge change (apologies), but conceptually straightforward. We simply
add an EventHandler parameter to ConfigurationTransition.apply to provide an
alternative place to record errors. This lifts StarlarkDefinedConfigTransition
of the burden to define its own.
Most of the hugeness in this change is that the ConfigurationTransition
interface is used in many places, so we have to pass / define this parameter
everywhere. A less invavise alternative could be to use Java interface defaults,
but I think the current approach is a better API (and the pain is limited just
to this change: there's no real extra API burden as a result).
Testing note: I'm impressed that this change only broke one test:
StarlarkRuleTransitionProviderTest#testAliasedBuildSetting, and for
straightforward reasons. See new comments in ConfigurationResolver for details.
PiperOrigin-RevId: 304043250
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 2d6c00c..41e5d3c 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -400,6 +400,7 @@
"analysis/config/FragmentOptions.java",
],
deps = [
+ ":events",
":util",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 65f99ac..141c310 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -908,7 +908,8 @@
.executionPlatform(getToolchainContext().executionPlatform().label())
.build());
BuildOptions fromOptions = getConfiguration().getOptions();
- Map<String, BuildOptions> splitOptions = transition.split(fromOptions);
+ Map<String, BuildOptions> splitOptions =
+ transition.split(fromOptions, getAnalysisEnvironment().getEventHandler());
List<ConfiguredTargetAndData> deps = getConfiguredTargetAndTargetDeps(attributeName);
if (SplitTransition.equals(fromOptions, splitOptions.values())) {
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 b1303d9..fcbd92e 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
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
@@ -548,10 +549,21 @@
}
// TODO(bazel-team): Add safety-check that this never mutates fromOptions.
- Map<String, BuildOptions> result = transition.apply(fromOptions);
+ StoredEventHandler handlerWithErrorStatus = new StoredEventHandler();
+ Map<String, BuildOptions> result = transition.apply(fromOptions, handlerWithErrorStatus);
if (doesStarlarkTransition) {
- StarlarkTransition.replayEvents(eventHandler, transition);
+ // We use a temporary StoredEventHandler instead of the caller's event handler because
+ // StarlarkTransition.validate assumes no errors occurred. We need a StoredEventHandler to be
+ // able to check that, and fail out early if there are errors.
+ //
+ // TODO(bazel-team): harden StarlarkTransition.validate so we can eliminate this step.
+ // StarlarkRuleTransitionProviderTest#testAliasedBuildSetting_outputReturnMismatch shows the
+ // effect.
+ handlerWithErrorStatus.replayOn(eventHandler);
+ if (handlerWithErrorStatus.hasErrors()) {
+ throw new TransitionException("Errors encountered while applying Starlark transition");
+ }
result = StarlarkTransition.validate(transition, buildSettingPackages, result);
}
return result;
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 37e671b..643f6fb 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
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import javax.annotation.Nullable;
@@ -88,7 +89,7 @@
private static final BuildOptionsCache<Label> cache = new BuildOptionsCache<>();
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
if (executionPlatform == null) {
// No execution platform is known, so don't change anything.
return options;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java
index 9749a4e..4eca3f3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java
@@ -16,6 +16,7 @@
import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
/** Dynamic transition to the host configuration. */
@@ -31,7 +32,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
if (options.get(CoreOptions.class).isHost) {
// If the input already comes from the host configuration, just return the existing values.
//
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
index fa64ced..48db17d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
@@ -18,8 +18,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigurationTransitionApi;
@@ -46,14 +46,12 @@
private final List<String> inputs;
private final List<String> outputs;
private final Location location;
- private final StoredEventHandler eventHandler;
private StarlarkDefinedConfigTransition(
List<String> inputs, List<String> outputs, Location location) {
this.inputs = inputs;
this.outputs = outputs;
this.location = location;
- this.eventHandler = new StoredEventHandler();
}
/**
@@ -86,10 +84,6 @@
return location;
}
- public StoredEventHandler getEventHandler() {
- return eventHandler;
- }
-
/**
* Given a map of a subset of the "previous" build settings, returns the changed build settings as
* a result of applying this transition.
@@ -104,7 +98,7 @@
* @throws InterruptedException if evaluating the transition is interrupted
*/
public abstract ImmutableMap<String, Map<String, Object>> evaluate(
- Map<String, Object> previousSettings, StructImpl attributeMap)
+ Map<String, Object> previousSettings, StructImpl attributeMap, EventHandler eventHandler)
throws EvalException, InterruptedException;
public static StarlarkDefinedConfigTransition newRegularTransition(
@@ -137,7 +131,9 @@
@Override
public ImmutableMap<String, Map<String, Object>> evaluate(
- Map<String, Object> previousSettings, StructImpl attributeMapper) {
+ Map<String, Object> previousSettings,
+ StructImpl attributeMapper,
+ EventHandler eventHandler) {
return ImmutableMap.of(PATCH_TRANSITION_KEY, changedSettings);
}
@@ -209,11 +205,12 @@
@Override
@SuppressWarnings("rawtypes")
public ImmutableMap<String, Map<String, Object>> evaluate(
- Map<String, Object> previousSettings, StructImpl attributeMapper)
+ Map<String, Object> previousSettings, StructImpl attributeMapper, EventHandler eventHandler)
throws EvalException, InterruptedException {
Object result;
try {
- result = evalFunction(impl, ImmutableList.of(previousSettings, attributeMapper));
+ result =
+ evalFunction(impl, ImmutableList.of(previousSettings, attributeMapper), eventHandler);
} catch (EvalException e) {
throw new EvalException(impl.getLocation(), e.getMessage());
}
@@ -282,14 +279,15 @@
}
/** Evaluate the input function with the given argument, and return the return value. */
- private Object evalFunction(BaseFunction function, ImmutableList<Object> args)
+ private Object evalFunction(
+ BaseFunction function, ImmutableList<Object> args, EventHandler eventHandler)
throws InterruptedException, EvalException {
try (Mutability mutability = Mutability.create("eval_transition_function")) {
StarlarkThread thread =
StarlarkThread.builder(mutability)
.setSemantics(semantics)
.build();
- thread.setPrintHandler(StarlarkThread.makeDebugPrintHandler(getEventHandler()));
+ thread.setPrintHandler(StarlarkThread.makeDebugPrintHandler(eventHandler));
starlarkContext.storeInThread(thread);
return Starlark.call(thread, function, args, /*kwargs=*/ ImmutableMap.of());
}
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 0abfd48..f1c159a 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
@@ -17,6 +17,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+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;
@@ -48,11 +49,12 @@
}
@Override
- public Map<String, BuildOptions> apply(BuildOptions buildOptions) {
+ public Map<String, BuildOptions> apply(BuildOptions buildOptions, EventHandler eventHandler) {
ImmutableMap.Builder<String, BuildOptions> toOptions = ImmutableMap.builder();
- for (Map.Entry<String, BuildOptions> entry1 : transition1.apply(buildOptions).entrySet()) {
+ for (Map.Entry<String, BuildOptions> entry1 :
+ transition1.apply(buildOptions, eventHandler).entrySet()) {
for (Map.Entry<String, BuildOptions> entry2 :
- transition2.apply(entry1.getValue()).entrySet()) {
+ transition2.apply(entry1.getValue(), eventHandler).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 cead682..fc3b961 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
@@ -15,6 +15,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.events.EventHandler;
import java.util.Map;
/**
@@ -34,7 +35,7 @@
*
* <p>Returning an empty or null map triggers a {@link RuntimeException}.
*/
- Map<String, BuildOptions> apply(BuildOptions buildOptions);
+ Map<String, BuildOptions> apply(BuildOptions 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/NoTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java
index 6bc2b0f..b8fcaa3 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.events.EventHandler;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
/** No-op configuration transition. */
@@ -25,7 +26,7 @@
private NoTransition() {}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
return options;
}
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 a8ba1f4..b287107 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.events.EventHandler;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
/** A {@link PatchTransition} to a null configuration. */
@@ -26,7 +27,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions 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 bab9fd2..16f95a8 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.events.EventHandler;
import java.util.Collections;
import java.util.Map;
@@ -61,13 +62,14 @@
*
* @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);
+ BuildOptions patch(BuildOptions options, EventHandler eventHandler);
@Override
- default Map<String, BuildOptions> apply(BuildOptions buildOptions) {
- return Collections.singletonMap(PATCH_TRANSITION_KEY, patch(buildOptions));
+ default Map<String, BuildOptions> apply(BuildOptions buildOptions, EventHandler eventHandler) {
+ return Collections.singletonMap(PATCH_TRANSITION_KEY, patch(buildOptions, eventHandler));
}
@Override
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 2fec5f6..5e630d2 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
@@ -18,6 +18,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import com.google.devtools.build.lib.events.EventHandler;
import java.util.Collection;
import java.util.Map;
@@ -26,11 +27,11 @@
* output {@link BuildOptions}. This provides the ability to transition to multiple configurations
* simultaneously.
*
- * <p>Also see {@link PatchTransition}, which maps a single input {@BuildOptions} to a single
- * output. If your transition never needs to produce multiple outputs, you should use a
- * {@link PatchTransition}.
+ * <p>Also see {@link PatchTransition}, which maps a single input {@link BuildOptions} to a single
+ * output. If your transition never needs to produce multiple outputs, you should use a {@link
+ * PatchTransition}.
*
- * Corresponding rule implementations may require special support to handle this in an organized
+ * <p>Corresponding rule implementations may require special support to handle this in an organized
* way (e.g. for determining which CPU corresponds to which dep for a multi-arch split dependency).
*/
@ThreadSafety.Immutable
@@ -43,10 +44,10 @@
*
* <p>Returning an empty or null list triggers a {@link RuntimeException}.
*/
- Map<String, BuildOptions> split(BuildOptions buildOptions);
+ Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler);
/**
- * Returns true iff {@code option} and {@splitOptions} are equal.
+ * Returns true iff {@code option} and {@code splitOptions} are equal.
*
* <p>This can be used to determine if a split is a noop.
*/
@@ -55,8 +56,8 @@
}
@Override
- default Map<String, BuildOptions> apply(BuildOptions buildOptions) {
- Map<String, BuildOptions> splitOptions = split(buildOptions);
+ default Map<String, BuildOptions> apply(BuildOptions 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");
return splitOptions;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java
index 58297d6..904ba67 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.syntax.Dict;
@@ -66,7 +67,8 @@
static Map<String, BuildOptions> applyAndValidate(
BuildOptions buildOptions,
StarlarkDefinedConfigTransition starlarkTransition,
- StructImpl attrObject)
+ StructImpl attrObject,
+ EventHandler eventHandler)
throws EvalException, InterruptedException {
// TODO(waltl): consider building this once and use it across different split
// transitions.
@@ -76,7 +78,7 @@
ImmutableMap.Builder<String, BuildOptions> splitBuildOptions = ImmutableMap.builder();
ImmutableMap<String, Map<String, Object>> transitions =
- starlarkTransition.evaluate(settings, attrObject);
+ starlarkTransition.evaluate(settings, attrObject, eventHandler);
validateFunctionOutputsMatchesDeclaredOutputs(transitions.values(), starlarkTransition);
for (Map.Entry<String, Map<String, Object>> entry : transitions.entrySet()) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkAttributeTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkAttributeTransitionProvider.java
index 64abac5..f4254bd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkAttributeTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkAttributeTransitionProvider.java
@@ -25,6 +25,7 @@
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.Event;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
@@ -102,25 +103,22 @@
* error was encountered during transition application/validation.
*/
@Override
- public final Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public final Map<String, BuildOptions> split(
+ BuildOptions buildOptions, EventHandler eventHandler) {
try {
- return applyAndValidate(buildOptions, starlarkDefinedConfigTransition, attrObject);
+ return applyAndValidate(
+ buildOptions, starlarkDefinedConfigTransition, attrObject, eventHandler);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
- starlarkDefinedConfigTransition
- .getEventHandler()
- .handle(
- Event.error(
- starlarkDefinedConfigTransition.getLocationForErrorReporting(),
- "Starlark transition interrupted during attribute transition implementation"));
+ eventHandler.handle(
+ Event.error(
+ starlarkDefinedConfigTransition.getLocationForErrorReporting(),
+ "Starlark transition interrupted during attribute transition implementation"));
return ImmutableMap.of("error", buildOptions.clone());
} catch (EvalException e) {
- starlarkDefinedConfigTransition
- .getEventHandler()
- .handle(
- Event.error(
- starlarkDefinedConfigTransition.getLocationForErrorReporting(),
- e.getMessage()));
+ eventHandler.handle(
+ Event.error(
+ starlarkDefinedConfigTransition.getLocationForErrorReporting(), e.getMessage()));
return ImmutableMap.of("error", buildOptions.clone());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkRuleTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkRuleTransitionProvider.java
index 123f3ec..921cc18 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkRuleTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkRuleTransitionProvider.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
@@ -105,35 +106,30 @@
// TODO(b/121134880): validate that the targets these transitions are applied on don't read any
// attributes that are then configured by the outputs of these transitions.
@Override
- public BuildOptions patch(BuildOptions buildOptions) {
+ public BuildOptions patch(BuildOptions buildOptions, EventHandler eventHandler) {
Map<String, BuildOptions> result;
try {
- result = applyAndValidate(buildOptions, starlarkDefinedConfigTransition, attrObject);
+ result =
+ applyAndValidate(
+ buildOptions, starlarkDefinedConfigTransition, attrObject, eventHandler);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
- starlarkDefinedConfigTransition
- .getEventHandler()
- .handle(
- Event.error(
- starlarkDefinedConfigTransition.getLocationForErrorReporting(),
- "Starlark transition interrupted during rule transition implementation"));
+ eventHandler.handle(
+ Event.error(
+ starlarkDefinedConfigTransition.getLocationForErrorReporting(),
+ "Starlark transition interrupted during rule transition implementation"));
return buildOptions.clone();
} catch (EvalException e) {
- starlarkDefinedConfigTransition
- .getEventHandler()
- .handle(
- Event.error(
- starlarkDefinedConfigTransition.getLocationForErrorReporting(),
- e.getMessage()));
+ eventHandler.handle(
+ Event.error(
+ starlarkDefinedConfigTransition.getLocationForErrorReporting(), e.getMessage()));
return buildOptions.clone();
}
if (result.size() != 1) {
- starlarkDefinedConfigTransition
- .getEventHandler()
- .handle(
- Event.error(
- starlarkDefinedConfigTransition.getLocationForErrorReporting(),
- "Rule transition only allowed to return a single transitioned configuration."));
+ eventHandler.handle(
+ Event.error(
+ starlarkDefinedConfigTransition.getLocationForErrorReporting(),
+ "Rule transition only allowed to return a single transitioned configuration."));
return buildOptions.clone();
}
return Iterables.getOnlyElement(result.values());
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkTransition.java
index d8743c7..1add6f1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkTransition.java
@@ -26,7 +26,6 @@
import com.google.devtools.build.lib.analysis.config.transitions.ComposingTransition;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
@@ -68,15 +67,6 @@
this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
}
- public void replayOn(ExtendedEventHandler eventHandler) {
- starlarkDefinedConfigTransition.getEventHandler().replayOn(eventHandler);
- starlarkDefinedConfigTransition.getEventHandler().clear();
- }
-
- public boolean hasErrors() {
- return starlarkDefinedConfigTransition.getEventHandler().hasErrors();
- }
-
private List<String> getInputs() {
return starlarkDefinedConfigTransition.getInputs();
}
@@ -512,25 +502,6 @@
.collect(Collectors.toSet()));
}
- /**
- * For a given transition, for any Starlark-defined transitions that compose it, replay events. If
- * any events were errors, throw an error.
- */
- public static void replayEvents(ExtendedEventHandler eventHandler, ConfigurationTransition root)
- throws TransitionException {
- root.visit(
- (StarlarkTransitionVisitor)
- transition -> {
- // Replay events and errors and throw if there were errors
- boolean hasErrors = transition.hasErrors();
- transition.replayOn(eventHandler);
- if (hasErrors) {
- throw new TransitionException(
- "Errors encountered while applying Starlark transition");
- }
- });
- }
-
@Override
public boolean equals(Object object) {
if (object == this) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
index 7dc38b6..772f2f4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.common.options.Options;
@@ -30,7 +31,7 @@
*/
public final class TestTrimmingTransitionFactory implements TransitionFactory<Rule> {
- private static final Set<String> TEST_OPTIONS =
+ private static final ImmutableSet<String> TEST_OPTIONS =
ImmutableSet.copyOf(Options.getDefaults(TestOptions.class).asMap().keySet());
/**
@@ -40,7 +41,7 @@
INSTANCE;
@Override
- public BuildOptions patch(BuildOptions originalOptions) {
+ public BuildOptions patch(BuildOptions originalOptions, EventHandler eventHandler) {
if (!originalOptions.contains(TestOptions.class)) {
// nothing to do, already trimmed this fragment
return originalOptions;
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 4c3b76d..12e244c 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
@@ -145,7 +145,11 @@
}
Dependency dep = attributeAndDep.getValue();
BuildOptions fromOptions = config.getOptions();
- Collection<BuildOptions> toOptions = dep.getTransition().apply(fromOptions).values();
+ // TODO(bazel-team): support transitions on Starlark-defined build flags. These require
+ // 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();
String hostConfigurationChecksum = hostConfiguration.checksum();
String dependencyName;
if (attributeAndDep.getKey() == DependencyResolver.TOOLCHAIN_DEPENDENCY) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index 895a3cb..62d31b8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -67,7 +67,6 @@
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.util.List;
-import java.util.Map;
/** Rule definitions for Android rules. */
public final class AndroidRuleClasses {
@@ -244,7 +243,8 @@
}
@Override
- public Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public ImmutableMap<String, BuildOptions> split(
+ BuildOptions buildOptions, EventHandler eventHandler) {
AndroidConfiguration.Options androidOptions =
buildOptions.get(AndroidConfiguration.Options.class);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
index ae446f7..d076307 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
@@ -19,6 +19,7 @@
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib:build-configuration-option-details",
"//src/main/java/com/google/devtools/build/lib:core-rules",
+ "//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/actions",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java
index cede350..18b3546 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -48,7 +49,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
if (!(options.contains(ConfigFeatureFlagOptions.class)
&& options.get(ConfigFeatureFlagOptions.class)
.enforceTransitiveConfigsForConfigFeatureFlag
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
index b482729..aa4f4fc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -55,7 +56,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
if (!options.contains(ConfigFeatureFlagOptions.class)) {
return options;
}
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 399412d..8084e56 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
@@ -21,6 +21,7 @@
import com.google.devtools.build.lib.analysis.config.CoreOptions;
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;
import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
@@ -39,7 +40,7 @@
public static final PatchTransition APPLE_CROSSTOOL_TRANSITION = new AppleCrosstoolTransition();
@Override
- public BuildOptions patch(BuildOptions buildOptions) {
+ public BuildOptions patch(BuildOptions buildOptions, EventHandler eventHandler) {
BuildOptions result = buildOptions.clone();
AppleCommandLineOptions appleOptions = buildOptions.get(AppleCommandLineOptions.class);
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 b566386..8f6af96 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
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.analysis.config.CoreOptions;
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 com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions;
@@ -197,7 +198,8 @@
}
@Override
- public final Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public final Map<String, BuildOptions> split(
+ BuildOptions buildOptions, EventHandler eventHandler) {
List<String> cpus;
DottedVersion actualMinimumOsVersion;
ConfigurationDistinguisher configurationDistinguisher;
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 50678b06..f6b094b 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
@@ -24,6 +24,8 @@
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
@@ -130,8 +132,20 @@
if (!buildRequestOptions.experimentalCreatePySymlinks) {
return ImmutableSet.of();
}
+ EventHandler e =
+ new EventHandler() {
+ @Override
+ public void handle(Event event) {
+ throw new UnsupportedOperationException(
+ "This transition shouldn't do anything that could fail.\n"
+ + "TODO(bazel-team): refactor this to not call patch(). Blaze code should"
+ + " not apply transitions unless it absolutely has to, since that requires"
+ + " sequencing (likesupporting Starlark flags and handling exceptions)"
+ + " that's easy to get wrong.");
+ }
+ };
return targetConfigs.stream()
- .map(config -> configGetter.apply(transition.patch(config.getOptions())))
+ .map(config -> configGetter.apply(transition.patch(config.getOptions(), 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 e0d89b8..d505c10 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
@@ -18,6 +18,7 @@
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.transitions.PatchTransition;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.errorprone.annotations.Immutable;
import java.util.Objects;
@@ -68,7 +69,7 @@
private static final BuildOptionsCache<PythonVersion> cache = new BuildOptionsCache<>();
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
PythonVersion newVersion = determineNewVersion(options);
Preconditions.checkArgument(newVersion.isTargetValue());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 446d2a2..4ddb6d5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -466,7 +466,7 @@
BuildOptions toolchainOptions =
((ConfiguredRuleClassProvider) ruleClassProvider)
.getToolchainTaggedTrimmingTransition()
- .patch(configuration.getOptions());
+ .patch(configuration.getOptions(), env.getListener());
BuildConfigurationValue.Key toolchainConfig =
BuildConfigurationValue.keyWithoutPlatformMapping(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java
index 6b8c885..49fcef7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java
@@ -80,7 +80,7 @@
BuildOptions targetOptions = defaultBuildOptions.applyDiff(options.getOptionsDiff());
BuildOptions hostOptions =
targetOptions.get(CoreOptions.class).useDistinctHostConfiguration
- ? HostTransition.INSTANCE.patch(targetOptions)
+ ? HostTransition.INSTANCE.patch(targetOptions, env.getListener())
: targetOptions;
ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> allFragments =
@@ -345,7 +345,6 @@
ConfigurationResolver.applyTransition(
fromOptions, transition, buildSettingPackages, env.getListener())
.values();
- StarlarkTransition.replayEvents(env.getListener(), transition);
for (BuildOptions toOption : toOptions) {
configSkyKeys.add(
BuildConfigurationValue.keyWithPlatformMapping(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 66e4dd9..16c0c3a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1512,7 +1512,7 @@
BuildOptions targetOptions = firstTargetConfig.getOptions();
BuildOptions hostOptions =
targetOptions.get(CoreOptions.class).useDistinctHostConfiguration
- ? HostTransition.INSTANCE.patch(targetOptions)
+ ? HostTransition.INSTANCE.patch(targetOptions, eventHandler)
: targetOptions;
BuildConfiguration hostConfig = getConfiguration(eventHandler, hostOptions, keepGoing);
@@ -2045,7 +2045,6 @@
ConfigurationResolver.applyTransition(
fromOptions, transition, buildSettingPackages, eventHandler)
.values();
- StarlarkTransition.replayEvents(eventHandler, transition);
} catch (TransitionException e) {
eventHandler.handle(Event.error(e.getMessage()));
builder.setHasError();
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 36143c7..094de9a 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
@@ -546,7 +546,7 @@
public static final OptionDefinition ALSO_IRRELEVANT_OPTION =
OptionsParser.getOptionDefinitionByName(DiffResetOptions.class, "also_irrelevant");
public static final PatchTransition CLEAR_IRRELEVANT =
- (options) -> {
+ (options, eventHandler) -> {
BuildOptions cloned = options.clone();
cloned.get(DiffResetOptions.class).probablyIrrelevantOption = "(cleared)";
cloned.get(DiffResetOptions.class).alsoIrrelevantOption = "(cleared)";
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 e82ada5..bb9fed6 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
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
@@ -265,7 +266,7 @@
new TransitionFactory<AttributeTransitionData>() {
@Override
public SplitTransition create(AttributeTransitionData data) {
- return (BuildOptions options) -> {
+ return (BuildOptions options, EventHandler eventHandler) -> {
String define = data.attributes().get("define", STRING);
BuildOptions newOptions = options.clone();
CoreOptions optionsFragment = newOptions.get(CoreOptions.class);
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 4ffaaa1..e2f3cb6 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
@@ -47,11 +47,12 @@
@TestSpec(size = Suite.SMALL_TESTS)
@RunWith(JUnit4.class)
public class ConfigurationsForLateBoundTargetsTest extends AnalysisTestCase {
- private static final PatchTransition CHANGE_FOO_FLAG_TRANSITION = options -> {
- BuildOptions toOptions = options.clone();
- toOptions.get(LateBoundSplitUtil.TestOptions.class).fooFlag = "PATCHED!";
- return toOptions;
- };
+ private static final PatchTransition CHANGE_FOO_FLAG_TRANSITION =
+ (options, eventHandler) -> {
+ BuildOptions toOptions = options.clone();
+ toOptions.get(LateBoundSplitUtil.TestOptions.class).fooFlag = "PATCHED!";
+ return toOptions;
+ };
/** Rule definition with a latebound dependency. */
private static final RuleDefinition LATE_BOUND_DEP_RULE =
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
index 95e7f02..de5a706 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
@@ -1203,4 +1203,58 @@
"transition outputs [//command_line_option:test_arg] were "
+ "not defined by transition function");
}
+
+ @Test
+ public void composingTransitionReportsAllStarlarkErrors() throws Exception {
+ writeWhitelistFile();
+ scratch.file(
+ "test/build_settings.bzl",
+ "def _impl(ctx):",
+ " return []",
+ "string_flag = rule(implementation = _impl, build_setting = config.string(flag=True))");
+ scratch.file(
+ "test/transitions.bzl",
+ "def _impl(settings, attr):",
+ " return {}",
+ "attr_transition = transition(implementation = _impl, inputs = [],",
+ " outputs = ['//test:attr_transition_output_flag'])",
+ "self_transition = transition(implementation = _impl, inputs = [],",
+ " outputs = ['//test:self_transition_output_flag'])");
+ scratch.file(
+ "test/rules.bzl",
+ "load('//test:transitions.bzl', 'attr_transition', 'self_transition')",
+ "def _impl(ctx):",
+ " return []",
+ "rule_with_attr_transition = rule(",
+ " implementation = _impl,",
+ " attrs = {",
+ " '_whitelist_function_transition': attr.label(",
+ " default = '//tools/whitelists/function_transition_whitelist'),",
+ " 'deps': attr.label_list(cfg = attr_transition),",
+ " })",
+ "rule_with_self_transition = rule(",
+ " implementation = _impl,",
+ " cfg = self_transition,",
+ " attrs = {",
+ " '_whitelist_function_transition': attr.label(",
+ " default = '//tools/whitelists/function_transition_whitelist'),",
+ " })");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:rules.bzl', 'rule_with_attr_transition', 'rule_with_self_transition')",
+ "load('//test:build_settings.bzl', 'string_flag')",
+ "string_flag(name = 'attr_transition_output_flag', build_setting_default='')",
+ "string_flag(name = 'self_transition_output_flag', build_setting_default='')",
+ "rule_with_attr_transition(name = 'buildme', deps = [':adep'])",
+ "rule_with_self_transition(name = 'adep')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:buildme");
+ assertContainsEvent(
+ "transition outputs [//test:attr_transition_output_flag] were not defined by transition "
+ + "function");
+ assertContainsEvent(
+ "transition outputs [//test:self_transition_output_flag] were not defined by transition "
+ + "function");
+ }
}
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 d71fbe7..f70e716 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
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.analysis.PlatformOptions;
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.StoredEventHandler;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.testutil.FakeAttributeMapper;
import com.google.devtools.common.options.OptionsParsingException;
@@ -50,7 +51,7 @@
ImmutableList.of(CoreOptions.class, PlatformOptions.class),
"--platforms=//platform:target");
- BuildOptions result = transition.patch(options);
+ BuildOptions result = transition.patch(options, new StoredEventHandler());
assertThat(result).isNotNull();
assertThat(result).isNotSameInstanceAs(options);
@@ -80,7 +81,7 @@
ImmutableList.of(CoreOptions.class, PlatformOptions.class),
"--platforms=//platform:target");
- BuildOptions result = transition.patch(options);
+ BuildOptions result = transition.patch(options, new StoredEventHandler());
assertThat(result).isNotNull();
assertThat(result).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 542504f..aee0d37 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
@@ -59,7 +59,8 @@
public void splitTransition() {
TransitionFactory<Object> factory =
TransitionFactories.of(
- (SplitTransition) buildOptions -> ImmutableMap.of("test0", buildOptions.clone()));
+ (SplitTransition)
+ (buildOptions, eventHandler) -> 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 88bbbd1..85855b5 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
@@ -23,9 +23,12 @@
import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
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 java.util.Collection;
import java.util.Map;
import java.util.stream.IntStream;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -36,6 +39,12 @@
// Use starlark flags for the test since they are easy to set and check.
private static final Label FLAG_1 = Label.parseAbsoluteUnchecked("//flag1");
private static final Label FLAG_2 = Label.parseAbsoluteUnchecked("//flag2");
+ private EventHandler eventHandler;
+
+ @Before
+ public void init() {
+ eventHandler = new StoredEventHandler();
+ }
@Test
public void compose_patch_patch() {
@@ -48,7 +57,8 @@
assertThat(composed).isNotNull();
assertThat(composed.isSplit()).isFalse();
ConfigurationTransition transition = composed.create(new StubData());
- Collection<BuildOptions> results = transition.apply(BuildOptions.builder().build()).values();
+ Collection<BuildOptions> results =
+ transition.apply(BuildOptions.builder().build(), eventHandler).values();
assertThat(results).isNotNull();
assertThat(results).hasSize(1);
BuildOptions result = Iterables.getOnlyElement(results);
@@ -67,7 +77,8 @@
assertThat(composed).isNotNull();
assertThat(composed.isSplit()).isTrue();
ConfigurationTransition transition = composed.create(new StubData());
- Map<String, BuildOptions> results = transition.apply(BuildOptions.builder().build());
+ Map<String, BuildOptions> results =
+ transition.apply(BuildOptions.builder().build(), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -93,7 +104,8 @@
assertThat(composed).isNotNull();
assertThat(composed.isSplit()).isTrue();
ConfigurationTransition transition = composed.create(new StubData());
- Map<String, BuildOptions> results = transition.apply(BuildOptions.builder().build());
+ Map<String, BuildOptions> results =
+ transition.apply(BuildOptions.builder().build(), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -200,7 +212,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
return updateOptions(options, flagLabel, flagValue);
}
}
@@ -215,7 +227,7 @@
}
@Override
- public Map<String, BuildOptions> split(BuildOptions options) {
+ public Map<String, BuildOptions> split(BuildOptions options, EventHandler eventHandler) {
return IntStream.range(0, flagValues.size())
.boxed()
.collect(
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 d804b2f..acc758e 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
@@ -22,8 +22,11 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
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 java.util.Map;
import java.util.stream.IntStream;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -34,6 +37,12 @@
// Use starlark flags for the test since they are easy to set and check.
private static final Label FLAG_1 = Label.parseAbsoluteUnchecked("//flag1");
private static final Label FLAG_2 = Label.parseAbsoluteUnchecked("//flag2");
+ private EventHandler eventHandler;
+
+ @Before
+ public void init() {
+ eventHandler = new StoredEventHandler();
+ }
@Test
public void compose_patch_patch() {
@@ -42,7 +51,8 @@
ComposingTransition.of(new StubPatch(FLAG_1, "value1"), new StubPatch(FLAG_1, "value2"));
assertThat(composed).isNotNull();
- Map<String, BuildOptions> results = composed.apply(BuildOptions.builder().build());
+ Map<String, BuildOptions> results =
+ composed.apply(BuildOptions.builder().build(), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(1);
BuildOptions result = Iterables.getOnlyElement(results.values());
@@ -58,7 +68,8 @@
new StubPatch(FLAG_1, "value1"), new StubSplit(FLAG_2, "value2a", "value2b"));
assertThat(composed).isNotNull();
- Map<String, BuildOptions> results = composed.apply(BuildOptions.builder().build());
+ Map<String, BuildOptions> results =
+ composed.apply(BuildOptions.builder().build(), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -81,7 +92,8 @@
new StubSplit(FLAG_1, "value1a", "value1b"), new StubPatch(FLAG_2, "value2"));
assertThat(composed).isNotNull();
- Map<String, BuildOptions> results = composed.apply(BuildOptions.builder().build());
+ Map<String, BuildOptions> results =
+ composed.apply(BuildOptions.builder().build(), eventHandler);
assertThat(results).isNotNull();
assertThat(results).hasSize(2);
@@ -105,7 +117,9 @@
new StubSplit(FLAG_2, "value2a", "value2b"));
assertThat(composed).isNotNull();
- assertThrows(IllegalStateException.class, () -> composed.apply(BuildOptions.builder().build()));
+ assertThrows(
+ IllegalStateException.class,
+ () -> composed.apply(BuildOptions.builder().build(), eventHandler));
}
@Test
@@ -159,7 +173,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
return updateOptions(options, flagLabel, flagValue);
}
}
@@ -174,7 +188,7 @@
}
@Override
- public Map<String, BuildOptions> split(BuildOptions options) {
+ public Map<String, BuildOptions> split(BuildOptions options, EventHandler eventHandler) {
return IntStream.range(0, flagValues.size())
.boxed()
.collect(
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 78d74f5..0338df6 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
@@ -23,6 +23,8 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions;
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.packages.AttributeTransitionData;
import com.google.devtools.build.lib.testutil.FakeAttributeMapper;
import com.google.devtools.common.options.OptionsParsingException;
@@ -42,7 +44,7 @@
BuildOptions.of(
ImmutableList.of(CoreOptions.class, TestOptions.class), "--trim_test_configuration");
- BuildOptions result = TRIM_TRANSITION.patch(options);
+ BuildOptions result = TRIM_TRANSITION.patch(options, new StoredEventHandler());
// Verify the transitions actually applied.
assertThat(result).isNotNull();
@@ -56,7 +58,7 @@
BuildOptions.of(
ImmutableList.of(CoreOptions.class, TestOptions.class), "--notrim_test_configuration");
- BuildOptions result = TRIM_TRANSITION.patch(options);
+ BuildOptions result = TRIM_TRANSITION.patch(options, new StoredEventHandler());
// Verify the transitions actually applied.
assertThat(result).isNotNull();
@@ -75,7 +77,7 @@
.addStarlarkOption(starlarkOptionKey, starlarkOptionValue)
.build();
- BuildOptions result = TRIM_TRANSITION.patch(options);
+ BuildOptions result = TRIM_TRANSITION.patch(options, new StoredEventHandler());
// Verify the transitions actually applied.
assertThat(result).isNotNull();
@@ -103,8 +105,11 @@
"--platforms=//platform:target",
"--trim_test_configuration");
- BuildOptions execThenTrim = TRIM_TRANSITION.patch(execTransition.patch(options));
- BuildOptions trimThenExec = execTransition.patch(TRIM_TRANSITION.patch(options));
+ EventHandler handler = new StoredEventHandler();
+ BuildOptions execThenTrim =
+ TRIM_TRANSITION.patch(execTransition.patch(options, handler), handler);
+ BuildOptions trimThenExec =
+ execTransition.patch(TRIM_TRANSITION.patch(options, handler), 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 b25d8a8..c7c8ae3 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
@@ -1771,7 +1771,9 @@
} else {
try {
return skyframeExecutor.getConfigurationForTesting(
- reporter, fromConfig.fragmentClasses(), transition.patch(fromConfig.getOptions()));
+ reporter,
+ fromConfig.fragmentClasses(),
+ transition.patch(fromConfig.getOptions(), eventCollector));
} catch (OptionsParsingException | InvalidConfigurationException e) {
throw new AssertionError(e);
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
index a6140bb..2fe73f4 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.util.TestAspects;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassNamePredicate;
import com.google.devtools.build.lib.testutil.FakeAttributeMapper;
import com.google.devtools.build.lib.util.FileType;
@@ -286,7 +287,7 @@
private static class TestSplitTransition implements SplitTransition {
@Override
- public Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
return ImmutableMap.of("test0", buildOptions.clone(), "test1", buildOptions.clone());
}
}
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 027d59b..7b179ef 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
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
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;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.syntax.StarlarkValue;
@@ -77,7 +78,8 @@
private static final ConfigurationTransition TEST_HOST_TRANSITION =
new ConfigurationTransition() {
@Override
- public ImmutableMap<String, BuildOptions> apply(BuildOptions buildOptions) {
+ public ImmutableMap<String, BuildOptions> apply(
+ BuildOptions buildOptions, EventHandler eventHandler) {
return ImmutableMap.of("", buildOptions);
}
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
index 1c7b7b3..6ebcaf2 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
@@ -56,6 +56,7 @@
deps = [
":configured_target_query_helper",
"//src/main/java/com/google/devtools/build/lib:build-base",
+ "//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib:filetype",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/packages",
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java
index 9287511..55f18da 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryTest.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions;
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.BuildType;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting;
@@ -272,7 +273,7 @@
}
@Override
- public Map<String, BuildOptions> split(BuildOptions options) {
+ public Map<String, BuildOptions> split(BuildOptions options, EventHandler eventHandler) {
BuildOptions result1 = options.clone();
BuildOptions result2 = options.clone();
result1.get(TestOptions.class).testArguments = Collections.singletonList(toOption1);
@@ -537,7 +538,7 @@
/** Return an empty BuildOptions for testing fragment dropping. * */
public static class RemoveTestOptionsTransition implements PatchTransition {
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
BuildOptions.Builder builder = BuildOptions.builder();
for (FragmentOptions option : options.getNativeOptions()) {
if (!(option instanceof TestOptions)) {
diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java
index c984d1a..92d872d 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java
@@ -26,6 +26,7 @@
import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions;
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.packages.AttributeMap;
import com.google.devtools.build.lib.packages.Type;
@@ -348,7 +349,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
BuildOptions result = options.clone();
result.get(TestOptions.class).testArguments = Collections.singletonList(toOption);
return result;
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactoryTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactoryTest.java
index c0be600..4e1297e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactoryTest.java
@@ -61,7 +61,7 @@
PatchTransition transition = new ConfigFeatureFlagTransitionFactory("flag_values").create(rule);
BuildOptions original = getOptionsWithoutFlagFragment();
- BuildOptions converted = transition.patch(original);
+ BuildOptions converted = transition.patch(original, eventCollector);
assertThat(converted).isSameInstanceAs(original);
assertThat(original.contains(ConfigFeatureFlagOptions.class)).isFalse();
@@ -83,7 +83,7 @@
PatchTransition transition = new ConfigFeatureFlagTransitionFactory("flag_values").create(rule);
BuildOptions original = getOptionsWithoutFlagFragment();
- BuildOptions converted = transition.patch(original);
+ BuildOptions converted = transition.patch(original, eventCollector);
assertThat(converted).isSameInstanceAs(original);
assertThat(original.contains(ConfigFeatureFlagOptions.class)).isFalse();
@@ -97,7 +97,7 @@
ImmutableMap.of(Label.parseAbsolute("//a:flag", ImmutableMap.of()), "value");
BuildOptions original = getOptionsWithFlagFragment(originalFlagMap);
- BuildOptions converted = transition.patch(original);
+ BuildOptions converted = transition.patch(original, eventCollector);
assertThat(converted).isNotSameInstanceAs(original);
assertThat(FeatureFlagValue.getFlagValues(original)).containsExactlyEntriesIn(originalFlagMap);
@@ -125,7 +125,7 @@
ImmutableMap.of(Label.parseAbsolute("//a:flag", ImmutableMap.of()), "a");
BuildOptions original = getOptionsWithFlagFragment(originalFlagMap);
- BuildOptions converted = transition.patch(original);
+ BuildOptions converted = transition.patch(original, eventCollector);
assertThat(converted).isNotSameInstanceAs(original);
assertThat(FeatureFlagValue.getFlagValues(original)).containsExactlyEntriesIn(originalFlagMap);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
index 392eb23..00d2244 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
@@ -885,7 +885,7 @@
BuildOptions depOptions =
new ConfigFeatureFlagTaggedTrimmingTransitionFactory(BaseRuleClasses.TAGGED_TRIMMING_ATTR)
.create((Rule) getTarget("//test:dep"))
- .patch(topLevelOptions);
+ .patch(topLevelOptions, eventCollector);
assertThat(depOptions).isSameInstanceAs(topLevelOptions);
}
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 60daf41..8aa98f4 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
@@ -314,7 +314,8 @@
throws InterruptedException, OptionsParsingException, InvalidConfigurationException {
ImmutableList.Builder<BuildConfiguration> splitConfigs = ImmutableList.builder();
- for (BuildOptions splitOptions : splitTransition.split(configuration.getOptions()).values()) {
+ for (BuildOptions splitOptions :
+ splitTransition.split(configuration.getOptions(), eventCollector).values()) {
splitConfigs.add(getSkyframeExecutor().getConfigurationForTesting(
reporter, configuration.fragmentClasses(), splitOptions));
}
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 db77ed6..d5eb832 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
@@ -44,6 +44,7 @@
import com.google.devtools.build.lib.analysis.util.TestAspects;
import com.google.devtools.build.lib.analysis.util.TestAspects.DummyRuleFactory;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
@@ -70,14 +71,14 @@
private static class NoopSplitTransition implements SplitTransition {
@Override
- public Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
return ImmutableMap.of("noop", buildOptions);
}
}
private static class SetsHostCpuSplitTransition implements SplitTransition {
@Override
- public Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
BuildOptions result = buildOptions.clone();
result.get(CoreOptions.class).hostCpu = "SET BY SPLIT";
return ImmutableMap.of("hostCpu", result);
@@ -87,7 +88,7 @@
private static class SetsCpuSplitTransition implements SplitTransition {
@Override
- public Map<String, BuildOptions> split(BuildOptions buildOptions) {
+ public Map<String, BuildOptions> split(BuildOptions buildOptions, EventHandler eventHandler) {
BuildOptions result = buildOptions.clone();
result.get(CoreOptions.class).cpu = "SET BY SPLIT";
return ImmutableMap.of("cpu", result);
@@ -97,7 +98,7 @@
private static class SetsCpuPatchTransition implements PatchTransition {
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
BuildOptions result = options.clone();
result.get(CoreOptions.class).cpu = "SET BY PATCH";
return result;
@@ -151,7 +152,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
BuildOptions result = options.clone();
result.get(TestConfiguration.TestOptions.class).testFilter = "SET BY PATCH FACTORY: " + value;
return result;
@@ -194,7 +195,7 @@
}
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
if (!options.contains(TestConfiguration.TestOptions.class)) {
return options;
}
@@ -550,7 +551,7 @@
private static PatchTransition newPatchTransition(final String value) {
return new PatchTransition() {
@Override
- public BuildOptions patch(BuildOptions options) {
+ public BuildOptions patch(BuildOptions options, EventHandler eventHandler) {
BuildOptions toOptions = options.clone();
TestConfiguration.TestOptions baseOptions =
toOptions.get(TestConfiguration.TestOptions.class);
@@ -566,7 +567,7 @@
* prefix + "2"}.
*/
private static SplitTransition newSplitTransition(final String prefix) {
- return buildOptions -> {
+ return (buildOptions, eventHandler) -> {
ImmutableMap.Builder<String, BuildOptions> result = ImmutableMap.builder();
for (int index = 1; index <= 2; index++) {
BuildOptions toOptions = buildOptions.clone();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
index 6ef1485..0f73928 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/BUILD
@@ -20,6 +20,7 @@
deps = [
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib:core-rules",
+ "//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib:filetype",
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib/actions",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java
index 4a8bec9..bc55bcb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction;
@@ -612,7 +613,7 @@
}
@Override
- public BuildOptions patch(BuildOptions target) {
+ public BuildOptions patch(BuildOptions target, EventHandler eventHandler) {
BuildOptions output = target.clone();
if (alpha != null) {
output.get(AOptions.class).alpha = alpha;
