LabelConverter cleanup
- LabelConverter doesn't actually need a base Label; it just needs a base package. Changed the constructor to take a PackageIdentifier.
- Nobody ever actually reuses a "convertedLabelsInPackage" map; such a map is only ever created once per Package.Builder, or per LabelConverter instance. We can simply construct a LabelConverter in Package.Builder and just get rid of this cache map as an input parameter.
- This also means that convertedLabelsInPackage can be removed from BazelStarlarkContext.
- Fixed LabelConverter tests and documentation accordingly.
- As a next step, we should actually change the "context" parameter of `Type.convert(Object x, Object what, Object context)` to *always* be a LabelConverter, perhaps even non-nullable. Using a base Label as the conversion context is insufficient because it doesn't contain the repo mapping.
Work towards https://github.com/bazelbuild/bazel/issues/15658.
PiperOrigin-RevId: 454658146
Change-Id: I9219da9eec785d76a040e72f6901df466ae19799
diff --git a/src/main/java/com/google/devtools/build/lib/packages/LabelConverter.java b/src/main/java/com/google/devtools/build/lib/packages/LabelConverter.java
index eaca82d..cf3b7d7 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/LabelConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/LabelConverter.java
@@ -14,54 +14,46 @@
package com.google.devtools.build.lib.packages;
-import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.cmdline.BazelModuleContext;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import java.util.HashMap;
import java.util.Map;
import net.starlark.java.eval.Module;
import net.starlark.java.eval.StarlarkThread;
-/** Class which can evaluate a label with repository remappings. */
+/**
+ * Converts a label literal string into a {@link Label} object, using the appropriate base package
+ * and repo mapping.
+ */
public class LabelConverter {
- public static LabelConverter forThread(StarlarkThread thread) {
+ /**
+ * Returns a label converter for the given thread, which MUST be evaluating a .bzl file. It uses
+ * the package containing the .bzl file as the base package, and the repo mapping of the repo
+ * containing the .bzl file.
+ */
+ public static LabelConverter forBzlEvaluatingThread(StarlarkThread thread) {
BazelModuleContext moduleContext =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
- BazelStarlarkContext bazelStarlarkContext = BazelStarlarkContext.from(thread);
return new LabelConverter(
- moduleContext.label(),
- moduleContext.repoMapping(),
- bazelStarlarkContext.getConvertedLabelsInPackage());
+ moduleContext.label().getPackageIdentifier(), moduleContext.repoMapping());
}
- public static LabelConverter forModuleContext(BazelModuleContext moduleContext) {
- return new LabelConverter(moduleContext.label(), moduleContext.repoMapping());
- }
-
- private final Label base;
+ private final PackageIdentifier base;
private final RepositoryMapping repositoryMapping;
- private final Map<String, Label> labelCache;
+ private final Map<String, Label> labelCache = new HashMap<>();
- public LabelConverter(Label base, RepositoryMapping repositoryMapping) {
- this(
- base,
- repositoryMapping,
- // Only cache labels seen by this converter.
- new HashMap<>());
- }
-
- public LabelConverter(
- Label base, RepositoryMapping repositoryMapping, Map<String, Label> labelCache) {
+ /** Creates a label converter using the given base package and repo mapping. */
+ public LabelConverter(PackageIdentifier base, RepositoryMapping repositoryMapping) {
this.base = base;
this.repositoryMapping = repositoryMapping;
- this.labelCache = labelCache;
}
- /** Returns the base label that relative labels will be resolved against. */
- Label getBase() {
+ /** Returns the base package identifier that relative labels will be resolved against. */
+ PackageIdentifier getBasePackage() {
return base;
}
@@ -73,17 +65,12 @@
// label-strings across all their attribute values.
Label converted = labelCache.get(input);
if (converted == null) {
- converted = base.getRelativeWithRemapping(input, repositoryMapping);
+ converted = Label.parseWithPackageContext(input, base, repositoryMapping);
labelCache.put(input, converted);
}
return converted;
}
- @VisibleForTesting
- Map<String, Label> getLabelCache() {
- return labelCache;
- }
-
@Override
public String toString() {
return base.toString();