Skip to content

Commit

Permalink
Make print() work from starlark transition implementation functions.
Browse files Browse the repository at this point in the history
Give each starlark transition a stored event handler. After starlark transitions are applied (after top level transitions and inside configuration resolver), replay and then clear events.

Declare a new abstract class for starlark transitions to consolidate fields/behavior.

Add behavior to ComposingTransitions to allow decomposition. This is helpful here but will also be helpful in future work where we'll need to post-process starlark transitions after they've been applied (e.g. output type verification).

#5574

RELNOTES: None.
PiperOrigin-RevId: 229417148
  • Loading branch information
juliexxia authored and Copybara-Service committed Jan 15, 2019
1 parent 9ba4663 commit cf4d89e
Show file tree
Hide file tree
Showing 12 changed files with 294 additions and 22 deletions.
Expand Up @@ -27,8 +27,10 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.Dependency;
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
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.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.events.Event;
Expand Down Expand Up @@ -217,6 +219,8 @@ public static OrderedSetMultimap<Attribute, Dependency> resolveConfigurations(
transitionsMap.put(transitionKey, toOptions);
}

postProcessStarlarkTransitions(env, transition);

// If the transition doesn't change the configuration, trivially re-use the original
// configuration.
if (sameFragments && toOptions.size() == 1
Expand Down Expand Up @@ -296,6 +300,16 @@ public static OrderedSetMultimap<Attribute, Dependency> resolveConfigurations(
return sortResolvedDeps(originalDeps, resolvedDeps, attributesAndLabels);
}

private static void postProcessStarlarkTransitions(
SkyFunction.Environment env, ConfigurationTransition transition) {
ImmutableList<ConfigurationTransition> transitions =
ComposingTransition.decomposeTransition(transition);
ExtendedEventHandler eventHandler = env.getListener();
transitions.stream()
.filter(t -> t instanceof StarlarkTransition)
.forEach(t -> ((StarlarkTransition) t).replayOn(eventHandler));
}

/**
* Encapsulates a set of config fragments and a config transition. This can be used to determine
* the exact build options needed to set a configuration.
Expand All @@ -322,6 +336,9 @@ public boolean equals(Object o) {
} else if (o == null) {
return false;
} else {
if (!(o instanceof FragmentsAndTransition)) {
return false;
}
FragmentsAndTransition other = (FragmentsAndTransition) o;
return other.transition.equals(transition) && other.fragments.equals(fragments);
}
Expand Down
Expand Up @@ -16,8 +16,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.StructImpl;
import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigurationTransitionApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
Expand All @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/**
* Implementation of {@link ConfigurationTransitionApi}.
Expand All @@ -41,12 +42,14 @@ public abstract class StarlarkDefinedConfigTransition implements ConfigurationTr
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();
}

/**
Expand All @@ -56,17 +59,16 @@ private StarlarkDefinedConfigTransition(
public abstract Boolean isForAnalysisTesting();

/**
* Returns the input option keys for this transition. Only option keys contained in this
* list will be provided in the 'settings' argument given to the transition implementation
* function.
* Returns the input option keys for this transition. Only option keys contained in this list will
* be provided in the 'settings' argument given to the transition implementation function.
*/
public List<String> getInputs() {
return inputs;
}

/**
* Returns the output option keys for this transition. The transition implementation function
* must return a dictionary where the option keys exactly match the elements of this list.
* Returns the output option keys for this transition. The transition implementation function must
* return a dictionary where the option keys exactly match the elements of this list.
*/
public List<String> getOutputs() {
return outputs;
Expand All @@ -80,6 +82,10 @@ public Location getLocationForErrorReporting() {
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.
Expand All @@ -101,9 +107,8 @@ public static StarlarkDefinedConfigTransition newRegularTransition(
List<String> inputs,
List<String> outputs,
SkylarkSemantics semantics,
EventHandler eventHandler,
StarlarkContext context) {
return new RegularTransition(impl, inputs, outputs, semantics, eventHandler, context);
return new RegularTransition(impl, inputs, outputs, semantics, context);
}

public static StarlarkDefinedConfigTransition newAnalysisTestTransition(
Expand Down Expand Up @@ -134,25 +139,41 @@ public ImmutableList<Map<String, Object>> getChangedSettings(
public void repr(SkylarkPrinter printer) {
printer.append("<analysis_test_transition object>");
}

@Override
public boolean equals(Object object) {
if (object == this) {
return true;
}
if (object instanceof AnalysisTestTransition) {
AnalysisTestTransition otherTransition = (AnalysisTestTransition) object;
return Objects.equals(otherTransition.getInputs(), this.getInputs())
&& Objects.equals(otherTransition.getOutputs(), this.getOutputs())
&& Objects.equals(otherTransition.changedSettings, this.changedSettings);
}
return false;
}

@Override
public int hashCode() {
return Objects.hash(this.getInputs(), this.getOutputs(), this.changedSettings);
}
}

private static class RegularTransition extends StarlarkDefinedConfigTransition {
private final BaseFunction impl;
private final SkylarkSemantics semantics;
private final EventHandler eventHandler;
private final StarlarkContext starlarkContext;

public RegularTransition(
BaseFunction impl,
List<String> inputs,
List<String> outputs,
SkylarkSemantics semantics,
EventHandler eventHandler,
StarlarkContext context) {
super(inputs, outputs, impl.getLocation());
this.impl = impl;
this.semantics = semantics;
this.eventHandler = eventHandler;
this.starlarkContext = context;
}

Expand Down Expand Up @@ -233,12 +254,31 @@ private Object evalFunction(BaseFunction function, ImmutableList<Object> args)
Environment env =
Environment.builder(mutability)
.setSemantics(semantics)
.setEventHandler(eventHandler)
.setEventHandler(getEventHandler())
.setStarlarkContext(starlarkContext)
.build();

return function.call(args, ImmutableMap.of(), null, env);
}
}

@Override
public boolean equals(Object object) {
if (object == this) {
return true;
}
if (object instanceof RegularTransition) {
RegularTransition otherTransition = (RegularTransition) object;
return Objects.equals(otherTransition.getInputs(), this.getInputs())
&& Objects.equals(otherTransition.getOutputs(), this.getOutputs())
&& Objects.equals(otherTransition.impl, this.impl);
}
return false;
}

@Override
public int hashCode() {
return Objects.hash(this.getInputs(), this.getOutputs(), this.impl);
}
}
}
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -82,4 +83,29 @@ public boolean equals(Object other) {
&& ((ComposingTransition) other).transition1.equals(this.transition1)
&& ((ComposingTransition) other).transition2.equals(this.transition2);
}

/**
* Recursively decompose a composing transition into all the {@link ConfigurationTransition}
* instances that it holds.
*
* @param root {@link ComposingTransition} to decompose
*/
public static ImmutableList<ConfigurationTransition> decomposeTransition(
ConfigurationTransition root) {
ArrayList<ConfigurationTransition> toBeInspected = new ArrayList<>();
ImmutableList.Builder<ConfigurationTransition> transitions = new ImmutableList.Builder<>();
toBeInspected.add(root);
ConfigurationTransition current;
while (!toBeInspected.isEmpty()) {
current = toBeInspected.remove(0);
if (current instanceof ComposingTransition) {
ComposingTransition composingCurrent = (ComposingTransition) current;
toBeInspected.addAll(
ImmutableList.of(composingCurrent.transition1, composingCurrent.transition2));
} else {
transitions.add(current);
}
}
return transitions.build();
}
}
Expand Up @@ -55,7 +55,7 @@ public class FunctionTransitionUtil {
* incoming {@link BuildOptions}. For native options, this involves a preprocess step of
* converting options to their "command line form".
*
* Also Validate that transitions output sensical results.
* <p>Also validate that transitions output sensical results.
*
* @param buildOptions the pre-transition build options
* @param starlarkTransition the transition to apply
Expand Down
Expand Up @@ -17,6 +17,7 @@
import static com.google.devtools.build.lib.analysis.skylark.FunctionTransitionUtil.applyAndValidate;
import static com.google.devtools.build.lib.analysis.skylark.SkylarkAttributesCollection.ERROR_MESSAGE_FOR_NO_ATTR;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
Expand Down Expand Up @@ -52,21 +53,25 @@ public class StarlarkAttributeTransitionProvider implements SplitTransitionProvi
this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
}

@VisibleForTesting
public StarlarkDefinedConfigTransition getStarlarkDefinedConfigTransitionForTesting() {
return starlarkDefinedConfigTransition;
}

@Override
public SplitTransition apply(AttributeMap attributeMap) {
Preconditions.checkArgument(attributeMap instanceof ConfiguredAttributeMapper);
return new FunctionSplitTransition(
starlarkDefinedConfigTransition, (ConfiguredAttributeMapper) attributeMap);
}

private static class FunctionSplitTransition implements SplitTransition {
private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition;
class FunctionSplitTransition extends StarlarkTransition implements SplitTransition {
private final StructImpl attrObject;

FunctionSplitTransition(
StarlarkDefinedConfigTransition starlarkDefinedConfigTransition,
ConfiguredAttributeMapper attributeMap) {
this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
super(starlarkDefinedConfigTransition);

LinkedHashMap<String, Object> attributes = new LinkedHashMap<>();
for (String attribute : attributeMap.getAttributeNames()) {
Expand Down
Expand Up @@ -16,6 +16,7 @@

import static com.google.devtools.build.lib.analysis.skylark.FunctionTransitionUtil.applyAndValidate;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
Expand Down Expand Up @@ -53,19 +54,22 @@ public class StarlarkRuleTransitionProvider implements RuleTransitionFactory {
this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
}

@VisibleForTesting
public StarlarkDefinedConfigTransition getStarlarkDefinedConfigTransitionForTesting() {
return starlarkDefinedConfigTransition;
}

@Override
public PatchTransition buildTransitionFor(Rule rule) {
return new FunctionPatchTransition(starlarkDefinedConfigTransition, rule);
}

private static class FunctionPatchTransition implements PatchTransition {

private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition;
class FunctionPatchTransition extends StarlarkTransition implements PatchTransition {
private final StructImpl attrObject;

FunctionPatchTransition(
StarlarkDefinedConfigTransition starlarkDefinedConfigTransition, Rule rule) {
this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
super(starlarkDefinedConfigTransition);

LinkedHashMap<String, Object> attributes = new LinkedHashMap<>();
RawAttributeMapper attributeMapper = RawAttributeMapper.of(rule);
Expand Down
@@ -0,0 +1,52 @@
// Copyright 2019 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.skylark;

import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import java.util.Objects;

/** A marker class for configuration transitions that are defined in Starlark. */
public abstract class StarlarkTransition implements ConfigurationTransition {

private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition;

StarlarkTransition(StarlarkDefinedConfigTransition starlarkDefinedConfigTransition) {
this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
}

public void replayOn(ExtendedEventHandler eventHandler) {
starlarkDefinedConfigTransition.getEventHandler().replayOn(eventHandler);
starlarkDefinedConfigTransition.getEventHandler().clear();
}

@Override
public boolean equals(Object object) {
if (object == this) {
return true;
}
if (object instanceof StarlarkTransition) {
StarlarkDefinedConfigTransition starlarkDefinedConfigTransition =
((StarlarkTransition) object).starlarkDefinedConfigTransition;
return Objects.equals(starlarkDefinedConfigTransition, this.starlarkDefinedConfigTransition);
}
return false;
}

@Override
public int hashCode() {
return Objects.hashCode(starlarkDefinedConfigTransition);
}
}
Expand Up @@ -2150,6 +2150,11 @@ public SplitTransition getSplitTransition(AttributeMap attributeMapper) {
return splitTransitionProvider.apply(attributeMapper);
}

@VisibleForTesting
public SplitTransitionProvider getSplitTransitionProviderForTesting() {
return splitTransitionProvider;
}

/**
* Returns true if this attribute transitions on a split transition.
* See {@link SplitTransition}.
Expand Down
Expand Up @@ -59,7 +59,7 @@ public ConfigurationTransitionApi transition(
validateBuildSettingKeys(inputs, "input", location);
validateBuildSettingKeys(outputs, "output", location);
return StarlarkDefinedConfigTransition.newRegularTransition(
implementation, inputs, outputs, semantics, env.getEventHandler(), context);
implementation, inputs, outputs, semantics, context);
}

@Override
Expand Down

0 comments on commit cf4d89e

Please sign in to comment.