Skip to content

Commit

Permalink
Change uses of SplitTransitionProvider to TransitionFactory.
Browse files Browse the repository at this point in the history
Removed SplitTransitionProvider.

Part of #7814.

Closes #7833.

PiperOrigin-RevId: 240598880
  • Loading branch information
katre authored and copybara-github committed Mar 27, 2019
1 parent 75b7ed4 commit 0a9e1ed
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
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.Location;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
import com.google.devtools.build.lib.packages.Attribute.ImmutableAttributeFactory;
import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate;
import com.google.devtools.build.lib.packages.Attribute.SplitTransitionProvider;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.Provider;
Expand Down Expand Up @@ -252,7 +252,7 @@ && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
Object trans = arguments.get(CONFIGURATION_ARG);
boolean isSplit =
trans instanceof SplitTransition
|| trans instanceof SplitTransitionProvider
|| trans instanceof TransitionFactory
|| trans instanceof StarlarkDefinedConfigTransition;
if (isSplit && defaultValue instanceof SkylarkLateBoundDefault) {
throw new EvalException(
Expand All @@ -263,8 +263,8 @@ && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
builder.cfg(HostTransition.INSTANCE);
} else if (trans instanceof SplitTransition) {
builder.cfg((SplitTransition) trans);
} else if (trans instanceof SplitTransitionProvider) {
builder.cfg((SplitTransitionProvider) trans);
} else if (trans instanceof TransitionFactory) {
builder.cfg((TransitionFactory) trans);
} else if (trans instanceof StarlarkDefinedConfigTransition) {
StarlarkDefinedConfigTransition starlarkDefinedTransition =
(StarlarkDefinedConfigTransition) trans;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
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.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.SplitTransitionProvider;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
import com.google.devtools.build.lib.packages.RuleTransitionData;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.packages.StructProvider;
import com.google.devtools.build.lib.skylarkbuildapi.SplitTransitionProviderApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;
Expand All @@ -39,17 +41,18 @@
import java.util.List;

/**
* This class implements a {@link SplitTransitionProvider} to provide a starlark-defined transition
* that rules can apply to their dependencies' configurations. This transition has access to (1) the
* a map of the current configuration's build settings and (2) the configured attributes of the
* given rule (not its dependencies').
* This class implements {@link TransitionFactory} to provide a starlark-defined transition that
* rules can apply to their dependencies' configurations. This transition has access to (1) the a
* map of the current configuration's build settings and (2) the configured attributes of the given
* rule (not its dependencies').
*
* <p>For starlark defined rule class transitions, see {@link StarlarkRuleTransitionProvider}.
*
* <p>TODO(bazel-team): Consider allowing dependency-typed attributes to actually return providers
* instead of just labels (see {@link SkylarkAttributesCollection#addAttribute}).
*/
public class StarlarkAttributeTransitionProvider implements SplitTransitionProvider {
public class StarlarkAttributeTransitionProvider
implements TransitionFactory<RuleTransitionData>, SplitTransitionProviderApi {
private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition;

StarlarkAttributeTransitionProvider(
Expand All @@ -63,12 +66,18 @@ public StarlarkDefinedConfigTransition getStarlarkDefinedConfigTransitionForTest
}

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

@Override
public boolean isSplit() {
return true;
}

@Override
public void repr(SkylarkPrinter printer) {
printer.append("<transition object>");
Expand Down
113 changes: 46 additions & 67 deletions src/main/java/com/google/devtools/build/lib/packages/Attribute.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,13 @@
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
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.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.RuleClass.Builder.RuleClassNamePredicate;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkbuildapi.SplitTransitionProviderApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
Expand Down Expand Up @@ -287,44 +286,6 @@ public String checkValid(Rule from, Rule to) {
}
};

/**
* Provides a {@link SplitTransition} given the originating target {@link Rule}. The split
* transition may be constant for all instances of the originating rule, or it may differ
* based on attributes of that rule. For instance, a split transition on a rule's deps may differ
* depending on the 'platform' attribute of the rule.
*/
public interface SplitTransitionProvider extends SplitTransitionProviderApi {
/**
* Returns the {@link SplitTransition} given the attribute mapper of the originating rule.
*/
SplitTransition apply(AttributeMap attributeMap);
}

/**
* Implementation of {@link SplitTransitionProvider} that returns a single {@link SplitTransition}
* regardless of the originating rule.
*/
@AutoCodec.VisibleForSerialization
@AutoCodec
static class BasicSplitTransitionProvider implements SplitTransitionProvider {
private final SplitTransition splitTransition;

@AutoCodec.VisibleForSerialization
BasicSplitTransitionProvider(SplitTransition splitTransition) {
this.splitTransition = splitTransition;
}

@Override
public SplitTransition apply(AttributeMap attributeMap) {
return splitTransition;
}

@Override
public void repr(SkylarkPrinter printer) {
printer.append("<transition object>");
}
}

/** A predicate class to check if the value of the attribute comes from a predefined set. */
@AutoCodec
public static class AllowedValueSet implements PredicateWithMessage<Object> {
Expand Down Expand Up @@ -393,7 +354,8 @@ public static class ImmutableAttributeFactory {
private final ConfigurationTransition configTransition;
private final RuleClassNamePredicate allowedRuleClassesForLabels;
private final RuleClassNamePredicate allowedRuleClassesForLabelsWarning;
private final SplitTransitionProvider splitTransitionProvider;
// TODO(https://github.com/bazelbuild/bazel/issues/7814): Merge this with configTransition.
private final TransitionFactory<RuleTransitionData> splitTransitionFactory;
private final FileTypeSet allowedFileTypesForLabels;
private final ValidityPredicate validityPredicate;
private final Object value;
Expand All @@ -414,7 +376,7 @@ public static class ImmutableAttributeFactory {
ConfigurationTransition configTransition,
RuleClassNamePredicate allowedRuleClassesForLabels,
RuleClassNamePredicate allowedRuleClassesForLabelsWarning,
SplitTransitionProvider splitTransitionProvider,
TransitionFactory<RuleTransitionData> splitTransitionFactory,
FileTypeSet allowedFileTypesForLabels,
ValidityPredicate validityPredicate,
AttributeValueSource valueSource,
Expand All @@ -428,7 +390,7 @@ public static class ImmutableAttributeFactory {
this.configTransition = configTransition;
this.allowedRuleClassesForLabels = allowedRuleClassesForLabels;
this.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning;
this.splitTransitionProvider = splitTransitionProvider;
this.splitTransitionFactory = splitTransitionFactory;
this.allowedFileTypesForLabels = allowedFileTypesForLabels;
this.validityPredicate = validityPredicate;
this.value = value;
Expand All @@ -453,7 +415,7 @@ public Attribute build(String name) {
Preconditions.checkState(!name.isEmpty(), "name has not been set");
if (valueSource == AttributeValueSource.LATE_BOUND) {
Preconditions.checkState(isLateBound(name));
Preconditions.checkState(splitTransitionProvider == null);
Preconditions.checkState(splitTransitionFactory == null);
}
// TODO(bazel-team): Set the default to be no file type, then remove this check, and also
// remove all allowedFileTypes() calls without parameters.
Expand All @@ -480,7 +442,7 @@ public Attribute build(String name) {
propertyFlags,
value,
configTransition,
splitTransitionProvider,
splitTransitionFactory,
allowedRuleClassesForLabels,
allowedRuleClassesForLabelsWarning,
allowedFileTypesForLabels,
Expand All @@ -504,7 +466,8 @@ public static class Builder <TYPE> {
private ConfigurationTransition configTransition = NoTransition.INSTANCE;
private RuleClassNamePredicate allowedRuleClassesForLabels = ANY_RULE;
private RuleClassNamePredicate allowedRuleClassesForLabelsWarning = NO_RULE;
private SplitTransitionProvider splitTransitionProvider;
// TODO(https://github.com/bazelbuild/bazel/issues/7814): Merge this with configTransition.
private TransitionFactory<RuleTransitionData> splitTransitionFactory;
private FileTypeSet allowedFileTypesForLabels;
private ValidityPredicate validityPredicate = ANY_EDGE;
private Object value;
Expand Down Expand Up @@ -637,14 +600,16 @@ public Builder<TYPE> hasAnalysisTestTransition() {
"analysis-test split transition");
}

/**
* Defines the configuration transition for this attribute.
*/
public Builder<TYPE> cfg(SplitTransitionProvider splitTransitionProvider) {
/** Defines the configuration transition for this attribute. */
// TODO(https://github.com/bazelbuild/bazel/issues/7814): Currently only for split transitions.
public Builder<TYPE> cfg(TransitionFactory<RuleTransitionData> splitTransitionFactory) {
Preconditions.checkState(this.configTransition == NoTransition.INSTANCE,
"the configuration transition is already set");
Preconditions.checkState(
splitTransitionFactory.isSplit(),
"cfg(TransitionFactory) is only valid for split transitions at this time.");

this.splitTransitionProvider = Preconditions.checkNotNull(splitTransitionProvider);
this.splitTransitionFactory = Preconditions.checkNotNull(splitTransitionFactory);
return this;
}

Expand All @@ -657,7 +622,18 @@ public Builder<TYPE> cfg(ConfigurationTransition configTransition) {
Preconditions.checkState(this.configTransition == NoTransition.INSTANCE,
"the configuration transition is already set");
if (configTransition instanceof SplitTransition) {
return cfg(new BasicSplitTransitionProvider((SplitTransition) configTransition));
return cfg(
new TransitionFactory<RuleTransitionData>() {
@Override
public ConfigurationTransition create(RuleTransitionData data) {
return configTransition;
}

@Override
public boolean isSplit() {
return true;
}
});
} else {
this.configTransition = configTransition;
return this;
Expand Down Expand Up @@ -1200,7 +1176,7 @@ public ImmutableAttributeFactory buildPartial() {
configTransition,
allowedRuleClassesForLabels,
allowedRuleClassesForLabelsWarning,
splitTransitionProvider,
splitTransitionFactory,
allowedFileTypesForLabels,
validityPredicate,
valueSource,
Expand Down Expand Up @@ -1951,7 +1927,8 @@ public static LabelListLateBoundDefault<Void> fromRuleAndAttributesOnly(

private final ConfigurationTransition configTransition;

private final SplitTransitionProvider splitTransitionProvider;
// TODO(https://github.com/bazelbuild/bazel/issues/7814): Merge this with configTransition.
private final TransitionFactory<RuleTransitionData> splitTransitionFactory;

/**
* For label or label-list attributes, this predicate returns which rule
Expand Down Expand Up @@ -2009,7 +1986,7 @@ public static LabelListLateBoundDefault<Void> fromRuleAndAttributesOnly(
Set<PropertyFlag> propertyFlags,
Object defaultValue,
ConfigurationTransition configTransition,
SplitTransitionProvider splitTransitionProvider,
TransitionFactory<RuleTransitionData> splitTransitionFactory,
RuleClassNamePredicate allowedRuleClassesForLabels,
RuleClassNamePredicate allowedRuleClassesForLabelsWarning,
FileTypeSet allowedFileTypesForLabels,
Expand Down Expand Up @@ -2041,7 +2018,7 @@ public static LabelListLateBoundDefault<Void> fromRuleAndAttributesOnly(
this.propertyFlags = propertyFlags;
this.defaultValue = defaultValue;
this.configTransition = configTransition;
this.splitTransitionProvider = splitTransitionProvider;
this.splitTransitionFactory = splitTransitionFactory;
this.allowedRuleClassesForLabels = allowedRuleClassesForLabels;
this.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning;
this.allowedFileTypesForLabels = allowedFileTypesForLabels;
Expand All @@ -2058,7 +2035,7 @@ public static LabelListLateBoundDefault<Void> fromRuleAndAttributesOnly(
propertyFlags,
defaultValue,
configTransition,
splitTransitionProvider,
splitTransitionFactory,
allowedRuleClassesForLabels,
allowedRuleClassesForLabelsWarning,
allowedFileTypesForLabels,
Expand Down Expand Up @@ -2187,20 +2164,22 @@ public ConfigurationTransition getConfigurationTransition(AttributeMap usused) {
*/
public SplitTransition getSplitTransition(AttributeMap attributeMapper) {
Preconditions.checkState(hasSplitConfigurationTransition());
return splitTransitionProvider.apply(attributeMapper);
return (SplitTransition)
splitTransitionFactory.create(RuleTransitionData.create(attributeMapper));
}

// TODO(https://github.com/bazelbuild/bazel/issues/7814): Remove this.
@VisibleForTesting
public SplitTransitionProvider getSplitTransitionProviderForTesting() {
return splitTransitionProvider;
public TransitionFactory<RuleTransitionData> getSplitTransitionProviderForTesting() {
return splitTransitionFactory;
}

/**
* Returns true if this attribute transitions on a split transition. See {@link SplitTransition}.
*/
// TODO(https://github.com/bazelbuild/bazel/issues/7814) Remove this.
public boolean hasSplitConfigurationTransition() {
return (splitTransitionProvider != null);
return (splitTransitionFactory != null);
}

/**
Expand Down Expand Up @@ -2288,11 +2267,11 @@ public Predicate<RuleClass> getAllowedRuleClassesPredicate() {
}

/**
* Returns a predicate that evaluates to true for rule classes that are
* allowed labels in this attribute. If this is not a label or label-list
* attribute, the returned predicate always evaluates to true.
* Returns a predicate that evaluates to true for rule classes that are allowed labels in this
* attribute. If this is not a label or label-list attribute, the returned predicate always
* evaluates to true.
*/
// TODO(b/69917891) Remove these methods once checkbuilddeps no longer depends on them.
// TODO(b/69917891): Remove these methods once checkbuilddeps no longer depends on them.
public Predicate<String> getAllowedRuleClassNamesPredicate() {
return allowedRuleClassesForLabels.asPredicateOfRuleClassName();
}
Expand Down Expand Up @@ -2476,7 +2455,7 @@ public boolean equals(Object o) {
&& Objects.equals(propertyFlags, attribute.propertyFlags)
&& Objects.equals(defaultValue, attribute.defaultValue)
&& Objects.equals(configTransition, attribute.configTransition)
&& Objects.equals(splitTransitionProvider, attribute.splitTransitionProvider)
&& Objects.equals(splitTransitionFactory, attribute.splitTransitionFactory)
&& Objects.equals(allowedRuleClassesForLabels, attribute.allowedRuleClassesForLabels)
&& Objects.equals(
allowedRuleClassesForLabelsWarning, attribute.allowedRuleClassesForLabelsWarning)
Expand Down Expand Up @@ -2507,7 +2486,7 @@ public <TYPE> Attribute.Builder<TYPE> cloneBuilder(Type<TYPE> tp) {
builder.validityPredicate = validityPredicate;
builder.condition = condition;
builder.configTransition = configTransition;
builder.splitTransitionProvider = splitTransitionProvider;
builder.splitTransitionFactory = splitTransitionFactory;
builder.propertyFlags = newEnumSet(propertyFlags, PropertyFlag.class);
builder.value = defaultValue;
builder.valueSet = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
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.Location;
import com.google.devtools.build.lib.packages.Attribute.SplitTransitionProvider;
import com.google.devtools.build.lib.packages.NativeProvider;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
Expand All @@ -44,6 +43,7 @@
import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key;
import com.google.devtools.build.lib.skylarkbuildapi.FileApi;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkRuleContextApi;
import com.google.devtools.build.lib.skylarkbuildapi.SplitTransitionProviderApi;
import com.google.devtools.build.lib.skylarkbuildapi.apple.AppleCommonApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.syntax.Environment;
Expand Down Expand Up @@ -175,7 +175,7 @@ public ImmutableMap<String, String> getTargetAppleEnvironment(
}

@Override
public SplitTransitionProvider getMultiArchSplitProvider() {
public SplitTransitionProviderApi getMultiArchSplitProvider() {
return new MultiArchSplitTransitionProvider();
}

Expand Down
Loading

0 comments on commit 0a9e1ed

Please sign in to comment.