Allow BlazeModules to expose a helper that PackageFactory will use for creating fresh Package instances. Also make a few Package methods public.
--
MOS_MIGRATED_REVID=123247246
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index dd0df6a..8a666b0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -209,7 +209,7 @@
* @precondition {@code name} must be a suffix of
* {@code filename.getParentDirectory())}.
*/
- private Package(PackageIdentifier packageId, String runfilesPrefix) {
+ protected Package(PackageIdentifier packageId, String runfilesPrefix) {
this.packageIdentifier = packageId;
this.workspaceName = runfilesPrefix;
this.nameFragment = Canonicalizer.fragments().intern(packageId.getPackageFragment());
@@ -531,13 +531,23 @@
suffix = "";
}
+ throw makeNoSuchTargetException(targetName, suffix);
+ }
+
+ protected NoSuchTargetException makeNoSuchTargetException(String targetName, String suffix) {
+ Label label;
try {
- throw new NoSuchTargetException(createLabel(targetName), "target '" + targetName
- + "' not declared in package '" + name + "'" + suffix + " defined by "
- + this.filename);
+ label = createLabel(targetName);
} catch (LabelSyntaxException e) {
throw new IllegalArgumentException(targetName);
}
+ String msg = String.format(
+ "target '%s' not declared in package '%s'%s defined by %s",
+ targetName,
+ name,
+ suffix,
+ filename);
+ return new NoSuchTargetException(label, msg);
}
/**
@@ -666,16 +676,36 @@
}
}
- public static Builder newExternalPackageBuilder(Path workspacePath, String runfilesPrefix) {
- Builder b = new Builder(Label.EXTERNAL_PACKAGE_IDENTIFIER, runfilesPrefix);
+ public static Builder newExternalPackageBuilder(Builder.Helper helper, Path workspacePath,
+ String runfilesPrefix) {
+ Builder b = new Builder(helper.createFreshPackage(
+ Label.EXTERNAL_PACKAGE_IDENTIFIER, runfilesPrefix));
b.setFilename(workspacePath);
b.setMakeEnv(new MakeEnvironment.Builder());
return b;
}
+ /** A builder for {@link Package} objects. Only intended to be used by {@link PackageFactory}. */
public static class Builder {
- protected static Package newPackage(PackageIdentifier packageId, String runfilesPrefix) {
- return new Package(packageId, runfilesPrefix);
+ public static interface Helper {
+ /**
+ * Returns a fresh {@link Package} instance that a {@link Builder} will internally mutate
+ * during package loading.
+ */
+ Package createFreshPackage(PackageIdentifier packageId, String runfilesPrefix);
+ }
+
+ /** {@link Helper} that simply calls the {@link Package} constructor. */
+ public static class DefaultHelper implements Helper {
+ public static final DefaultHelper INSTANCE = new DefaultHelper();
+
+ private DefaultHelper() {
+ }
+
+ @Override
+ public Package createFreshPackage(PackageIdentifier packageId, String runfilesPrefix) {
+ return new Package(packageId, runfilesPrefix);
+ }
}
/**
@@ -740,8 +770,8 @@
}
}
- public Builder(PackageIdentifier id, String runfilesPrefix) {
- this(newPackage(id, runfilesPrefix));
+ public Builder(Helper helper, PackageIdentifier id, String runfilesPrefix) {
+ this(helper.createFreshPackage(id, runfilesPrefix));
}
protected PackageIdentifier getPackageIdentifier() {
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 b1ad355..73ff55d 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
@@ -331,6 +331,8 @@
private final ImmutableList<EnvironmentExtension> environmentExtensions;
private final ImmutableMap<String, PackageArgument<?>> packageArguments;
+ private final Package.Builder.Helper packageBuilderHelper;
+
/**
* Constructs a {@code PackageFactory} instance with the given rule factory.
*/
@@ -341,7 +343,8 @@
null,
AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY,
ImmutableList.<EnvironmentExtension>of(),
- "test");
+ "test",
+ Package.Builder.DefaultHelper.INSTANCE);
}
@VisibleForTesting
@@ -352,7 +355,8 @@
null,
AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY,
ImmutableList.of(environmentExtension),
- "test");
+ "test",
+ Package.Builder.DefaultHelper.INSTANCE);
}
@VisibleForTesting
@@ -363,7 +367,8 @@
null,
AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY,
environmentExtensions,
- "test");
+ "test",
+ Package.Builder.DefaultHelper.INSTANCE);
}
/**
@@ -375,7 +380,8 @@
Map<String, String> platformSetRegexps,
Function<RuleClass, AttributeContainer> attributeContainerFactory,
Iterable<EnvironmentExtension> environmentExtensions,
- String version) {
+ String version,
+ Package.Builder.Helper packageBuilderHelper) {
this.platformSetRegexps = platformSetRegexps;
this.ruleFactory = new RuleFactory(ruleClassProvider, attributeContainerFactory);
this.ruleClassProvider = ruleClassProvider;
@@ -388,6 +394,7 @@
this.packageArguments = createPackageArguments();
this.nativeModule = newNativeModule();
this.workspaceNativeModule = WorkspaceFactory.newNativeModule(ruleClassProvider, version);
+ this.packageBuilderHelper = packageBuilderHelper;
}
/**
@@ -1276,14 +1283,24 @@
}
@VisibleForTesting
+ public Package.Builder newExternalPackageBuilder(Path workspacePath, String runfilesPrefix) {
+ return Package.newExternalPackageBuilder(packageBuilderHelper, workspacePath, runfilesPrefix);
+ }
+
+ @VisibleForTesting
+ public Package.Builder newPackageBuilder(PackageIdentifier packageId, String runfilesPrefix) {
+ return new Package.Builder(packageBuilderHelper, packageId, runfilesPrefix);
+ }
+
+ @VisibleForTesting
public Package createPackageForTesting(
PackageIdentifier packageId,
Path buildFile,
CachingPackageLocator locator,
EventHandler eventHandler)
throws NoSuchPackageException, InterruptedException {
- Package externalPkg =
- Package.newExternalPackageBuilder(buildFile.getRelative("WORKSPACE"), "TESTING").build();
+ Package externalPkg = newExternalPackageBuilder(
+ buildFile.getRelative("WORKSPACE"), "TESTING").build();
return createPackageForTesting(packageId, externalPkg, buildFile, locator, eventHandler);
}
@@ -1533,8 +1550,8 @@
Map<String, Extension> imports,
ImmutableList<Label> skylarkFileDependencies)
throws InterruptedException {
- Package.Builder pkgBuilder = new Package.Builder(
- packageId, ruleClassProvider.getRunfilesPrefix());
+ Package.Builder pkgBuilder = new Package.Builder(packageBuilderHelper.createFreshPackage(
+ packageId, ruleClassProvider.getRunfilesPrefix()));
StoredEventHandler eventHandler = new StoredEventHandler();
try (Mutability mutability = Mutability.create("package %s", packageId)) {
@@ -1621,8 +1638,8 @@
.setPhase(Phase.LOADING)
.build();
- Package.Builder pkgBuilder = new Package.Builder(packageId,
- ruleClassProvider.getRunfilesPrefix());
+ Package.Builder pkgBuilder = new Package.Builder(packageBuilderHelper.createFreshPackage(
+ packageId, ruleClassProvider.getRunfilesPrefix()));
pkgBuilder.setFilename(buildFilePath)
.setMakeEnv(pkgMakeEnv)
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 25f692a..743cff5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.exec.OutputService;
import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.NoSuchThingException;
+import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.Preprocessor;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -342,6 +343,15 @@
}
/**
+ * Returns a helper that the {@link PackageFactory} will use during package loading. If the module
+ * does not provide any heloer, it should return null. Note that only one helper per Bazel/Blaze
+ * runtime is allowed.
+ */
+ public Package.Builder.Helper getPackageBuilderHelper() {
+ return null;
+ }
+
+ /**
* Returns a factory for creating {@link AbstractBlazeQueryEnvironment} objects.
* If the module does not provide any {@link QueryEnvironmentFactory}, it should return null. Note
* that only one factory per Bazel/Blaze runtime is allowed.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 8c9af36..ce6b1f5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.events.OutputFilter;
import com.google.devtools.build.lib.flags.CommandNameCache;
import com.google.devtools.build.lib.packages.AttributeContainer;
+import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.Preprocessor;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -1260,13 +1261,27 @@
extensions.add(module.getPackageEnvironmentExtension());
}
+ Package.Builder.Helper packageBuilderHelper = null;
+ for (BlazeModule module : blazeModules) {
+ Package.Builder.Helper candidateHelper = module.getPackageBuilderHelper();
+ if (candidateHelper != null) {
+ Preconditions.checkState(packageBuilderHelper == null,
+ "more than one module defines a package builder helper");
+ packageBuilderHelper = candidateHelper;
+ }
+ }
+ if (packageBuilderHelper == null) {
+ packageBuilderHelper = Package.Builder.DefaultHelper.INSTANCE;
+ }
+
PackageFactory packageFactory =
new PackageFactory(
ruleClassProvider,
platformRegexps,
attributeContainerFactory,
extensions,
- BlazeVersionInfo.instance().getVersion());
+ BlazeVersionInfo.instance().getVersion(),
+ packageBuilderHelper);
if (configurationFactory == null) {
configurationFactory = new ConfigurationFactory(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index c84e420..87ec20c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -67,8 +67,8 @@
}
Path repoWorkspace = workspaceRoot.getRoot().getRelative(workspaceRoot.getRelativePath());
- Package.Builder builder =
- Package.newExternalPackageBuilder(repoWorkspace, ruleClassProvider.getRunfilesPrefix());
+ Package.Builder builder = packageFactory.newExternalPackageBuilder(
+ repoWorkspace, ruleClassProvider.getRunfilesPrefix());
if (workspaceASTValue.getASTs().isEmpty()) {
return new WorkspaceFileValue(
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
index 246b13d..cbfa252 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
@@ -79,7 +79,8 @@
protected void setUpContextForRule(Map<String, Object> kwargs, Attribute... attributes)
throws Exception {
- Package.Builder packageBuilder = Package.newExternalPackageBuilder(workspaceFile, "runfiles");
+ Package.Builder packageBuilder = Package.newExternalPackageBuilder(
+ Package.Builder.DefaultHelper.INSTANCE, workspaceFile, "runfiles");
FuncallExpression ast =
new FuncallExpression(new Identifier("test"), ImmutableList.<Passed>of());
ast.setLocation(Location.BUILTIN);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index 6185190..2496581 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -50,7 +50,6 @@
import com.google.devtools.build.lib.events.Location.LineAndColumn;
import com.google.devtools.build.lib.packages.Attribute.ValidityPredicate;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
-import com.google.devtools.build.lib.packages.Package.Builder;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleClass.Configurator;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory;
@@ -261,7 +260,8 @@
}
private Package.Builder createDummyPackageBuilder() {
- return new Builder(PackageIdentifier.createInMainRepo(TEST_PACKAGE_NAME), "TESTING")
+ return packageFactory.newPackageBuilder(
+ PackageIdentifier.createInMainRepo(TEST_PACKAGE_NAME), "TESTING")
.setFilename(testBuildfilePath)
.setMakeEnv(new MakeEnvironment.Builder());
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
index a716e22..95d3c45 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
@@ -53,7 +53,7 @@
public void testCreateRule() throws Exception {
Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD");
Package.Builder pkgBuilder =
- new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
+ packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
.setFilename(myPkgPath)
.setMakeEnv(new MakeEnvironment.Builder());
@@ -116,7 +116,7 @@
@Test
public void testCreateWorkspaceRule() throws Exception {
Path myPkgPath = scratch.resolve("/foo/workspace/WORKSPACE");
- Package.Builder pkgBuilder = Package.newExternalPackageBuilder(myPkgPath, "TESTING");
+ Package.Builder pkgBuilder = packageFactory.newExternalPackageBuilder(myPkgPath, "TESTING");
Map<String, Object> attributeValues = new HashMap<>();
attributeValues.put("name", "foo");
@@ -140,7 +140,7 @@
public void testWorkspaceRuleFailsInBuildFile() throws Exception {
Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD");
Package.Builder pkgBuilder =
- new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
+ packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
.setFilename(myPkgPath)
.setMakeEnv(new MakeEnvironment.Builder());
@@ -169,7 +169,7 @@
public void testBuildRuleFailsInWorkspaceFile() throws Exception {
Path myPkgPath = scratch.resolve("/foo/workspace/WORKSPACE");
Package.Builder pkgBuilder =
- new Package.Builder(Label.EXTERNAL_PACKAGE_IDENTIFIER, "TESTING")
+ packageFactory.newPackageBuilder(Label.EXTERNAL_PACKAGE_IDENTIFIER, "TESTING")
.setFilename(myPkgPath)
.setMakeEnv(new MakeEnvironment.Builder());
@@ -210,7 +210,7 @@
public void testOutputFileNotEqualDot() throws Exception {
Path myPkgPath = scratch.resolve("/foo");
Package.Builder pkgBuilder =
- new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
+ packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
.setFilename(myPkgPath)
.setMakeEnv(new MakeEnvironment.Builder());
@@ -243,7 +243,7 @@
public void testTestRules() throws Exception {
Path myPkgPath = scratch.resolve("/foo/workspace/mypkg/BUILD");
Package pkg =
- new Package.Builder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
+ packageFactory.newPackageBuilder(PackageIdentifier.createInMainRepo("mypkg"), "TESTING")
.setFilename(myPkgPath)
.setMakeEnv(new MakeEnvironment.Builder())
.build();
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
index 8bcbd5a..cd0e3d2 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
@@ -115,7 +115,8 @@
fail("Shouldn't happen: " + e.getMessage());
}
StoredEventHandler eventHandler = new StoredEventHandler();
- builder = Package.newExternalPackageBuilder(workspaceFilePath, "");
+ builder = Package.newExternalPackageBuilder(
+ Package.Builder.DefaultHelper.INSTANCE, workspaceFilePath, "");
this.factory = new WorkspaceFactory(
builder,
TestRuleClassProvider.getRuleClassProvider(),
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index ac3f401..ddda9ff 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -57,7 +57,8 @@
null,
AttributeContainer.ATTRIBUTE_CONTAINER_FACTORY,
ImmutableList.copyOf(environmentExtensions),
- "test");
+ "test",
+ Package.Builder.DefaultHelper.INSTANCE);
}
/**
@@ -123,7 +124,7 @@
TestUtils.getPool());
LegacyGlobber globber = new LegacyGlobber(globCache);
Package externalPkg =
- Package.newExternalPackageBuilder(
+ factory.newExternalPackageBuilder(
buildFile.getParentDirectory().getRelative("WORKSPACE"), "TESTING")
.build();
Builder resultBuilder =
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
index 4cbff52..e160d6d 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
@@ -69,6 +69,7 @@
private static final int GLOBBING_THREADS = 7;
protected ConfiguredRuleClassProvider ruleClassProvider;
+ protected PackageFactory packageFactory;
protected SkyframeExecutor skyframeExecutor;
@Before
@@ -84,8 +85,8 @@
} else {
ruleClassProvider = TestRuleClassProvider.getRuleClassProvider();
}
- skyframeExecutor = createSkyframeExecutor(getEnvironmentExtensions(),
- getPreprocessorFactorySupplier());
+ packageFactory = new PackageFactory(ruleClassProvider, getEnvironmentExtensions());
+ skyframeExecutor = createSkyframeExecutor(getPreprocessorFactorySupplier());
setUpSkyframe(parsePackageCacheOptions());
}
@@ -95,11 +96,10 @@
}
private SkyframeExecutor createSkyframeExecutor(
- Iterable<EnvironmentExtension> environmentExtensions,
Preprocessor.Factory.Supplier preprocessorFactorySupplier) {
SkyframeExecutor skyframeExecutor =
SequencedSkyframeExecutor.create(
- new PackageFactory(ruleClassProvider, environmentExtensions),
+ packageFactory,
new BlazeDirectories(outputBase, outputBase, rootDirectory, TestConstants.PRODUCT_NAME),
null, /* BinTools */
null, /* workspaceStatusActionFactory */
diff --git a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java
index 60a347c..53af210 100644
--- a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java
+++ b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/WorkspaceResolver.java
@@ -76,8 +76,10 @@
* Converts the WORKSPACE file content into an ExternalPackage.
*/
public Package parse(Path workspacePath) {
- Package.Builder builder =
- Package.newExternalPackageBuilder(workspacePath, ruleClassProvider.getRunfilesPrefix());
+ Package.Builder builder = Package.newExternalPackageBuilder(
+ Package.Builder.DefaultHelper.INSTANCE,
+ workspacePath,
+ ruleClassProvider.getRunfilesPrefix());
try (Mutability mutability = Mutability.create("External Package %s", workspacePath)) {
new WorkspaceFactory(builder, ruleClassProvider, environmentExtensions, mutability)
.parse(ParserInputSource.create(workspacePath));
