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/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index af0b5ed..cfa5379 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -373,10 +373,13 @@
java_library(
name = "packages-internal",
- srcs = glob([
- "packages/*.java",
- "pkgcache/*.java",
- ]),
+ srcs = glob(
+ [
+ "packages/*.java",
+ "pkgcache/*.java",
+ ],
+ exclude = ["packages/BuilderFactoryForTesting.java"],
+ ),
exports = [
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -410,6 +413,17 @@
)
java_library(
+ name = "packages/BuilderFactoryForTesting",
+ srcs = ["packages/BuilderFactoryForTesting.java"],
+ deps = [
+ ":build-base",
+ ":packages-internal",
+ "//third_party:guava",
+ "//third_party:jsr305",
+ ],
+)
+
+java_library(
name = "packages",
exports = [
"//src/main/java/com/google/devtools/build/lib:foundation",
@@ -612,8 +626,35 @@
)
java_library(
+ name = "bazel/BazelRepositoryModule",
+ srcs = ["bazel/BazelRepositoryModule.java"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib:bazel-commands",
+ "//src/main/java/com/google/devtools/build/lib:bazel-repository",
+ "//src/main/java/com/google/devtools/build/lib:bazel-rules",
+ "//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:io",
+ "//src/main/java/com/google/devtools/build/lib:packages-internal",
+ "//src/main/java/com/google/devtools/build/lib:runtime",
+ "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
+ "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
+ "//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/vfs",
+ "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
+ "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+ "//src/main/java/com/google/devtools/common/options:options_internal",
+ "//third_party:guava",
+ "//third_party:jsr305",
+ ],
+)
+
+java_library(
name = "bazel-main",
- srcs = glob(["bazel/*.java"]),
+ srcs = glob(
+ ["bazel/*.java"],
+ exclude = ["bazel/BazelRepositoryModule.java"],
+ ),
resources = [
"bazel/rules/java/java_stub_template.txt",
"bazel/rules/java/java_stub_template_windows.txt",
@@ -621,6 +662,7 @@
"bazel/rules/sh/sh_stub_template_windows.txt",
],
deps = [
+ ":bazel/BazelRepositoryModule",
"//src/main/java/com/google/devtools/build/lib:bazel",
"//src/main/java/com/google/devtools/build/lib:bazel-commands",
"//src/main/java/com/google/devtools/build/lib:bazel-repository",
@@ -648,6 +690,7 @@
"//src/main/java/com/google/devtools/build/lib/worker",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
+ "//src/test/java/com/google/devtools/build/lib:testutil/BazelPackageBuilderHelperForTesting",
"//third_party:guava",
"//third_party:jsr305",
],
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java
index ac89158..dc9d60e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ServerDirectories.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.hash.HashCode;
@@ -89,6 +90,11 @@
/** Returns the installed embedded binaries directory, under the shared installBase location. */
public Path getEmbeddedBinariesRoot() {
+ return getEmbeddedBinariesRoot(installBase);
+ }
+
+ @VisibleForTesting
+ public static Path getEmbeddedBinariesRoot(Path installBase) {
return installBase.getChild("_embedded_binaries");
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 8a18f3c..d6793da 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -89,9 +89,7 @@
import java.util.stream.Collectors;
import javax.annotation.Nullable;
-/**
- * Adds support for fetching external code.
- */
+/** Adds support for fetching external code. */
public class BazelRepositoryModule extends BlazeModule {
// Default location (relative to output user root) of the repository cache.
@@ -111,21 +109,25 @@
public BazelRepositoryModule() {
this.skylarkRepositoryFunction = new SkylarkRepositoryFunction(httpDownloader);
- this.repositoryHandlers =
- ImmutableMap.<String, RepositoryFunction>builder()
- .put(LocalRepositoryRule.NAME, new LocalRepositoryFunction())
- .put(HttpArchiveRule.NAME, new HttpArchiveFunction(httpDownloader))
- .put(GitRepositoryRule.NAME, new GitRepositoryFunction(httpDownloader))
- .put(HttpJarRule.NAME, new HttpJarFunction(httpDownloader))
- .put(HttpFileRule.NAME, new HttpFileFunction(httpDownloader))
- .put(MavenJarRule.NAME, new MavenJarFunction(mavenDownloader))
- .put(NewHttpArchiveRule.NAME, new NewHttpArchiveFunction(httpDownloader))
- .put(NewGitRepositoryRule.NAME, new NewGitRepositoryFunction(httpDownloader))
- .put(NewLocalRepositoryRule.NAME, new NewLocalRepositoryFunction())
- .put(AndroidSdkRepositoryRule.NAME, new AndroidSdkRepositoryFunction())
- .put(AndroidNdkRepositoryRule.NAME, new AndroidNdkRepositoryFunction())
- .put(MavenServerRule.NAME, new MavenServerRepositoryFunction())
- .build();
+ this.repositoryHandlers = repositoryRules(httpDownloader, mavenDownloader);
+ }
+
+ public static ImmutableMap<String, RepositoryFunction> repositoryRules(
+ HttpDownloader httpDownloader, MavenDownloader mavenDownloader) {
+ return ImmutableMap.<String, RepositoryFunction>builder()
+ .put(LocalRepositoryRule.NAME, new LocalRepositoryFunction())
+ .put(HttpArchiveRule.NAME, new HttpArchiveFunction(httpDownloader))
+ .put(GitRepositoryRule.NAME, new GitRepositoryFunction(httpDownloader))
+ .put(HttpJarRule.NAME, new HttpJarFunction(httpDownloader))
+ .put(HttpFileRule.NAME, new HttpFileFunction(httpDownloader))
+ .put(MavenJarRule.NAME, new MavenJarFunction(mavenDownloader))
+ .put(NewHttpArchiveRule.NAME, new NewHttpArchiveFunction(httpDownloader))
+ .put(NewGitRepositoryRule.NAME, new NewGitRepositoryFunction(httpDownloader))
+ .put(NewLocalRepositoryRule.NAME, new NewLocalRepositoryFunction())
+ .put(AndroidSdkRepositoryRule.NAME, new AndroidSdkRepositoryFunction())
+ .put(AndroidNdkRepositoryRule.NAME, new AndroidNdkRepositoryFunction())
+ .put(MavenServerRule.NAME, new MavenServerRepositoryFunction())
+ .build();
}
/**
@@ -185,9 +187,11 @@
for (Entry<String, RepositoryFunction> handler : repositoryHandlers.entrySet()) {
RuleDefinition ruleDefinition;
try {
- ruleDefinition = handler.getValue().getRuleDefinition().getDeclaredConstructor()
- .newInstance();
- } catch (IllegalAccessException | InstantiationException | NoSuchMethodException
+ ruleDefinition =
+ handler.getValue().getRuleDefinition().getDeclaredConstructor().newInstance();
+ } catch (IllegalAccessException
+ | InstantiationException
+ | NoSuchMethodException
| InvocationTargetException e) {
throw new IllegalStateException(e);
}
@@ -266,8 +270,7 @@
@Override
public ImmutableList<Injected> getPrecomputedValues() {
return ImmutableList.of(
- PrecomputedValue.injected(
- RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides));
+ PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides));
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuilderFactoryForTesting.java b/src/main/java/com/google/devtools/build/lib/packages/BuilderFactoryForTesting.java
new file mode 100644
index 0000000..c807714
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuilderFactoryForTesting.java
@@ -0,0 +1,27 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.packages;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+
+/**
+ * Factory for {@link com.google.devtools.build.lib.packages.PackageFactory.BuilderForTesting}
+ * instances. Intended to only be used by unit tests.
+ */
+@VisibleForTesting
+public interface BuilderFactoryForTesting {
+ PackageFactory.BuilderForTesting builder(BlazeDirectories directories);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 13e9045..370df06 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -372,15 +372,6 @@
public abstract PackageFactory build(RuleClassProvider ruleClassProvider, FileSystem fs);
}
- /**
- * Factory for {@link PackageFactory.BuilderForTesting} instances. Intended to only be used by
- * unit tests.
- */
- @VisibleForTesting
- public abstract static class BuilderFactoryForTesting {
- public abstract BuilderForTesting builder();
- }
-
@VisibleForTesting
public Package.Builder.Helper getPackageBuilderHelperForTesting() {
return packageBuilderHelper;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java
index 7c4c798..37f111f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentFunction.java
@@ -55,7 +55,7 @@
private final AtomicReference<Map<String, String>> clientEnv;
- ClientEnvironmentFunction(AtomicReference<Map<String, String>> clientEnv) {
+ public ClientEnvironmentFunction(AtomicReference<Map<String, String>> clientEnv) {
this.clientEnv = clientEnv;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
index 1d92dc2..ad91c60 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
@@ -90,7 +90,7 @@
static final Precomputed<UUID> BUILD_ID = new Precomputed<>(Key.create("build_id"));
- static final Precomputed<Map<String, String>> ACTION_ENV =
+ public static final Precomputed<Map<String, String>> ACTION_ENV =
new Precomputed<>(Key.create("action_env"));
static final Precomputed<ImmutableList<ActionAnalysisMetadata>> COVERAGE_REPORT_KEY =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index fa5bbd6..382010e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -107,18 +107,19 @@
private static final int MAX_DIRECTORIES_TO_EAGERLY_VISIT_IN_GLOBBING = 3000;
private final ImmutableDiff preinjectedDiff;
- private final Differencer preinjectedDifferencer = new Differencer() {
- @Override
- public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVersion)
- throws InterruptedException {
- return preinjectedDiff;
- }
- };
+ private final Differencer preinjectedDifferencer =
+ new Differencer() {
+ @Override
+ public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVersion)
+ throws InterruptedException {
+ return preinjectedDiff;
+ }
+ };
private final Reporter reporter;
protected final RuleClassProvider ruleClassProvider;
protected SkylarkSemantics skylarkSemantics;
protected final ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions;
- protected final AtomicReference<PathPackageLocator> pkgLocatorRef;
+ private final AtomicReference<PathPackageLocator> pkgLocatorRef;
protected final ExternalFilesHelper externalFilesHelper;
protected final BlazeDirectories directories;
private final int legacyGlobbingThreads;
@@ -128,24 +129,36 @@
public abstract static class Builder {
protected final Path workspaceDir;
protected final BlazeDirectories directories;
+ protected final PathPackageLocator pkgLocator;
+ final AtomicReference<PathPackageLocator> pkgLocatorRef;
+ protected final ExternalFilesHelper externalFilesHelper;
protected RuleClassProvider ruleClassProvider = getDefaultRuleClassProvider();
protected SkylarkSemantics skylarkSemantics;
protected Reporter reporter = new Reporter(new EventBus());
protected Map<SkyFunctionName, SkyFunction> extraSkyFunctions = new HashMap<>();
- protected List<PrecomputedValue.Injected> extraPrecomputedValues = new ArrayList<>();
- protected String defaultsPackageContents = getDefaultDefaultPackageContents();
- protected int legacyGlobbingThreads = 1;
+ List<PrecomputedValue.Injected> extraPrecomputedValues = new ArrayList<>();
+ String defaultsPackageContents = getDefaultDefaultPackageContents();
+ int legacyGlobbingThreads = 1;
int skyframeThreads = 1;
- protected Builder(Path workspaceDir) {
+ protected Builder(Path workspaceDir, Path installBase, Path outputBase) {
this.workspaceDir = workspaceDir;
- // The 'installBase' and 'outputBase' directories won't be meaningfully used by
- // WorkspaceFileFunction, so we pass in a dummy Path.
- // TODO(nharmata): Refactor WorkspaceFileFunction to make this a non-issue.
Path devNull = workspaceDir.getFileSystem().getPath("/dev/null");
directories =
new BlazeDirectories(
- new ServerDirectories(devNull, devNull, devNull), workspaceDir, "blaze");
+ new ServerDirectories(installBase, outputBase, devNull), workspaceDir, "blaze");
+
+ this.pkgLocator =
+ new PathPackageLocator(
+ directories.getOutputBase(),
+ ImmutableList.of(Root.fromPath(workspaceDir)),
+ BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY);
+ this.pkgLocatorRef = new AtomicReference<>(pkgLocator);
+ this.externalFilesHelper =
+ ExternalFilesHelper.create(
+ pkgLocatorRef,
+ ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
+ directories);
}
public Builder setRuleClassProvider(RuleClassProvider ruleClassProvider) {
@@ -214,30 +227,22 @@
protected abstract String getDefaultDefaultPackageContents();
}
- protected AbstractPackageLoader(Builder builder) {
- Path workspaceDir = builder.workspaceDir;
- PathPackageLocator pkgLocator =
- new PathPackageLocator(
- null,
- ImmutableList.of(Root.fromPath(workspaceDir)),
- BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY);
+ AbstractPackageLoader(Builder builder) {
this.ruleClassProvider = builder.ruleClassProvider;
this.skylarkSemantics = builder.skylarkSemantics;
this.reporter = builder.reporter;
this.extraSkyFunctions = ImmutableMap.copyOf(builder.extraSkyFunctions);
- this.pkgLocatorRef = new AtomicReference<>(pkgLocator);
+ this.pkgLocatorRef = builder.pkgLocatorRef;
this.legacyGlobbingThreads = builder.legacyGlobbingThreads;
this.skyframeThreads = builder.skyframeThreads;
this.directories = builder.directories;
- this.externalFilesHelper = ExternalFilesHelper.create(
- pkgLocatorRef,
- ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
- directories);
+ this.externalFilesHelper = builder.externalFilesHelper;
+
this.preinjectedDiff =
makePreinjectedDiff(
skylarkSemantics,
- pkgLocator,
+ builder.pkgLocator,
builder.defaultsPackageContents,
ImmutableList.copyOf(builder.extraPrecomputedValues));
}
@@ -267,7 +272,7 @@
PrecomputedValue.DEFAULT_VISIBILITY.set(injectable, ConstantRuleVisibility.PRIVATE);
PrecomputedValue.SKYLARK_SEMANTICS.set(injectable, skylarkSemantics);
PrecomputedValue.DEFAULTS_PACKAGE_CONTENTS.set(injectable, defaultsPackageContents);
- return new ImmutableDiff(ImmutableList.<SkyKey>of(), valuesToInject);
+ return new ImmutableDiff(ImmutableList.of(), valuesToInject);
}
@Override
@@ -333,20 +338,25 @@
}
protected abstract String getName();
+
protected abstract ImmutableList<EnvironmentExtension> getEnvironmentExtensions();
+
protected abstract CrossRepositoryLabelViolationStrategy
getCrossRepositoryLabelViolationStrategy();
+
protected abstract ImmutableList<BuildFileName> getBuildFilesByPriority();
+
protected abstract ActionOnIOExceptionReadingBuildFile getActionOnIOExceptionReadingBuildFile();
- protected final ImmutableMap<SkyFunctionName, SkyFunction> makeFreshSkyFunctions() {
+ private ImmutableMap<SkyFunctionName, SkyFunction> makeFreshSkyFunctions() {
AtomicReference<TimestampGranularityMonitor> tsgm =
new AtomicReference<>(new TimestampGranularityMonitor(BlazeClock.instance()));
Cache<PackageIdentifier, LoadedPackageCacheEntry> packageFunctionCache =
CacheBuilder.newBuilder().build();
Cache<PackageIdentifier, AstParseResult> astCache = CacheBuilder.newBuilder().build();
- AtomicReference<PerBuildSyscallCache> syscallCacheRef = new AtomicReference<>(
- PerBuildSyscallCache.newBuilder().setConcurrencyLevel(legacyGlobbingThreads).build());
+ AtomicReference<PerBuildSyscallCache> syscallCacheRef =
+ new AtomicReference<>(
+ PerBuildSyscallCache.newBuilder().setConcurrencyLevel(legacyGlobbingThreads).build());
PackageFactory pkgFactory =
new PackageFactory(
ruleClassProvider,
@@ -359,13 +369,14 @@
pkgFactory.setSyscalls(syscallCacheRef);
pkgFactory.setMaxDirectoriesToEagerlyVisitInGlobbing(
MAX_DIRECTORIES_TO_EAGERLY_VISIT_IN_GLOBBING);
- CachingPackageLocator cachingPackageLocator = new CachingPackageLocator() {
- @Override
- @Nullable
- public Path getBuildFileForPackage(PackageIdentifier packageName) {
- return pkgLocatorRef.get().getPackageBuildFileNullable(packageName, syscallCacheRef);
- }
- };
+ CachingPackageLocator cachingPackageLocator =
+ new CachingPackageLocator() {
+ @Override
+ @Nullable
+ public Path getBuildFileForPackage(PackageIdentifier packageName) {
+ return pkgLocatorRef.get().getPackageBuildFileNullable(packageName, syscallCacheRef);
+ }
+ };
ImmutableMap.Builder<SkyFunctionName, SkyFunction> builder = ImmutableMap.builder();
builder
.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction())
@@ -378,10 +389,11 @@
.put(
SkyFunctions.PACKAGE_LOOKUP,
new PackageLookupFunction(
- /* deletedPackages= */ new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()),
+ /* deletedPackages= */ new AtomicReference<>(ImmutableSet.of()),
getCrossRepositoryLabelViolationStrategy(),
getBuildFilesByPriority()))
- .put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES,
+ .put(
+ SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES,
new BlacklistedPackagePrefixesFunction(
/*hardcodedBlacklistedPackagePrefixes=*/ ImmutableSet.of(),
/*additionalBlacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT))
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD
index 7ca7242..7cb108a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD
@@ -10,11 +10,15 @@
name = "packages",
srcs = glob(["*.java"]),
deps = [
+ "//src/main/java/com/google/devtools/build/lib:bazel-repository",
"//src/main/java/com/google/devtools/build/lib:bazel-rules",
+ "//src/main/java/com/google/devtools/build/lib:bazel/BazelRepositoryModule",
"//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:io",
"//src/main/java/com/google/devtools/build/lib:packages-internal",
+ "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
+ "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/vfs",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java
index dfed692..cdaa07c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java
@@ -17,17 +17,25 @@
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.bazel.rules.BazelRuleClassProvider;
+import com.google.devtools.build.lib.bazel.BazelRepositoryModule;
+import com.google.devtools.build.lib.bazel.repository.MavenDownloader;
+import com.google.devtools.build.lib.bazel.repository.MavenServerFunction;
+import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
+import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
+import com.google.devtools.build.lib.bazel.repository.skylark.SkylarkRepositoryFunction;
+import com.google.devtools.build.lib.bazel.rules.BazelRulesModule;
import com.google.devtools.build.lib.packages.BuildFileName;
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
import com.google.devtools.build.lib.packages.RuleClassProvider;
-import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
-import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
+import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
+import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
+import com.google.devtools.build.lib.skyframe.DirectoryListingFunction;
+import com.google.devtools.build.lib.skyframe.DirectoryListingStateFunction;
import com.google.devtools.build.lib.skyframe.LocalRepositoryLookupFunction;
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
@@ -37,37 +45,54 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
/**
- * Concrete implementation of {@link PackageLoader} that uses skyframe under the covers, but with
- * no caching or incrementality.
+ * Concrete implementation of {@link PackageLoader} that uses skyframe under the covers, but with no
+ * caching or incrementality.
*/
public class BazelPackageLoader extends AbstractPackageLoader {
/** Returns a fresh {@link Builder} instance. */
- public static Builder builder(Path workspaceDir) {
- Builder builder = new Builder(workspaceDir);
+ public static Builder builder(Path workspaceDir, Path installBase, Path outputBase) {
+ Builder builder = new Builder(workspaceDir, installBase, outputBase);
+
+ RepositoryCache repositoryCache = new RepositoryCache();
+ HttpDownloader httpDownloader = new HttpDownloader(repositoryCache);
// Set up SkyFunctions and PrecomputedValues needed to make local repositories work correctly.
ImmutableMap<String, RepositoryFunction> repositoryHandlers =
- ImmutableMap.of(
- LocalRepositoryRule.NAME, (RepositoryFunction) new LocalRepositoryFunction());
+ BazelRepositoryModule.repositoryRules(httpDownloader, new MavenDownloader(repositoryCache));
+
+ // Prevent PackageLoader from fetching any remote repositories; these should only be fetched by
+ // Bazel before calling PackageLoader.
+ AtomicBoolean isFetch = new AtomicBoolean(false);
builder.addExtraSkyFunctions(
- ImmutableMap.<SkyFunctionName, SkyFunction>of(
- SkyFunctions.LOCAL_REPOSITORY_LOOKUP,
- new LocalRepositoryLookupFunction(),
- SkyFunctions.REPOSITORY_DIRECTORY,
- new RepositoryDelegatorFunction(
- repositoryHandlers,
- null,
- new AtomicBoolean(true),
- ImmutableMap::of,
- builder.directories),
- SkyFunctions.REPOSITORY,
- new RepositoryLoaderFunction()));
+ ImmutableMap.<SkyFunctionName, SkyFunction>builder()
+ .put(
+ SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE,
+ new ClientEnvironmentFunction(new AtomicReference<>(ImmutableMap.of())))
+ .put(
+ SkyFunctions.DIRECTORY_LISTING_STATE,
+ new DirectoryListingStateFunction(builder.externalFilesHelper))
+ .put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction())
+ .put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction())
+ .put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction())
+ .put(
+ SkyFunctions.REPOSITORY_DIRECTORY,
+ new RepositoryDelegatorFunction(
+ repositoryHandlers,
+ new SkylarkRepositoryFunction(httpDownloader),
+ isFetch,
+ ImmutableMap::of,
+ builder.directories))
+ .put(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction())
+ .put(MavenServerFunction.NAME, new MavenServerFunction(builder.directories))
+ .build());
// Set extra precomputed values.
builder.addExtraPrecomputedValues(
+ PrecomputedValue.injected(PrecomputedValue.ACTION_ENV, ImmutableMap.of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES,
Suppliers.ofInstance(ImmutableMap.of())));
@@ -78,10 +103,17 @@
/** Builder for {@link BazelPackageLoader} instances. */
public static class Builder extends AbstractPackageLoader.Builder {
private static final ConfiguredRuleClassProvider DEFAULT_RULE_CLASS_PROVIDER =
- BazelRuleClassProvider.create();
+ createRuleClassProvider();
- private Builder(Path workspaceDir) {
- super(workspaceDir);
+ private static ConfiguredRuleClassProvider createRuleClassProvider() {
+ ConfiguredRuleClassProvider.Builder classProvider = new ConfiguredRuleClassProvider.Builder();
+ new BazelRepositoryModule().initializeRuleClasses(classProvider);
+ new BazelRulesModule().initializeRuleClasses(classProvider);
+ return classProvider.build();
+ }
+
+ private Builder(Path workspaceDir, Path installBase, Path outputBase) {
+ super(workspaceDir, installBase, outputBase);
}
@Override
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;
