Skip to content

Commit

Permalink
Remove unused NullTransition.
Browse files Browse the repository at this point in the history
Work towards composable starlark transitions: #22248.

PiperOrigin-RevId: 636209749
Change-Id: I9ecdec8b376a9c95b06dea36bacbf9268d0c5195
  • Loading branch information
katre authored and copybara-github committed May 22, 2024
1 parent 89432df commit 56739f4
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 166 deletions.
19 changes: 0 additions & 19 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ java_library(
":config/transitions/composing_transition_factory",
":config/transitions/configuration_transition",
":config/transitions/no_transition",
":config/transitions/null_transition",
":config/transitions/patch_transition",
":config/transitions/split_transition",
":config/transitions/transition_factory",
Expand Down Expand Up @@ -2050,7 +2049,6 @@ java_library(
deps = [
":config/transitions/configuration_transition",
":config/transitions/no_transition",
":config/transitions/null_transition",
":config/transitions/split_transition",
":config/transitions/transition_factory",
"//third_party:auto_value",
Expand Down Expand Up @@ -2119,7 +2117,6 @@ java_library(
":config/build_options",
":config/transitions/configuration_transition",
":config/transitions/no_transition",
":config/transitions/null_transition",
":required_config_fragments_provider",
"//src/main/java/com/google/devtools/build/lib/events",
"//third_party:guava",
Expand All @@ -2133,7 +2130,6 @@ java_library(
":config/transitions/composing_transition",
":config/transitions/configuration_transition",
":config/transitions/no_transition",
":config/transitions/null_transition",
":config/transitions/transition_factory",
"//third_party:auto_value",
"//third_party:guava",
Expand Down Expand Up @@ -2172,21 +2168,6 @@ java_library(
],
)

java_library(
name = "config/transitions/null_transition",
srcs = ["config/transitions/NullTransition.java"],
deps = [
":config/build_options",
":config/transitions/configuration_transition",
":config/transitions/patch_transition",
":config/transitions/transition_factory",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//third_party:auto_value",
],
)

java_library(
name = "config/transitions/no_config_transition",
srcs = ["config/transitions/NoConfigTransition.java"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.google.auto.value.AutoValue;
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.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;

Expand All @@ -33,8 +32,6 @@ public static <T extends TransitionFactory.Data> TransitionFactory<T> of(
ConfigurationTransition transition) {
if (transition instanceof NoTransition) {
return NoTransition.createFactory();
} else if (transition instanceof NullTransition) {
return NullTransition.createFactory();
} else if (transition instanceof SplitTransition splitTransition) {
return split(splitTransition);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,29 +119,17 @@ public static ConfigurationTransition of(
Preconditions.checkNotNull(transition1);
Preconditions.checkNotNull(transition2);

if (isFinal(transition1)) {
// Since no other transition can be composed with transition1, use it directly.
return transition1;
} else if (transition1 == NoTransition.INSTANCE) {
if (transition1 == NoTransition.INSTANCE) {
// Since transition1 causes no changes, use transition2 directly.
return transition2;
}

if (transition2 == NoTransition.INSTANCE) {
} else if (transition2 == NoTransition.INSTANCE) {
// Since transition2 causes no changes, use transition 1 directly.
return transition1;
} else if (isFinal(transition2)) {
// When the second transition is null, there's no need to compose.
return transition2;
}

return new ComposingTransition(transition1, transition2);
}

private static boolean isFinal(ConfigurationTransition transition) {
return transition == NullTransition.INSTANCE;
}

/**
* Composes a new key out of two given keys. Composing two split transitions is not allowed at the
* moment, so what this essentially does are (1) make sure not both transitions are split and (2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,30 +51,17 @@ public static <T extends TransitionFactory.Data> TransitionFactory<T> of(
!transitionFactory1.isSplit() || !transitionFactory2.isSplit(),
"can't compose two split transition factories");

if (isFinal(transitionFactory1)) {
// Since no other transition can be composed with transitionFactory1, use it directly.
return transitionFactory1;
} else if (NoTransition.isInstance(transitionFactory1)) {
if (NoTransition.isInstance(transitionFactory1)) {
// Since transitionFactory1 causes no changes, use transitionFactory2 directly.
return transitionFactory2;
}

if (NoTransition.isInstance(transitionFactory2)) {
} else if (NoTransition.isInstance(transitionFactory2)) {
// Since transitionFactory2 causes no changes, use transitionFactory1 directly.
return transitionFactory1;
} else if (isFinal(transitionFactory2)) {
// When the second transition is null there's no need to compose.
return transitionFactory2;
}

return create(transitionFactory1, transitionFactory2);
}

private static <T extends TransitionFactory.Data> boolean isFinal(
TransitionFactory<T> transitionFactory) {
return NullTransition.isInstance(transitionFactory);
}

private static <T extends TransitionFactory.Data> TransitionFactory<T> create(
TransitionFactory<T> transitionFactory1, TransitionFactory<T> transitionFactory2) {
return new AutoValue_ComposingTransitionFactory<T>(transitionFactory1, transitionFactory2);
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/query2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/composing_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/null_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,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.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.constraints.IncompatibleTargetChecker.IncompatibleTargetException;
import com.google.devtools.build.lib.cmdline.Label;
Expand All @@ -55,9 +54,10 @@
import javax.annotation.Nullable;

/**
* TransitionResolver resolves the dependencies of a {@link ConfiguredTarget}, reporting which
* configurations its dependencies are actually needed in according to the transitions applied to
* them. See {@link TransitionsOutputFormatterCallback}.
* TransitionResolver resolves the dependencies of a {@link
* com.google.devtools.build.lib.analysis.ConfiguredTarget}, reporting which configurations its
* dependencies are actually needed in according to the transitions applied to them. See {@link
* TransitionsOutputFormatterCallback}.
*/
public class CqueryTransitionResolver {

Expand Down Expand Up @@ -187,15 +187,12 @@ public ImmutableSet<ResolvedTransition> dependencies(CqueryNode configuredTarget
Label label = labelEntry.getKey();
Collection<ConfiguredTargetAndData> targets = labelEntry.getValue();

ConfigurationTransition noOrNullTransition =
getTransitionIfNoOrNull(configuration, targets);
if (noOrNullTransition != null) {
// The most common case, so short-circuit this.
String transitionName = usesNoTransition(configuration, targets);
if (transitionName != null) {
resolved.add(
ResolvedTransition.create(
label,
/* buildOptions= */ ImmutableList.of(),
dependencyName,
noOrNullTransition.getName()));
label, /* buildOptions= */ ImmutableList.of(), dependencyName, transitionName));
continue;
}

Expand Down Expand Up @@ -224,15 +221,16 @@ private EvaluateException(String message) {
}

@Nullable
private static ConfigurationTransition getTransitionIfNoOrNull(
private static String usesNoTransition(
BuildConfigurationValue fromConfiguration, Collection<ConfiguredTargetAndData> targets) {
ConfiguredTargetAndData first = targets.iterator().next();
// Check whether the configuration changed.
if (targets.size() == 1 && Objects.equals(fromConfiguration, first.getConfiguration())) {
return NoTransition.INSTANCE;
return NoTransition.INSTANCE.getName();
}
// If any target has a null configuration, they all do, so it's sufficient to check the first.
if (first.getConfiguration() == null) {
return NullTransition.INSTANCE;
return "(null transition)";
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/run_under_converter",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/null_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/split_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import org.junit.Test;
Expand All @@ -37,15 +36,6 @@ public void noTransition() {
assertThat(factory.isSplit()).isFalse();
}

@Test
public void nullTransition() {
TransitionFactory<TransitionFactory.Data> factory =
TransitionFactories.of(NullTransition.INSTANCE);
assertThat(factory).isNotNull();
assertThat(NullTransition.isInstance(factory)).isTrue();
assertThat(factory.isSplit()).isFalse();
}

@Test
public void splitTransition() {
TransitionFactory<TransitionFactory.Data> factory =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_config_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/null_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/split_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,28 +157,6 @@ public void compose_noTrans_second() {
assertThat(composed).isEqualTo(patch);
}

@Test
public void compose_nullTrans_first() {
StubPatch patch = new StubPatch(FLAG_1, "value");
TransitionFactory<StubData> composed =
ComposingTransitionFactory.of(
NullTransition.createFactory(), TransitionFactories.of(patch));

assertThat(composed).isNotNull();
assertThat(NullTransition.isInstance(composed)).isTrue();
}

@Test
public void compose_nullTrans_second() {
StubPatch patch = new StubPatch(FLAG_1, "value");
TransitionFactory<StubData> composed =
ComposingTransitionFactory.of(
TransitionFactories.of(patch), NullTransition.createFactory());

assertThat(composed).isNotNull();
assertThat(NullTransition.isInstance(composed)).isTrue();
}

private static final class StubData implements TransitionFactory.Data {}

// Helper methods and classes for the tests.
Expand Down

0 comments on commit 56739f4

Please sign in to comment.