aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/test/java/com/google/devtools/build/lib/analysis
diff options
context:
space:
mode:
authorGravatar gregce <gregce@google.com>2017-08-09 23:04:40 +0200
committerGravatar Marcel Hlopko <hlopko@google.com>2017-08-10 13:48:01 +0200
commit44b71aae7afa1a1d01a7d2e0cb7549d0cae3b5d2 (patch)
tree7d1bb82d27aa6eab2b1b9fe1ac71e9274c14f080 /src/test/java/com/google/devtools/build/lib/analysis
parent7740f68c7ed2b925628f2a60e774b5c04ca5bbed (diff)
Remove --experimental_dynamic_configs=off.
This is the first in a series of changes stripping out Bazel's static configuration code. This change removes the ability to request static configurations but not the (now orphaned) logic behind them. Because that logic is all over the code base, it will be removed in layers in followup changes. PiperOrigin-RevId: 164769846
Diffstat (limited to 'src/test/java/com/google/devtools/build/lib/analysis')
-rw-r--r--src/test/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTargetTest.java84
-rw-r--r--src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java12
2 files changed, 4 insertions, 92 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTargetTest.java
index 228472c30f..df2abf27ec 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTargetTest.java
@@ -44,90 +44,6 @@ public class OutputFileConfiguredTargetTest extends BuildViewTestBase {
}
/**
- * This regression-tests the fix for a bug in which Bazel could crash with a NullPointerException
- * when two builds were invoked with different configurations because an output file ended up
- * unexpectedly having a null generating rule.
- *
- * <p>The reasons for this are subtle and complex. In short, the bug only happens when
- * --experimental_dynamic_configs=off and BuildConfiguration.equals provides value equality.
- *
- * <p>In that scenario, when we call {@code bazel build //foo:gen1 --copt a=b}, Bazel creates a
- * host configuration H1 for this build that attaches to {@code
- * //foo:host_generated_file_producer} (which is the generator of {@code host_src1.cc}).
- *
- * <p>When we then call {@code bazel build //foo:gen2 --copt a=c}, the options have changed so
- * Bazel clears all configurations and configured targets. This includes the host configuration,
- * even though - importantly - none of its options have changed. So Bazel uses a new host config
- * H2 that's value-equal to H1 (but not reference-equal) and assigns it to {@code host_src2.cc}.
- *
- * <p>Here's the problem: during configured target analysis, Bazel creates the configured target
- * <host_src2.cc, H2>, then requests from Skyframe the dependency with SkyKey
- * <host_generated_file_producer, H2>. But Skyframe uses an interner to reference-collapse
- * equivalent keys (SkyKey.SKY_KEY_INTERNER}). And that doesn't get cleared between builds. So
- * the actual SkyKey used is <host_generated_file_producer, H1>, which produces a ConfiguredTarget
- * with config=H1. This breaks {@link TargetContext#findDirectPrerequisite}, which output files
- * use to find their generating rules. That method looks for
- * {@code label.equals("host_generated_file_producer")} and {@code config == H2}, which finds
- * no match so returns a null reference.
- *
- * <p>When configs only use reference equality, the problem goes away because the interner can no
- * longer safely merge H1 and H2. An alternative fix might be to change
- * {@link TargetContext#findDirectPrerequisite} to check {@code config.equals(H2)} instead of
- * {@code config == H2}. But this isn't really safe because static configs embed references to
- * other configs in their transition table. So returning H1 when you expect H2 creates the
- * possibility of "leaking out" to the wrong configuration during a transition, even if H1 and H2
- * have the same exact build options.
- *
- * <p>All of these problems go away with dynamic configurations. This is for two reasons: 1)
- * dynamic configs don't embed transition tables, so "leaking out" is no longer possible. And
- * 2) a dynamic config is evaluated through Skyframe keyed on its BuildOptions, so semantic
- * equality implies reference equality anyway, 2 may change in the future, but if/when it does
- * this test should reliably fail so appropriate measures can be taken.
- */
- @Test
- public void regressionTestForStaticConfigsWithValueEquality() throws Exception {
- scratch.file("foo/BUILD",
- "genrule(",
- " name = 'host_generated_file_producer',",
- " srcs = [],",
- " outs = [",
- " 'host_src1.cc',",
- " 'host_src2.cc',",
- " ],",
- " cmd = 'echo hi > $(location host_src1.cc); echo hi > $(location host_src2.cc)')",
- "",
- "cc_binary(",
- " name = 'host_generated_file_consumer1',",
- " srcs = ['host_src1.cc'])",
- "",
- "cc_binary(",
- " name = 'host_generated_file_consumer2',",
- " srcs = ['host_src2.cc'])",
- "",
- "genrule(",
- " name = 'gen1',",
- " srcs = [],",
- " outs = ['gen1.out'],",
- " cmd = 'echo hi > $@',",
- " tools = [':host_generated_file_consumer1'])",
- "",
- "genrule(",
- " name = 'gen2',",
- " srcs = [],",
- " outs = ['gen2.out'],",
- " cmd = 'echo hi > $@',",
- " tools = [':host_generated_file_consumer2'])"
- );
- useConfiguration("--copt", "a=b");
- update("//foo:gen1");
- useConfiguration("--copt", "a=c");
- update("//foo:gen2");
- OutputFileConfiguredTarget hostSrc2 = (OutputFileConfiguredTarget)
- getConfiguredTarget("//foo:host_src2.cc", getHostConfiguration());
- assertThat(hostSrc2.getGeneratingRule()).isNotNull();
- }
-
- /**
* Dynamic configurations maintain a host configuration cache that stores configurations
* instantiated outside of Skyframe. We shouldn't, in general, instantiate configurations
* outside of {@link BuildConfigurationValue}. But in this specific case Bazel performance
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 99f8d118ce..4b31bbd1cf 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
@@ -180,7 +180,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
protected BuildConfigurationCollection masterConfig;
protected BuildConfiguration targetConfig; // "target" or "build" config
private List<String> configurationArgs;
- private DynamicConfigsMode dynamicConfigsMode = DynamicConfigsMode.OFF;
+ private DynamicConfigsMode dynamicConfigsMode = DynamicConfigsMode.NOTRIM;
protected OptionsParser optionsParser;
private PackageCacheOptions packageCacheOptions;
@@ -442,13 +442,9 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
*/
protected void useConfiguration(String... args) throws Exception {
String[] actualArgs;
- if (dynamicConfigsMode != DynamicConfigsMode.OFF) {
- actualArgs = Arrays.copyOf(args, args.length + 1);
- actualArgs[args.length] = "--experimental_dynamic_configs="
- + dynamicConfigsMode.toString().toLowerCase();
- } else {
- actualArgs = args;
- }
+ actualArgs = Arrays.copyOf(args, args.length + 1);
+ actualArgs[args.length] = "--experimental_dynamic_configs="
+ + dynamicConfigsMode.toString().toLowerCase();
masterConfig = createConfigurations(actualArgs);
targetConfig = getTargetConfiguration();
configurationArgs = Arrays.asList(actualArgs);