Unit tests respect top-level rule-class transitions.
--
PiperOrigin-RevId: 151129669
MOS_MIGRATED_REVID=151129669
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 6043cda..3431013 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -54,8 +54,12 @@
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
+import com.google.devtools.build.lib.packages.Attribute.Transition;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.NativeAspectClass;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.PackageSpecification;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
@@ -1088,14 +1092,39 @@
return result;
}
+ private Transition getTopLevelTransitionForTarget(Label label, ExtendedEventHandler handler) {
+ Rule rule;
+ try {
+ rule = skyframeExecutor
+ .getPackageManager()
+ .getTarget(handler, label)
+ .getAssociatedRule();
+ } catch (NoSuchPackageException | NoSuchTargetException e) {
+ return ConfigurationTransition.NONE;
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new AssertionError("Configuration of " + label + " interrupted");
+ }
+ if (rule == null) {
+ return ConfigurationTransition.NONE;
+ }
+ RuleTransitionFactory factory = rule
+ .getRuleClassObject()
+ .getTransitionFactory();
+ return (factory == null) ? ConfigurationTransition.NONE : factory.buildTransitionFor(rule);
+ }
+
/**
- * Returns a configured target for the specified target and configuration. Returns {@code null} if
- * something goes wrong.
+ * Returns a configured target for the specified target and configuration. If dynamic
+ * configurations are activated, and the target in question has a top-level rule class transition,
+ * that transition is applied in the returned ConfiguredTarget. Returns {@code null} if something
+ * goes wrong.
*/
@VisibleForTesting
public ConfiguredTarget getConfiguredTargetForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config) {
- return skyframeExecutor.getConfiguredTargetForTesting(eventHandler, label, config);
+ return skyframeExecutor.getConfiguredTargetForTesting(eventHandler, label, config,
+ getTopLevelTransitionForTarget(label, eventHandler));
}
/**
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 2dc660f..c6d088d 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
@@ -82,6 +82,7 @@
import com.google.devtools.build.lib.exec.OutputService;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -1442,13 +1443,32 @@
/**
* Returns a particular configured target.
- *
- * <p>Used only for testing.
*/
@VisibleForTesting
@Nullable
public ConfiguredTarget getConfiguredTargetForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration configuration) {
+ return getConfiguredTargetForTesting(
+ eventHandler, label, configuration, ConfigurationTransition.NONE);
+ }
+
+ /**
+ * Returns a particular configured target. If dynamic configurations are active, applies the given
+ * configuration transition.
+ */
+ @VisibleForTesting
+ @Nullable
+ public ConfiguredTarget getConfiguredTargetForTesting(
+ ExtendedEventHandler eventHandler,
+ Label label,
+ BuildConfiguration configuration,
+ Attribute.Transition transition) {
+
+ Preconditions.checkArgument(configuration == null
+ || configuration.useDynamicConfigurations()
+ || transition == ConfigurationTransition.NONE,
+ "Dynamic configurations required for test configuration using a transition");
+
if (memoizingEvaluator.getExistingValueForTesting(
PrecomputedValue.WORKSPACE_STATUS_KEY.getKeyForTesting()) == null) {
injectWorkspaceStatusData(label.getWorkspaceRoot());
@@ -1458,8 +1478,7 @@
if (configuration == null) {
dep = Dependency.withNullConfiguration(label);
} else if (configuration.useDynamicConfigurations()) {
- dep = Dependency.withTransitionAndAspects(label, Attribute.ConfigurationTransition.NONE,
- AspectCollection.EMPTY);
+ dep = Dependency.withTransitionAndAspects(label, transition, AspectCollection.EMPTY);
} else {
dep = Dependency.withConfiguration(label, configuration);
}
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 20e767b..e827137 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
@@ -677,7 +677,8 @@
/**
* Returns the ConfiguredTarget for the specified label, configured for the "build" (aka "target")
- * configuration.
+ * configuration. If the label corresponds to a target with a top-level configuration transition,
+ * that transition is applied to the given config in the returned ConfiguredTarget.
*/
public ConfiguredTarget getConfiguredTarget(String label)
throws LabelSyntaxException {
@@ -685,8 +686,9 @@
}
/**
- * Returns the ConfiguredTarget for the specified label, using the
- * given build configuration.
+ * Returns the ConfiguredTarget for the specified label, using the given build configuration. If
+ * the label corresponds to a target with a top-level configuration transition, that transition is
+ * applied to the given config in the returned ConfiguredTarget.
*/
protected ConfiguredTarget getConfiguredTarget(String label, BuildConfiguration config)
throws LabelSyntaxException {
@@ -695,7 +697,8 @@
/**
* Returns the ConfiguredTarget for the specified label, using the
- * given build configuration.
+ * given build configuration. If the label corresponds to a target with a top-level configuration
+ * transition, that transition is applied to the given config in the returned ConfiguredTarget.
*
* <p>If the evaluation of the SkyKey corresponding to the configured target fails, this
* method may return null. In that case, use a debugger to inspect the {@link ErrorInfo}
@@ -704,8 +707,7 @@
* {@link SkyframeExecutor#getConfiguredTargetForTesting}. See also b/26382502.
*/
protected ConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration config) {
- return view.getConfiguredTargetForTesting(
- reporter, BlazeTestUtils.convertLabel(label), config);
+ return view.getConfiguredTargetForTesting(reporter, BlazeTestUtils.convertLabel(label), config);
}
/**
@@ -969,7 +971,11 @@
* be "foo.o".
*/
protected Artifact getBinArtifact(String packageRelativePath, String owner) {
- return getBinArtifact(packageRelativePath, makeLabelAndConfiguration(owner));
+ ConfiguredTargetKey config = makeLabelAndConfiguration(owner);
+ return getPackageRelativeDerivedArtifact(
+ packageRelativePath,
+ config.getConfiguration().getBinDirectory(RepositoryName.MAIN),
+ config);
}
/**
@@ -1025,18 +1031,6 @@
}
/**
- * Gets a derived Artifact for testing in the subdirectory of the {@link
- * BuildConfiguration#getBinDirectory} corresponding to the package of {@code owner}. So
- * to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just
- * be "foo.o".
- */
- private Artifact getBinArtifact(String packageRelativePath, ArtifactOwner owner) {
- return getPackageRelativeDerivedArtifact(packageRelativePath,
- targetConfig.getBinDirectory(RepositoryName.MAIN),
- owner);
- }
-
- /**
* Gets a derived Artifact for testing in the {@link BuildConfiguration#getGenfilesDirectory}.
* This method should only be used for tests that do no analysis, and so there is no
* ConfiguredTarget to own this artifact. If the test runs the analysis phase, {@link
@@ -1055,7 +1049,8 @@
* just be "foo.o".
*/
protected Artifact getGenfilesArtifact(String packageRelativePath, String owner) {
- return getGenfilesArtifact(packageRelativePath, makeLabelAndConfiguration(owner));
+ ConfiguredTargetKey configKey = makeLabelAndConfiguration(owner);
+ return getGenfilesArtifact(packageRelativePath, configKey, configKey.getConfiguration());
}
/**
@@ -1065,7 +1060,8 @@
* just be "foo.o".
*/
protected Artifact getGenfilesArtifact(String packageRelativePath, ConfiguredTarget owner) {
- return getGenfilesArtifact(packageRelativePath, new ConfiguredTargetKey(owner));
+ ConfiguredTargetKey configKey = new ConfiguredTargetKey(owner);
+ return getGenfilesArtifact(packageRelativePath, configKey, configKey.getConfiguration());
}
/**
@@ -1114,14 +1110,14 @@
/**
* Gets a derived Artifact for testing in the subdirectory of the {@link
- * BuildConfiguration#getGenfilesDirectory} corresponding to the package of {@code owner}.
- * So to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should
- * just be "foo.o".
+ * BuildConfiguration#getGenfilesDirectory} corresponding to the package of {@code owner}. So to
+ * specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just be
+ * "foo.o".
*/
- private Artifact getGenfilesArtifact(String packageRelativePath, ArtifactOwner owner) {
- return getPackageRelativeDerivedArtifact(packageRelativePath,
- targetConfig.getGenfilesDirectory(RepositoryName.MAIN),
- owner);
+ private Artifact getGenfilesArtifact(
+ String packageRelativePath, ArtifactOwner owner, BuildConfiguration config) {
+ return getPackageRelativeDerivedArtifact(
+ packageRelativePath, config.getGenfilesDirectory(RepositoryName.MAIN), owner);
}
/**
@@ -1246,11 +1242,17 @@
}
private ConfiguredTargetKey makeLabelAndConfiguration(String label) {
- BuildConfiguration config = targetConfig;
+ BuildConfiguration config;
+ try {
+ config = getConfiguredTarget(label).getConfiguration();
+ } catch (LabelSyntaxException e) {
+ throw new IllegalArgumentException(e);
+ }
if (targetConfig.useDynamicConfigurations()) {
try {
- config = view.getDynamicConfigurationForTesting(getTarget(label), targetConfig, reporter);
+ config = view.getDynamicConfigurationForTesting(getTarget(label), config, reporter);
} catch (Exception e) {
+ //TODO(b/36585204): Clean this up
throw new RuntimeException(e);
}
}
