bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Dependency on legacy @@rules_cc even with bzlmod enabled

Open jacky8hyf opened this issue 1 year ago • 4 comments

Description of the bug:

It appears that there is an implicit dependency to the legacy @@rules_cc (note: not the bzlmod version @@rules_cc~<version>) even with bzlmod enabled. This can be mitigated by --noenable_workspace or adding an empty WORKSPACE file.

Which category does this issue belong to?

External Dependency

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Extract the following tarball:

example.tar.gz

Or create these files manually:

# MODULE.bazel
local_path_override(
    module_name = "apple_support",
    path = "external/bazelbuild-apple_support",
)

local_path_override(
    module_name = "bazel_features",
    path = "external/bazelbuild-bazel_features",
)

local_path_override(
    module_name = "bazel_skylib",
    path = "external/bazel-skylib",
)

local_path_override(
    module_name = "platforms",
    path = "external/bazelbuild-platforms",
)

local_path_override(
    module_name = "rules_cc",
    path = "external/bazelbuild-rules_cc",
)

local_path_override(
    module_name = "rules_java",
    path = "external/bazelbuild-rules_java",
)

local_path_override(
    module_name = "rules_license",
    path = "external/bazelbuild-rules_license",
)

local_path_override(
    module_name = "rules_pkg",
    path = "external/bazelbuild-rules_pkg",
)

local_path_override(
    module_name = "rules_python",
    path = "external/bazelbuild-rules_python",
)
# init.sh
wget https://github.com/bazelbuild/apple_support/releases/download/1.15.1/apple_support.1.15.1.tar.gz && mkdir -p external/bazelbuild-apple_support/ && tar xf apple_support.1.15.1.tar.gz -C external/bazelbuild-apple_support/
wget https://github.com/bazelbuild/bazel-skylib/releases/download/1.6.1/bazel-skylib-1.6.1.tar.gz && mkdir -p external/bazel-skylib/ && tar xf bazel-skylib-1.6.1.tar.gz -C external/bazel-skylib/
wget https://github.com/bazelbuild/platforms/releases/download/0.0.10/platforms-0.0.10.tar.gz && mkdir -p external/bazelbuild-platforms/ && tar xf platforms-0.0.10.tar.gz -C external/bazelbuild-platforms/
wget https://github.com/bazelbuild/rules_cc/releases/download/0.0.10-rc1/rules_cc-0.0.10-rc1.tar.gz && mkdir -p external/bazelbuild-rules_cc/ && tar xf rules_cc-0.0.10-rc1.tar.gz -C external/bazelbuild-rules_cc/
wget https://github.com/bazelbuild/rules_java/releases/download/7.6.1/rules_java-7.6.1.tar.gz && mkdir -p external/bazelbuild-rules_java/ && tar xf rules_java-7.6.1.tar.gz -C external/bazelbuild-rules_java/
wget https://github.com/bazelbuild/rules_license/releases/download/0.0.8/rules_license-0.0.8.tar.gz && mkdir -p external/bazelbuild-rules_license/ && tar xf rules_license-0.0.8.tar.gz -C external/bazelbuild-rules_license/
wget https://github.com/bazelbuild/rules_pkg/releases/download/0.10.1/rules_pkg-0.10.1.tar.gz && mkdir -p external/bazelbuild-rules_pkg/ && tar xf rules_pkg-0.10.1.tar.gz -C external/bazelbuild-rules_pkg/
wget https://github.com/bazelbuild/rules_python/releases/download/0.32.2/rules_python-0.32.2.tar.gz && tar xf rules_python-0.32.2.tar.gz -C external/ rules_python-0.32.2 && mv external/rules_python-0.32.2 external/bazelbuild-rules_python
wget https://github.com/bazel-contrib/bazel_features/releases/download/v1.11.0/bazel_features-v1.11.0.tar.gz && tar xf bazel_features-v1.11.0.tar.gz -C external/ && mv external/bazel_features-1.11.0 external/bazelbuild-bazel_features
# .bazelrc
common:no_internet --repository_cache=
common:no_internet --distdir=
common:no_internet --experimental_repository_disable_download
common --config=no_internet
# BUILD
cc_binary(
    name = "x",
    srcs = [],
)

Then run the following:

# Fetch external dependencies
/bin/sh init.sh

bazel build //:x

Expected output

The build should fail at the CppLink action, e.g.

ERROR: /mnt/sdc/android/tmpdir/BUILD:1:10: Linking x failed: (Exit 1): gcc failed: error executing CppLink command (from target //:x) /usr/bin/gcc @bazel-out/k8-fastbuild/bin/x-2.params

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
/usr/lib/gcc/x86_64-linux-gnu/13/../../../x86_64-linux-gnu/Scrt1.o:function _start:(.text+0x17): error: undefined reference to 'main'

because cc_binary.srcs is empty. If I add an actual source file with a main function it should build properly.

Actual output

The build fails early with this message:

$ bazel build //:x -k
INFO: Repository rules_cc instantiated at:
  /DEFAULT.WORKSPACE.SUFFIX:50:6: in <toplevel>
  /home/elsk/.cache/bazel/_bazel_elsk/84eca1b8431f69ca7f27198b88cc3819/external/bazel_tools/tools/build_defs/repo/utils.bzl:240:18: in maybe
Repository rule http_archive defined at:
  /home/elsk/.cache/bazel/_bazel_elsk/84eca1b8431f69ca7f27198b88cc3819/external/bazel_tools/tools/build_defs/repo/http.bzl:369:31: in <toplevel>
ERROR: An error occurred during the fetch of repository 'rules_cc':
   Traceback (most recent call last):
	File "/home/elsk/.cache/bazel/_bazel_elsk/84eca1b8431f69ca7f27198b88cc3819/external/bazel_tools/tools/build_defs/repo/http.bzl", line 133, column 45, in _http_archive_impl
		download_info = ctx.download_and_extract(
Error in download_and_extract: java.io.IOException: Failed to download repository @@rules_cc: download is disabled.
ERROR: /DEFAULT.WORKSPACE.SUFFIX:50:6: fetching http_archive rule //external:rules_cc: Traceback (most recent call last):
	File "/home/elsk/.cache/bazel/_bazel_elsk/84eca1b8431f69ca7f27198b88cc3819/external/bazel_tools/tools/build_defs/repo/http.bzl", line 133, column 45, in _http_archive_impl
		download_info = ctx.download_and_extract(
Error in download_and_extract: java.io.IOException: Failed to download repository @@rules_cc: download is disabled.
ERROR: /mnt/sdc/android/tmpdir/BUILD:1:10: While resolving toolchains for target //:x (312a038): invalid registered toolchain '//toolchains:all': while parsing '//toolchains:all': no such package '@@rules_cc//cc': java.io.IOException: Failed to download repository @@rules_cc: download is disabled.
INFO: Analyzed target //:x (67 packages loaded, 6 targets configured).
WARNING: errors encountered while analyzing target '//:x', it will not be built.
INFO: Found 0 targets...
ERROR: command succeeded, but not all targets were analyzed
INFO: Elapsed time: 0.356s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully

Workarounds

Adding an empty WORKSPACE.bzlmod file at the root of the repository fixes the issue (shows "expected output").

Adding bazel_dep(name = "rules_cc") to MODULE.bazel fixes the issue (shows "expected output").

Adding --noenable_workspace also fixes the issue (shows "expected output").

Analysis

Note that the error specifies a dependency on @@rules_cc, not @@rules_cc~, indicating that there's a legacy non-bzlmod dependency on the non-bzlmod rules_cc. In fact, if I drop --experimental_repository_disable_download in .bazelrc and build, I can see under <output_base>/external that there are a rules_cc and a rules_cc~ directory.

I suspect that @@rules_cc dependency might come from some auto-generated WORKSPACE file when there are no WORKSPACE file at the root module.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 7.1.2 / release 7.2.0rc1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

jacky8hyf avatar May 29 '24 05:05 jacky8hyf

I would say this is working as intended: In WORKSPACE mode, Bazel has always provided default definitions for certain repos, including rules_cc. This logic is still active for backwards compatibility unless you opt out by creating WORKSPACE.bzlmod or via --noenable_workspace.

Would you have liked better docs for this?

fmeum avatar May 29 '24 07:05 fmeum

I think the main UI issue is that the implicit dependency is not clear.

In my original PoC, we can say that the user should add bazel_dep(name = "rules_cc") to its MODULE.bazel because they are using cc_binary. Though it comes from the native module, it is arguably replacable by the cc_binary from rules_cc.

Let me provide another example when this becomes more confusing to the user. Let's say I am the author of a Bazel module named mymodule. Here's its contents:

# mymodule/BUILD
# empty file
# mymodule/cc.bzl
load("@rules_cc//cc:defs.bzl", "cc_binary")

def my_binary(**kwargs):
    cc_binary(**kwargs)
# mymodule/MODULE.bazel
module(name = "mymodule")
bazel_dep(name = "rules_cc")

In short, mymodule defines a rule called my_binary that relies on rules_cc.

However, the user of mymodule must be advised that they need an empty WORKSPACE.bzlmod file or --noenable_workspace. In the original example, let's say the user adds:

# MODULE.bazel
bazel_dep(name = "mymodule")
local_path_override(
    module_name = "mymodule",
    path = "mymodule"
)
# BUILD
load("@mymodule//:cc.bzl", "my_binary")
my_binary(
    name = "x",
    srcs = [],
)

The user of mymodule would expect that the bazel_dep(name = "mymodule") is self-contained. They should not need to know that my_binary is backed by rules_cc or any CC toolchains. It is an implementation detail of mymodule. Therefore, they should not need to know that they need to add the empty WORKSPACE.bzlmod file (or --noenable_workspace, omitted below). But that's not true. And as the author of mymodule, I'll also need to tell my users about adding an empty WORKSPACE.bzlmod file as an extra step.

So, if you add docs like "If you use rules_cc or compile any CC stuff or use the CC toolchain for whatever reason, you need to add an empty WORKSPACE[.bzlmod] file", then the user might think "I am only using mymodule not rules_cc, so I should be fine", and ignore these docs. The author of mymodule (I) will need to ask the user to take this extra step.


In WORKSPACE mode, Bazel has always provided default definitions for certain repos, including rules_cc.

I am not sure what "WORKSPACE mode" means here. By default (--enable_bzlmod and --enable_workspace are both set), I am in a hybrid Bzlmod+WORKSPACE mode. So, if the root module depends on Bzlmod's @@rules_cc~ (which appears to be always the case anyways because it is a built-in dependency, shown with bazel mod graph --include_builtin), shouldn't that override the WORKSPACE's default definition of legacy @@rules_cc?

In other words, in pure WORKSPACE mode, I put local_repository to vendor rules_cc so it does not reach out to the Internet. After I migrate to bzlmod with local_path_override(module_name = "rules_cc", ...), it would seem natrual to me to delete the local_repository and the remaining empty WORKSPACE file. But it seems that I can't do that yet without --noenable_workspace.

jacky8hyf avatar May 29 '24 17:05 jacky8hyf

OK, this is actually wild. Thanks for providing such a detailed report, by the way -- I really appreciate it!

The reason that anyone is depending on @@rules_cc (instead of @@rules_cc~) is actually very, very hidden. It's not because the user used cc_binary or anything. It's actually because one of the WORKSPACE prefixes also declares @@rules_java_builtin, whose //toolchains package (BUILD file) contains this line:

load("@rules_cc//cc:defs.bzl", "cc_library")

Because @@rules_java_builtin is itself a WORKSPACE repo, its repo mapping is calculated from the root module's mappings. Because the root module doesn't have a dependency on rules_cc, @@rules_java_builtin resolves @rules_cc to mean @@rules_cc. (This is why adding a bazel_dep on rules_cc in your root module fixes your issue -- it would resolve to @@rules_cc~ instead.)

Following the chain, the @@rules_java_builtin//toolchains package is loaded by the WORKSPACE macro @@rules_java_builtin//java:repositories.bzl%rules_java_toolchains, which calls register_toolchains on @@rules_java_builtin//toolchains:all (link). This WORKSPACE macro is in turn called in a WORKSPACE suffix (link).

And toolchain registration is triggered whenever anything that requires any toolchain is built, because we don't know the type of a registered toolchain until we actually load it.


OK, so what does this mean?

It means that @@rules_cc will always be fetched as long as --enable_workspace is true AND there is no WORKSPACE.bzlmod file at the workspace root. (The WORKSPACE.bzlmod file disables all WORKSPACE prefixes and suffixes.) It doesn't actually matter if anyone's building any C++ code at all. This also (hopefully) resolves your questions around the mymodule example.

And can we fix this? It's a solid maybe. Right now, WORKSPACE repos have a repo mapping computed based on the root module's mappings. So if the root module sees @rules_cc as @@rules_cc~, then all WORKSPACE repos will do so too (certain details ignored here). In this case, the root module doesn't see @rules_cc, so @rules_cc is simply mapped to @@rules_cc.

We used to compute WORKSPACE repo mappings based on what's in the transitive module dependency graph. That is, as long as the module rules_cc exists in the final transitive dep graph (and it always does, as you observed, because it's a dependency of bazel_tools), WORKSPACE repos will see @rules_cc as @@rules_cc~. This was changed to the current approach in https://github.com/bazelbuild/bazel/commit/dfdf63a398ef3909364d31d00d1860bf42d6151f, because it's less surprising (users have no real visibility into their transitive dep graph), and is more amenable to on-the-spot fixes (something's wrong? ok I'll change my root module).

So I don't think we should roll back https://github.com/bazelbuild/bazel/commit/dfdf63a398ef3909364d31d00d1860bf42d6151f, but it's not unthinkable to also do the "if in transitive dep graph then X" thing. But given the higher complexity of that solution and the relatively low impact of this issue, I'm leaning towards leaving this as is.

Wyverald avatar May 30 '24 00:05 Wyverald

Thanks for the detailed analysis; you have explained all the confusion I have very clearly!

I am fine with leaving this as-is given that WORKSPACE is supposed to be the legacy thing and going away. I'll pick one of the workaround I've written above.

jacky8hyf avatar May 31 '24 00:05 jacky8hyf