Make tests that mess with the WORKSPACE file work with untrimmed dynamic configs.
They already work with static configs because those configs get created in
test setup, before WORKSPACE is modified. That means they get the necessary
bindings needed to load their fragments (e.g. local_config_xcode for
AppleConfiguration).
But dynamic configs get instantiated on-demand. Which means in this case
*after* WORKSPACE is overwritten. So they can't find the bindings and fail.
This fix has offending tests *append* WORKSPACE instead of overwrite.
This unblocks defaulting --experimental_dynamic_configs to "notrim_partial".
--
PiperOrigin-RevId: 147377195
MOS_MIGRATED_REVID=147377195
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 76d24e0..438c0d4 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -23,6 +23,7 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
import static org.junit.Assert.fail;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
@@ -137,7 +138,10 @@
"honest(name='b', foo=[])");
scratch.overwriteFile("WORKSPACE",
- "bind(name='b', actual='//a:b')");
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("bind(name='b', actual='//a:b')")
+ .build());
skyframeExecutor.invalidateFilesUnderPathForTesting(reporter,
ModifiedFileSet.EVERYTHING_MODIFIED, rootDirectory);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index aff2791..9ca17e7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -58,24 +58,27 @@
}
@Override
- public void setupMockClient(MockToolsConfig config) throws IOException {
+ public List<String> getWorkspaceContents(MockToolsConfig config) {
String bazelToolWorkspace = config.getPath("/bazel_tools_workspace").getPathString();
- ArrayList<String> workspaceContents =
- new ArrayList<>(
- ImmutableList.of(
- "local_repository(name = 'bazel_tools', path = '" + bazelToolWorkspace + "')",
- "local_repository(name = 'local_config_xcode', path = '/local_config_xcode')",
- "bind(",
- " name = 'objc_proto_lib',",
- " actual = '//objcproto:ProtocolBuffers_lib',",
- ")",
- "bind(",
- " name = 'objc_protobuf_lib',",
- " actual = '//objcproto:protobuf_lib',",
- ")",
- "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')",
- "bind(name = 'tools/python', actual='//tools/python')"));
+ return new ArrayList<>(
+ ImmutableList.of(
+ "local_repository(name = 'bazel_tools', path = '" + bazelToolWorkspace + "')",
+ "local_repository(name = 'local_config_xcode', path = '/local_config_xcode')",
+ "bind(",
+ " name = 'objc_proto_lib',",
+ " actual = '//objcproto:ProtocolBuffers_lib',",
+ ")",
+ "bind(",
+ " name = 'objc_protobuf_lib',",
+ " actual = '//objcproto:protobuf_lib',",
+ ")",
+ "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')",
+ "bind(name = 'tools/python', actual='//tools/python')"));
+ }
+ @Override
+ public void setupMockClient(MockToolsConfig config) throws IOException {
+ List<String> workspaceContents = getWorkspaceContents(config);
config.create(
"/local_config_xcode/BUILD", "xcode_config(name = 'host_xcodes')");
config.overwrite("WORKSPACE", workspaceContents.toArray(new String[workspaceContents.size()]));
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
index 04d1bd2..3d47097 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
@@ -88,6 +88,11 @@
public abstract void setupMockClient(MockToolsConfig mockToolsConfig) throws IOException;
/**
+ * Returns the contents of WORKSPACE.
+ */
+ public abstract List<String> getWorkspaceContents(MockToolsConfig config);
+
+ /**
* This is called from test setup to create any necessary mock workspace files in the
* <code>_embedded_binaries</code> directory.
*/
@@ -141,6 +146,11 @@
}
@Override
+ public List<String> getWorkspaceContents(MockToolsConfig mockToolsConfig) {
+ return delegate.getWorkspaceContents(mockToolsConfig);
+ }
+
+ @Override
public void setupMockWorkspaceFiles(Path embeddedBinariesRoot) throws IOException {
delegate.setupMockWorkspaceFiles(embeddedBinariesRoot);
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
index 915ec37..1ffa3bd 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -133,10 +134,12 @@
" local=True,",
" attrs={'path': attr.string(mandatory=True)})");
scratch.file(rootDirectory.getRelative("BUILD").getPathString());
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "load('//:def.bzl', 'repo')",
- "repo(name='foo', path='/repo2')");
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("load('//:def.bzl', 'repo')")
+ .add("repo(name='foo', path='/repo2')")
+ .build());
invalidatePackages();
ConfiguredTarget target = getConfiguredTarget("@foo//:bar");
Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -158,11 +161,13 @@
" implementation=_impl,",
" local=True)");
scratch.file(rootDirectory.getRelative("BUILD").getPathString());
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "local_repository(name='repo2', path='/repo2')",
- "load('//:def.bzl', 'repo')",
- "repo(name='foo')");
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='repo2', path='/repo2')")
+ .add("load('//:def.bzl', 'repo')")
+ .add("repo(name='foo')")
+ .build());
invalidatePackages();
ConfiguredTarget target = getConfiguredTarget("@foo//:bar");
Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -185,11 +190,13 @@
" implementation=_impl,",
" local=True)");
scratch.file(rootDirectory.getRelative("BUILD").getPathString());
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "local_repository(name='repo2', path='/repo2')",
- "load('//:def.bzl', 'repo')",
- "repo(name='foo')");
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='repo2', path='/repo2')")
+ .add("load('//:def.bzl', 'repo')")
+ .add("repo(name='foo')")
+ .build());
invalidatePackages();
ConfiguredTarget target = getConfiguredTarget("@foo//:bar");
Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -213,11 +220,13 @@
" implementation=_impl,",
" local=True)");
scratch.file(rootDirectory.getRelative("BUILD").getPathString());
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "local_repository(name='repo2', path='/repo2')",
- "load('//:def.bzl', 'repo')",
- "repo(name='foobar')");
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='repo2', path='/repo2')")
+ .add("load('//:def.bzl', 'repo')")
+ .add("repo(name='foobar')")
+ .build());
invalidatePackages();
ConfiguredTarget target = getConfiguredTarget("@foobar//:bar");
Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -305,12 +314,15 @@
scratch.file("/repo2/BUILD", "exports_files_(['data.txt'])");
scratch.file("/repo2/def.bzl", "def macro():", " print('bleh')");
scratch.file("/repo2/WORKSPACE");
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "local_repository(name='bleh')",
- "local_repository(name='foo', path='/repo2')",
- "load('@foo//:def.bzl', 'repo')",
- "repo(name='foobar')");
+
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='bleh')")
+ .add("local_repository(name='foo', path='/repo2')")
+ .add("load('@foo//:def.bzl', 'repo')")
+ .add("repo(name='foobar')")
+ .build());
try {
invalidatePackages();
getTarget("@foo//:data.txt");
@@ -325,11 +337,12 @@
@Test
public void testLoadDoesNotHideWorkspaceFunction() throws Exception {
scratch.file("def.bzl", "def macro():", " print('bleh')");
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "workspace(name='bleh')",
- "local_repository(name='bazel_tools', path=__workspace_dir__)",
- "load('//:def.bzl', 'macro')");
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("workspace(name='bleh')")
+ .add("load('//:def.bzl', 'macro')")
+ .build());
scratch.file("data.txt");
scratch.file("BUILD", "filegroup(", " name='files', ", " srcs=['data.txt'])");
invalidatePackages();
@@ -365,15 +378,17 @@
@Test
public void testMultipleLoadSameExtension() throws Exception {
- scratch.overwriteFile(
- rootDirectory.getRelative("WORKSPACE").getPathString(),
- "load('//:def.bzl', 'f1')",
- "f1()",
- "load('//:def.bzl', 'f2')",
- "f2()",
- "load('//:def.bzl', 'f1')",
- "f1()",
- "local_repository(name = 'foo', path = '')");
+ scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("load('//:def.bzl', 'f1')")
+ .add("f1()")
+ .add("load('//:def.bzl', 'f2')")
+ .add("f2()")
+ .add("load('//:def.bzl', 'f1')")
+ .add("f1()")
+ .add("local_repository(name = 'foo', path = '')")
+ .build());
scratch.file(
rootDirectory.getRelative("BUILD").getPathString(), "filegroup(name = 'bar', srcs = [])");
scratch.file(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 063da16..078d6af 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -339,7 +339,11 @@
public void testPackageBoundaryError_ExternalRepository() throws Exception {
scratch.file("/r/BUILD", "cc_library(name = 'cclib',", " srcs = ['sub/my_sub_lib.h'])");
scratch.file("/r/sub/BUILD", "cc_library(name = 'my_sub_lib', srcs = ['my_sub_lib.h'])");
- scratch.overwriteFile("WORKSPACE", "local_repository(name='r', path='/r')");
+ scratch.overwriteFile("WORKSPACE",
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='r', path='/r')")
+ .build());
invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
reporter.removeHandler(failFastHandler);
getConfiguredTarget("@r//:cclib");
@@ -825,7 +829,10 @@
"external_rule(name='r')");
scratch.overwriteFile("WORKSPACE",
- "local_repository(name='r', path='/r')");
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='r', path='/r')")
+ .build());
invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
SkylarkRuleContext context = createRuleContext("@r//a:r");
@@ -856,7 +863,10 @@
"external_rule(name='r')");
scratch.overwriteFile("WORKSPACE",
- "local_repository(name='r', path='/r')");
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='r', path='/r')")
+ .build());
invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
SkylarkRuleContext context = createRuleContext("@r//a:r");
@@ -885,15 +895,19 @@
" print(name + ': ' + path)"
);
scratch.file("BUILD");
- scratch.overwriteFile(
- "WORKSPACE",
- "local_repository(name='r2', path='/r2')",
- "load('@r2//:test.bzl', 'macro')",
- "macro('r1', '/r1')",
- "NEXT_NAME = 'r3'",
- "load('@r2//:other_test.bzl', 'other_macro')", // We can still refer to r2 in other chunks.
- "macro(NEXT_NAME, '/r2')" // and we can still use macro outside of its chunk.
- );
+
+ scratch.overwriteFile("WORKSPACE",
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='r2', path='/r2')")
+ .add("load('@r2//:test.bzl', 'macro')")
+ .add("macro('r1', '/r1')")
+ .add("NEXT_NAME = 'r3'")
+ // We can still refer to r2 in other chunks:
+ .add("load('@r2//:other_test.bzl', 'other_macro')")
+ .add("macro(NEXT_NAME, '/r2')") // and we can still use macro outside of its chunk.
+ .build());
+
invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
assertThat(getConfiguredTarget("@r1//:test")).isNotNull();
}
@@ -908,10 +922,13 @@
scratch.file("/baz/WORKSPACE");
scratch.file("/baz/baz.txt");
scratch.file("/baz/BUILD", "filegroup(name = 'baz', srcs = ['baz.txt'])");
- scratch.overwriteFile(
- "WORKSPACE",
- "local_repository(name = 'foo', path = '/bar')",
- "local_repository(name = 'foo', path = '/baz')");
+ scratch.overwriteFile("WORKSPACE",
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name = 'foo', path = '/bar')")
+ .add("local_repository(name = 'foo', path = '/baz')")
+ .build());
+
invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
assertThat(
(List<Label>)
@@ -924,19 +941,23 @@
scratch.overwriteFile("BUILD");
scratch.overwriteFile("bar.bzl", "dummy = 1");
- scratch.overwriteFile(
- "WORKSPACE",
- "local_repository(name = 'foo', path = '/bar')",
- "load('//:bar.bzl', 'dummy')",
- "local_repository(name = 'foo', path = '/baz')");
+
+ scratch.overwriteFile("WORKSPACE",
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name = 'foo', path = '/bar')")
+ .add("load('//:bar.bzl', 'dummy')")
+ .add("local_repository(name = 'foo', path = '/baz')")
+ .build());
+
try {
invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchains.
createRuleContext("@foo//:baz");
fail("Should have failed because repository 'foo' is overloading after a load!");
} catch (Exception ex) {
assertContainsEvent(
- "ERROR /workspace/WORKSPACE:3:1: Cannot redefine repository after any load statement "
- + "in the WORKSPACE file (for repository 'foo')");
+ "Cannot redefine repository after any load statement in the WORKSPACE file "
+ + "(for repository 'foo')");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java b/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
index 6004661..0cca601 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
@@ -24,6 +24,7 @@
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
+import java.util.Collection;
/**
* Allow tests to easily manage scratch files in a FileSystem.
@@ -141,6 +142,14 @@
* Like {@code scratch.file}, but the file is first deleted if it already
* exists.
*/
+ public Path overwriteFile(String pathName, Collection<String> lines) throws IOException {
+ return overwriteFile(pathName, lines.toArray(new String[lines.size()]));
+ }
+
+ /**
+ * Like {@code scratch.file}, but the file is first deleted if it already
+ * exists.
+ */
public Path overwriteFile(String pathName, String... lines) throws IOException {
return overwriteFile(pathName, DEFAULT_CHARSET, lines);
}
