Allow BazelPackageLoader to load external repositories.
Also, disallow BazelPackageLoader from fetching missing external repos.
Integration tests for BazelPackageLoader wrt external repos will be left for a follow-up CL.
RELNOTES: None.
PiperOrigin-RevId: 188967694
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index aa77f34..438a480 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -77,10 +77,14 @@
java_library(
name = "testutil",
testonly = 1,
- srcs = glob(["testutil/*.java"]),
+ srcs = glob(
+ ["testutil/*.java"],
+ exclude = ["testutil/BazelPackageBuilderHelperForTesting.java"],
+ ),
visibility = ["//visibility:public"],
deps = [
":guava_junit_truth",
+ ":testutil/BazelPackageBuilderHelperForTesting",
"//src/main/java/com/google/devtools/build/lib:bazel-main",
"//src/main/java/com/google/devtools/build/lib:bazel-rules",
"//src/main/java/com/google/devtools/build/lib:build-base",
@@ -88,6 +92,7 @@
"//src/main/java/com/google/devtools/build/lib:io",
"//src/main/java/com/google/devtools/build/lib:os_util",
"//src/main/java/com/google/devtools/build/lib:packages",
+ "//src/main/java/com/google/devtools/build/lib:packages/BuilderFactoryForTesting",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/clock",
@@ -102,6 +107,19 @@
)
java_library(
+ name = "testutil/BazelPackageBuilderHelperForTesting",
+ testonly = 0,
+ srcs = ["testutil/BazelPackageBuilderHelperForTesting.java"],
+ visibility = ["//visibility:public"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib:build-base",
+ "//src/main/java/com/google/devtools/build/lib:packages",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/packages",
+ "//third_party:guava",
+ ],
+)
+
+java_library(
name = "foundations_testutil",
testonly = 1,
srcs = glob([
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 eb6605a..a9960b0 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
@@ -271,8 +271,8 @@
@Override
public void setupMockWorkspaceFiles(Path embeddedBinariesRoot) throws IOException {
+ embeddedBinariesRoot.createDirectoryAndParents();
Path jdkWorkspacePath = embeddedBinariesRoot.getRelative("jdk.WORKSPACE");
- FileSystemUtils.createDirectoryAndParents(jdkWorkspacePath.getParentDirectory());
FileSystemUtils.writeContentAsLatin1(jdkWorkspacePath, "");
}
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 a60b3ec..7084281 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
@@ -205,6 +205,10 @@
@Before
public final void initializeSkyframeExecutor() throws Exception {
+ initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ true);
+ }
+
+ public void initializeSkyframeExecutor(boolean doPackageLoadingChecks) throws Exception {
analysisMock = getAnalysisMock();
directories =
new BlazeDirectories(
@@ -227,13 +231,16 @@
PrecomputedValue.injected(
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
ImmutableMap.<RepositoryName, PathFragment>of()));
- pkgFactory =
+ PackageFactory.BuilderForTesting pkgFactoryBuilder =
analysisMock
.getPackageFactoryBuilderForTesting(directories)
.setExtraPrecomputeValues(extraPrecomputedValues)
.setEnvironmentExtensions(getEnvironmentExtensions())
- .setPlatformSetRegexps(getPlatformSetRegexps())
- .build(ruleClassProvider, scratch.getFileSystem());
+ .setPlatformSetRegexps(getPlatformSetRegexps());
+ if (!doPackageLoadingChecks) {
+ pkgFactoryBuilder.disableChecks();
+ }
+ pkgFactory = pkgFactoryBuilder.build(ruleClassProvider, scratch.getFileSystem());
tsgm = new TimestampGranularityMonitor(BlazeClock.instance());
skyframeExecutor =
SequencedSkyframeExecutor.create(
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java b/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
index 0c924aa..d50b43d 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
@@ -33,7 +33,7 @@
public PackageFactoryBuilderWithSkyframeForTesting getPackageFactoryBuilderForTesting(
BlazeDirectories directories) {
return (PackageFactoryBuilderWithSkyframeForTesting)
- TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING.builder();
+ TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING.builder(directories);
}
public ConfiguredRuleClassProvider createRuleClassProvider() {
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
index b934efe..4863530 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
@@ -60,9 +60,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Tests for package loading.
- */
+/** Tests for package loading. */
@RunWith(JUnit4.class)
public class PackageCacheTest extends FoundationTestCase {
@@ -76,6 +74,10 @@
initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ true);
}
+ /**
+ * @param doPackageLoadingChecks when true, a PackageLoader will be called after each package load
+ * this test performs, and the results compared to SkyFrame's result.
+ */
private void initializeSkyframeExecutor(boolean doPackageLoadingChecks) throws Exception {
analysisMock = AnalysisMock.get();
ruleClassProvider = analysisMock.createRuleClassProvider();
@@ -110,8 +112,7 @@
}
private void setUpSkyframe(
- PackageCacheOptions packageCacheOptions,
- SkylarkSemanticsOptions skylarkSemanticsOptions) {
+ PackageCacheOptions packageCacheOptions, SkylarkSemanticsOptions skylarkSemanticsOptions) {
PathPackageLocator pkgLocator =
PathPackageLocator.create(
null,
@@ -136,8 +137,8 @@
}
private OptionsParser parse(String... options) throws Exception {
- OptionsParser parser = OptionsParser.newOptionsParser(
- PackageCacheOptions.class, SkylarkSemanticsOptions.class);
+ OptionsParser parser =
+ OptionsParser.newOptionsParser(PackageCacheOptions.class, SkylarkSemanticsOptions.class);
parser.parse("--default_visibility=public");
parser.parse(options);
@@ -160,9 +161,7 @@
}
protected void setOptions(String... options) throws Exception {
- setUpSkyframe(
- parsePackageCacheOptions(options),
- parseSkylarkSemanticsOptions(options));
+ setUpSkyframe(parsePackageCacheOptions(options), parseSkylarkSemanticsOptions(options));
}
private PackageManager getPackageManager() {
@@ -176,8 +175,8 @@
private Package getPackage(String packageName)
throws NoSuchPackageException, InterruptedException {
- return getPackageManager().getPackage(reporter,
- PackageIdentifier.createInMainRepo(packageName));
+ return getPackageManager()
+ .getPackage(reporter, PackageIdentifier.createInMainRepo(packageName));
}
private Target getTarget(Label label)
@@ -222,9 +221,8 @@
@Test
public void testGetNonexistentPackage() throws Exception {
- checkGetPackageFails("not-there",
- "no such package 'not-there': "
- + "BUILD file not found on package path");
+ checkGetPackageFails(
+ "not-there", "no such package 'not-there': " + "BUILD file not found on package path");
}
@Test
@@ -250,34 +248,32 @@
getTarget("//pkg1:not-there");
fail();
} catch (NoSuchTargetException e) {
- assertThat(e).hasMessage("no such target '//pkg1:not-there': target 'not-there' "
- + "not declared in package 'pkg1' defined by /workspace/pkg1/BUILD");
+ assertThat(e)
+ .hasMessage(
+ "no such target '//pkg1:not-there': target 'not-there' "
+ + "not declared in package 'pkg1' defined by /workspace/pkg1/BUILD");
}
}
/**
- * A missing package is one for which no BUILD file can be found. The
- * PackageCache caches failures of this kind until the next sync.
+ * A missing package is one for which no BUILD file can be found. The PackageCache caches failures
+ * of this kind until the next sync.
*/
@Test
public void testRepeatedAttemptsToParseMissingPackage() throws Exception {
- checkGetPackageFails("missing",
- "no such package 'missing': "
- + "BUILD file not found on package path");
+ checkGetPackageFails(
+ "missing", "no such package 'missing': " + "BUILD file not found on package path");
// Still missing:
- checkGetPackageFails("missing",
- "no such package 'missing': "
- + "BUILD file not found on package path");
+ checkGetPackageFails(
+ "missing", "no such package 'missing': " + "BUILD file not found on package path");
// Update the BUILD file on disk so "missing" is no longer missing:
- scratch.file("missing/BUILD",
- "# an ok build file");
+ scratch.file("missing/BUILD", "# an ok build file");
// Still missing:
- checkGetPackageFails("missing",
- "no such package 'missing': "
- + "BUILD file not found on package path");
+ checkGetPackageFails(
+ "missing", "no such package 'missing': " + "BUILD file not found on package path");
invalidatePackages();
@@ -297,7 +293,7 @@
*
* <p>Note: since the PackageCache.setStrictPackageCreation method was deleted (since it wasn't
* used by any significant clients) creating a "broken" build file got trickier--syntax errors are
- * not enough. For now, we create an unreadable BUILD file, which will cause an IOException to be
+ * not enough. For now, we create an unreadable BUILD file, which will cause an IOException to be
* thrown. This test seems less valuable than it once did.
*/
@Test
@@ -316,8 +312,7 @@
eventCollector.clear();
// Update the BUILD file on disk so "broken" is no longer broken:
- scratch.overwriteFile("broken/BUILD",
- "# an ok build file");
+ scratch.overwriteFile("broken/BUILD", "# an ok build file");
invalidatePackages(); // resets cache of failures
@@ -328,10 +323,11 @@
@Test
public void testMovedBuildFileCausesReloadAfterSync() throws Exception {
- Path buildFile1 = scratch.file("pkg/BUILD",
- "cc_library(name = 'foo')");
- Path buildFile2 = scratch.file("/otherroot/pkg/BUILD",
- "cc_library(name = 'bar')");
+ // PackageLoader doesn't support --package_path.
+ initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ false);
+
+ Path buildFile1 = scratch.file("pkg/BUILD", "cc_library(name = 'foo')");
+ Path buildFile2 = scratch.file("/otherroot/pkg/BUILD", "cc_library(name = 'bar')");
setOptions("--package_path=/workspace:/otherroot");
Package oldPkg = getPackage("pkg");
@@ -386,8 +382,8 @@
setOptions("--package_path=/workspace:/otherroot");
}
- protected Path createBuildFile(Path workspace, String packageName,
- String... targets) throws IOException {
+ protected Path createBuildFile(Path workspace, String packageName, String... targets)
+ throws IOException {
String[] lines = new String[targets.length];
for (int i = 0; i < targets.length; i++) {
@@ -397,8 +393,7 @@
return scratch.file(workspace + "/" + packageName + "/BUILD", lines);
}
- private void assertLabelValidity(boolean expected, String labelString)
- throws Exception {
+ private void assertLabelValidity(boolean expected, String labelString) throws Exception {
Label label = Label.parseAbsolute(labelString);
boolean actual = false;
@@ -410,9 +405,16 @@
error = e.getMessage();
}
if (actual != expected) {
- fail("assertLabelValidity(" + label + ") "
- + actual + ", not equal to expected value " + expected
- + " (error=" + error + ")");
+ fail(
+ "assertLabelValidity("
+ + label
+ + ") "
+ + actual
+ + ", not equal to expected value "
+ + expected
+ + " (error="
+ + error
+ + ")");
}
}
@@ -425,9 +427,7 @@
@Test
public void testLocationForLabelCrossingSubpackage() throws Exception {
scratch.file("e/f/BUILD");
- scratch.file("e/BUILD",
- "# Whatever",
- "filegroup(name='fg', srcs=['f/g'])");
+ scratch.file("e/BUILD", "# Whatever", "filegroup(name='fg', srcs=['f/g'])");
reporter.removeHandler(failFastHandler);
List<Event> events = getPackage("e").getEvents();
assertThat(events).hasSize(1);
@@ -437,6 +437,9 @@
/** Static tests (i.e. no changes to filesystem, nor calls to sync). */
@Test
public void testLabelValidity() throws Exception {
+ // PackageLoader doesn't support --package_path.
+ initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ false);
+
reporter.removeHandler(failFastHandler);
setUpCacheWithTwoRootLocator();
@@ -476,9 +479,8 @@
@Test
public void testAddedBuildFileCausesLabelToBecomeInvalid() throws Exception {
reporter.removeHandler(failFastHandler);
- scratch.file("pkg/BUILD",
- " cc_library(name = 'foo', ",
- " srcs = ['x/y.cc'])");
+ scratch.file(
+ "pkg/BUILD", " cc_library(name = 'foo', ", " srcs = ['x/y.cc'])");
assertLabelValidity(true, "//pkg:x/y.cc");
@@ -491,9 +493,10 @@
invalidatePackages();
// now:
- assertPackageLoadingFails("pkg",
+ assertPackageLoadingFails(
+ "pkg",
"Label '//pkg:x/y.cc' crosses boundary of subpackage 'pkg/x' "
- + "(perhaps you meant to put the colon here: '//pkg/x:y.cc'?)");
+ + "(perhaps you meant to put the colon here: '//pkg/x:y.cc'?)");
}
@Test
@@ -513,9 +516,10 @@
assertLabelValidity(true, "//c/d:foo.txt");
// ...and this crosses package boundaries:
assertLabelValidity(false, "//c:d/x");
- assertPackageLoadingFails("c",
+ assertPackageLoadingFails(
+ "c",
"Label '//c:d/x' crosses boundary of subpackage 'c/d' (have you deleted c/d/BUILD? "
- + "If so, use the --deleted_packages=c/d option)");
+ + "If so, use the --deleted_packages=c/d option)");
assertThat(getPackageManager().isPackage(reporter, PackageIdentifier.createInMainRepo("c/d")))
.isTrue();
@@ -532,8 +536,9 @@
getPackage("c/d");
fail();
} catch (NoSuchPackageException e) {
- assertThat(e).hasMessage(
- "no such package 'c/d': Package is considered deleted due to --deleted_packages");
+ assertThat(e)
+ .hasMessage(
+ "no such package 'c/d': Package is considered deleted due to --deleted_packages");
}
// Labels in the subpackage are no longer valid...
@@ -544,7 +549,8 @@
@Test
public void testPackageFeatures() throws Exception {
- scratch.file("peach/BUILD",
+ scratch.file(
+ "peach/BUILD",
"package(features = ['crosstool_default_false'])",
"cc_library(name = 'cc', srcs = ['cc.cc'])");
Rule cc = (Rule) getTarget("//peach:cc");
@@ -556,11 +562,7 @@
reporter.removeHandler(failFastHandler);
setOptions("--package_path=.:.");
scratch.file("x/y/BUILD");
- scratch.file("x/BUILD",
- "genrule(name = 'x',",
- "srcs = [],",
- "outs = ['y/z.h'],",
- "cmd = '')");
+ scratch.file("x/BUILD", "genrule(name = 'x',", "srcs = [],", "outs = ['y/z.h'],", "cmd = '')");
Package p = getPackage("x");
assertThat(p.containsErrors()).isTrue();
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index d685a46..4781496 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -73,6 +73,17 @@
public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Before
+ public final void turnOffPackageLoadingChecks() throws Exception {
+ // By default, PackageLoader loads every package the test harness loads, in order to verify
+ // the PackageLoader works correctly. In this test, however, PackageLoader sometimes fails to
+ // load packages and causes the test to become flaky.
+ // Since PackageLoader gets generally good coverage from the rest of Bazel's tests, and because
+ // we believe there's nothing special from the point of view of package loading in this test,
+ // we disable this verification here.
+ initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ false);
+ }
+
+ @Before
public void createFiles() throws Exception {
scratch.file("java/android/BUILD",
"android_binary(name = 'app',",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
index cc23c7c..2c832dd 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
@@ -44,6 +44,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -51,6 +52,17 @@
/** Test case for apple_binary. */
@RunWith(JUnit4.class)
public class AppleBinaryTest extends ObjcRuleTestCase {
+ @Before
+ public final void turnOffPackageLoadingChecks() throws Exception {
+ // By default, PackageLoader loads every package the test harness loads, in order to verify
+ // the PackageLoader works correctly. In this test, however, PackageLoader sometimes fails to
+ // load packages and causes the test to become flaky.
+ // Since PackageLoader gets generally good coverage from the rest of Bazel's tests, and because
+ // we believe there's nothing special from the point of view of package loading in this test,
+ // we disable this verification here.
+ initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ false);
+ }
+
static final RuleType RULE_TYPE = new RuleType("apple_binary") {
@Override
Iterable<String> requiredAttributes(Scratch scratch, String packageDir,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
index f3ab343..0c9357d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
@@ -117,7 +117,7 @@
new WorkspaceFileFunction(
TestRuleClassProvider.getRuleClassProvider(),
TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING
- .builder()
+ .builder(directories)
.build(
TestRuleClassProvider.getRuleClassProvider(), root.getFileSystem()),
directories))
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
index 3b83fa8..681e4aa 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
@@ -111,7 +111,7 @@
new WorkspaceFileFunction(
TestRuleClassProvider.getRuleClassProvider(),
TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING
- .builder()
+ .builder(directories)
.build(
TestRuleClassProvider.getRuleClassProvider(), root.getFileSystem()),
directories))
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 6a8543f..2c0c772 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -166,7 +166,7 @@
new WorkspaceFileFunction(
TestRuleClassProvider.getRuleClassProvider(),
TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING
- .builder()
+ .builder(directories)
.build(TestRuleClassProvider.getRuleClassProvider(), fs),
directories))
.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction())
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 0edfea3..a0c16e4 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -132,10 +132,13 @@
BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY));
skyFunctions.put(SkyFunctions.WORKSPACE_AST,
new WorkspaceASTFunction(TestRuleClassProvider.getRuleClassProvider()));
- skyFunctions.put(SkyFunctions.WORKSPACE_FILE,
- new WorkspaceFileFunction(TestRuleClassProvider.getRuleClassProvider(),
- TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING.builder().build(
- TestRuleClassProvider.getRuleClassProvider(), fs),
+ skyFunctions.put(
+ SkyFunctions.WORKSPACE_FILE,
+ new WorkspaceFileFunction(
+ TestRuleClassProvider.getRuleClassProvider(),
+ TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING
+ .builder(directories)
+ .build(TestRuleClassProvider.getRuleClassProvider(), fs),
directories));
skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunctionTest.java
index ac45a7e..679d276 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunctionTest.java
@@ -51,6 +51,7 @@
@Before
public final void createSkyframeExecutor() throws Exception {
+ initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ false);
skyframeExecutor = getSkyframeExecutor();
}
@@ -63,7 +64,7 @@
private RecursivePkgValue buildRecursivePkgValue(Path root, PathFragment rootRelativePath)
throws Exception {
- return buildRecursivePkgValue(root, rootRelativePath, ImmutableSet.<PathFragment>of());
+ return buildRecursivePkgValue(root, rootRelativePath, ImmutableSet.of());
}
private RecursivePkgValue buildRecursivePkgValue(
@@ -139,8 +140,7 @@
WalkableGraph graph = Preconditions.checkNotNull(evaluationResult.getWalkableGraph());
assertThat(
exists(
- buildRecursivePkgKey(
- rootDirectory, excludedPathFragment, ImmutableSet.<PathFragment>of()),
+ buildRecursivePkgKey(rootDirectory, excludedPathFragment, ImmutableSet.of()),
graph))
.isFalse();
@@ -148,8 +148,7 @@
// because that key was evaluated.
assertThat(
exists(
- buildRecursivePkgKey(
- rootDirectory, PathFragment.create("a/c"), ImmutableSet.<PathFragment>of()),
+ buildRecursivePkgKey(rootDirectory, PathFragment.create("a/c"), ImmutableSet.of()),
graph))
.isTrue();
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index 3348527..8554b25 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -220,7 +220,7 @@
new WorkspaceFileFunction(
TestRuleClassProvider.getRuleClassProvider(),
TestConstants.PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING
- .builder()
+ .builder(directories)
.build(
TestRuleClassProvider.getRuleClassProvider(),
scratch.getFileSystem()),
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java
index 1ac5266..31c25f5 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java
@@ -37,28 +37,26 @@
/** Abstract base class of a unit test for a {@link AbstractPackageLoader} implementation. */
public abstract class AbstractPackageLoaderTest {
- private Path pkgRoot;
+ private Path workspaceDir;
protected StoredEventHandler handler;
- protected PackageLoader pkgLoader;
+ protected FileSystem fs;
+ private Reporter reporter;
@Before
public final void init() throws Exception {
- FileSystem fs = new InMemoryFileSystem();
- pkgRoot = fs.getPath("/").getRelative("pkgRoot");
- FileSystemUtils.createDirectoryAndParents(pkgRoot);
- Reporter reporter = new Reporter(new EventBus());
+ fs = new InMemoryFileSystem();
+ workspaceDir = fs.getPath("/workspace/");
+ workspaceDir.createDirectoryAndParents();
+ reporter = new Reporter(new EventBus());
handler = new StoredEventHandler();
reporter.addHandler(handler);
- pkgLoader = makeFreshBuilder(pkgRoot)
- .useDefaultSkylarkSemantics()
- .setReporter(reporter)
- .build();
}
- protected abstract AbstractPackageLoader.Builder makeFreshBuilder(Path pkgRoot);
+ protected abstract AbstractPackageLoader.Builder newPackageLoaderBuilder(Path workspaceDir);
@Test
public void simpleNoPackage() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("nope"));
try {
pkgLoader.loadPackage(pkgId);
@@ -73,6 +71,7 @@
@Test
public void simpleBadPackage() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
file("bad/BUILD", "invalidBUILDsyntax");
PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("bad"));
Package badPkg = pkgLoader.loadPackage(pkgId);
@@ -83,18 +82,20 @@
@Test
public void simpleGoodPackage() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
file("good/BUILD", "sh_library(name = 'good')");
PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("good"));
Package goodPkg = pkgLoader.loadPackage(pkgId);
assertThat(goodPkg.containsErrors()).isFalse();
- assertThat(
- goodPkg.getTarget("good").getAssociatedRule().getRuleClass()).isEqualTo("sh_library");
+ assertThat(goodPkg.getTarget("good").getAssociatedRule().getRuleClass())
+ .isEqualTo("sh_library");
assertNoEvents(goodPkg.getEvents());
assertNoEvents(handler.getEvents());
}
@Test
public void simpleMultipleGoodPackage() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
file("good1/BUILD", "sh_library(name = 'good1')");
file("good2/BUILD", "sh_library(name = 'good2')");
PackageIdentifier pkgId1 = PackageIdentifier.createInMainRepo(PathFragment.create("good1"));
@@ -114,6 +115,7 @@
@Test
public void loadPackagesToleratesDuplicates() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
file("good1/BUILD", "sh_library(name = 'good1')");
PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("good1"));
ImmutableMap<PackageIdentifier, PackageLoader.PackageOrException> pkgs =
@@ -127,29 +129,33 @@
@Test
public void simpleGoodPackage_Skylark() throws Exception {
- file("good/good.bzl",
- "def f(x):",
- " native.sh_library(name = x)");
- file("good/BUILD",
- "load('//good:good.bzl', 'f')",
- "f('good')");
+ PackageLoader pkgLoader = newPackageLoader();
+ file("good/good.bzl", "def f(x):", " native.sh_library(name = x)");
+ file("good/BUILD", "load('//good:good.bzl', 'f')", "f('good')");
PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("good"));
Package goodPkg = pkgLoader.loadPackage(pkgId);
assertThat(goodPkg.containsErrors()).isFalse();
- assertThat(
- goodPkg.getTarget("good").getAssociatedRule().getRuleClass()).isEqualTo("sh_library");
+ assertThat(goodPkg.getTarget("good").getAssociatedRule().getRuleClass())
+ .isEqualTo("sh_library");
assertNoEvents(goodPkg.getEvents());
assertNoEvents(handler.getEvents());
}
protected Path path(String rootRelativePath) {
- return pkgRoot.getRelative(PathFragment.create(rootRelativePath));
+ return workspaceDir.getRelative(PathFragment.create(rootRelativePath));
}
protected Path file(String fileName, String... contents) throws Exception {
Path path = path(fileName);
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
FileSystemUtils.writeContentAsLatin1(path, Joiner.on("\n").join(contents));
return path;
}
+
+ protected PackageLoader newPackageLoader() {
+ return newPackageLoaderBuilder(workspaceDir)
+ .useDefaultSkylarkSemantics()
+ .setReporter(reporter)
+ .build();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/packages/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/packages/BUILD
index 105843b..137ae9d 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/packages/BUILD
@@ -17,11 +17,14 @@
"BazelPackageLoaderTest.java",
],
deps = [
+ "//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib:packages",
"//src/main/java/com/google/devtools/build/lib/skyframe/packages",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
+ "//src/test/java/com/google/devtools/build/lib:analysis_testutil",
+ "//src/test/java/com/google/devtools/build/lib:packages_testutil",
"//src/test/java/com/google/devtools/build/lib:testutil",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java
index fb53082..a676586 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoaderTest.java
@@ -16,11 +16,15 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertNoEvents;
+import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.IOException;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -33,13 +37,47 @@
*/
@RunWith(JUnit4.class)
public final class BazelPackageLoaderTest extends AbstractPackageLoaderTest {
+
+ private Path installBase;
+ private Path outputBase;
+
+ @Before
+ public void setUp() throws Exception {
+ installBase = fs.getPath("/installBase/");
+ installBase.createDirectoryAndParents();
+ outputBase = fs.getPath("/outputBase/");
+ outputBase.createDirectoryAndParents();
+ Path embeddedBinaries = ServerDirectories.getEmbeddedBinariesRoot(installBase);
+ embeddedBinaries.createDirectoryAndParents();
+
+ mockEmbeddedTools(embeddedBinaries);
+ }
+
+ private void mockEmbeddedTools(Path embeddedBinaries) throws IOException {
+ Path tools = embeddedBinaries.getRelative("embedded_tools");
+ tools.getRelative("tools/cpp").createDirectoryAndParents();
+ tools.getRelative("tools/osx").createDirectoryAndParents();
+ FileSystemUtils.writeIsoLatin1(tools.getRelative("WORKSPACE"), "");
+ FileSystemUtils.writeIsoLatin1(tools.getRelative("tools/cpp/BUILD"), "");
+ FileSystemUtils.writeIsoLatin1(
+ tools.getRelative("tools/cpp/cc_configure.bzl"),
+ "def cc_configure(*args, **kwargs):",
+ " pass");
+ FileSystemUtils.writeIsoLatin1(tools.getRelative("tools/osx/BUILD"), "");
+ FileSystemUtils.writeIsoLatin1(
+ tools.getRelative("tools/osx/xcode_configure.bzl"),
+ "def xcode_configure(*args, **kwargs):",
+ " pass");
+ }
+
@Override
- protected BazelPackageLoader.Builder makeFreshBuilder(Path pkgRoot) {
- return BazelPackageLoader.builder(pkgRoot);
+ protected AbstractPackageLoader.Builder newPackageLoaderBuilder(Path workspaceDir) {
+ return BazelPackageLoader.builder(workspaceDir, installBase, outputBase);
}
@Test
public void simpleLocalRepositoryPackage() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
file("WORKSPACE", "local_repository(name = 'r', path='r')");
file("r/WORKSPACE", "workspace(name = 'r')");
file("r/good/BUILD", "sh_library(name = 'good')");
@@ -52,4 +90,22 @@
assertNoEvents(goodPkg.getEvents());
assertNoEvents(handler.getEvents());
}
+
+ @Test
+ public void newLocalRepository() throws Exception {
+ PackageLoader pkgLoader = newPackageLoader();
+ file(
+ "WORKSPACE",
+ "new_local_repository(name = 'r', path = '/r', "
+ + "build_file_content = 'sh_library(name = \"good\")')");
+ fs.getPath("/r").createDirectoryAndParents();
+ PackageIdentifier pkgId =
+ PackageIdentifier.create(RepositoryName.create("@r"), PathFragment.create(""));
+ Package goodPkg = pkgLoader.loadPackage(pkgId);
+ assertThat(goodPkg.containsErrors()).isFalse();
+ assertThat(goodPkg.getTarget("good").getAssociatedRule().getRuleClass())
+ .isEqualTo("sh_library");
+ assertNoEvents(goodPkg.getEvents());
+ assertNoEvents(handler.getEvents());
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BazelPackageBuilderHelperForTesting.java b/src/test/java/com/google/devtools/build/lib/testutil/BazelPackageBuilderHelperForTesting.java
index 2bd6cf6..6f9cd61 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/BazelPackageBuilderHelperForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/BazelPackageBuilderHelperForTesting.java
@@ -17,18 +17,16 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.skyframe.PackageFunction;
import com.google.devtools.build.lib.skyframe.packages.BazelPackageLoader;
import com.google.devtools.build.lib.skyframe.packages.PackageLoader;
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
-import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
/**
* A Package.Builder.Helper for use in tests that a sanity check with {@link BazelPackageLoader} for
@@ -36,9 +34,12 @@
*/
public class BazelPackageBuilderHelperForTesting implements Package.Builder.Helper {
private final RuleClassProvider ruleClassProvider;
+ private final BlazeDirectories directories;
- public BazelPackageBuilderHelperForTesting(RuleClassProvider ruleClassProvider) {
+ public BazelPackageBuilderHelperForTesting(
+ RuleClassProvider ruleClassProvider, BlazeDirectories directories) {
this.ruleClassProvider = ruleClassProvider;
+ this.directories = directories;
}
@Override
@@ -66,23 +67,14 @@
RuleClassProvider ruleClassProvider,
SkylarkSemantics skylarkSemantics) {
PackageIdentifier pkgId = pkg.getPackageIdentifier();
- if (pkgId.equals(Label.EXTERNAL_PACKAGE_IDENTIFIER)
- || !pkg.getPackageIdentifier().getRepository().isMain()
- || PackageFunction.isDefaultsPackage(pkg.getPackageIdentifier())) {
- // TODO(nharmata): Support these packages.
- return;
- }
- int numNameSegments = pkg.getNameFragment().segmentCount();
- PathFragment fullFilenameFragment = pkg.getFilename().asFragment();
- int numFullFilenameFragmentSegments = fullFilenameFragment.segmentCount();
- Path workspaceRoot = pkg.getFilename().getFileSystem().getPath(
- fullFilenameFragment.subFragment(
- 0,
- numFullFilenameFragmentSegments - (numNameSegments + 1)));
- PackageLoader packageLoader = BazelPackageLoader.builder(workspaceRoot)
- .setSkylarkSemantics(skylarkSemantics)
- .setRuleClassProvider(ruleClassProvider)
- .build();
+ PackageLoader packageLoader =
+ BazelPackageLoader.builder(
+ directories.getWorkspace(),
+ directories.getInstallBase(),
+ directories.getOutputBase())
+ .setSkylarkSemantics(skylarkSemantics)
+ .setRuleClassProvider(ruleClassProvider)
+ .build();
Package newlyLoadedPkg;
try {
newlyLoadedPkg = packageLoader.loadPackage(pkg.getPackageIdentifier());
@@ -97,17 +89,31 @@
ImmutableSet.copyOf(
Iterables.transform(newlyLoadedPkg.getTargets().values(), TARGET_TO_LABEL));
if (!targetsInPkg.equals(targetsInNewlyLoadedPkg)) {
- throw new IllegalStateException(String.format(
- "The Package for %s had a different set of targets (<targetsInPkg> - "
- + "<targetsInNewlyLoadedPkg> = %s, <targetsInNewlyLoadedPkg> - <targetsInPkg> = %s) when "
- + "loaded normally during execution of the current test than it did when loaded via "
- + "BazelPackageLoader (done automatically by the BazelPackageBuilderHelperForTesting "
- + "hook). This either means: (i) Skyframe package loading semantics have diverged from "
- + "BazelPackageLoader semantics (ii) The test in question is doing something that "
- + "confuses BazelPackageBuilderHelperForTesting.",
- pkgId,
- Sets.difference(targetsInPkg, targetsInNewlyLoadedPkg),
- Sets.difference(targetsInNewlyLoadedPkg, targetsInPkg)));
+ Sets.SetView<Label> unsatisfied = Sets.difference(targetsInPkg, targetsInNewlyLoadedPkg);
+ if (pkgId.compareTo(PackageIdentifier.createInMainRepo("tools/defaults")) == 0
+ && unsatisfied.isEmpty()) {
+ // The tools/defaults package is populated from command-line options
+ // (=configuration fragments) which the user specifies in tests using
+ // BuildViewTestCase.useConfiguration().
+ // We'd like PackageLoader to work as much as possible without duplicating the entire
+ // configuration of Bazel, so we prefer not to pass these flags to PackageLoader.
+ // As a result, the contents of tools/defaults might differ.
+ // As long as PackageLoader returns a superset of what Bazel returns, everything should load
+ // fine.
+ return;
+ }
+ Sets.SetView<Label> unexpected = Sets.difference(targetsInNewlyLoadedPkg, targetsInPkg);
+ throw new IllegalStateException(
+ String.format(
+ "The Package for %s had a different set of targets (<targetsInPkg> - "
+ + "<targetsInNewlyLoadedPkg> = %s, <targetsInNewlyLoadedPkg> - <targetsInPkg> = "
+ + "%s) when loaded normally during execution of the current test than it did "
+ + "when loaded via BazelPackageLoader (done automatically by the "
+ + "BazelPackageBuilderHelperForTesting hook). This either means: (i) Skyframe "
+ + "package loading semantics have diverged from "
+ + "BazelPackageLoader semantics (ii) The test in question is doing something "
+ + "that confuses BazelPackageBuilderHelperForTesting.",
+ pkgId, unsatisfied, unexpected));
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java b/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java
index 04e8e03..5cb3772 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.testutil;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.packages.BuilderFactoryForTesting;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -20,11 +22,10 @@
import com.google.devtools.build.lib.vfs.FileSystem;
/**
- * A {@link PackageFactory.BuilderFactoryForTesting} implementation that injects a
- * {@link BazelPackageBuilderHelperForTesting}.
+ * A {@link BuilderFactoryForTesting} implementation that injects a {@link
+ * BazelPackageBuilderHelperForTesting}.
*/
-class PackageFactoryBuilderFactoryForBazelUnitTests
- extends PackageFactory.BuilderFactoryForTesting {
+class PackageFactoryBuilderFactoryForBazelUnitTests implements BuilderFactoryForTesting {
static final PackageFactoryBuilderFactoryForBazelUnitTests INSTANCE =
new PackageFactoryBuilderFactoryForBazelUnitTests();
@@ -32,17 +33,25 @@
}
@Override
- public PackageFactoryBuilderWithSkyframeForTesting builder() {
- return new PackageFactoryBuilderForBazelUnitTests();
+ public PackageFactoryBuilderWithSkyframeForTesting builder(BlazeDirectories directories) {
+ return new PackageFactoryBuilderForBazelUnitTests(directories);
}
private static class PackageFactoryBuilderForBazelUnitTests
extends PackageFactoryBuilderWithSkyframeForTesting {
+
+ private final BlazeDirectories directories;
+
+ public PackageFactoryBuilderForBazelUnitTests(BlazeDirectories directories) {
+ this.directories = directories;
+ }
+
@Override
public PackageFactory build(RuleClassProvider ruleClassProvider, FileSystem fs) {
- Package.Builder.Helper packageBuilderHelperForTesting = doChecksForTesting
- ? new BazelPackageBuilderHelperForTesting(ruleClassProvider)
- : Package.Builder.DefaultHelper.INSTANCE;
+ Package.Builder.Helper packageBuilderHelperForTesting =
+ doChecksForTesting
+ ? new BazelPackageBuilderHelperForTesting(ruleClassProvider, directories)
+ : Package.Builder.DefaultHelper.INSTANCE;
return new PackageFactory(
ruleClassProvider,
platformSetRegexps,
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
index d7abdf6..7a59246 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
@@ -15,7 +15,7 @@
package com.google.devtools.build.lib.testutil;
import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.packages.PackageFactory.BuilderFactoryForTesting;
+import com.google.devtools.build.lib.packages.BuilderFactoryForTesting;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
