Embed builtins bzls as a Java resource
Currently the builtins .bzl files are packaged into, and read from, the install base. However, there is no install base in our Java unit tests, as well as in BazelPackageLoader. To support these use cases, we now instead embed the builtins_bzl.zip as a Java resource, and extract it to a fresh InMemoryFileSystem upon ConfiguredRuleClassProvider construction. The flag value --experimental_builtins_bzl_path=%install_base% has been renamed to "%bundled%".
A previous prototype attempted to make ServerDirectories responsible for knowing how to access the bundled bzls. This was problematic because it required updating all instantiators of ServerDirectories (mostly tests), and complicated the target graph. The new design puts this knowledge in BazelRuleClassProvider, alongside the other rule logic registration. This avoids an undesirable situation whereby any un-updated test could block proper use of builtins injection in production.
The only loss of not having builtins bzls live in the install base is that their sources are not immediately available to a debugger; you have to use --experimental_builtins_bzl_path=<some staging location> instead. This matches the intended workflow when developing builtins bzl code anyway.
Our typical way of locating resource files (ResourceFileLoader) is to resolve them relative to a given Java class. But the builtins zip does not live underneath the java tree. If we tried to add it directly as a resource, then the java_binary rule would create the jar entry under its absolute package path instead of its relative path within the java root. So instead we use a genrule to copy it into the java tree where needed.
Misc:
- Expose a method in ResourceFileLoader to compute a resource path without retrieving it yet. This avoids the inconvenience of having to pass two args (a Class and a relative resource name) around.
- Drive-by removal of a bunch of BUILD deps made obsolete by a recent configuration machinery cleanup. Likewise, drive-by auto-formatting of some files affected by the prior version of this CL.
- Added the builtins zip to the distfile, for bootstrapping. This didn't belong in either the "srcs" tree or the "derived_java_srcs" target, so I added it directly as a distfile dep.
- Fixed a hashCode/equals bug in ServerDirectories.
Work toward #11437.
PiperOrigin-RevId: 347697763
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 37091e9..d468ebd0a8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -434,6 +434,7 @@
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
"//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/lib/vfs/inmemoryfs",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index b869ace..24221c9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.analysis.RuleContext.PrerequisiteValidator;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
@@ -52,9 +53,15 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.SymbolGenerator;
import com.google.devtools.build.lib.starlarkbuildapi.core.Bootstrap;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionsProvider;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
@@ -66,6 +73,8 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
import javax.annotation.Nullable;
import net.starlark.java.annot.StarlarkAnnotations;
import net.starlark.java.annot.StarlarkBuiltin;
@@ -99,7 +108,8 @@
private Label preludeLabel;
private String runfilesPrefix;
private String toolsRepository;
- private String builtinsPackagePathInSource;
+ @Nullable private String builtinsBzlZipResource;
+ private String builtinsBzlPackagePathInSource;
private final List<Class<? extends Fragment>> configurationFragmentClasses = new ArrayList<>();
private final List<BuildInfoFactory> buildInfoFactories = new ArrayList<>();
private final Set<Class<? extends FragmentOptions>> configurationOptions =
@@ -107,11 +117,9 @@
private final Map<String, RuleClass> ruleClassMap = new HashMap<>();
private final Map<String, RuleDefinition> ruleDefinitionMap = new HashMap<>();
- private final Map<String, NativeAspectClass> nativeAspectClassMap =
- new HashMap<>();
+ private final Map<String, NativeAspectClass> nativeAspectClassMap = new HashMap<>();
private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>();
- private final Digraph<Class<? extends RuleDefinition>> dependencyGraph =
- new Digraph<>();
+ private final Digraph<Class<? extends RuleDefinition>> dependencyGraph = new Digraph<>();
private final List<Class<? extends Fragment>> universalFragments = new ArrayList<>();
@Nullable private TransitionFactory<Rule> trimmingTransitionFactory = null;
@Nullable private PatchTransition toolchainTaggedTrimmingTransition = null;
@@ -170,10 +178,22 @@
return this;
}
+ /**
+ * Sets the resource path to the builtins_bzl.zip resource.
+ *
+ * <p>This value is required for production uses. For uses in tests, this may be left null, but
+ * the resulting rule class provider will not work with {@code
+ * --experimental_builtins_bzl_path=%bundled%}.
+ */
+ public Builder setBuiltinsBzlZipResource(String name) {
+ this.builtinsBzlZipResource = name;
+ return this;
+ }
+
// This is required if the rule class provider will be used with
// "--experimental_builtins_bzl_path=%workspace%", but can be skipped in unit tests.
- public Builder setBuiltinsPackagePathInSource(String path) {
- this.builtinsPackagePathInSource = path;
+ public Builder setBuiltinsBzlPackagePathInSource(String path) {
+ this.builtinsBzlPackagePathInSource = path;
return this;
}
@@ -356,15 +376,20 @@
try {
Constructor<? extends RuleConfiguredTargetFactory> ctor = factoryClass.getConstructor();
return ctor.newInstance();
- } catch (NoSuchMethodException | IllegalAccessException | InstantiationException
+ } catch (NoSuchMethodException
+ | IllegalAccessException
+ | InstantiationException
| InvocationTargetException e) {
throw new IllegalStateException(e);
}
}
private RuleClass commitRuleDefinition(Class<? extends RuleDefinition> definitionClass) {
- RuleDefinition instance = checkNotNull(ruleDefinitionMap.get(definitionClass.getName()),
- "addRuleDefinition(new %s()) should be called before build()", definitionClass.getName());
+ RuleDefinition instance =
+ checkNotNull(
+ ruleDefinitionMap.get(definitionClass.getName()),
+ "addRuleDefinition(new %s()) should be called before build()",
+ definitionClass.getName());
RuleDefinition.Metadata metadata = instance.getMetadata();
checkArgument(
@@ -374,19 +399,18 @@
List<Class<? extends RuleDefinition>> ancestors = metadata.ancestors();
checkArgument(
- metadata.type() == ABSTRACT ^ metadata.factoryClass()
- != RuleConfiguredTargetFactory.class);
+ metadata.type() == ABSTRACT
+ ^ metadata.factoryClass() != RuleConfiguredTargetFactory.class);
checkArgument(
- (metadata.type() != TEST)
- || ancestors.contains(BaseRuleClasses.TestBaseRule.class));
+ (metadata.type() != TEST) || ancestors.contains(BaseRuleClasses.TestBaseRule.class));
RuleClass[] ancestorClasses = new RuleClass[ancestors.size()];
for (int i = 0; i < ancestorClasses.length; i++) {
ancestorClasses[i] = ruleMap.get(ancestors.get(i));
if (ancestorClasses[i] == null) {
// Ancestors should have been initialized by now
- throw new IllegalStateException("Ancestor " + ancestors.get(i) + " of "
- + metadata.name() + " is not initialized");
+ throw new IllegalStateException(
+ "Ancestor " + ancestors.get(i) + " of " + metadata.name() + " is not initialized");
}
}
@@ -395,8 +419,8 @@
factory = createFactory(metadata.factoryClass());
}
- RuleClass.Builder builder = new RuleClass.Builder(
- metadata.name(), metadata.type(), false, ancestorClasses);
+ RuleClass.Builder builder =
+ new RuleClass.Builder(metadata.name(), metadata.type(), false, ancestorClasses);
builder.factory(factory);
builder.setThirdPartyLicenseExistencePolicy(thirdPartyLicenseExistencePolicy);
RuleClass ruleClass = instance.build(builder, this);
@@ -407,17 +431,58 @@
return ruleClass;
}
+ /**
+ * Unpacks the builtins zip file into an InMemoryFileSystem. The zip file is located as a Java
+ * resource file.
+ *
+ * <p>The files underneath the zip's {@code builtins_bzl/} directory are moved to a top-level
+ * {@code /virtual_builtins_bzl} directory. The Path to that directory is returned.
+ */
+ private static Path unpackBuiltinsBzlZipResource(String builtinsResourceName) {
+ ClassLoader loader = ConfiguredRuleClassProvider.class.getClassLoader();
+ try (InputStream builtinsZip = loader.getResourceAsStream(builtinsResourceName)) {
+ Preconditions.checkArgument(
+ builtinsZip != null, "No resource with name %s", builtinsResourceName);
+
+ InMemoryFileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256);
+ Path root = fs.getPath("/virtual_builtins_bzl");
+
+ try (ZipInputStream zip = new ZipInputStream(builtinsZip)) {
+ for (ZipEntry entry = zip.getNextEntry(); entry != null; entry = zip.getNextEntry()) {
+ String entryName = entry.getName();
+ Preconditions.checkArgument(entryName.startsWith("builtins_bzl/"));
+ Path dest = root.getRelative(entryName.substring("builtins_bzl/".length()));
+
+ dest.getParentDirectory().createDirectoryAndParents();
+ try (OutputStream os = dest.getOutputStream()) {
+ ByteStreams.copy(zip, os);
+ }
+ }
+ }
+ return root;
+ } catch (IOException ex) {
+ throw new IllegalArgumentException(
+ "Error while unpacking builtins_bzl zip resource file", ex);
+ }
+ }
+
public ConfiguredRuleClassProvider build() {
for (Node<Class<? extends RuleDefinition>> ruleDefinition :
dependencyGraph.getTopologicalOrder()) {
commitRuleDefinition(ruleDefinition.getLabel());
}
+ Path builtinsBzlRoot =
+ builtinsBzlZipResource != null
+ ? unpackBuiltinsBzlZipResource(builtinsBzlZipResource)
+ : null;
+
return new ConfiguredRuleClassProvider(
preludeLabel,
runfilesPrefix,
toolsRepository,
- builtinsPackagePathInSource,
+ builtinsBzlRoot,
+ builtinsBzlPackagePathInSource,
ImmutableMap.copyOf(ruleClassMap),
ImmutableMap.copyOf(ruleDefinitionMap),
ImmutableMap.copyOf(nativeAspectClassMap),
@@ -451,53 +516,43 @@
}
}
- /**
- * Default content that should be added at the beginning of the WORKSPACE file.
- */
+ /** Default content that should be added at the beginning of the WORKSPACE file. */
private final String defaultWorkspaceFilePrefix;
- /**
- * Default content that should be added at the end of the WORKSPACE file.
- */
+ /** Default content that should be added at the end of the WORKSPACE file. */
private final String defaultWorkspaceFileSuffix;
-
- /**
- * Label for the prelude file.
- */
+ /** Label for the prelude file. */
private final Label preludeLabel;
- /**
- * The default runfiles prefix.
- */
+ /** The default runfiles prefix. */
private final String runfilesPrefix;
- /**
- * The path to the tools repository.
- */
+ /** The path to the tools repository. */
private final String toolsRepository;
- /** The relative location of the builtins_bzl directory within a Bazel source tree. */
- private final String builtinsPackagePathInSource;
-
/**
- * Maps rule class name to the metaclass instance for that rule.
+ * Where the builtins bzl files are located (if not overridden by
+ * --experimental_builtins_bzl_path). Note that this lives in a separate InMemoryFileSystem.
+ *
+ * <p>May be null in tests, in which case --experimental_builtins_bzl_path must point to a
+ * builtins root.
*/
+ @Nullable private final Path builtinsBzlRoot;
+
+ /** The relative location of the builtins_bzl directory within a Bazel source tree. */
+ private final String builtinsBzlPackagePathInSource;
+
+ /** Maps rule class name to the metaclass instance for that rule. */
private final ImmutableMap<String, RuleClass> ruleClassMap;
- /**
- * Maps rule class name to the rule definition objects.
- */
+ /** Maps rule class name to the rule definition objects. */
private final ImmutableMap<String, RuleDefinition> ruleDefinitionMap;
- /**
- * Maps aspect name to the aspect factory meta class.
- */
+ /** Maps aspect name to the aspect factory meta class. */
private final ImmutableMap<String, NativeAspectClass> nativeAspectClassMap;
- /**
- * The configuration options that affect the behavior of the rules.
- */
+ /** The configuration options that affect the behavior of the rules. */
private final ImmutableList<Class<? extends FragmentOptions>> configurationOptions;
/** The set of configuration fragment factories. */
@@ -549,7 +604,8 @@
Label preludeLabel,
String runfilesPrefix,
String toolsRepository,
- String builtinsPackagePathInSource,
+ @Nullable Path builtinsBzlRoot,
+ String builtinsBzlPackagePathInSource,
ImmutableMap<String, RuleClass> ruleClassMap,
ImmutableMap<String, RuleDefinition> ruleDefinitionMap,
ImmutableMap<String, NativeAspectClass> nativeAspectClassMap,
@@ -573,7 +629,8 @@
this.preludeLabel = preludeLabel;
this.runfilesPrefix = runfilesPrefix;
this.toolsRepository = toolsRepository;
- this.builtinsPackagePathInSource = builtinsPackagePathInSource;
+ this.builtinsBzlRoot = builtinsBzlRoot;
+ this.builtinsBzlPackagePathInSource = builtinsBzlPackagePathInSource;
this.ruleClassMap = ruleClassMap;
this.ruleDefinitionMap = ruleDefinitionMap;
this.nativeAspectClassMap = nativeAspectClassMap;
@@ -650,8 +707,14 @@
}
@Override
- public String getBuiltinsPackagePathInSource() {
- return builtinsPackagePathInSource;
+ @Nullable
+ public Path getBuiltinsBzlRoot() {
+ return builtinsBzlRoot;
+ }
+
+ @Override
+ public String getBuiltinsBzlPackagePathInSource() {
+ return builtinsBzlPackagePathInSource;
}
@Override
@@ -716,16 +779,12 @@
return shouldInvalidateCacheForOptionDiff.apply(newOptions, changedOption, oldValue, newValue);
}
- /**
- * Returns the set of configuration options that are supported in this module.
- */
+ /** Returns the set of configuration options that are supported in this module. */
public ImmutableList<Class<? extends FragmentOptions>> getConfigurationOptions() {
return configurationOptions;
}
- /**
- * Returns the definition of the rule class definition with the specified name.
- */
+ /** Returns the definition of the rule class definition with the specified name. */
public RuleDefinition getRuleClassDefinition(String ruleClassName) {
return ruleDefinitionMap.get(ruleClassName);
}
@@ -738,9 +797,7 @@
return universalFragments;
}
- /**
- * Creates a BuildOptions class for the given options taken from an optionsProvider.
- */
+ /** Creates a BuildOptions class for the given options taken from an optionsProvider. */
public BuildOptions createBuildOptions(OptionsProvider optionsProvider) {
return BuildOptions.of(configurationOptions, optionsProvider);
}
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 5a8bb70..4b1ada1 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
@@ -25,8 +25,8 @@
import javax.annotation.Nullable;
/**
- * Represents the relevant directories for the server: the location of the embedded binaries
- * and the output directories.
+ * Represents the relevant directories for the server: the location of the embedded binaries and the
+ * output directories.
*/
@Immutable
public final class ServerDirectories {
@@ -34,15 +34,21 @@
/** Top-level user output directory; used, e.g., as default location for caches. */
private final Path outputUserRoot;
+
/** Where Blaze gets unpacked. */
private final Path installBase;
/** The content hash of everything in installBase. */
@Nullable private final HashCode installMD5;
+
/** The root of the temp and output trees. */
private final Path outputBase;
private final Path execRootBase;
+ // TODO(bazel-team): Use a builder to simplify/unify these constructors. This makes it easier to
+ // have sensible defaults, e.g. execRootBase = outputBase + "/execroot". Then reorder the fields
+ // to be consistent throughout this class.
+
public ServerDirectories(
Path installBase,
Path outputBase,
@@ -58,7 +64,11 @@
}
public ServerDirectories(Path installBase, Path outputBase, Path outputUserRoot) {
- this(installBase, outputBase, outputUserRoot, outputBase.getRelative(EXECROOT), null);
+ this(
+ // Some tests set installBase to null.
+ // TODO(bazel-team): Be more consistent about whether nulls are permitted (e.g. equals()
+ // presently doesn't tolerate them). We should probably just disallow them.
+ installBase, outputBase, outputUserRoot, outputBase.getRelative(EXECROOT), null);
}
private static HashCode checkMD5(HashCode hash) {
@@ -116,7 +126,7 @@
@Override
public int hashCode() {
- return Objects.hash(installBase, installMD5, outputBase, outputUserRoot);
+ return Objects.hash(installBase, installMD5, outputBase, execRootBase, outputUserRoot);
}
@Override
@@ -131,6 +141,7 @@
return this.installBase.equals(that.installBase)
&& Objects.equals(this.installMD5, that.installMD5)
&& this.outputBase.equals(that.outputBase)
+ && this.execRootBase.equals(that.execRootBase)
&& this.outputUserRoot.equals(that.outputUserRoot);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
index 89cdaa4..0f674f4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD
@@ -22,7 +22,7 @@
java_library(
name = "rules",
srcs = glob(["*.java"]),
- resources = glob(["*.WORKSPACE"]),
+ resources = glob(["*.WORKSPACE"]) + [":builtins_bzl_zip"],
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
@@ -98,3 +98,14 @@
"//third_party:jsr305",
],
)
+
+# We need this redirect target so that the builtins zip can be packaged as a
+# resource next to the BazelBuiltins class (com/google/devtools/...). Otherwise
+# it gets placed under the package path of the original zip target.
+genrule(
+ name = "builtins_bzl_zip",
+ srcs = ["//src/main/starlark/builtins_bzl:builtins_bzl.zip"],
+ outs = ["builtins_bzl.zip"],
+ cmd = "cp $(SRCS) $@",
+ visibility = ["//:__pkg__"],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 532590c..a2b6858 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -240,7 +240,9 @@
/** Adds this class's definitions to a builder. */
public static void setup(ConfiguredRuleClassProvider.Builder builder) {
builder.setToolsRepository(TOOLS_REPOSITORY);
- builder.setBuiltinsPackagePathInSource("src/main/starlark/builtins_bzl");
+ builder.setBuiltinsBzlZipResource(
+ ResourceFileLoader.resolveResource(BazelRuleClassProvider.class, "builtins_bzl.zip"));
+ builder.setBuiltinsBzlPackagePathInSource("src/main/starlark/builtins_bzl");
builder.setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePolicy.NEVER_CHECK);
for (RuleSet ruleSet : RULE_SETS) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
index 1f7765b..8791fd4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
+import com.google.devtools.build.lib.vfs.Path;
import java.util.Map;
import net.starlark.java.eval.StarlarkThread;
@@ -38,8 +39,17 @@
*/
String getRunfilesPrefix();
+ /**
+ * Where the builtins bzl files are located (if not overridden by
+ * --experimental_builtins_bzl_path). Note that this lives in a separate InMemoryFileSystem.
+ *
+ * <p>May be null in tests, in which case --experimental_builtins_bzl_path must point to a
+ * builtins root.
+ */
+ Path getBuiltinsBzlRoot();
+
/** The relative location of the builtins_bzl directory within a Bazel source tree. */
- String getBuiltinsPackagePathInSource();
+ String getBuiltinsBzlPackagePathInSource();
/**
* Returns a map from rule names to rule class objects.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
index 4a5ff48..4925142 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
@@ -84,8 +84,8 @@
"This flag tells Bazel how to find the \"@_builtins\" .bzl files that govern how "
+ "predeclared symbols for BUILD and .bzl files are defined. This flag is only "
+ "intended for Bazel developers, to help when writing @_builtins .bzl code. "
- + "Ordinarily this value is set to \"%install_base%\", which means to use the "
- + "builtins_bzl/ directory located in the install base. However, it can be set to "
+ + "Ordinarily this value is set to \"%bundled%\", which means to use the "
+ + "builtins_bzl/ directory packaged in the Bazel binary. However, it can be set to "
+ "the path (relative to the root of the current workspace) of an alternate "
+ "builtins_bzl/ directory, such as one in a Bazel source tree workspace. A literal "
+ "value of \"%workspace%\" is equivalent to the relative package path of "
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
index 7d2e2df..079b375 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.RecordingSkyFunctionEnvironment;
@@ -102,7 +103,7 @@
// computeInline() entry point.
private final PackageFactory packageFactory;
- // Used for determining paths to builtins bzls.
+ // Used for determining paths to builtins bzls that live in the workspace.
private final BlazeDirectories directories;
// Handles retrieving BzlCompileValues, either by calling Skyframe or by inlining
@@ -585,20 +586,29 @@
}
private Root getBuiltinsRoot(StarlarkSemantics starlarkSemantics) throws BzlLoadFailedException {
- String path = starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_BZL_PATH);
- if (path.isEmpty()) {
+ String flag = starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_BZL_PATH);
+ if (flag.isEmpty()) {
throw new IllegalStateException("Requested builtins root, but injection is disabled");
- } else if (path.equals("%install_base%")) {
- return Root.fromPath(directories.getInstallBase().getRelative("builtins_bzl"));
- } else if (path.equals("%workspace%")) {
- return Root.fromPath(
- directories
- .getWorkspace()
- .getRelative(packageFactory.getRuleClassProvider().getBuiltinsPackagePathInSource()));
- } else {
- // TODO(#11437): Should we consider interning these roots?
- return Root.fromPath(directories.getWorkspace().getRelative(path));
}
+
+ Path path;
+ if (flag.equals("%bundled%")) {
+ // May be null in tests, but in that case the flag shouldn't be set to %bundled%.
+ path =
+ Preconditions.checkNotNull(
+ packageFactory.getRuleClassProvider().getBuiltinsBzlRoot(),
+ "rule class provider does not specify a builtins root; either call"
+ + " setBuiltinsBzlZipResource() or else set --experimental_builtins_bzl_path to"
+ + " a root");
+ } else if (flag.equals("%workspace%")) {
+ String packagePath =
+ packageFactory.getRuleClassProvider().getBuiltinsBzlPackagePathInSource();
+ path = directories.getWorkspace().getRelative(packagePath);
+ } else {
+ path = directories.getWorkspace().getRelative(flag);
+ }
+ // TODO(#11437): Should we consider interning these roots?
+ return Root.fromPath(path);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/util/ResourceFileLoader.java b/src/main/java/com/google/devtools/build/lib/util/ResourceFileLoader.java
index a773f4a..3f32fc0 100644
--- a/src/main/java/com/google/devtools/build/lib/util/ResourceFileLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/util/ResourceFileLoader.java
@@ -20,9 +20,9 @@
import java.io.InputStream;
/**
- * A little utility to load resources (property files) from jars or
- * the classpath. Recommended for longer texts that do not fit nicely into
- * a piece of Java code - e.g. a template for a lengthy email.
+ * A little utility to load resources (property files) from jars or the classpath. Recommended for
+ * longer texts that do not fit nicely into a piece of Java code - e.g. a template for a lengthy
+ * email.
*/
public final class ResourceFileLoader {
@@ -37,11 +37,10 @@
}
/**
- * Loads a text resource that is located in a directory on the Java classpath that
- * corresponds to the package of <code>relativeToClass</code> using UTF8 encoding.
- * E.g.
- * <code>loadResource(Class.forName("com.google.foo.Foo", "bar.txt"))</code>
- * will look for <code>com/google/foo/bar.txt</code> in the classpath.
+ * Loads a text resource that is located in a directory on the Java classpath that corresponds to
+ * the package of <code>relativeToClass</code> using UTF8 encoding. E.g. <code>
+ * loadResource(Class.forName("com.google.foo.Foo", "bar.txt"))</code> will look for <code>
+ * com/google/foo/bar.txt</code> in the classpath.
*/
public static String loadResource(Class<?> relativeToClass, String resourceName)
throws IOException {
@@ -55,11 +54,19 @@
private static InputStream getResourceAsStream(Class<?> relativeToClass, String resourceName) {
ClassLoader loader = relativeToClass.getClassLoader();
+ String resource = resolveResource(relativeToClass, resourceName);
+ return loader.getResourceAsStream(resource);
+ }
+
+ /**
+ * Converts a relative resource name and Java class to a full resource path, using the same logic
+ * as {@link #loadResource}.
+ */
+ public static String resolveResource(Class<?> relativeToClass, String resourceName) {
// TODO(bazel-team): use relativeToClass.getPackage().getName().
String className = relativeToClass.getName();
String packageName = className.substring(0, className.lastIndexOf('.'));
String path = packageName.replace('.', '/');
- String resource = path + '/' + resourceName;
- return loader.getResourceAsStream(resource);
+ return path + '/' + resourceName;
}
}
