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(
