diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
5 files changed, 67 insertions, 31 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 777bfc9579..ade168c05d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.actions.CommandLines.CommandLineLimits; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; -import com.google.devtools.build.lib.analysis.config.transitions.ComposingPatchTransition; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.buildeventstream.BuildEventId; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; @@ -1979,7 +1978,8 @@ public class BuildConfiguration { } else if (currentTransition == null) { currentTransition = fragmentTransition; } else { - currentTransition = new ComposingPatchTransition(currentTransition, fragmentTransition); + currentTransition = + TransitionResolver.composePatchTransitions(currentTransition, fragmentTransition); } } return currentTransition; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java index ca23d24e9f..ec8cfc35f6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java @@ -15,9 +15,6 @@ package com.google.devtools.build.lib.analysis.config; -import com.google.common.base.Preconditions; -import com.google.devtools.build.lib.analysis.config.transitions.ComposingPatchTransition; -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.packages.Rule; import com.google.devtools.build.lib.packages.RuleTransitionFactory; @@ -41,17 +38,7 @@ public class ComposingRuleTransitionFactory implements RuleTransitionFactory { @Override public PatchTransition buildTransitionFor(Rule rule) { - PatchTransition transition1 = Preconditions.checkNotNull(rtf1.buildTransitionFor(rule)); - PatchTransition transition2 = Preconditions.checkNotNull(rtf2.buildTransitionFor(rule)); - - if (transition1 == NoTransition.INSTANCE) { - return transition2; - } - - if (transition2 == NoTransition.INSTANCE) { - return transition1; - } - - return new ComposingPatchTransition(transition1, transition2); + return TransitionResolver.composePatchTransitions( + rtf1.buildTransitionFor(rule), rtf2.buildTransitionFor(rule)); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java index 946f9599fb..11bbc27402 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.analysis.config; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.config.transitions.ComposingSplitTransition; @@ -157,9 +156,10 @@ public final class TransitionResolver { /** * Composes two transitions together efficiently. */ - @VisibleForTesting public static ConfigurationTransition composeTransitions(ConfigurationTransition transition1, ConfigurationTransition transition2) { + Preconditions.checkNotNull(transition1); + Preconditions.checkNotNull(transition2); if (isFinal(transition1) || transition2 == NoTransition.INSTANCE) { return transition1; } else if (isFinal(transition2) || transition1 == NoTransition.INSTANCE) { @@ -173,6 +173,28 @@ public final class TransitionResolver { } } + /** Composes two patch transitions together efficiently. */ + public static PatchTransition composePatchTransitions( + PatchTransition transition1, PatchTransition transition2) { + // composeTransitions knows all the tricks for composing transitions, so call into that rather + // than reimplementing it here. It will return one of the following: + // * one of the two input transitions (which is a PatchTransition by this method's signature, + // and needs only be cast) + // -or- + // * a new ComposingSplitTransition of (transition1, transition2) - which consists only of + // PatchTransitions and can thus be converted to a ComposingPatchTransition with asPatch. + ConfigurationTransition composed = composeTransitions(transition1, transition2); + if (composed instanceof ComposingSplitTransition) { + return ((ComposingSplitTransition) composed).asPatch(); + } else if (composed instanceof PatchTransition) { + return (PatchTransition) composed; + } else { + throw new AssertionError( + "composeTransitions returned something other than a ComposingSplitTransition or " + + "one of the input PatchTransitions"); + } + } + /** * Returns true if once the given transition is applied to a dep no followup transitions should * be composed after it. @@ -191,9 +213,7 @@ public final class TransitionResolver { "cannot apply splits after null transitions (null transitions are expected to be final)"); Preconditions.checkState(currentTransition != HostTransition.INSTANCE, "cannot apply splits after host transitions (host transitions are expected to be final)"); - return currentTransition == NoTransition.INSTANCE - ? split - : new ComposingSplitTransition(currentTransition, split); + return composeTransitions(currentTransition, split); } /** @@ -221,14 +241,8 @@ public final class TransitionResolver { return currentTransition; } if (transitionFactory != null) { - PatchTransition ruleClassTransition = - Preconditions.checkNotNull( - transitionFactory.buildTransitionFor(toTarget.getAssociatedRule())); - if (currentTransition == NoTransition.INSTANCE) { - return ruleClassTransition; - } else if (ruleClassTransition != NoTransition.INSTANCE) { - return new ComposingSplitTransition(currentTransition, ruleClassTransition); - } + return composeTransitions( + currentTransition, transitionFactory.buildTransitionFor(toTarget.getAssociatedRule())); } return currentTransition; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingPatchTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingPatchTransition.java index 2f99542151..6703b5c3a7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingPatchTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingPatchTransition.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.analysis.config.transitions; +import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -27,13 +28,23 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; public class ComposingPatchTransition implements PatchTransition { private final ComposingSplitTransition delegate; + /** + * Creates a {@link ComposingPatchTransition} that applies the sequence: {@code fromOptions -> + * transition1 -> transition2 -> toOptions }. + * + * <p>Note that it's possible to create silly transitions with this constructor (e.g., if one or + * both of the transitions is NoTransition). Use composePatchTransitions instead, which checks for + * these states and avoids instantiation appropriately. + * + * @see TransitionResolver#composePatchTransitions + */ public ComposingPatchTransition(PatchTransition transition1, PatchTransition transition2) { this(new ComposingSplitTransition(transition1, transition2)); } @AutoCodec.Instantiator - @AutoCodec.VisibleForSerialization ComposingPatchTransition(ComposingSplitTransition delegate) { + Preconditions.checkArgument(delegate.isPatchOnly()); this.delegate = delegate; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java index 340a5b7b12..73fa1cf4e5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java @@ -48,6 +48,12 @@ public class ComposingSplitTransition implements SplitTransition { /** * Creates a {@link ComposingSplitTransition} that applies the sequence: {@code fromOptions -> * transition1 -> transition2 -> toOptions }. + * + * <p>Note that it's possible to create silly transitions with this constructor (e.g., if one or + * both of the transitions is NoTransition). Use composeTransitions instead, which checks for + * these states and avoids instantiation appropriately. + * + * @see TransitionResolver#composeTransitions */ @AutoCodec.Instantiator public ComposingSplitTransition( @@ -76,6 +82,24 @@ public class ComposingSplitTransition implements SplitTransition { } /** + * Returns whether this transition contains only patches (and is thus suitable as a delegate + * for {@link ComposingPatchTransition}). + */ + public boolean isPatchOnly() { + return transition1 instanceof PatchTransition && transition2 instanceof PatchTransition; + } + + /** + * Allows this transition to be used in patch-only contexts if it contains only + * {@link PatchTransition}s. + * + * <p>Can only be called if {@link #isPatchOnly()} returns true. + */ + public ComposingPatchTransition asPatch() { + return new ComposingPatchTransition(this); + } + + /** * Applies the given transition over the given {@link BuildOptions}, returns the result. */ // TODO(gregce): move this somewhere more general. This isn't intrinsic to composed splits. |