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/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 49e9e36..b3ff10d 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
@@ -875,7 +875,6 @@
BazelStarlarkContext.Phase.LOADING,
toolsRepository,
configurationFragmentMap,
- /*convertedLabelsInPackage=*/ new HashMap<>(),
new SymbolGenerator<>(fileLabel),
/*analysisRuleLabel=*/ null,
networkAllowlistForTests)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index ec41d70..95927c7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1129,7 +1129,6 @@
BazelStarlarkContext.Phase.ANALYSIS,
ruleClassProvider.getToolsRepository(),
/*fragmentNameToClass=*/ null,
- /*convertedLabelsInPackage=*/ new HashMap<>(),
getSymbolGenerator(),
getLabel(),
/*networkAllowlistForTests=*/ null)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
index f45a493..6d564d5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
@@ -350,7 +350,6 @@
Phase.ANALYSIS,
/*toolsRepository=*/ null,
/*fragmentNameToClass=*/ null,
- /*convertedLabelsInPackage=*/ new HashMap<>(),
dummySymbolGenerator,
parentLabel,
/*networkAllowlistForTests=*/ null);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
index 5ddcc30..e859fd2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
@@ -143,10 +143,8 @@
// instance to avoid adding a dependency to the C++ package.
builder.value((NativeComputedDefaultApi) defaultValue);
} else {
- BazelModuleContext moduleContext =
- BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
builder.defaultValue(
- defaultValue, LabelConverter.forModuleContext(moduleContext), DEFAULT_ARG);
+ defaultValue, LabelConverter.forBzlEvaluatingThread(thread), DEFAULT_ARG);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
index 7fff123..b21ed4c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -539,7 +539,7 @@
private static ImmutableSet<Label> parseExecCompatibleWith(
Sequence<?> inputs, StarlarkThread thread) throws EvalException {
ImmutableSet.Builder<Label> parsedLabels = new ImmutableSet.Builder<>();
- LabelConverter converter = LabelConverter.forThread(thread);
+ LabelConverter converter = LabelConverter.forBzlEvaluatingThread(thread);
for (String input : Sequence.cast(inputs, String.class, "exec_compatible_with")) {
try {
Label label = converter.convert(input);
@@ -998,7 +998,7 @@
private static ImmutableSet<ToolchainTypeRequirement> parseToolchainTypes(
Sequence<?> rawToolchains, StarlarkThread thread) throws EvalException {
Map<Label, ToolchainTypeRequirement> toolchainTypes = new HashMap<>();
- LabelConverter converter = LabelConverter.forThread(thread);
+ LabelConverter converter = LabelConverter.forBzlEvaluatingThread(thread);
for (Object rawToolchain : rawToolchains) {
ToolchainTypeRequirement toolchainType = parseToolchainType(converter, rawToolchain);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java
index 1e6a355..cbfd009 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java
@@ -87,7 +87,7 @@
return ((ToolchainTypeInfo) key).typeLabel();
} else if (key instanceof String) {
try {
- LabelConverter converter = LabelConverter.forThread(starlarkThread);
+ LabelConverter converter = LabelConverter.forBzlEvaluatingThread(starlarkThread);
return converter.convert((String) key);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Unable to parse toolchain label '%s': %s", key, e.getMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java
index f2f80ad..4d88cb1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java
@@ -27,11 +27,13 @@
import com.google.devtools.build.lib.bazel.bzlmod.Selection.SelectionResult;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.LabelConverter;
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -133,7 +135,7 @@
for (Module module : depGraph.values()) {
LabelConverter labelConverter =
new LabelConverter(
- StarlarkBazelModule.createModuleRootLabel(module.getCanonicalRepoName()),
+ PackageIdentifier.create(module.getCanonicalRepoName(), PathFragment.EMPTY_FRAGMENT),
module.getRepoMappingWithBazelDepsOnly());
for (ModuleExtensionUsage usage : module.getExtensionUsages()) {
try {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java
index cd3c9dc..da0f70d 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java
@@ -18,10 +18,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
-import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.LabelConverter;
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -89,16 +87,6 @@
}
/**
- * Creates a label pointing to the root package of the repo with the given canonical repo name.
- * This label can be used to anchor (relativize) labels with no "@foo" part.
- */
- static Label createModuleRootLabel(RepositoryName canonicalRepoName) {
- return Label.createUnvalidated(
- PackageIdentifier.create(canonicalRepoName, PathFragment.EMPTY_FRAGMENT),
- "unused_dummy_target_name");
- }
-
- /**
* Creates a new {@link StarlarkBazelModule} object representing the given {@link AbridgedModule},
* with its scope limited to the given {@link ModuleExtension}. It'll be populated with the tags
* present in the given {@link ModuleExtensionUsage}. Any labels present in tags will be converted
@@ -111,7 +99,9 @@
@Nullable ModuleExtensionUsage usage)
throws ExternalDepsException {
LabelConverter labelConverter =
- new LabelConverter(createModuleRootLabel(module.getCanonicalRepoName()), repoMapping);
+ new LabelConverter(
+ PackageIdentifier.create(module.getCanonicalRepoName(), PathFragment.EMPTY_FRAGMENT),
+ repoMapping);
ImmutableList<Tag> tags = usage == null ? ImmutableList.of() : usage.getTags();
HashMap<String, ArrayList<TypeCheckedTag>> typeCheckedTags = new HashMap<>();
for (String tagClassName : extension.getTagClasses().keySet()) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
index 9b85c55..eb1bb30 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
@@ -49,7 +49,6 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import java.io.IOException;
-import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
@@ -163,7 +162,6 @@
BazelStarlarkContext.Phase.LOADING, // ("fetch")
/*toolsRepository=*/ null,
/*fragmentNameToClass=*/ null,
- /*convertedLabelsInPackage=*/ new HashMap<>(),
new SymbolGenerator<>(key),
/*analysisRuleLabel=*/ null,
/*networkAllowlistForTests=*/ null)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java
index 947ebeb..ad5f277 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
-import java.util.HashMap;
import java.util.Optional;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
@@ -60,7 +59,6 @@
@Nullable private final RepositoryName toolsRepository;
// Only necessary for loading phase threads to construct configuration_field.
@Nullable private final ImmutableMap<String, Class<?>> fragmentNameToClass;
- private final HashMap<String, Label> convertedLabelsInPackage;
private final SymbolGenerator<?> symbolGenerator;
@Nullable private final Label analysisRuleLabel;
// TODO(b/192694287): Remove once we migrate all tests from the allowlist
@@ -73,12 +71,9 @@
* @param fragmentNameToClass a map from configuration fragment name to configuration fragment
* class, such as "apple" to AppleConfiguration.class for loading phase threads, null for
* other threads.
- * @param convertedLabelsInPackage a mutable map from String to Label, used during package loading
- * of a single package.
* @param symbolGenerator a {@link SymbolGenerator} to be used when creating objects to be
* compared using reference equality.
* @param analysisRuleLabel is the label of the rule for an analysis phase (rule or aspect
- * 'implementation') thread, or null for all other threads.
*/
// TODO(adonovan): clearly demarcate which fields are defined in which kinds of threads (loading,
// analysis, workspace, implicit outputs, computed defaults, etc), perhaps by splitting these into
@@ -91,14 +86,12 @@
Phase phase,
@Nullable RepositoryName toolsRepository,
@Nullable ImmutableMap<String, Class<?>> fragmentNameToClass,
- HashMap<String, Label> convertedLabelsInPackage,
SymbolGenerator<?> symbolGenerator,
@Nullable Label analysisRuleLabel,
@Nullable Label networkAllowlistForTests) {
this.phase = Preconditions.checkNotNull(phase);
this.toolsRepository = toolsRepository;
this.fragmentNameToClass = fragmentNameToClass;
- this.convertedLabelsInPackage = convertedLabelsInPackage;
this.symbolGenerator = Preconditions.checkNotNull(symbolGenerator);
this.analysisRuleLabel = analysisRuleLabel;
this.networkAllowlistForTests = networkAllowlistForTests;
@@ -117,16 +110,6 @@
return fragmentNameToClass;
}
- /**
- * Returns a String -> Label map of all the Strings that have already been converted to Labels
- * during package loading of the current package.
- *
- * <p>This is used for a performance optimization during package loading, and unused otherwise.
- */
- public HashMap<String, Label> getConvertedLabelsInPackage() {
- return convertedLabelsInPackage;
- }
-
public SymbolGenerator<?> getSymbolGenerator() {
return symbolGenerator;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
index 2e03f40..de1632d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
@@ -419,13 +419,13 @@
LabelConverter converter = (LabelConverter) context;
Label result = converter.convert(value);
- if (!result.getPackageIdentifier().equals(converter.getBase().getPackageIdentifier())) {
+ if (!result.getPackageIdentifier().equals(converter.getBasePackage())) {
throw new ConversionException("label '" + value + "' is not in the current package");
}
return result;
} catch (LabelSyntaxException e) {
throw new ConversionException(
- "illegal output file name '" + value + "' in rule " + context + ": " + e.getMessage());
+ "illegal output file name '" + value + "': " + e.getMessage());
}
}
}
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();
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 aca053d..2e24b12 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
@@ -876,9 +876,6 @@
*/
public static class Builder {
- static final ImmutableMap<RepositoryName, RepositoryName> EMPTY_REPOSITORY_MAPPING =
- ImmutableMap.of();
-
/** Defines configuration to control the runtime behavior of {@link Package}s. */
public interface PackageSettings {
/**
@@ -933,6 +930,8 @@
* workspace.
*/
private final RepositoryMapping repositoryMapping;
+ /** Converts label literals to Label objects within this package. */
+ private final LabelConverter labelConverter;
private RootedPath filename = null;
private Label buildFileLabel = null;
@@ -1007,8 +1006,6 @@
private final Interner<ImmutableList<?>> listInterner = new ThreadCompatibleInterner<>();
- private final HashMap<String, Label> convertedLabelsInPackage = new HashMap<>();
-
private ImmutableMap<Location, String> generatorMap = ImmutableMap.of();
private final TestSuiteImplicitTestsAccumulator testSuiteImplicitTestsAccumulator =
@@ -1063,6 +1060,7 @@
this.pkg = new Package(id, workspaceName, packageSettings.succinctTargetNotFoundErrors());
this.noImplicitFileExport = noImplicitFileExport;
this.repositoryMapping = repositoryMapping;
+ this.labelConverter = new LabelConverter(id, repositoryMapping);
if (pkg.getName().startsWith("javatests/")) {
setDefaultTestonly(true);
}
@@ -1116,19 +1114,14 @@
return this;
}
- /** Get the repository mapping for this package */
- RepositoryMapping getRepositoryMapping() {
- return this.repositoryMapping;
+ LabelConverter getLabelConverter() {
+ return labelConverter;
}
Interner<ImmutableList<?>> getListInterner() {
return listInterner;
}
- HashMap<String, Label> getConvertedLabelsInPackage() {
- return convertedLabelsInPackage;
- }
-
/** Sets the name of this package's BUILD file. */
public Builder setFilename(RootedPath filename) {
this.filename = filename;
@@ -1516,7 +1509,7 @@
* Returns a lightweight snapshot view of the names of all rule targets belonging to this
* package at the time of this call.
*
- * @throws IllegalStateException if this method is called after {@link beforeBuild} has been
+ * @throws IllegalStateException if this method is called after {@link #beforeBuild} has been
* called.
*/
Map<String, Rule> getRulesSnapshotView() {
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 98ff211..e9eaeb2 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
@@ -668,7 +668,6 @@
BazelStarlarkContext.Phase.LOADING,
ruleClassProvider.getToolsRepository(),
/*fragmentNameToClass=*/ null,
- pkgBuilder.getConvertedLabelsInPackage(),
new SymbolGenerator<>(pkgBuilder.getPackageIdentifier()),
/*analysisRuleLabel=*/ null,
/*networkAllowlistForTests=*/ null)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 4b38b15..359ec57 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -43,7 +43,6 @@
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 com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
@@ -2122,10 +2121,9 @@
BitSet definedAttrIndices =
populateDefinedRuleAttributeValues(
rule,
- pkgBuilder.getRepositoryMapping(),
+ pkgBuilder.getLabelConverter(),
attributeValues,
pkgBuilder.getListInterner(),
- pkgBuilder.getConvertedLabelsInPackage(),
eventHandler);
populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler);
// Now that all attributes are bound to values, collect and store configurable attribute keys.
@@ -2145,10 +2143,9 @@
*/
private <T> BitSet populateDefinedRuleAttributeValues(
Rule rule,
- RepositoryMapping repositoryMapping,
+ LabelConverter labelConverter,
AttributeValues<T> attributeValues,
Interner<ImmutableList<?>> listInterner,
- Map<String, Label> convertedLabelsInPackage,
EventHandler eventHandler) {
BitSet definedAttrIndices = new BitSet();
for (T attributeAccessor : attributeValues.getAttributeAccessors()) {
@@ -2181,13 +2178,7 @@
if (attributeValues.valuesAreBuildLanguageTyped()) {
try {
nativeAttributeValue =
- convertFromBuildLangType(
- rule,
- attr,
- attributeValue,
- repositoryMapping,
- listInterner,
- convertedLabelsInPackage);
+ convertFromBuildLangType(rule, attr, attributeValue, labelConverter, listInterner);
} catch (ConversionException e) {
rule.reportError(String.format("%s: %s", rule.getLabel(), e.getMessage()), eventHandler);
continue;
@@ -2483,18 +2474,15 @@
Rule rule,
Attribute attr,
Object buildLangValue,
- RepositoryMapping repositoryMapping,
- Interner<ImmutableList<?>> listInterner,
- Map<String, Label> convertedLabelsInPackage)
+ LabelConverter labelConverter,
+ Interner<ImmutableList<?>> listInterner)
throws ConversionException {
- LabelConverter context =
- new LabelConverter(rule.getLabel(), repositoryMapping, convertedLabelsInPackage);
Object converted =
BuildType.selectableConvert(
attr.getType(),
buildLangValue,
new AttributeConversionContext(attr.getName(), rule.getRuleClass()),
- context);
+ labelConverter);
if ((converted instanceof SelectorList<?>) && !attr.isConfigurable()) {
throw new ConversionException(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index aa015c5..3b69ca0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -137,7 +137,6 @@
BazelStarlarkContext.Phase.WORKSPACE,
/*toolsRepository=*/ null,
/*fragmentNameToClass=*/ null,
- /*convertedLabelsInPackage=*/ new HashMap<>(),
new SymbolGenerator<>(workspaceFileKey),
/*analysisRuleLabel=*/ null,
/*networkAllowlistForTests=*/ null)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigStarlarkCommon.java
index 1379784..59e28c9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigStarlarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigStarlarkCommon.java
@@ -48,7 +48,7 @@
if (name instanceof Label) {
label = (Label) name;
} else if (name instanceof String) {
- LabelConverter converter = LabelConverter.forThread(thread);
+ LabelConverter converter = LabelConverter.forBzlEvaluatingThread(thread);
try {
label = converter.convert((String) name);
} catch (LabelSyntaxException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTagTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTagTest.java
index 1e0e64f..f6eec32 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTagTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTagTest.java
@@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.LabelConverter;
@@ -77,7 +78,7 @@
"foo", StarlarkList.immutableOf(":thing1", "//pkg:thing2", "@repo//pkg:thing3"))
.build(),
new LabelConverter(
- Label.parseAbsoluteUnchecked("@myrepo//mypkg:defs.bzl"),
+ PackageIdentifier.parse("@myrepo//mypkg"),
createRepositoryMapping(createModuleKey("test", "1.0"), "repo", "other_repo")));
assertThat(typeCheckedTag.getFieldNames()).containsExactly("foo");
assertThat(getattr(typeCheckedTag, "foo"))
@@ -96,7 +97,7 @@
attr("foo", BuildType.LABEL).allowedFileTypes(FileTypeSet.ANY_FILE).build()),
buildTag("tag_name").build(),
new LabelConverter(
- Label.parseAbsoluteUnchecked("@myrepo//mypkg:defs.bzl"),
+ PackageIdentifier.parse("@myrepo//mypkg"),
createRepositoryMapping(createModuleKey("test", "1.0"), "repo", "other_repo")));
assertThat(typeCheckedTag.getFieldNames()).containsExactly("foo");
assertThat(getattr(typeCheckedTag, "foo")).isEqualTo(Starlark.NONE);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
index 0cd0c1d..51679bc 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
@@ -38,7 +38,7 @@
private final Label currentRule = Label.parseAbsoluteUnchecked("//quux:baz");
private final LabelConverter labelConversionContext =
- new LabelConverter(currentRule, RepositoryMapping.ALWAYS_FALLBACK);
+ new LabelConverter(currentRule.getPackageIdentifier(), RepositoryMapping.ALWAYS_FALLBACK);
@Test
public void testKeepsDictOrdering() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/LabelConverterTest.java b/src/test/java/com/google/devtools/build/lib/packages/LabelConverterTest.java
index 576663e..95d9d50 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/LabelConverterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/LabelConverterTest.java
@@ -17,10 +17,10 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
-import com.google.devtools.build.lib.packages.Type.ConversionException;
-import java.util.HashMap;
+import com.google.devtools.build.lib.vfs.PathFragment;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -28,30 +28,18 @@
/** Test of {@link LabelConverter}. */
@RunWith(JUnit4.class)
public class LabelConverterTest {
- private final Label currentRule = Label.parseAbsoluteUnchecked("//quux:baz");
@Test
- public void testLabelWithRemapping() throws Exception {
- LabelConverter context =
+ public void convertLabel() throws Exception {
+ PackageIdentifier basePackage = PackageIdentifier.create("quux", PathFragment.create("baz"));
+ LabelConverter converter =
new LabelConverter(
- currentRule,
+ basePackage,
RepositoryMapping.createAllowingFallback(
- ImmutableMap.of("orig_repo", RepositoryName.create("new_repo"))),
- /* labelCache= */ new HashMap<>());
- Label label = BuildType.LABEL.convert("@orig_repo//foo:bar", null, context);
- assertThat(label)
- .isEquivalentAccordingToCompareTo(
- Label.parseAbsolute("@new_repo//foo:bar", ImmutableMap.of()));
- }
-
- @Test
- public void testLabelConversionContextCaches() throws ConversionException {
- LabelConverter labelConverter =
- new LabelConverter(
- currentRule, RepositoryMapping.ALWAYS_FALLBACK, /* labelCache= */ new HashMap<>());
-
- assertThat(labelConverter.getLabelCache()).doesNotContainKey("//some:label");
- BuildType.LABEL.convert("//some:label", "doesntmatter", labelConverter);
- assertThat(labelConverter.getLabelCache()).containsKey("//some:label");
+ ImmutableMap.of("orig_repo", RepositoryName.create("new_repo"))));
+ assertThat(converter.convert("@orig_repo//foo:bar"))
+ .isEqualTo(Label.parseCanonical("@new_repo//foo:bar"));
+ assertThat(converter.convert("//foo:bar")).isEqualTo(Label.parseCanonical("@quux//foo:bar"));
+ assertThat(converter.convert(":bar")).isEqualTo(Label.parseCanonical("@quux//baz:bar"));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/OutputFileTest.java b/src/test/java/com/google/devtools/build/lib/packages/OutputFileTest.java
index ee8487b..88dcae1 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/OutputFileTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/OutputFileTest.java
@@ -135,7 +135,7 @@
scratch.file("bad_out_name/BUILD", "genrule(name='a', cmd='ls', outs=['!@#:'])");
reporter.removeHandler(failFastHandler);
getTarget("//bad_out_name:BUILD");
- assertContainsEvent("illegal output file name '!@#:' in rule //bad_out_name:a");
+ assertContainsEvent("illegal output file name '!@#:'");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/util/BazelEvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/starlark/util/BazelEvaluationTestCase.java
index 98a7923..81a5e6d 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/util/BazelEvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/util/BazelEvaluationTestCase.java
@@ -35,7 +35,6 @@
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Module;
@@ -129,7 +128,6 @@
BazelStarlarkContext.Phase.LOADING,
TestConstants.TOOLS_REPOSITORY,
/*fragmentNameToClass=*/ null,
- /*convertedLabelsInPackage=*/ new HashMap<>(),
new SymbolGenerator<>(new Object()),
/*analysisRuleLabel=*/ null,
/*networkAllowlistForTests=*/ null) // dummy value for tests
