Skip to content

Commit

Permalink
[7.0.0] Can't load from @bazel_tools//tools/jdk .bzl files from m…
Browse files Browse the repository at this point in the history
…odule extension (#20018)

Since Bazel now forwards many `.bzl` symbols under
`@bazel_tools//tools/jdk` to `@rules_java`, `@bazel_tools` needs to use
its proper repo mapping when resolving `.bzl` loads, otherwise loading
these symbols will fail from `.bzl` files loaded transitively during
module extension evaluation.

The only exception this is when loading a repo rule that can host a
Bazel module, recognized by the package
`@bazel_tools//tools/build_defs/repo`. The full repo mapping of
`@bazel_tools` isn't available at this point, so we use a fixed mapping
that only contains `bazel_tools` itself.

Fixes #20000

Closes #20006.

Commit
5f92b9d

PiperOrigin-RevId: 578612514
Change-Id: Icd50098f5ff8b4bd3ea21574a7c1d3fa4ed8611e

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
bazel-io and fmeum authored Nov 1, 2023
1 parent 1ccd9cc commit 843ac00
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -622,16 +622,18 @@ private static boolean requiresBuiltinsInjection(BzlLoadValue.Key key) {
|| (key instanceof BzlLoadValue.KeyForBzlmod && !isFileSafeForUninjectedEvaluation(key));
}

private static final PackageIdentifier BAZEL_TOOLS_BOOTSTRAP_RULES_PACKAGE =
PackageIdentifier.create(
RepositoryName.BAZEL_TOOLS, PathFragment.create("tools/build_defs/repo"));

private static boolean isFileSafeForUninjectedEvaluation(BzlLoadValue.Key key) {
// We don't inject _builtins for repo rules to avoid a Skyframe cycle.
// The cycle is caused only with bzlmod because the `@_builtins` repo does not declare its own
// module deps and requires `@bazel_tools` to re-use the latter's repo mapping. This triggers
// Bazel module resolution, and if there are any non-registry overrides in the root MODULE.bazel
// file (such as `git_override` or `archive_override`), the corresponding bzl files will be
// evaluated.
return PackageIdentifier.create(
RepositoryName.BAZEL_TOOLS, PathFragment.create("tools/build_defs/repo"))
.equals(key.getLabel().getPackageIdentifier());
return key.getLabel().getPackageIdentifier().equals(BAZEL_TOOLS_BOOTSTRAP_RULES_PACKAGE);
}

/**
Expand Down Expand Up @@ -944,12 +946,12 @@ private static RepositoryMapping getRepositoryMapping(
}

if (key instanceof BzlLoadValue.KeyForBzlmod) {
if (repoName.equals(RepositoryName.BAZEL_TOOLS)) {
// Special case: we're only here to get the @bazel_tools repo (for example, for
// http_archive). This repo shouldn't have visibility into anything else (during repo
// generation), so we just return an empty repo mapping.
// TODO(wyv): disallow fallback.
return RepositoryMapping.ALWAYS_FALLBACK;
if (key.getLabel().getPackageIdentifier().equals(BAZEL_TOOLS_BOOTSTRAP_RULES_PACKAGE)) {
// Special case: we're only here to get one of the rules in the @bazel_tools repo that
// load Bazel modules. At this point we can't load from any other modules and thus use a
// repository mapping that contains only @bazel_tools itself.
return RepositoryMapping.create(
ImmutableMap.of("bazel_tools", RepositoryName.BAZEL_TOOLS), RepositoryName.BAZEL_TOOLS);
}
if (repoName.isMain()) {
// Special case: when we try to run an extension in the main repo, we need to grab the repo
Expand Down
30 changes: 30 additions & 0 deletions src/test/py/bazel/bzlmod/bazel_module_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,36 @@ def testLocationNonRegistry(self):
_, _, stderr = self.RunBazel(['build', '@what'], allow_failure=True)
self.assertIn('ERROR: @@hello~override//:MODULE.bazel', '\n'.join(stderr))

def testLoadRulesJavaSymbolThroughBazelTools(self):
"""Tests that loads from @bazel_tools that delegate to other modules resolve."""
self.ScratchFile(
'MODULE.bazel',
[
'ext = use_extension("//:ext.bzl", "ext")',
'use_repo(ext, "data")',
],
)
self.ScratchFile('BUILD')
self.ScratchFile(
'ext.bzl',
[
(
"load('@bazel_tools//tools/jdk:toolchain_utils.bzl',"
" 'find_java_toolchain')"
),
'def _repo_impl(ctx):',
" ctx.file('WORKSPACE')",
" ctx.file('BUILD', 'exports_files([\"data.txt\"])')",
" ctx.file('data.txt', 'hi')",
'repo = repository_rule(implementation = _repo_impl)',
'def _ext_impl(ctx):',
" repo(name='data')",
'ext = module_extension(implementation = _ext_impl)',
],
)

self.RunBazel(['build', '@data//:data.txt'])


if __name__ == '__main__':
absltest.main()

0 comments on commit 843ac00

Please sign in to comment.