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 04c5548e046789..44fb113ecad0db 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 @@ -630,8 +630,8 @@ private static ModuleThreadContext execModuleFile( } }); - injectRepos(injectedRepositories, context, thread); compiledRootModuleFile.runOnThread(thread); + injectRepos(injectedRepositories, context, thread); } catch (EvalException e) { eventHandler.handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for %s", moduleKey); 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 eafb8a4829d03f..4f16fc9f6ca9cf 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 @@ -108,11 +108,12 @@ Location location() { public void addRepoNameUsage( String repoName, String how, ImmutableList stack) throws EvalException { - RepoNameUsage collision = repoNameUsages.put(repoName, new RepoNameUsage(how, stack)); + RepoNameUsage incoming = new RepoNameUsage(how, stack); + RepoNameUsage collision = repoNameUsages.put(repoName, incoming); if (collision != null) { throw Starlark.errorf( - "The repo name '%s' is already being used %s at %s", - repoName, collision.how(), collision.location()); + "The repo name '%s' cannot be defined %s at %s as it is already defined %s at %s", + repoName, incoming.how(), incoming.location(), collision.how(), collision.location()); } } 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 4f361dc8cdac9f..94b4ffca99d6be 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 @@ -522,7 +522,8 @@ public void testRootModule_include_bad_repoNameCollision() throws Exception { evaluator.evaluate( ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); assertThat(result.hasError()).isTrue(); - assertContainsEvent("The repo name 'foo' is already being used"); + assertContainsEvent("The repo name 'foo' cannot be defined"); + assertContainsEvent("as it is already defined"); } @Test @@ -1144,8 +1145,8 @@ public void testModuleExtensions_repoNameCollision_localRepoName() throws Except reporter.removeHandler(failFastHandler); // expect failures evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); - assertContainsEvent( - "The repo name 'mymod' is already being used as the current module name at"); + assertContainsEvent("The repo name 'mymod' cannot be defined"); + assertContainsEvent("as it is already defined as the current module name"); } @Test @@ -1491,7 +1492,8 @@ public void moduleRepoName_conflict() throws Exception { reporter.removeHandler(failFastHandler); // expect failures evaluator.evaluate(ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - assertContainsEvent("The repo name 'bbb' is already being used as the module's own repo name"); + assertContainsEvent("The repo name 'bbb' cannot be defined"); + assertContainsEvent("as it is already defined as the module's own repo name"); } @Test diff --git a/src/test/py/bazel/bzlmod/bazel_overrides_test.py b/src/test/py/bazel/bzlmod/bazel_overrides_test.py index 3fd7bd18a503d1..1cd1f064ddb160 100644 --- a/src/test/py/bazel/bzlmod/bazel_overrides_test.py +++ b/src/test/py/bazel/bzlmod/bazel_overrides_test.py @@ -720,11 +720,61 @@ def testInjectRepositoryOnExistingRepo(self): allow_failure=True, ) self.AssertNotExitCode(exit_code, 0, stderr) + stderr = "\n".join(stderr) self.assertIn( - "Error in use_repo: The repo name 'my_repo' is already being used by" - ' --inject_repository at ', + "ERROR: The repo name 'my_repo' cannot be defined by" + ' --inject_repository at as it is already defined by' + ' a use_repo() call at', stderr, ) + self.assertIn( + 'MODULE.bazel:2:9', + stderr, + ) + + def testInjectRepositoryRepoNameSideEffects(self): + self.ScratchFile( + 'MODULE.bazel', + [ + 'local_repository = use_repo_rule("@//:defs.bzl", "local_repository")', + f'local_repository(name = "repo")', + ], + ) + self.ScratchFile( + 'defs.bzl', + [ + 'def _impl(ctx):', + ' ctx.file("BUILD")', + # Deliberate collision with `@bazel_tools//tools/build_defs/repo:local.bzl%local_repository` + 'local_repository = repository_rule(implementation=_impl)', + ], + ) + self.ScratchFile('BUILD') + + self.ScratchFile('other_module/REPO.bazel') + self.ScratchFile('other_repo/BUILD', ['filegroup(name="target")']) + + _, stdout, _ = self.RunBazel(['mod', 'dump_repo_mapping', '']) + self.assertIn( + '"+local_repository+repo"', + "\n".join(stdout), + ) + + # --inject_repository _must not_ affect `use_repo_rule` generated repo names. + _, stdout, _ = self.RunBazel([ + 'mod', + 'dump_repo_mapping', + '', + '--inject_repository=injected_repo=%workspace%/other_repo', + ]) + self.assertIn( + '"+local_repository+repo"', + "\n".join(stdout), + ) + self.assertIn( + '"+local_repository2+injected_repo"', + "\n".join(stdout), + ) def testOverrideRepositoryOnNonExistentRepo(self): self.ScratchFile('other_repo/REPO.bazel')