From bcf309b88949fe1bbff1776d88fdaa5c3e1d2d37 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 23 Mar 2023 07:05:34 -0700 Subject: [PATCH] Add native.module_{name,version} Extension authors often want to write some macro and change its behavior depending on which module is using it. For example, for rules_go, they want to look at the `go_deps` tags in a certain module and only allow access to repos generated from those tags. We do this by introducing two new methods on `native`, callable only during the loading phase. They return the name and version of the module associated with the current repo. If the repo is from WORKSPACE, they return `None`. If the repo is generated by an extension, they return info about the module hosting the extension. The implementation works by storing the "associated module" information in `RepositoryMappingValue`. I had attempted to store them in `BzlmodRepoRuleValue` or even `RepositoryDirectoryValue`, but those are not the right places since they might happen before we even evaluate the MODULE.bazel file (i.e. for non-registry overrides). Fixes https://github.com/bazelbuild/bazel/issues/17652. RELNOTES: Added `native.module_name()` and `native.module_version()` to allow BUILD macro authors to acquire information about which Bazel module the current repo is associated with. PiperOrigin-RevId: 518849334 Change-Id: I06b4bc95b5a57de2412ee02544240b054c708165 --- .../devtools/build/lib/packages/Package.java | 66 ++++++++-- .../build/lib/packages/PackageFactory.java | 5 + .../lib/packages/StarlarkNativeModule.java | 14 +++ .../google/devtools/build/lib/skyframe/BUILD | 2 +- .../build/lib/skyframe/PackageFunction.java | 2 + .../skyframe/RepositoryMappingFunction.java | 77 +++++++----- .../lib/skyframe/RepositoryMappingValue.java | 88 +++++++------ .../StarlarkNativeModuleApi.java | 27 ++++ .../FakeStarlarkNativeModuleApi.java | 12 ++ .../build/lib/packages/PackageTest.java | 5 +- .../build/lib/packages/RuleClassTest.java | 3 + .../build/lib/packages/RuleFactoryTest.java | 3 + .../RepositoryMappingFunctionTest.java | 117 +++++++++++------- src/test/py/bazel/bzlmod/bazel_module_test.py | 107 ++++++++++++++++ 14 files changed, 403 insertions(+), 125 deletions(-) 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 36ec720f49b654..c0775eef4447c8 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 @@ -128,17 +128,31 @@ private NameConflictException(String message) { private RootedPath filename; /** - * The directory in which this package's BUILD file resides. All InputFile - * members of the packages are located relative to this directory. + * The directory in which this package's BUILD file resides. All InputFile members of the packages + * are located relative to this directory. */ private Path packageDirectory; /** - * The name of the workspace this package is in. Used as a prefix for the runfiles directory. - * This can be set in the WORKSPACE file. This must be a valid target name. + * The name of the workspace this package is in. Used as a prefix for the runfiles directory. This + * can be set in the WORKSPACE file. This must be a valid target name. */ private String workspaceName; + /** + * The name of the Bzlmod module associated with the repo this package is in. If this package is + * not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the + * name of the module hosting the extension. + */ + private final Optional associatedModuleName; + + /** + * The version of the Bzlmod module associated with the repo this package is in. If this package + * is not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the + * version of the module hosting the extension. + */ + private final Optional associatedModuleVersion; + /** * The root of the source tree in which this package was found. It is an invariant that {@code * sourceRoot.getRelative(packageId.getSourceRoot()).equals(packageDirectory)}. Returns {@link @@ -147,8 +161,8 @@ private NameConflictException(String message) { private Optional sourceRoot; /** - * The "Make" environment of this package, containing package-local - * definitions of "Make" variables. + * The "Make" environment of this package, containing package-local definitions of "Make" + * variables. */ private ImmutableMap makeEnv; @@ -290,9 +304,15 @@ public ImmutableMap getLoads() { *

{@code name} MUST be a suffix of {@code filename.getParentDirectory())}. */ private Package( - PackageIdentifier packageId, String workspaceName, boolean succinctTargetNotFoundErrors) { + PackageIdentifier packageId, + String workspaceName, + Optional associatedModuleName, + Optional associatedModuleVersion, + boolean succinctTargetNotFoundErrors) { this.packageIdentifier = packageId; this.workspaceName = workspaceName; + this.associatedModuleName = associatedModuleName; + this.associatedModuleVersion = associatedModuleVersion; this.succinctTargetNotFoundErrors = succinctTargetNotFoundErrors; } @@ -871,6 +891,8 @@ public static Builder newExternalPackageBuilder( helper, LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER, workspaceName, + /* associatedModuleName= */ Optional.empty(), + /* associatedModuleVersion= */ Optional.empty(), starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), mainRepoMapping, mainRepoMapping) @@ -886,6 +908,8 @@ public static Builder newExternalPackageBuilderForBzlmod( DefaultPackageSettings.INSTANCE, basePackageId, DUMMY_WORKSPACE_NAME_FOR_BZLMOD_PACKAGES, + /* associatedModuleName= */ Optional.empty(), + /* associatedModuleVersion= */ Optional.empty(), starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), repoMapping, // This mapping is *not* the main repository's mapping, but since it is only used to @@ -1098,10 +1122,18 @@ public T intern(T sample) { PackageSettings packageSettings, PackageIdentifier id, String workspaceName, + Optional associatedModuleName, + Optional associatedModuleVersion, boolean noImplicitFileExport, RepositoryMapping repositoryMapping, RepositoryMapping mainRepositoryMapping) { - this.pkg = new Package(id, workspaceName, packageSettings.succinctTargetNotFoundErrors()); + this.pkg = + new Package( + id, + workspaceName, + associatedModuleName, + associatedModuleVersion, + packageSettings.succinctTargetNotFoundErrors()); this.noImplicitFileExport = noImplicitFileExport; this.repositoryMapping = repositoryMapping; this.mainRepositoryMapping = mainRepositoryMapping; @@ -1127,6 +1159,24 @@ String getPackageWorkspaceName() { return pkg.getWorkspaceName(); } + /** + * Returns the name of the Bzlmod module associated with the repo this package is in. If this + * package is not from a Bzlmod repo, this is empty. For repos generated by module extensions, + * this is the name of the module hosting the extension. + */ + Optional getAssociatedModuleName() { + return pkg.associatedModuleName; + } + + /** + * Returns the version of the Bzlmod module associated with the repo this package is in. If this + * package is not from a Bzlmod repo, this is empty. For repos generated by module extensions, + * this is the version of the module hosting the extension. + */ + Optional getAssociatedModuleVersion() { + return pkg.associatedModuleVersion; + } + /** * Updates the externalPackageRepositoryMappings entry for {@code repoWithin}. Adds new entry * from {@code localName} to {@code mappedName} in {@code repoWithin}'s map. 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 f7ceb9ed623343..8ee679225485f5 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 @@ -52,6 +52,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.OptionalLong; import java.util.Set; import java.util.concurrent.ForkJoinPool; @@ -464,6 +465,8 @@ public Package.Builder newExternalPackageBuilder( public Package.Builder newPackageBuilder( PackageIdentifier packageId, String workspaceName, + Optional associatedModuleName, + Optional associatedModuleVersion, StarlarkSemantics starlarkSemantics, RepositoryMapping repositoryMapping, RepositoryMapping mainRepositoryMapping) { @@ -471,6 +474,8 @@ public Package.Builder newPackageBuilder( packageSettings, packageId, workspaceName, + associatedModuleName, + associatedModuleVersion, starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT), repositoryMapping, mainRepositoryMapping); diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java index 2e494f48e198d4..3017d40514159a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java @@ -635,6 +635,20 @@ public Label packageRelativeLabel(Object input, StarlarkThread thread) throws Ev } } + @Override + @Nullable + public String moduleName(StarlarkThread thread) throws EvalException { + BazelStarlarkContext.from(thread).checkLoadingPhase("native.module_name"); + return PackageFactory.getContext(thread).getBuilder().getAssociatedModuleName().orElse(null); + } + + @Override + @Nullable + public String moduleVersion(StarlarkThread thread) throws EvalException { + BazelStarlarkContext.from(thread).checkLoadingPhase("native.module_version"); + return PackageFactory.getContext(thread).getBuilder().getAssociatedModuleVersion().orElse(null); + } + private static Dict getRuleDict(Rule rule, Mutability mu) throws EvalException { Dict.Builder values = Dict.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 90aa99db450ca2..79cfcdf85d27d0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -2246,8 +2246,8 @@ java_library( srcs = ["RepositoryMappingValue.java"], deps = [ ":sky_functions", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:exit_code", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index fbed05411c5ffb..ebeeccdb2fc566 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -1348,6 +1348,8 @@ private LoadedPackage loadPackage( .newPackageBuilder( packageId, workspaceName, + repositoryMappingValue.getAssociatedModuleName(), + repositoryMappingValue.getAssociatedModuleVersion(), starlarkBuiltinsValue.starlarkSemantics, repositoryMapping, mainRepositoryMappingValue.getRepositoryMapping()) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java index 9aed67e22d9d35..36363780e1e1a0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java @@ -16,9 +16,11 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue; +import com.google.devtools.build.lib.bazel.bzlmod.Module; import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionId; import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; import com.google.devtools.build.lib.bazel.bzlmod.SingleExtensionEvalValue; +import com.google.devtools.build.lib.bazel.bzlmod.Version; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -61,19 +63,21 @@ public SkyValue compute(SkyKey skyKey, Environment env) return null; } // We need to make sure that @_builtins maps to @_builtins too. - return RepositoryMappingValue.withMapping( + return RepositoryMappingValue.createForBzlmodRepo( RepositoryMapping.create( ImmutableMap.of( StarlarkBuiltinsValue.BUILTINS_NAME, StarlarkBuiltinsValue.BUILTINS_REPO, - // TODO(wyv): Google internal tests that have blzmod enabled fail because + // TODO(wyv): Google internal tests that have Bzlmod enabled fail because // they try to access cpp tools targets in the main repo from inside the // @_builtin repo. This is just a workaround and needs a proper way to // inject this mapping for google internal tests only. "", RepositoryName.MAIN), StarlarkBuiltinsValue.BUILTINS_REPO) - .withAdditionalMappings(bazelToolsMapping.getRepositoryMapping())); + .withAdditionalMappings(bazelToolsMapping.getRepositoryMapping()), + "bazel_tools", + Version.EMPTY); } BazelDepGraphValue bazelDepGraphValue = @@ -101,21 +105,20 @@ public SkyValue compute(SkyKey skyKey, Environment env) .collect( Collectors.toMap( Entry::getKey, entry -> RepositoryName.createUnvalidated(entry.getKey()))); - return RepositoryMappingValue.withMapping( - computeForBazelModuleRepo(repositoryName, bazelDepGraphValue) - .get() - // For the transitional period, we need to map the workspace name to the main repo. - .withAdditionalMappings( - ImmutableMap.of( - externalPackageValue.getPackage().getWorkspaceName(), RepositoryName.MAIN)) - .withAdditionalMappings(additionalMappings)); + return computeForBazelModuleRepo(repositoryName, bazelDepGraphValue) + .get() + // For the transitional period, we need to map the workspace name to the main repo. + .withAdditionalMappings( + ImmutableMap.of( + externalPackageValue.getPackage().getWorkspaceName(), RepositoryName.MAIN)) + .withAdditionalMappings(additionalMappings); } // Try and see if this is a repo generated from a Bazel module. - Optional mapping = + Optional mappingValue = computeForBazelModuleRepo(repositoryName, bazelDepGraphValue); - if (mapping.isPresent()) { - return RepositoryMappingValue.withMapping(mapping.get()); + if (mappingValue.isPresent()) { + return mappingValue.get(); } // Now try and see if this is a repo generated from a module extension. @@ -129,9 +132,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (extensionEvalValue == null) { return null; } - return RepositoryMappingValue.withMapping( - computeForModuleExtensionRepo( - repositoryName, moduleExtensionId.get(), extensionEvalValue, bazelDepGraphValue)); + return computeForModuleExtensionRepo( + repositoryName, moduleExtensionId.get(), extensionEvalValue, bazelDepGraphValue); } } @@ -154,31 +156,33 @@ public SkyValue compute(SkyKey skyKey, Environment env) } /** - * Calculate repo mappings for a repo generated from a Bazel module. Such a repo can see all its + * Calculates repo mappings for a repo generated from a Bazel module. Such a repo can see all its * {@code bazel_dep}s, as well as any repos generated by an extension it has a {@code use_repo} * clause for. * * @return the repo mappings for the repo if it's generated from a Bazel module, otherwise return * Optional.empty(). */ - private Optional computeForBazelModuleRepo( + private Optional computeForBazelModuleRepo( RepositoryName repositoryName, BazelDepGraphValue bazelDepGraphValue) { ModuleKey moduleKey = bazelDepGraphValue.getCanonicalRepoNameLookup().get(repositoryName); if (moduleKey == null) { return Optional.empty(); } - return Optional.of(bazelDepGraphValue.getFullRepoMapping(moduleKey)); + Module module = bazelDepGraphValue.getDepGraph().get(moduleKey); + return Optional.of( + RepositoryMappingValue.createForBzlmodRepo( + bazelDepGraphValue.getFullRepoMapping(moduleKey), + module.getName(), + module.getVersion())); } /** - * Calculate repo mappings for a repo generated from a module extension. Such a repo can see all + * Calculates repo mappings for a repo generated from a module extension. Such a repo can see all * repos generated by the same module extension, as well as all repos that the Bazel module * hosting the extension can see (see above). - * - * @return the repo mappings for the repo if it's generated from a module extension, otherwise - * return Optional.empty(). */ - private RepositoryMapping computeForModuleExtensionRepo( + private RepositoryMappingValue computeForModuleExtensionRepo( RepositoryName repositoryName, ModuleExtensionId extensionId, SingleExtensionEvalValue extensionEvalValue, @@ -190,16 +194,24 @@ private RepositoryMapping computeForModuleExtensionRepo( bazelDepGraphValue .getCanonicalRepoNameLookup() .get(extensionId.getBzlFileLabel().getRepository()); + Module module = bazelDepGraphValue.getDepGraph().get(moduleKey); // NOTE(wyv): This means that if "foo" has a bazel_dep with the repo name "bar", and the // extension generates an internal repo name "bar", then within a repo generated by the // extension, "bar" will refer to the latter. We should explore a way to differentiate between // the two to avoid any surprises. - return RepositoryMapping.create( - extensionEvalValue.getCanonicalRepoNameToInternalNames().inverse(), repositoryName) - .withAdditionalMappings(bazelDepGraphValue.getFullRepoMapping(moduleKey)); + return RepositoryMappingValue.createForBzlmodRepo( + RepositoryMapping.create( + extensionEvalValue.getCanonicalRepoNameToInternalNames().inverse(), repositoryName) + .withAdditionalMappings(bazelDepGraphValue.getFullRepoMapping(moduleKey)), + module.getName(), + module.getVersion()); } - private SkyValue computeFromWorkspace( + /** + * Calculate repo mappings for a repo generated from WORKSPACE. Such a repo is not subject to + * strict deps, and can additionally see all repos that the root module can see. + */ + private RepositoryMappingValue computeFromWorkspace( RepositoryName repositoryName, PackageValue externalPackageValue, @Nullable RepositoryMapping rootModuleRepoMapping) @@ -213,12 +225,13 @@ private SkyValue computeFromWorkspace( externalPackage.getRepositoryMapping(repositoryName)); if (rootModuleRepoMapping == null) { // This means Bzlmod is disabled. - return RepositoryMappingValue.withMapping(workspaceMapping); + return RepositoryMappingValue.createForWorkspaceRepo(workspaceMapping); } - // If Bzlmod is in play, we need to make sure that WORKSPACE repos see all repos that root + // If Bzlmod is in play, we need to make sure that WORKSPACE repos see all repos that the root // module can see, taking care to compose the existing WORKSPACE mapping with the main repo // mapping from Bzlmod. - return RepositoryMappingValue.withMapping(workspaceMapping.composeWith(rootModuleRepoMapping)); + return RepositoryMappingValue.createForWorkspaceRepo( + workspaceMapping.composeWith(rootModuleRepoMapping)); } private static Optional maybeGetModuleExtensionForRepo( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java index 6af4ac15d60f87..a6a0346b13e4b9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java @@ -15,7 +15,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.auto.value.AutoValue; -import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.bazel.bzlmod.Version; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -24,7 +24,8 @@ import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.util.Objects; +import java.util.Map; +import java.util.Optional; /** * A value that represents the 'mappings' of an external Bazel workspace, as defined in the main @@ -48,53 +49,68 @@ * changes to things in the WORKSPACE other than the mappings (and name) won't require reloading all * packages. If the mappings are changed then the external packages need to be reloaded. */ -public class RepositoryMappingValue implements SkyValue { +@AutoValue +public abstract class RepositoryMappingValue implements SkyValue { public static final Key KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS = - Key.create(RepositoryName.MAIN, /*rootModuleShouldSeeWorkspaceRepos=*/ false); + Key.create(RepositoryName.MAIN, /* rootModuleShouldSeeWorkspaceRepos= */ false); public static final RepositoryMappingValue VALUE_FOR_ROOT_MODULE_WITHOUT_REPOS = - new RepositoryMappingValue(RepositoryMapping.ALWAYS_FALLBACK); + RepositoryMappingValue.createForWorkspaceRepo(RepositoryMapping.ALWAYS_FALLBACK); - private final RepositoryMapping repositoryMapping; - - private RepositoryMappingValue(RepositoryMapping repositoryMapping) { - Preconditions.checkNotNull(repositoryMapping); - this.repositoryMapping = repositoryMapping; + /** + * Returns a {@link RepositoryMappingValue} for a repo defined in MODULE.bazel, which has an + * associated module. + */ + public static RepositoryMappingValue createForBzlmodRepo( + RepositoryMapping repositoryMapping, + String associatedModuleName, + Version associatedModuleVersion) { + return new AutoValue_RepositoryMappingValue( + repositoryMapping, + Optional.of(associatedModuleName), + Optional.of(associatedModuleVersion.getOriginal())); } - /** Returns the workspace mappings. */ - public RepositoryMapping getRepositoryMapping() { - return repositoryMapping; + /** + * Returns a {@link RepositoryMappingValue} for a repo defined in WORKSPACE, which has no + * associated module. + */ + public static RepositoryMappingValue createForWorkspaceRepo(RepositoryMapping repositoryMapping) { + return new AutoValue_RepositoryMappingValue( + repositoryMapping, Optional.empty(), Optional.empty()); } - /** Returns the {@link Key} for {@link RepositoryMappingValue}s. */ - public static Key key(RepositoryName repositoryName) { - return RepositoryMappingValue.Key.create( - repositoryName, /*rootModuleShouldSeeWorkspaceRepos=*/ true); - } + public abstract RepositoryMapping getRepositoryMapping(); - /** Returns a {@link RepositoryMappingValue} for a workspace with the given name. */ - public static RepositoryMappingValue withMapping(RepositoryMapping repositoryMapping) { - return new RepositoryMappingValue(Preconditions.checkNotNull(repositoryMapping)); - } + /** + * Returns the name of the Bzlmod module associated with the requested repo. If the requested repo + * is defined in WORKSPACE, this is empty. For repos generated by module extensions, this is the + * name of the module hosting the extension. + */ + public abstract Optional getAssociatedModuleName(); - @Override - public boolean equals(Object o) { - if (!(o instanceof RepositoryMappingValue)) { - return false; - } - RepositoryMappingValue other = (RepositoryMappingValue) o; - return Objects.equals(repositoryMapping, other.repositoryMapping); - } + /** + * Returns the version of the Bzlmod module associated with the requested repo. If the requested + * repo is defined in WORKSPACE, this is empty. For repos generated by module extensions, this is + * the version of the module hosting the extension. + */ + public abstract Optional getAssociatedModuleVersion(); - @Override - public int hashCode() { - return Objects.hash(repositoryMapping); + /** + * Replaces the inner {@link #getRepositoryMapping() repository mapping} with one returned by + * calling its {@link RepositoryMapping#withAdditionalMappings} method. + */ + public final RepositoryMappingValue withAdditionalMappings(Map mappings) { + return new AutoValue_RepositoryMappingValue( + getRepositoryMapping().withAdditionalMappings(mappings), + getAssociatedModuleName(), + getAssociatedModuleVersion()); } - @Override - public String toString() { - return repositoryMapping.toString(); + /** Returns the {@link Key} for {@link RepositoryMappingValue}s. */ + public static Key key(RepositoryName repositoryName) { + return RepositoryMappingValue.Key.create( + repositoryName, /* rootModuleShouldSeeWorkspaceRepos= */ true); } /** {@link SkyKey} for {@link RepositoryMappingValue}. */ diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java index f61f0b993596e7..e23efd7389dd16 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java @@ -16,6 +16,7 @@ import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.lib.cmdline.Label; +import javax.annotation.Nullable; import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; import net.starlark.java.annot.StarlarkBuiltin; @@ -274,6 +275,32 @@ NoneType exportsFiles(Sequence srcs, Object visibility, Object licenses, Star useStarlarkThread = true) Label packageRelativeLabel(Object input, StarlarkThread thread) throws EvalException; + @StarlarkMethod( + name = "module_name", + doc = + "The name of the Bazel module associated with the repo this package is in. If this" + + " package is from a repo defined in WORKSPACE instead of MODULE.bazel, this is" + + " empty. For repos generated by module extensions, this is the name of the module" + + " hosting the extension. It's the same as the module.name field seen" + + " in module_ctx.modules.", + allowReturnNones = true, + useStarlarkThread = true) + @Nullable + String moduleName(StarlarkThread thread) throws EvalException; + + @StarlarkMethod( + name = "module_version", + doc = + "The version of the Bazel module associated with the repo this package is in. If this" + + " package is from a repo defined in WORKSPACE instead of MODULE.bazel, this is" + + " empty. For repos generated by module extensions, this is the version of the" + + " module hosting the extension. It's the same as the module.version" + + " field seen in module_ctx.modules.", + allowReturnNones = true, + useStarlarkThread = true) + @Nullable + String moduleVersion(StarlarkThread thread) throws EvalException; + @StarlarkMethod( name = "subpackages", doc = diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkNativeModuleApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkNativeModuleApi.java index e2e58aea15b662..44d58039cc1dfb 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkNativeModuleApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkNativeModuleApi.java @@ -83,6 +83,18 @@ public Label packageRelativeLabel(Object input, StarlarkThread thread) throws Ev return Label.parseCanonicalUnchecked("//:fake_label"); } + @Nullable + @Override + public String moduleName(StarlarkThread thread) throws EvalException { + return null; + } + + @Nullable + @Override + public String moduleVersion(StarlarkThread thread) throws EvalException { + return null; + } + @Override public Sequence subpackages( Sequence include, Sequence exclude, boolean allowEmpty, StarlarkThread thread) diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageTest.java index 75d5a091c4b1ba..904bd637cad227 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/PackageTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/PackageTest.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.util.List; +import java.util.Optional; import net.starlark.java.eval.StarlarkCallable; import net.starlark.java.syntax.Location; import org.junit.Before; @@ -163,7 +164,9 @@ private Package.Builder pkgBuilder(String name) { DefaultPackageSettings.INSTANCE, PackageIdentifier.createInMainRepo(name), "workspace", - /*noImplicitFileExport=*/ true, + Optional.empty(), + Optional.empty(), + /* noImplicitFileExport= */ true, RepositoryMapping.ALWAYS_FALLBACK, RepositoryMapping.ALWAYS_FALLBACK); result.setFilename( diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index 8864cf4d5e0b97..e67e8fb9bf620f 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java @@ -68,6 +68,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import javax.annotation.Nullable; import net.starlark.java.eval.StarlarkFunction; @@ -262,6 +263,8 @@ private Package.Builder createDummyPackageBuilder() { .newPackageBuilder( PackageIdentifier.createInMainRepo(TEST_PACKAGE_NAME), "TESTING", + Optional.empty(), + Optional.empty(), StarlarkSemantics.DEFAULT, RepositoryMapping.ALWAYS_FALLBACK, RepositoryMapping.ALWAYS_FALLBACK) diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java index e104cfe44aeab9..d3559118aca699 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.vfs.RootedPath; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import net.starlark.java.eval.StarlarkSemantics; import net.starlark.java.eval.StarlarkThread; import net.starlark.java.syntax.Location; @@ -60,6 +61,8 @@ private Package.Builder newBuilder(PackageIdentifier id, Path filename) { .newPackageBuilder( id, "TESTING", + Optional.empty(), + Optional.empty(), StarlarkSemantics.DEFAULT, RepositoryMapping.ALWAYS_FALLBACK, RepositoryMapping.ALWAYS_FALLBACK) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java index 44c9a3955eca31..3aa18259bdb461 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction; import com.google.devtools.build.lib.bazel.bzlmod.FakeRegistry; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; +import com.google.devtools.build.lib.bazel.bzlmod.Version; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; import com.google.devtools.build.lib.cmdline.RepositoryMapping; @@ -108,27 +109,36 @@ public ImmutableMap getSkyFunctions( }; } - public static RepositoryMappingValue withMappingAllowingFallback( + private static RepositoryMappingValue valueForWorkspace( ImmutableMap repositoryMapping) { - return RepositoryMappingValue.withMapping( + return RepositoryMappingValue.createForWorkspaceRepo( RepositoryMapping.createAllowingFallback(repositoryMapping)); } - public static RepositoryMappingValue withMapping( - ImmutableMap repositoryMapping, RepositoryName ownerRepo) { - return RepositoryMappingValue.withMapping( - RepositoryMapping.create(repositoryMapping, ownerRepo)); + private static RepositoryMappingValue valueForBzlmod( + ImmutableMap repositoryMapping, + RepositoryName ownerRepo, + String associatedModuleName, + String associatedModuleVersion) + throws Exception { + return RepositoryMappingValue.createForBzlmodRepo( + RepositoryMapping.create(repositoryMapping, ownerRepo), + associatedModuleName, + Version.parse(associatedModuleVersion)); } - public RepositoryMappingValue withMappingForRootModule( - ImmutableMap repositoryMapping, RepositoryName ownerRepo) { + private RepositoryMappingValue valueForRootModule( + ImmutableMap repositoryMapping, + String rootModuleName, + String rootModuleVersion) + throws Exception { ImmutableMap.Builder allMappings = ImmutableMap.builder(); allMappings.putAll(repositoryMapping); for (String name : analysisMock.getWorkspaceRepos()) { allMappings.put(name, RepositoryName.createUnvalidated(name)); } - return RepositoryMappingValue.withMapping( - RepositoryMapping.create(allMappings.buildOrThrow(), ownerRepo)); + return valueForBzlmod( + allMappings.buildOrThrow(), RepositoryName.MAIN, rootModuleName, rootModuleVersion); } @Test @@ -148,7 +158,7 @@ public void testSimpleMapping() throws Exception { assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.of( "a", RepositoryName.create("b"), @@ -166,15 +176,14 @@ public void testRepoNameMapping_asRootModule() throws Exception { "bazel_dep(name='bbb',version='1.0', repo_name = 'com_foo_bar_b')"); registry.addModule(createModuleKey("bbb", "1.0"), "module(name='bbb', version='1.0')"); - RepositoryName name = RepositoryName.MAIN; - SkyKey skyKey = RepositoryMappingValue.key(name); + SkyKey skyKey = RepositoryMappingValue.key(RepositoryName.MAIN); EvaluationResult result = eval(skyKey); assertThat(result.hasError()).isFalse(); assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMappingForRootModule( + valueForRootModule( ImmutableMap.of( "", RepositoryName.MAIN, @@ -184,7 +193,8 @@ public void testRepoNameMapping_asRootModule() throws Exception { RepositoryName.MAIN, "com_foo_bar_b", RepositoryName.create("bbb~1.0")), - name)); + "aaa", + "0.1")); } @Test @@ -195,15 +205,14 @@ public void testRepoNameMapping_asRootModule_withOwnRepoName() throws Exception "bazel_dep(name='bbb',version='1.0', repo_name = 'com_foo_bar_b')"); registry.addModule(createModuleKey("bbb", "1.0"), "module(name='bbb', version='1.0')"); - RepositoryName name = RepositoryName.MAIN; - SkyKey skyKey = RepositoryMappingValue.key(name); + SkyKey skyKey = RepositoryMappingValue.key(RepositoryName.MAIN); EvaluationResult result = eval(skyKey); assertThat(result.hasError()).isFalse(); assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMappingForRootModule( + valueForRootModule( ImmutableMap.of( "", RepositoryName.MAIN, @@ -213,7 +222,8 @@ public void testRepoNameMapping_asRootModule_withOwnRepoName() throws Exception RepositoryName.MAIN, "com_foo_bar_b", RepositoryName.create("bbb~1.0")), - name)); + "aaa", + "0.1")); } @Test @@ -238,11 +248,13 @@ public void testRepoNameMapping_asDependency() throws Exception { assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMapping( + valueForBzlmod( ImmutableMap.of( "ccc", RepositoryName.create("ccc~1.0"), "com_foo_bar_b", RepositoryName.create("bbb~1.0")), - name)); + name, + "ccc", + "1.0")); } @Test @@ -262,10 +274,12 @@ public void testRepoNameMapping_dependencyOnRootModule() throws Exception { assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMapping( + valueForBzlmod( ImmutableMap.of( "bbb", RepositoryName.create("bbb~1.0"), "aaa", RepositoryName.create("")), - name)); + name, + "bbb", + "1.0")); } @Test @@ -280,8 +294,7 @@ public void testRepoNameMapping_multipleVersionOverride_fork() throws Exception .addModule(createModuleKey("bbb", "1.0"), "module(name='bbb', version='1.0')") .addModule(createModuleKey("bbb", "2.0"), "module(name='bbb', version='2.0')"); - RepositoryName name = RepositoryName.MAIN; - SkyKey skyKey = RepositoryMappingValue.key(name); + SkyKey skyKey = RepositoryMappingValue.key(RepositoryName.MAIN); EvaluationResult result = eval(skyKey); if (result.hasError()) { @@ -290,7 +303,7 @@ public void testRepoNameMapping_multipleVersionOverride_fork() throws Exception assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMappingForRootModule( + valueForRootModule( ImmutableMap.of( "", RepositoryName.MAIN, @@ -302,7 +315,8 @@ public void testRepoNameMapping_multipleVersionOverride_fork() throws Exception RepositoryName.create("bbb~1.0"), "bbb2", RepositoryName.create("bbb~2.0")), - name)); + "aaa", + "0.1")); } @Test @@ -333,11 +347,13 @@ public void testRepoNameMapping_multipleVersionOverride_diamond() throws Excepti assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMapping( + valueForBzlmod( ImmutableMap.of( "bbb", RepositoryName.create("bbb~1.0"), "ddd", RepositoryName.create("ddd~1.0")), - name)); + name, + "bbb", + "1.0")); } @Test @@ -366,11 +382,13 @@ public void testRepoNameMapping_multipleVersionOverride_lookup() throws Exceptio assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMapping( + valueForBzlmod( ImmutableMap.of( "bbb", RepositoryName.create("bbb~1.0"), "com_foo_bar_c", RepositoryName.create("ccc~1.0")), - name)); + name, + "bbb", + "1.0")); } @Test @@ -395,7 +413,7 @@ public void testMultipleRepositoriesWithMapping() throws Exception { assertThatEvaluationResult(eval(skyKey1)) .hasEntryThat(skyKey1) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.of( "a", RepositoryName.create("b"), @@ -406,7 +424,7 @@ public void testMultipleRepositoriesWithMapping() throws Exception { assertThatEvaluationResult(eval(skyKey2)) .hasEntryThat(skyKey2) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.of( "x", RepositoryName.create("y"), @@ -431,7 +449,7 @@ public void testRepositoryWithMultipleMappings() throws Exception { assertThatEvaluationResult(eval(skyKey)) .hasEntryThat(skyKey) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.of( "a", RepositoryName.create("b"), @@ -473,7 +491,7 @@ public void testMixtureOfBothSystems_workspaceRepo() throws Exception { assertThatEvaluationResult(eval(skyKey)) .hasEntryThat(skyKey) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.builder() .put("", RepositoryName.MAIN) .put("aaa", RepositoryName.MAIN) @@ -513,14 +531,15 @@ public void testMixtureOfBothSystems_mainRepo() throws Exception { assertThatEvaluationResult(eval(skyKey)) .hasEntryThat(skyKey) .isEqualTo( - withMappingForRootModule( + valueForRootModule( ImmutableMap.of( "", RepositoryName.MAIN, "aaa", RepositoryName.MAIN, "bbb", RepositoryName.create("bbb~1.0"), "root", RepositoryName.MAIN, "ws_repo", RepositoryName.create("ws_repo")), - RepositoryName.MAIN)); + "aaa", + "0.1")); } @Test @@ -543,12 +562,14 @@ public void testMixtureOfBothSystems_mainRepo_shouldNotSeeWorkspaceRepos() throw assertThatEvaluationResult(eval(skyKey)) .hasEntryThat(skyKey) .isEqualTo( - withMapping( + valueForBzlmod( ImmutableMap.of( "", RepositoryName.MAIN, "aaa", RepositoryName.MAIN, "bbb", RepositoryName.create("bbb~1.0")), - RepositoryName.MAIN)); + RepositoryName.MAIN, + "aaa", + "0.1")); } @Test @@ -589,7 +610,7 @@ public void testDefaultMainRepoNameInMapping() throws Exception { assertThatEvaluationResult(eval(skyKey)) .hasEntryThat(skyKey) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.of( TestConstants.WORKSPACE_NAME, RepositoryName.MAIN, "", RepositoryName.MAIN))); } @@ -609,7 +630,7 @@ public void testExplicitMainRepoNameInMapping() throws Exception { assertThatEvaluationResult(eval(skyKey)) .hasEntryThat(skyKey) .isEqualTo( - withMappingAllowingFallback( + valueForWorkspace( ImmutableMap.of("good", RepositoryName.MAIN, "", RepositoryName.MAIN))); } @@ -635,7 +656,7 @@ public void builtinsRepo() throws Exception { assertThatEvaluationResult(result) .hasEntryThat(skyKey) .isEqualTo( - withMapping( + valueForBzlmod( ImmutableMap.of( "bazel_tools", RepositoryName.BAZEL_TOOLS, // bazel_tools is a well-known module @@ -645,18 +666,20 @@ public void builtinsRepo() throws Exception { RepositoryName.create("_builtins"), "", RepositoryName.MAIN), - name)); + name, + "bazel_tools", + "")); } @Test public void testEqualsAndHashCode() throws Exception { new EqualsTester() .addEqualityGroup( - withMappingAllowingFallback(ImmutableMap.of("foo", RepositoryName.create("bar"))), - withMappingAllowingFallback(ImmutableMap.of("foo", RepositoryName.create("bar")))) + valueForWorkspace(ImmutableMap.of("foo", RepositoryName.create("bar"))), + valueForWorkspace(ImmutableMap.of("foo", RepositoryName.create("bar")))) .addEqualityGroup( - withMappingAllowingFallback(ImmutableMap.of("fizz", RepositoryName.create("buzz"))), - withMappingAllowingFallback(ImmutableMap.of("fizz", RepositoryName.create("buzz")))) + valueForWorkspace(ImmutableMap.of("fizz", RepositoryName.create("buzz"))), + valueForWorkspace(ImmutableMap.of("fizz", RepositoryName.create("buzz")))) .testEquals(); } } diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py index eef684d86ca3c2..c645262f754e4a 100644 --- a/src/test/py/bazel/bzlmod/bazel_module_test.py +++ b/src/test/py/bazel/bzlmod/bazel_module_test.py @@ -1041,6 +1041,113 @@ def testArchiveWithArchiveType(self): _, stdout, _ = self.RunBazel(['run', '//:main'], allow_failure=False) self.assertIn('main function => aaa@1.2', stdout) + def testNativeModuleNameAndVersion(self): + self.main_registry.setModuleBasePath('projects') + projects_dir = self.main_registry.projects + + self.ScratchFile( + 'MODULE.bazel', + [ + 'module(name="root",version="0.1")', + 'bazel_dep(name="foo",version="1.0")', + 'report_ext = use_extension("@foo//:ext.bzl", "report_ext")', + 'use_repo(report_ext, "report_repo")', + 'bazel_dep(name="bar")', + 'local_path_override(module_name="bar",path="bar")', + ], + ) + self.ScratchFile('WORKSPACE') + self.ScratchFile( + 'WORKSPACE.bzlmod', ['local_repository(name="quux",path="quux")'] + ) + self.ScratchFile( + 'BUILD', + [ + 'load("@foo//:report.bzl", "report")', + 'report()', + ], + ) + # foo: a repo defined by a normal Bazel module. Also hosts the extension + # `report_ext` which generates a repo `report_repo`. + self.main_registry.createLocalPathModule('foo', '1.0', 'foo') + projects_dir.joinpath('foo').mkdir(exist_ok=True) + scratchFile(projects_dir.joinpath('foo', 'WORKSPACE')) + scratchFile( + projects_dir.joinpath('foo', 'BUILD'), + [ + 'load(":report.bzl", "report")', + 'report()', + ], + ) + scratchFile( + projects_dir.joinpath('foo', 'report.bzl'), + [ + 'def report():', + ' repo = native.repository_name()', + ' name = str(native.module_name())', + ' version = str(native.module_version())', + ' print("@" + repo + " reporting in: " + name + "@" + version)', + ' native.filegroup(name="a")', + ], + ) + scratchFile( + projects_dir.joinpath('foo', 'ext.bzl'), + [ + 'def _report_repo(rctx):', + ' rctx.file("BUILD",', + ' "load(\\"@foo//:report.bzl\\", \\"report\\")\\n" +', + ' "report()")', + 'report_repo = repository_rule(_report_repo)', + 'report_ext = module_extension(', + ' lambda mctx: report_repo(name="report_repo"))', + ], + ) + # bar: a repo defined by a Bazel module with a non-registry override + self.ScratchFile('bar/WORKSPACE') + self.ScratchFile( + 'bar/MODULE.bazel', + [ + 'module(name="bar", version="2.0")', + 'bazel_dep(name="foo",version="1.0")', + ], + ) + self.ScratchFile( + 'bar/BUILD', + [ + 'load("@foo//:report.bzl", "report")', + 'report()', + ], + ) + # quux: a repo defined by WORKSPACE + self.ScratchFile('quux/WORKSPACE') + self.ScratchFile( + 'quux/BUILD', + [ + 'load("@foo//:report.bzl", "report")', + 'report()', + ], + ) + + _, _, stderr = self.RunBazel( + [ + 'build', + ':a', + '@foo//:a', + '@report_repo//:a', + '@bar//:a', + '@quux//:a', + ], + allow_failure=False, + ) + stderr = '\n'.join(stderr) + self.assertIn('@@ reporting in: root@0.1', stderr) + self.assertIn('@@foo~1.0 reporting in: foo@1.0', stderr) + self.assertIn( + '@@foo~1.0~report_ext~report_repo reporting in: foo@1.0', stderr + ) + self.assertIn('@@bar~override reporting in: bar@2.0', stderr) + self.assertIn('@@quux reporting in: None@None', stderr) + if __name__ == '__main__': unittest.main()