From 46341b1a59c7a1cc8ca0dc604ad53b6de7fee653 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 24 Sep 2024 01:53:41 -0700 Subject: [PATCH] Add `override_repo` and `inject_repo` Work towards #19301 Fixes #23580 RELNOTES: `override_repo` and `inject_repo` can be used to override and inject repos in module extensions. Closes #23534. PiperOrigin-RevId: 678139661 Change-Id: Iea7caca949c00e701f056c1037e273fee9740e93 --- site/en/external/extension.md | 57 ++++ .../bazel/bzlmod/BazelDepGraphFunction.java | 39 ++- .../lib/bazel/bzlmod/BazelDepGraphValue.java | 48 ++- ...uleExtensionEvalStarlarkThreadContext.java | 13 +- ...leExtensionRepoMappingEntriesFunction.java | 1 + .../bazel/bzlmod/ModuleExtensionUsage.java | 27 ++ .../lib/bazel/bzlmod/ModuleFileFunction.java | 4 +- .../lib/bazel/bzlmod/ModuleFileGlobals.java | 117 +++++++- .../lib/bazel/bzlmod/ModuleThreadContext.java | 83 ++++++ .../bzlmod/RegularRunnableExtension.java | 1 + .../bazel/bzlmod/SingleExtensionFunction.java | 35 ++- .../bzlmod/SingleExtensionUsagesFunction.java | 3 +- .../bzlmod/SingleExtensionUsagesValue.java | 12 +- .../bzlmod/BazelDepGraphFunctionTest.java | 1 + .../bzlmod/ModuleExtensionResolutionTest.java | 280 ++++++++++++++++++ .../bazel/bzlmod/ModuleFileFunctionTest.java | 176 +++++++++++ .../bazel/bzlmod/StarlarkBazelModuleTest.java | 3 +- .../bzlmod/modcommand/ModExecutorTest.java | 4 + .../starlark/StarlarkDefinedAspectsTest.java | 1 + .../lib/starlark/StarlarkIntegrationTest.java | 16 +- .../google/devtools/build/lib/testutil/BUILD | 1 + .../lib/testutil/FoundationTestCase.java | 18 ++ src/test/tools/bzlmod/MODULE.bazel.lock | 10 +- 23 files changed, 914 insertions(+), 36 deletions(-) diff --git a/site/en/external/extension.md b/site/en/external/extension.md index 5ba774961105ba..cbaebb51f45fe8 100644 --- a/site/en/external/extension.md +++ b/site/en/external/extension.md @@ -174,6 +174,63 @@ several repo visibility rules: the repo visible to the module instead of the extension-generated repo of the same name. +### Overriding and injecting module extension repos + +The root module can use +[`override_repo`](/rules/lib/globals/module#override_repo) and +[`inject_repo`](/rules/lib/globals/module#inject_repo) to override or inject +module extension repos. + +#### Example: Replacing `rules_java`'s `java_tools` with a vendored copy + +```python +# MODULE.bazel +local_repository = use_repo_rule("@bazel_tools//tools/build_defs/repo:local.bzl", "local_repository") +local_repository( + name = "my_java_tools", + path = "vendor/java_tools", +) + +bazel_dep(name = "rules_java", version = "7.11.1") +java_toolchains = use_extension("@rules_java//java:extension.bzl", "toolchains") + +override_repo(java_toolchains, remote_java_tools = "my_java_tools") +``` + +#### Example: Patch a Go dependency to depend on `@zlib` instead of the system zlib + +```python +# MODULE.bazel +bazel_dep(name = "gazelle", version = "0.38.0") +bazel_dep(name = "zlib", version = "1.3.1.bcr.3") + +go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps") +go_deps.from_file(go_mod = "//:go.mod") +go_deps.module_override( + patches = [ + "//patches:my_module_zlib.patch", + ], + path = "example.com/my_module", +) +use_repo(go_deps, ...) + +inject_repo(go_deps, "zlib") +``` + +```diff +# patches/my_module_zlib.patch +--- a/BUILD.bazel ++++ b/BUILD.bazel +@@ -1,6 +1,6 @@ + go_binary( + name = "my_module", + importpath = "example.com/my_module", + srcs = ["my_module.go"], +- copts = ["-lz"], ++ cdeps = ["@zlib"], + ) +``` + ## Best practices This section describes best practices when writing extensions so they are diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java index ff990d223dc07c..af19b3939a31f9 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java @@ -80,7 +80,12 @@ public SkyValue compute(SkyKey skyKey, Environment env) canonicalRepoNameLookup, depGraph.values().stream().map(AbridgedModule::from).collect(toImmutableList()), extensionUsagesById, - extensionUniqueNames.inverse()); + extensionUniqueNames.inverse(), + resolveRepoOverrides( + depGraph, + extensionUsagesById, + extensionUniqueNames.inverse(), + canonicalRepoNameLookup)); } private static ImmutableTable @@ -198,6 +203,38 @@ private static String makeUniqueNameCandidate(ModuleExtensionId id, int attempt) + extensionNameDisambiguator); } + private static ImmutableTable resolveRepoOverrides( + ImmutableMap depGraph, + ImmutableTable extensionUsagesTable, + ImmutableMap extensionUniqueNames, + ImmutableBiMap canonicalRepoNameLookup) { + RepositoryMapping rootModuleMappingWithoutOverrides = + BazelDepGraphValue.getRepositoryMapping( + ModuleKey.ROOT, + depGraph, + extensionUsagesTable, + extensionUniqueNames, + canonicalRepoNameLookup, + // ModuleFileFunction ensures that repos that override other repos are not themselves + // overridden, so we can safely pass an empty table here instead of resolving chains + // of overrides. + ImmutableTable.of()); + ImmutableTable.Builder repoOverridesBuilder = + ImmutableTable.builder(); + for (var extensionId : extensionUsagesTable.rowKeySet()) { + var rootUsage = extensionUsagesTable.row(extensionId).get(ModuleKey.ROOT); + if (rootUsage != null) { + for (var override : rootUsage.getRepoOverrides().entrySet()) { + repoOverridesBuilder.put( + extensionId, + override.getKey(), + rootModuleMappingWithoutOverrides.get(override.getValue().overridingRepoName())); + } + } + } + return repoOverridesBuilder.buildOrThrow(); + } + static class BazelDepGraphFunctionException extends SkyFunctionException { BazelDepGraphFunctionException(ExternalDepsException e, Transience transience) { super(e, transience); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java index 17894f4123a4db..4a060d45a7bd98 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java @@ -41,13 +41,15 @@ public static BazelDepGraphValue create( ImmutableMap canonicalRepoNameLookup, ImmutableList abridgedModules, ImmutableTable extensionUsagesTable, - ImmutableMap extensionUniqueNames) { + ImmutableMap extensionUniqueNames, + ImmutableTable repoOverrides) { return new AutoValue_BazelDepGraphValue( depGraph, ImmutableBiMap.copyOf(canonicalRepoNameLookup), abridgedModules, extensionUsagesTable, - extensionUniqueNames); + extensionUniqueNames, + repoOverrides); } public static BazelDepGraphValue createEmptyDepGraph() { @@ -71,7 +73,8 @@ public static BazelDepGraphValue createEmptyDepGraph() { canonicalRepoNameLookup, ImmutableList.of(), ImmutableTable.of(), - ImmutableMap.of()); + ImmutableMap.of(), + ImmutableTable.of()); } /** @@ -103,27 +106,54 @@ public static BazelDepGraphValue createEmptyDepGraph() { */ public abstract ImmutableMap getExtensionUniqueNames(); + /** + * For each module extension, a mapping from the name of the repo exported by the extension to the + * canonical name of the repo that should override it (if any). + */ + public abstract ImmutableTable getRepoOverrides(); + /** * Returns the full {@link RepositoryMapping} for the given module, including repos from Bazel * module deps and module extensions. */ public final RepositoryMapping getFullRepoMapping(ModuleKey key) { + return getRepositoryMapping( + key, + getDepGraph(), + getExtensionUsagesTable(), + getExtensionUniqueNames(), + getCanonicalRepoNameLookup(), + getRepoOverrides()); + } + + static RepositoryMapping getRepositoryMapping( + ModuleKey key, + ImmutableMap depGraph, + ImmutableTable extensionUsagesTable, + ImmutableMap extensionUniqueNames, + ImmutableBiMap canonicalRepoNameLookup, + ImmutableTable repoOverrides) { ImmutableMap.Builder mapping = ImmutableMap.builder(); for (Map.Entry extIdAndUsage : - getExtensionUsagesTable().column(key).entrySet()) { + extensionUsagesTable.column(key).entrySet()) { ModuleExtensionId extensionId = extIdAndUsage.getKey(); ModuleExtensionUsage usage = extIdAndUsage.getValue(); - String repoNamePrefix = getExtensionUniqueNames().get(extensionId) + "+"; + String repoNamePrefix = extensionUniqueNames.get(extensionId) + "+"; for (ModuleExtensionUsage.Proxy proxy : usage.getProxies()) { for (Map.Entry entry : proxy.getImports().entrySet()) { - String canonicalRepoName = repoNamePrefix + entry.getValue(); - mapping.put(entry.getKey(), RepositoryName.createUnvalidated(canonicalRepoName)); + RepositoryName defaultCanonicalRepoName = + RepositoryName.createUnvalidated(repoNamePrefix + entry.getValue()); + mapping.put( + entry.getKey(), + repoOverrides + .row(extensionId) + .getOrDefault(entry.getValue(), defaultCanonicalRepoName)); } } } - return getDepGraph() + return depGraph .get(key) - .getRepoMappingWithBazelDepsOnly(getCanonicalRepoNameLookup().inverse()) + .getRepoMappingWithBazelDepsOnly(canonicalRepoNameLookup.inverse()) .withAdditionalMappings(mapping.buildOrThrow()); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java index 58c43612090a29..1e1ab2c72e7522 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java @@ -71,6 +71,7 @@ record RepoRuleCall( private final String repoPrefix; private final PackageIdentifier basePackageId; private final RepositoryMapping baseRepoMapping; + private final ImmutableMap repoOverrides; private final BlazeDirectories directories; private final ExtendedEventHandler eventHandler; private final Map deferredRepos = new LinkedHashMap<>(); @@ -80,6 +81,7 @@ public ModuleExtensionEvalStarlarkThreadContext( String repoPrefix, PackageIdentifier basePackageId, RepositoryMapping baseRepoMapping, + ImmutableMap repoOverrides, RepositoryMapping mainRepoMapping, BlazeDirectories directories, ExtendedEventHandler eventHandler) { @@ -88,6 +90,7 @@ public ModuleExtensionEvalStarlarkThreadContext( this.repoPrefix = repoPrefix; this.basePackageId = basePackageId; this.baseRepoMapping = baseRepoMapping; + this.repoOverrides = repoOverrides; this.directories = directories; this.eventHandler = eventHandler; } @@ -133,13 +136,15 @@ public ImmutableMap createRepos(StarlarkSemantics starlarkSema // Make it possible to refer to extension repos in the label attributes of another extension // repo. Wrapping a label in Label(...) ensures that it is evaluated with respect to the // containing module's repo mapping instead. - var extensionRepos = + ImmutableMap.Builder entries = ImmutableMap.builder(); + entries.putAll(baseRepoMapping.entries()); + entries.putAll( Maps.asMap( deferredRepos.keySet(), - apparentName -> RepositoryName.createUnvalidated(repoPrefix + apparentName)); + apparentName -> RepositoryName.createUnvalidated(repoPrefix + apparentName))); + entries.putAll(repoOverrides); RepositoryMapping fullRepoMapping = - RepositoryMapping.create(extensionRepos, baseRepoMapping.ownerRepo()) - .withAdditionalMappings(baseRepoMapping); + RepositoryMapping.create(entries.buildKeepingLast(), baseRepoMapping.ownerRepo()); // LINT.ThenChange(//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java) ImmutableMap.Builder repoSpecs = ImmutableMap.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java index 9253d38813ea04..8e400bc37093cb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java @@ -74,6 +74,7 @@ private ModuleExtensionRepoMappingEntriesValue computeRepoMappingEntries( ImmutableMap.Builder entries = ImmutableMap.builder(); entries.putAll(bazelDepGraphValue.getFullRepoMapping(moduleKey).entries()); entries.putAll(extensionEvalValue.getCanonicalRepoNameToInternalNames().inverse()); + entries.putAll(bazelDepGraphValue.getRepoOverrides().row(extensionId)); return ModuleExtensionRepoMappingEntriesValue.create(entries.buildKeepingLast(), moduleKey); // LINT.ThenChange(//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java) } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java index e5dc76f120e9fa..f4fc11683fe970 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.ryanharter.auto.value.gson.GenerateTypeAdapter; @@ -130,6 +131,24 @@ public final boolean getHasNonDevUseExtension() { return getProxies().stream().anyMatch(p -> !p.isDevDependency()); } + /** + * Represents a repo that overrides another repo within the scope of the extension. + * + * @param overridingRepoName The apparent name of the overriding repo in the root module. + * @param mustExist Whether this override should apply to an existing repo. + * @param location The location of the {@code override_repo} or {@code inject_repo} call. + */ + @GenerateTypeAdapter + public record RepoOverride(String overridingRepoName, boolean mustExist, Location location) {} + + /** + * Contains information about overrides that apply to repos generated by this extension. Keyed by + * the extension-local repo name. + * + *

This is only non-empty for root module usages. + */ + public abstract ImmutableMap getRepoOverrides(); + public abstract Builder toBuilder(); public static Builder builder() { @@ -152,6 +171,11 @@ ModuleExtensionUsage trimForEvaluation() { // Extension implementation functions do not see the imports, they are only validated // against the set of generated repos in a validation step that comes afterward. .setProxies(ImmutableList.of()) + // Tracked in SingleExtensionUsagesValue instead, using canonical instead of apparent names. + // Whether this override must apply to an existing repo as well as its source location also + // don't influence the evaluation of the extension as they are checked in + // SingleExtensionFunction. + .setRepoOverrides(ImmutableMap.of()) .build(); } @@ -185,6 +209,9 @@ public Builder addTag(Tag value) { return this; } + @CanIgnoreReturnValue + public abstract Builder setRepoOverrides(ImmutableMap repoOverrides); + public abstract ModuleExtensionUsage build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index a86b30c56de60a..c24d555d502e13 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -435,7 +435,7 @@ public static RootModuleFileValue evaluateRootModuleFile( try { module = moduleThreadContext.buildModule(/* registry= */ null); } catch (EvalException e) { - eventHandler.handle(Event.error(e.getMessageWithStack())); + eventHandler.handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for the root module"); } for (ModuleExtensionUsage usage : module.getExtensionUsages()) { @@ -521,7 +521,7 @@ private static ModuleThreadContext execModuleFile( }); compiledRootModuleFile.runOnThread(thread); } catch (EvalException e) { - eventHandler.handle(Event.error(e.getMessageWithStack())); + eventHandler.handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for %s", moduleKey); } return context; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java index a0942ea77add66..42859959887c66 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.bazel.bzlmod; - import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; @@ -548,6 +547,15 @@ void addImport(String localRepoName, String exportedName, String byWhat, Locatio proxyBuilder.addImport(localRepoName, exportedName); } + void addOverride( + String overriddenRepoName, + String overridingRepoName, + boolean mustExist, + ImmutableList stack) + throws EvalException { + usageBuilder.addRepoOverride(overriddenRepoName, overridingRepoName, mustExist, stack); + } + class TagCallable implements StarlarkValue { final String tagName; @@ -637,6 +645,113 @@ public void useRepo( } } + @StarlarkMethod( + name = "override_repo", + doc = + """ + Overrides one or more repos defined by the given module extension with the given repos + visible to the current module. This is ignored if the current module is not the root + module or `--ignore_dev_dependency` is enabled. + +

Use inject_repo instead to add a new repo. + """, + parameters = { + @Param( + name = "extension_proxy", + doc = "A module extension proxy object returned by a use_extension call."), + }, + extraPositionals = + @Param( + name = "args", + doc = + """ + The repos in the extension that should be overridden with the repos of the same + name in the current module."""), + extraKeywords = + @Param( + name = "kwargs", + doc = + """ + The overrides to apply to the repos generated by the extension, where the values + are the names of repos in the scope of the current module and the keys are the + names of the repos they will override in the extension."""), + useStarlarkThread = true) + public void overrideRepo( + ModuleExtensionProxy extensionProxy, + Tuple args, + Dict kwargs, + StarlarkThread thread) + throws EvalException { + ModuleThreadContext context = ModuleThreadContext.fromOrFail(thread, "override_repo()"); + context.setNonModuleCalled(); + if (context.shouldIgnoreDevDeps()) { + // Ignore calls early as they may refer to repos that are dev dependencies (or this is not the + // root module). + return; + } + ImmutableList stack = thread.getCallStack(); + for (String arg : Sequence.cast(args, String.class, "args")) { + extensionProxy.addOverride(arg, arg, /* mustExist= */ true, stack); + } + for (Map.Entry entry : + Dict.cast(kwargs, String.class, String.class, "kwargs").entrySet()) { + extensionProxy.addOverride(entry.getKey(), entry.getValue(), /* mustExist= */ true, stack); + } + } + + @StarlarkMethod( + name = "inject_repo", + doc = + """ + Injects one or more new repos into the given module extension. + This is ignored if the current module is not the root module or `--ignore_dev_dependency` + is enabled. +

Use override_repo instead to override an + existing repo.""", + parameters = { + @Param( + name = "extension_proxy", + doc = "A module extension proxy object returned by a use_extension call."), + }, + extraPositionals = + @Param( + name = "args", + doc = + """ + The repos visible to the current module that should be injected into the + extension under the same name."""), + extraKeywords = + @Param( + name = "kwargs", + doc = + """ + The new repos to inject into the extension, where the values are the names of + repos in the scope of the current module and the keys are the name they will be + visible under in the extension."""), + useStarlarkThread = true) + public void injectRepo( + ModuleExtensionProxy extensionProxy, + Tuple args, + Dict kwargs, + StarlarkThread thread) + throws EvalException { + ModuleThreadContext context = ModuleThreadContext.fromOrFail(thread, "inject_repo()"); + context.setNonModuleCalled(); + if (context.shouldIgnoreDevDeps()) { + // Ignore calls early as they may refer to repos that are dev dependencies (or this is not the + // root module). + return; + } + ImmutableList stack = thread.getCallStack(); + for (String arg : Sequence.cast(args, String.class, "args")) { + extensionProxy.addOverride(arg, arg, /* mustExist= */ false, stack); + } + for (Map.Entry entry : + Dict.cast(kwargs, String.class, String.class, "kwargs").entrySet()) { + extensionProxy.addOverride(entry.getKey(), entry.getValue(), /* mustExist= */ false, stack); + } + } + @StarlarkMethod( name = "use_repo_rule", doc = diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java index aa1b9eded7a80e..2bd90a32d20bdb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; @@ -54,6 +55,9 @@ public class ModuleThreadContext extends StarlarkThreadContext { private final Map overrides = new LinkedHashMap<>(); private final Map repoNameUsages = new HashMap<>(); + private final Map overriddenRepos = new HashMap<>(); + private final Map overridingRepos = new HashMap<>(); + public static ModuleThreadContext fromOrFail(StarlarkThread thread, String what) throws EvalException { StarlarkThreadContext context = thread.getThreadLocal(StarlarkThreadContext.class); @@ -75,6 +79,18 @@ public ModuleThreadContext( this.includeLabelToCompiledModuleFile = includeLabelToCompiledModuleFile; } + record RepoOverride( + String overriddenRepoName, + String overridingRepoName, + boolean mustExist, + String extensionName, + ImmutableList stack) { + Location location() { + // Skip over the override_repo builtin frame. + return stack.reverse().get(1).location; + } + } + record RepoNameUsage(String how, Location where) {} public void addRepoNameUsage(String repoName, String how, Location where) throws EvalException { @@ -121,12 +137,14 @@ List getExtensionUsageBuilders() { } static class ModuleExtensionUsageBuilder { + private final ModuleThreadContext context; private final String extensionBzlFile; private final String extensionName; private final boolean isolate; private final ArrayList proxyBuilders; private final HashBiMap imports; + private final Map repoOverrides; private final ImmutableList.Builder tags; ModuleExtensionUsageBuilder( @@ -140,6 +158,7 @@ static class ModuleExtensionUsageBuilder { this.isolate = isolate; this.proxyBuilders = new ArrayList<>(); this.imports = HashBiMap.create(); + this.repoOverrides = new HashMap<>(); this.tags = ImmutableList.builder(); } @@ -171,6 +190,27 @@ void addImport(String localRepoName, String exportedName, String byWhat, Locatio imports.put(localRepoName, exportedName); } + public void addRepoOverride( + String overriddenRepoName, + String overridingRepoName, + boolean mustExist, + ImmutableList stack) + throws EvalException { + RepositoryName.validateUserProvidedRepoName(overriddenRepoName); + RepositoryName.validateUserProvidedRepoName(overridingRepoName); + RepoOverride collision = + repoOverrides.put( + overriddenRepoName, + new RepoOverride( + overriddenRepoName, overridingRepoName, mustExist, extensionName, stack)); + if (collision != null) { + throw Starlark.errorf( + "The repo exported as '%s' by module extension '%s' is already overridden with '%s' at" + + " %s", + overriddenRepoName, extensionName, collision.overridingRepoName, collision.location()); + } + } + void addTag(Tag tag) { tags.add(tag); } @@ -197,6 +237,31 @@ ModuleExtensionUsage buildUsage() throws EvalException { } else { builder.setIsolationKey(Optional.empty()); } + + for (var override : repoOverrides.entrySet()) { + String overriddenRepoName = override.getKey(); + String overridingRepoName = override.getValue().overridingRepoName; + if (!context.repoNameUsages.containsKey(overridingRepoName)) { + throw Starlark.errorf( + "The repo exported as '%s' by module extension '%s' is overridden with '%s', but" + + " no repo is visible under this name", + overriddenRepoName, extensionName, overridingRepoName) + .withCallStack(override.getValue().stack); + } + String importedAs = imports.inverse().get(overriddenRepoName); + if (importedAs != null) { + context.overriddenRepos.put(importedAs, override.getValue()); + } + context.overridingRepos.put(overridingRepoName, override.getValue()); + } + builder.setRepoOverrides( + ImmutableMap.copyOf( + Maps.transformValues( + repoOverrides, + v -> + new ModuleExtensionUsage.RepoOverride( + v.overridingRepoName, v.mustExist, v.location())))); + return builder.build(); } } @@ -263,6 +328,24 @@ public InterimModule buildModule(@Nullable Registry registry) throws EvalExcepti } extensionUsages.add(extensionUsageBuilder.buildUsage()); } + // A repo cannot be both overriding and overridden. This ensures that repo overrides can be + // applied to repo mappings in a single step (and also prevents cycles). + Optional overridingAndOverridden = + overridingRepos.keySet().stream().filter(overriddenRepos::containsKey).findFirst(); + if (overridingAndOverridden.isPresent()) { + var override = overridingRepos.get(overridingAndOverridden.get()); + var overrideOnOverride = overriddenRepos.get(overridingAndOverridden.get()); + throw Starlark.errorf( + "The repo '%s' used as an override for '%s' in module extension '%s' is itself" + + " overridden with '%s' at %s, which is not supported.", + override.overridingRepoName, + override.overriddenRepoName, + override.extensionName, + overrideOnOverride.overridingRepoName, + overrideOnOverride.location()) + .withCallStack(override.stack); + } + return module .setRegistry(registry) .setDeps(ImmutableMap.copyOf(deps)) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java index d0ed560bf7568d..962ea1e03018a1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java @@ -256,6 +256,7 @@ private RunModuleExtensionResult runInternal( usagesValue.getExtensionUniqueName() + "+", extensionId.getBzlFileLabel().getPackageIdentifier(), BazelModuleContext.of(bzlLoadValue.getModule()).repoMapping(), + usagesValue.getRepoOverrides(), mainRepositoryMapping, directories, env.getListener()); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java index d0a8e98c8dc51d..fbce3dfd7dac3b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java @@ -52,7 +52,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) { for (ModuleExtensionUsage.Proxy proxy : usage.getProxies()) { for (Entry repoImport : proxy.getImports().entrySet()) { - if (!evalOnlyValue.getGeneratedRepoSpecs().containsKey(repoImport.getValue())) { + if (!evalOnlyValue.getGeneratedRepoSpecs().containsKey(repoImport.getValue()) + && !usagesValue.getRepoOverrides().containsKey(repoImport.getValue())) { throw new SingleExtensionFunctionException( ExternalDepsException.withMessage( Code.INVALID_EXTENSION_IMPORT, @@ -71,6 +72,38 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } + // Check that repo overrides apply as declared. + for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) { + for (var override : usage.getRepoOverrides().entrySet()) { + boolean repoExists = evalOnlyValue.getGeneratedRepoSpecs().containsKey(override.getKey()); + if (repoExists && !override.getValue().mustExist()) { + throw new SingleExtensionFunctionException( + ExternalDepsException.withMessage( + Code.INVALID_EXTENSION_IMPORT, + "module extension \"%s\" from \"%s\" generates repository \"%s\", yet" + + " it is injected via inject_repo() at %s. Use override_repo() instead to" + + " override an existing repository.", + extensionId.getExtensionName(), + extensionId.getBzlFileLabel(), + override.getKey(), + override.getValue().location()), + Transience.PERSISTENT); + } else if (!repoExists && override.getValue().mustExist()) { + throw new SingleExtensionFunctionException( + ExternalDepsException.withMessage( + Code.INVALID_EXTENSION_IMPORT, + "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet" + + " it is overridden via override_repo() at %s. Use inject_repo() instead to" + + " inject a new repository.", + extensionId.getExtensionName(), + extensionId.getBzlFileLabel(), + override.getKey(), + override.getValue().location()), + Transience.PERSISTENT); + } + } + } + return evalOnlyValue; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java index 870eb6c276b39b..4a3ca17e13f372 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java @@ -61,6 +61,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept .collect(toImmutableList()), // TODO(wyv): Maybe cache these mappings? usagesTable.row(id).keySet().stream() - .collect(toImmutableMap(key -> key, bazelDepGraphValue::getFullRepoMapping))); + .collect(toImmutableMap(key -> key, bazelDepGraphValue::getFullRepoMapping)), + bazelDepGraphValue.getRepoOverrides().row(id)); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java index b75e3955b02aa7..a812b25f906a84 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java @@ -21,6 +21,7 @@ import com.google.common.collect.Maps; import com.google.common.hash.Hashing; import com.google.devtools.build.lib.cmdline.RepositoryMapping; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -56,13 +57,17 @@ public abstract class SingleExtensionUsagesValue implements SkyValue { /** The repo mappings to use for each module that used this extension. */ public abstract ImmutableMap getRepoMappings(); + /** Maps an extension-local repo name to the canonical name of the repo it is overridden with. */ + public abstract ImmutableMap getRepoOverrides(); + public static SingleExtensionUsagesValue create( ImmutableMap extensionUsages, String extensionUniqueName, ImmutableList abridgedModules, - ImmutableMap repoMappings) { + ImmutableMap repoMappings, + ImmutableMap repoOverrides) { return new AutoValue_SingleExtensionUsagesValue( - extensionUsages, extensionUniqueName, abridgedModules, repoMappings); + extensionUsages, extensionUniqueName, abridgedModules, repoMappings, repoOverrides); } /** @@ -89,7 +94,8 @@ SingleExtensionUsagesValue trimForEvaluation() { // repoMappings: The usage of repo mappings by the extension's implementation function is // tracked on the level of individual entries and all label attributes are provided as // `Label`, which exclusively reference canonical repository names. - ImmutableMap.of()); + ImmutableMap.of(), + getRepoOverrides()); } public static Key key(ModuleExtensionId id) { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java index 763be302dd8ab1..218b298eecaa9f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java @@ -220,6 +220,7 @@ private static ModuleExtensionUsage createModuleExtensionUsage( .setExtensionBzlFile(bzlFile) .setExtensionName(name) .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setDevDependency(false) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index 5ad8da2c8a2101..668ea6b79d2a7e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -1431,6 +1431,7 @@ public void nonVisibleLabelInLabelAttr() throws Exception { """ ERROR /ws/defs.bzl:9:12: Traceback (most recent call last): \tFile "/ws/defs.bzl", line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data='@not_other_repo//:foo') Error in repository_rule: no repository visible as '@not_other_repo' in \ the extension '@@//:defs.bzl%ext', but referenced by label \ '@not_other_repo//:foo' in attribute 'data' of data_repo 'ext'."""); @@ -1475,6 +1476,7 @@ public void nonVisibleLabelInLabelAttrNonRootModule() throws Exception { Traceback (most recent call last): \tFile "/usr/local/google/_blaze_jrluser/FAKEMD5/external/ext_module+/defs.bzl", \ line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data='@not_other_repo//:foo') Error in repository_rule: no repository visible as '@not_other_repo' in the extension \ '@@ext_module+//:defs.bzl%ext', but referenced by label '@not_other_repo//:foo' in \ attribute 'data' of data_repo 'ext'."""); @@ -1553,6 +1555,7 @@ public void nonVisibleLabelInLabelListAttr() throws Exception { """ ERROR /ws/defs.bzl:9:12: Traceback (most recent call last): \tFile "/ws/defs.bzl", line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data=['@not_other_repo//:foo']) Error in repository_rule: no repository visible as '@not_other_repo' \ in the extension '@@//:defs.bzl%ext', but referenced by label \ '@not_other_repo//:foo' in attribute 'data' of data_repo 'ext'."""); @@ -1589,6 +1592,7 @@ public void nonVisibleLabelInLabelKeyedStringDictAttr() throws Exception { """ ERROR /ws/defs.bzl:9:12: Traceback (most recent call last): \tFile "/ws/defs.bzl", line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data={'@not_other_repo//:foo':'bar'}) Error in repository_rule: no repository visible as '@not_other_repo' \ in the extension '@@//:defs.bzl%ext', but referenced by label \ '@not_other_repo//:foo' in attribute 'data' of data_repo 'ext'."""); @@ -3098,4 +3102,280 @@ def _other_ext_impl(ctx): "@@+other_ext+other_bar//:bar") .inOrder(); } + + @Test + public void overrideRepo_override() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar", module_foo = "foo") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "override", data = "overridden_data") + override_repo(ext, foo = "override") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@override//:data.bzl", _override_data = "data") + load("@module_foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + override_data = _override_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@module_foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _fail_repo_impl(ctx): + fail("This rule should not be evaluated") + fail_repo = repository_rule(implementation = _fail_repo_impl) + def _ext_impl(ctx): + fail_repo(name = "foo") + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + "@module_foo//:target2", + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + if (result.hasError()) { + throw result.getError().getException(); + } + assertThat((List) result.get(skyKey).getModule().getGlobal("bar_list")) + .containsExactly( + "@@+_repo_rules+override//:target1", + "@@+_repo_rules+override//:target2", + "@@+_repo_rules+override//:target3", + "@@+_repo_rules+override//:target4") + .inOrder(); + Object overrideData = result.get(skyKey).getModule().getGlobal("override_data"); + assertThat(overrideData).isInstanceOf(String.class); + assertThat(overrideData).isEqualTo("overridden_data"); + Object fooData = result.get(skyKey).getModule().getGlobal("foo_data"); + assertThat(fooData).isSameInstanceAs(overrideData); + } + + @Test + public void overrideRepo_override_onNonExistentRepoFails() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar", module_foo = "foo") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "foo", data = "overridden_data") + override_repo(ext, "foo") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _ext_impl(ctx): + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + Label("@foo//:target2"), + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + assertThat(result.hasError()).isTrue(); + assertThat(result.getError().getException()) + .hasMessageThat() + .isEqualTo( + "module extension \"ext\" from \"//:defs.bzl\" does not generate repository \"foo\"," + + " yet it is overridden via override_repo() at /ws/MODULE.bazel:6:14. Use" + + " inject_repo() instead to inject a new repository."); + } + + @Test + public void overrideRepo_inject() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar", module_foo = "foo") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "foo", data = "overridden_data") + inject_repo(ext, "foo") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _ext_impl(ctx): + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + Label("@foo//:target2"), + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + if (result.hasError()) { + throw result.getError().getException(); + } + assertThat((List) result.get(skyKey).getModule().getGlobal("bar_list")) + .containsExactly( + "@@+_repo_rules+foo//:target1", + "@@+_repo_rules+foo//:target2", + "@@+_repo_rules+foo//:target3", + "@@+_repo_rules+foo//:target4") + .inOrder(); + Object fooData = result.get(skyKey).getModule().getGlobal("foo_data"); + assertThat(fooData).isInstanceOf(String.class); + assertThat(fooData).isEqualTo("overridden_data"); + } + + @Test + public void overrideRepo_inject_onExistingRepoFails() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar", module_foo = "foo") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "override", data = "overridden_data") + inject_repo(ext, foo = "override") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@override//:data.bzl", _override_data = "data") + load("@module_foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + override_data = _override_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@module_foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _fail_repo_impl(ctx): + fail("This rule should not be evaluated") + fail_repo = repository_rule(implementation = _fail_repo_impl) + def _ext_impl(ctx): + fail_repo(name = "foo") + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + "@module_foo//:target2", + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + assertThat(result.hasError()).isTrue(); + assertThat(result.getError().getException()) + .hasMessageThat() + .isEqualTo( + "module extension \"ext\" from \"//:defs.bzl\" generates repository \"foo\", yet it is" + + " injected via inject_repo() at /ws/MODULE.bazel:6:12. Use override_repo()" + + " instead to override an existing repository."); + } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index e8296c2c6abade..1d54fe454e0216 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -776,6 +776,7 @@ public void testModuleExtensions_good() throws Exception { .setExtensionBzlFile("@mymod//:defs.bzl") .setExtensionName("myext1") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -806,6 +807,7 @@ public void testModuleExtensions_good() throws Exception { .setExtensionBzlFile("@mymod//:defs.bzl") .setExtensionName("myext2") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -850,6 +852,7 @@ public void testModuleExtensions_good() throws Exception { .setExtensionBzlFile("@rules_jvm_external//:defs.bzl") .setExtensionName("maven") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -927,6 +930,7 @@ public void testModuleExtensions_duplicateProxy_asRoot() throws Exception { .setExtensionBzlFile("@//:defs.bzl") .setExtensionName("myext") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -1058,6 +1062,7 @@ public void testModuleExtensions_duplicateProxy_asDep() throws Exception { .setExtensionBzlFile("@mymod//:defs.bzl") .setExtensionName("myext") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -1177,6 +1182,7 @@ public void testModuleExtensions_innate() throws Exception { .setExtensionBzlFile("//:MODULE.bazel") .setExtensionName("_repo_rules") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -1705,4 +1711,174 @@ public void testInvalidUseExtensionLabel() throws Exception { + " name 'foo/bar:extensions.bzl': repo names may contain only A-Z, a-z, 0-9, '-'," + " '_', '.' and '+'"); } + + @Test + public void testOverrideRepo_overridingRepoDoesntExist() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + ext = use_extension('//:defs.bzl', 'ext') + override_repo(ext, 'foo') + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:3:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 3, column 14, in + \t\toverride_repo(ext, 'foo') + Error in override_repo: The repo exported as 'foo' by module extension 'ext' is \ + overridden with 'foo', but no repo is visible under this name"""); + } + + @Test + public void testOverrideRepo_duplicateOverride() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override1", version = "1.0") + bazel_dep(name = "override2", version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + override_repo(ext, foo = "override1") + override_repo(ext, foo = "override2") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:6:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 6, column 14, in + \t\toverride_repo(ext, foo = "override2") + Error in override_repo: The repo exported as 'foo' by module extension 'ext' is already \ + overridden with 'override1' at /workspace/MODULE.bazel:5:14"""); + } + + @Test + public void testOverrideRepo_chain_singleExtension() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override", version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + use_repo(ext, bar = "foo") + override_repo(ext, baz = "bar") + override_repo(ext, foo = "override") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext, baz = "bar") + Error in override_repo: The repo 'bar' used as an override for 'baz' in module extension \ + 'ext' is itself overridden with 'override' at /workspace/MODULE.bazel:6:14, which is not \ + supported."""); + } + + @Test + public void testOverrideRepo_chain_multipleExtensions() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override", version = "1.0") + ext1 = use_extension('//:defs.bzl', 'ext1') + ext2 = use_extension('//:defs.bzl', 'ext2') + override_repo(ext1, baz = "bar") + override_repo(ext2, foo = "override") + use_repo(ext2, bar = "foo") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext1, baz = "bar") + Error in override_repo: The repo 'bar' used as an override for 'baz' in module extension \ + 'ext1' is itself overridden with 'override' at /workspace/MODULE.bazel:6:14, which is not \ + supported."""); + } + + @Test + public void testOverrideRepo_cycle_singleExtension() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override", version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + use_repo(ext, my_foo = "foo", my_bar = "bar") + override_repo(ext, foo = "my_bar", bar = "my_foo") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext, foo = "my_bar", bar = "my_foo") + Error in override_repo: The repo 'my_foo' used as an override for 'bar' in module \ + extension 'ext' is itself overridden with 'my_bar' at /workspace/MODULE.bazel:5:14, which \ + is not supported."""); + } + + @Test + public void testOverrideRepo_cycle_multipleExtensions() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + ext1 = use_extension('//:defs.bzl', 'ext1') + ext2 = use_extension('//:defs.bzl', 'ext2') + override_repo(ext1, foo = "my_bar") + override_repo(ext2, bar = "my_foo") + use_repo(ext1, my_foo = "foo") + use_repo(ext2, my_bar = "bar") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext2, bar = "my_foo") + Error in override_repo: The repo 'my_foo' used as an override for 'bar' in module \ + extension 'ext2' is itself overridden with 'my_bar' at /workspace/MODULE.bazel:4:14, \ + which is not supported."""); + } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java index 3d0adf3bbaeb84..439f0c8cf449f4 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java @@ -46,7 +46,8 @@ private static ModuleExtensionUsage.Builder getBaseUsageBuilder() { return ModuleExtensionUsage.builder() .setExtensionBzlFile("//:rje.bzl") .setExtensionName("maven") - .setIsolationKey(Optional.empty()); + .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()); } /** A builder for ModuleExtension that sets all the mandatory but irrelevant fields. */ diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java index 6ac2a9d7714c4b..9deb60bf20849e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java @@ -607,6 +607,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("maven") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("C@1.0/MODULE.bazel", 2, 23)) @@ -621,6 +622,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("maven") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("D@1.0/MODULE.bazel", 1, 10)) @@ -635,6 +637,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("gradle") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("Y@2.0/MODULE.bazel", 2, 13)) @@ -649,6 +652,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("maven") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("Y@2.0/MODULE.bazel", 13, 10)) diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java index 73e76918d4384a..bbca344aac45a2 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java @@ -871,6 +871,7 @@ def _impl(target, ctx): + "//test:aspect.bzl%MyAspect aspect on java_library rule //test:xxx: \n" + "Traceback (most recent call last):\n" + "\tFile \"/workspace/test/aspect.bzl\", line 2, column 13, in _impl\n" + + "\t\treturn 1 // 0\n" + "Error: integer division by zero"); } diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java index b973a92225bde3..3bde346e2e6314 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java @@ -480,17 +480,17 @@ protected void runStackTraceTest(String expr, String errorMessage) throws Except "Traceback (most recent call last):", "\tFile \"/workspace/test/starlark/extension.bzl\", line 6, column 6, in" + " custom_rule_impl", - // "\t\tfoo()", + "\t\tfoo()", "\tFile \"/workspace/test/starlark/extension.bzl\", line 9, column 6, in foo", - // "\t\tbar(2, 4)", + "\t\tbar(2, 4)", "\tFile \"/workspace/test/starlark/extension.bzl\", line 11, column 8, in bar", - // "\t\tfirst(x, y, z)", + "\t\tfirst(x, y, z)", "\tFile \"/workspace/test/starlark/functions.bzl\", line 2, column 9, in first", - // "\t\tsecond(a, b)", + "\t\tsecond(a, b)", "\tFile \"/workspace/test/starlark/functions.bzl\", line 5, column 8, in second", - // "\t\tthird(\"legal\")", + "\t\tthird('legal')", "\tFile \"/workspace/test/starlark/functions.bzl\", line 7, column 12, in third", - // ... + "\t\t" + expr.stripLeading(), errorMessage); scratch.file( "test/starlark/extension.bzl", @@ -503,9 +503,9 @@ def custom_rule_impl(ctx): foo() return [MyInfo(provider_key = ftb)] def foo(): - bar(2,4) + bar(2, 4) def bar(x,y,z=1): - first(x,y, z) + first(x, y, z) custom_rule = rule(implementation = custom_rule_impl, attrs = {'attr1': attr.label_list(mandatory=True, allow_files=True)}) """); diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/testutil/BUILD index 199065a6cf6f47..bdf9e30ce7b839 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/BUILD +++ b/src/test/java/com/google/devtools/build/lib/testutil/BUILD @@ -75,6 +75,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", + "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", "//src/main/protobuf:failure_details_java_proto", "//third_party:error_prone_annotations", diff --git a/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java b/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java index 848adffb09f15e..deb0af17a4edca 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java @@ -13,8 +13,11 @@ // limitations under the License. package com.google.devtools.build.lib.testutil; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; +import com.google.common.base.Splitter; +import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.events.Event; @@ -24,11 +27,13 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.util.Set; import java.util.regex.Pattern; +import net.starlark.java.eval.EvalException; import org.junit.After; import org.junit.Before; @@ -78,6 +83,19 @@ public final void initializeFileSystemAndDirectories() throws Exception { scratch.file(rootDirectory.getRelative("WORKSPACE").getPathString()); scratch.file(rootDirectory.getRelative("MODULE.bazel").getPathString()); root = Root.fromPath(rootDirectory); + + // Let the Starlark interpreter know how to read source files. + EvalException.setSourceReaderSupplier( + () -> + loc -> { + try { + String content = FileSystemUtils.readContent(fileSystem.getPath(loc.file()), UTF_8); + return Iterables.get(Splitter.on("\n").split(content), loc.line() - 1, null); + } catch (Exception ignored) { + // ignore any exceptions reading the file -- this is just for extra info + return null; + } + }); } @Before diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 54502b8d16d3fe..03ed8d97af56c4 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -67,7 +67,7 @@ "@@platforms//host:extension.bzl%host_platform": { "general": { "bzlTransitiveDigest": "xelQcPZH8+tmuOHVjL9vDxMnnQNMlwj0SlvgoqBkm4U=", - "usagesDigest": "BsEIFMvXBOQ6RUjjEzt6CU2+w+vAjFwqp58drqCa2jo=", + "usagesDigest": "dV0XZuBT6Bl4dbmmjYrJStsZAD/hV31fLkUkDHmbHIM=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -84,7 +84,7 @@ "@@rules_jvm_external+//:extensions.bzl%maven": { "general": { "bzlTransitiveDigest": "tgBpQFC46MaT8n2UeSnG4GNQ8M01bKKTeEWQX+cuoSA=", - "usagesDigest": "sl3uEZVp3ixjaQzCfAguinRv5zcKQM68YRioCHBX0cE=", + "usagesDigest": "gwSjMkWKurhylh2CEkjffLf0iUOR4Ra7XvJyx5l1ECc=", "recordedFileInputs": { "@@rules_jvm_external+//rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" }, @@ -1108,7 +1108,7 @@ "@@rules_jvm_external+//:non-module-deps.bzl%non_module_deps": { "general": { "bzlTransitiveDigest": "7dxAT2NQpsWXJNV5Sy6o7E78vgRhVEN1vlZdrexTMZY=", - "usagesDigest": "xWSELLxz/xEmLvX/KEddTujJEVrOIQ4j7cs6BkF2J4s=", + "usagesDigest": "lOaODXUC3oEqMd8/8lv63loQ2pQB4QFWNzo9pqgifck=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -1136,7 +1136,7 @@ "@@rules_python+//python/extensions:python.bzl%python": { "general": { "bzlTransitiveDigest": "z35lvtk23Cj8pA0OHXIWJQ+sP4WORVrujhMDtWGyqo8=", - "usagesDigest": "tmZ6sMJ23K1YYNMIBcEX0sXPX3I2o7ICQethLTZQvuM=", + "usagesDigest": "u8OFDhNDDCVxFb/U4yT0JRRUDiGNgTPY/lKnK3FcvdM=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -1166,7 +1166,7 @@ "@@rules_python+//python/extensions/private:internal_deps.bzl%internal_deps": { "general": { "bzlTransitiveDigest": "59ucGaK2UFzBbVlrE4x4O1jM4EWKd3zAXRUH63T4j68=", - "usagesDigest": "T+GX/uEOF49O6I6zgLru1E6pJLzL0nzFKcKsMpGGm4I=", + "usagesDigest": "7eZGJGF9wrvQzVaMSqT5s+PIScCmeQUQ61hx1hyUvNI=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {},