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