Patching MODULE.bazel has no effect on module extension evaluation

[philwo]

Description of the bug:

@meteorcloudy @Wyverald I noticed this today while trying to upgrade remote-apis and remote-apis-sdks to Bazel 9.0.0 and the latest gRPC / protobuf dependencies. 😊 I think I can find a workaround for my particular case, but it would be really nice if this approach worked. Maybe I'm also doing something wrong, not sure - I included a minimum repro example below!

Bazel apparently doesn't apply patches from a single_version_override prior to evaluating the MODULE.bazel file downloaded from BCR. I believe that this should work, as it's also documented: "If a patch makes changes to the MODULE.bazel file, these changes will only be effective if the patch file is provided by the root module." (https://bazel.build/rules/lib/globals/module#parameters_12)

This means that if there's a bug affecting a BCR module, like currently in gRPC 1.78.0 where the MODULE.bazel passes an invalid Python version to python.toolchain (grpc/grpc#41438), I cannot patch that out locally like this:

$ touch BUILD.bazel
$ cat > MODULE.bazel <<'EOF'
module(
    name = "cant_patch_this",
    version = "",
)

bazel_dep(name = "grpc", version = "1.78.0")

# grpc pulls in a version of rules_foreign_cc that is incompatible with Bazel 9.0.0, so let's
# override it to a newer one.
single_version_override(
    module_name = "rules_foreign_cc",
    version = "0.15.1",
)

# grpc 1.78.0 has a bug in its MODULE.bazel file, where it passes the invalid Python version
# "3.14.0b2" to rules_python's python.toolchain module extension. This causes it to crash with
# the error "Error: key "3.14.0b2" not found in dictionary".
#
# Let's just patch that invalid Python version out of the MODULE.bazel file.
single_version_override(
    module_name = "grpc",
    patches = [
        "//:grpc_1_78_0_remove_invalid_python_version.patch",
    ],
)
EOF

$ cat > grpc_1_78_0_remove_invalid_python_version.patch <<'EOF'
diff --git a/MODULE.bazel b/MODULE.bazel
index 2a1c5aec1e..a2573a25c6 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -68,7 +68,6 @@ PYTHON_VERSIONS = [
     "3.11",
     "3.12",
     "3.13",
-    "3.14.0b2",
 ]

 python = use_extension("@rules_python//python/extensions:python.bzl", "python")
EOF

The patch has no effect, and Bazel still prints the error about the invalid Python version:

$ bazel mod show_repo grpc
Starting local Bazel server (9.0.0) and connecting to it...
ERROR: /home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/rules_python+/python/private/python_register_toolchains.bzl:106:31: Traceback (most recent call last):
        File "/home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/rules_python+/python/private/python.bzl", line 281, column 53, in _python_impl
                register_result = python_register_toolchains(
        File "/home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/rules_python+/python/private/python_register_toolchains.bzl", line 106, column 31, in python_register_toolchains
                sha256 = tool_versions[python_version]["sha256"].get(platform, None)
Error: key "3.14.0b2" not found in dictionary
ERROR: error evaluating module extension @@rules_python+//python/extensions:python.bzl%python
## grpc@1.78.0:
load("@@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
  name = "grpc+",
  urls = ["https://github.com/grpc/grpc/archive/refs/tags/v1.78.0.tar.gz"],
  integrity = "sha256-4qznkKXy0PgyWdE5CoFqM7AT6jTfLoYITZJ+WNqkxdk=",
  strip_prefix = "grpc-1.78.0",
  remote_patches = {"https://bcr.bazel.build/modules/grpc/1.78.0/patches/adopt_bzlmod.patch": "sha256-d7z79/WSEiNTZiQ/A50HMbyqbZj3qIESa4H2n/yhlhA=", "https://bcr.bazel.build/modules/grpc/1.78.0/patches/fix_googleapis_switched_rules.patch": "sha256-WTFS3rp23Cj/jlOWbLnu01AtTiun02+3MTXVAzDxpeA="},
  remote_file_urls = {},
  remote_file_integrity = {},
  remote_module_file_urls = ["https://bcr.bazel.build/modules/grpc/1.78.0/MODULE.bazel"],
  remote_module_file_integrity = "sha256-anLwsv6VA0L+Y7Ixx2GX320Reo6hrDONrECwUwIPWGo=",
  remote_patch_strip = 1,
  patches = [Label("//:grpc_1_78_0_remove_invalid_python_version.patch")],
  patch_cmds = [],
  patch_args = ["-p0"],
)

ERROR: Results may be incomplete as 2 extensions failed.
    Fetching module extension @@rules_python+//python/extensions:python.bzl%python; starting

By the way, it was very difficult to understand where this mysterious "3.14.0b2" string comes from, because Bazel doesn't save anything to disk when this evaluation fails, which means that there's absolutely nothing to be found in my output_base. The only place where you can find something is in the CAS of the repo cache.

$ fgrep -r "3.14.0b2" "$(bazel info output_base)"
<nothing>

$ fgrep -r "3.14.0b2" ~/.cache/bazel
/home/philwo/.cache/bazel/_bazel_philwo/cache/repos/v1/content_addressable/sha256/6a72f0b2fe950342fe63b231c76197df6d117a8ea1ac338dac40b053020f586a/file:    "3.14.0b2",

In the end, I used a debugger and set a breakpoint in the Starlark interpreter to find out where it comes from, where it was clearly visible. It would probably be nice to print this location in the error message.

Which category does this issue belong to?

No response

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

See above.

Which operating system are you running Bazel on?

I use Arch, btw

What is the output of bazel info release?

release 9.0.0 (reproducible with latest main branch, too)

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 ?

git@github.com:philwo/herb.git
32eef3485d66f448df4b66fe4f0d530e1a88f99b

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

No response

Have you found anything relevant by searching the web?

https://stackoverflow.com/questions/79018238/single-version-override-with-patch-has-no-effect

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

No response

[fmeum]

Does this work if you set version on the single_version_override with the patches?

single_version_override with patches but without version is a footgun since it attempts to apply the patches to all the different versions, not just the one you specified via bazel_dep.

@meteorcloudy @Wyverald What do you think, should we make version a required argument of single_version_override unless exactly registry is set? This would be a very simple migration for root modules and wouldn't affect registry modules where single_version_override is ignored anyway. I doubt this would need an incompatible flag for Bazel 10.

[fmeum]

Re the error message: Since the error is encountered during Starlark evaluation of a module extension, I don't think Bazel has a chance to report where the input string comes from (it does report the paths to the relevant .bzl files). rules_python would be in a position to emit a better error, pointing to the particular module that provided the invalid version.

[philwo]

Does this work if you set version on the single_version_override with the patches?

Thanks for the suggestion! No, that doesn't change anything.

single_version_override with patches but without version is a footgun since it attempts to apply the patches to all the different versions, not just the one you specified via bazel_dep.

As a user, that sounds like unexpected behavior to me. Why would it patch something except the one single version that I want to use and specify via bazel_dep? Of course, if I eventually no longer need the patch, for example when I bump dependencies again and it no longer applies cleanly, then I'd just remove the whole single_version_override statement from the MODULE.bazel.

I would find that to be easier to work with than to always have to keep two version numbers in sync (the one for the bazel_dep and the one for the single_version_override).

I don't think Bazel has a chance to report where the input string comes from

I think it definitely knows. See the data in my screenshot of the debugger. It might be tricky to get that data once we're in the code that prints the error, but at least it's there :)

(it does report the paths to the relevant .bzl files)

The *.bzl files printed are completely non-actionable for a user and confusing, because I don't call these *.bzl files anywhere in my whole project. The culprit is the MODULE.bazel file of a dependency (grpc), which is completely invisible from the error message and the state on the disk. Fixing this issue as a user would require me to know which dependency is causing the error in the first place, so that I could then file a bug with the maintainer of that, or fix it myself.

[philwo]

(it does report the paths to the relevant .bzl files)

Ah, maybe my example is confusing here, because I used the bazel mod show_repo command to trigger it. If you hit this error with a normal bazel build, it doesn't print anything about the grpc dependency.

Example with absolutely no "grpc" anywhere in the output:

$ bazel mod path
ERROR: /home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/rules_python+/python/private/python_register_toolchains.bzl:106:31: Traceback (most recent call last):
        File "/home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/rules_python+/python/private/python.bzl", line 281, column 53, in _python_impl
                register_result = python_register_toolchains(
        File "/home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/rules_python+/python/private/python_register_toolchains.bzl", line 106, column 31, in python_register_toolchains
                sha256 = tool_versions[python_version]["sha256"].get(platform, None)
Error: key "3.14.0b2" not found in dictionary
ERROR: error evaluating module extension @@rules_python+//python/extensions:python.bzl%python
No target modules specified.
ERROR: Results may be incomplete as 2 extensions failed.

[fmeum]

The patch doesn't apply because it's a git-style patch with a/b prefixes, so you need to set patch_strip = 1. Perhaps we should warn or error out if such a patch is applied with default patch_strip?

When I set patch_strip = 1, I then see the "expected" errors as the patch only applies to one particular version of grpc.

I think it definitely knows. See the data in my screenshot of the debugger. It might be tricky to get that data once we're in the code that prints the error, but at least it's there :)

The data is there and it would be very easy to report the context if rules_python did check that the version is valid and/or handled a missing version. It would be next to impossible for Bazel to accurately determine the tag that was responsible for this particular issue. That's why I would go with improved error handling in rules_python as the simpler alternative.

[philwo]

@fmeum Oh wow, yes. Setting patch_strip = 1 makes it work. Thank you!

I think Bazel should definitely print a user-friendly error when it cannot apply a patch. It looks to me like it usually does print a good message, which immediately shows what's wrong. But somehow that error about the patch not applying gets swallowed in this case and instead the evaluation error from Starlark gets shown.

For example trying to apply the same patch to grpc 1.76.0.bcr.1 without specifying patch_strip gives a good error: "Cannot find file to patch (near line 3), old file name (a/MODULE.bazel) doesn't exist, new file name (b/MODULE.bazel) doesn't exist"

$ cat > MODULE.bazel <<'EOF'
module(
    name = "cant_patch_this",
    version = "",
)

bazel_dep(name = "grpc", version = "1.76.0.bcr.1")

single_version_override(
    module_name = "grpc",
    patches = [
        "//:grpc_1_78_0_remove_invalid_python_version.patch",
    ],
    version = "1.76.0.bcr.1",
)
EOF

$ bazel clean --expunge; bazel mod show_repo grpc
ERROR: /home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/bazel_tools/tools/build_defs/repo/utils.bzl:251:22: An error occurred during the fetch of repository 'grpc+':
   Traceback (most recent call last):
        File "/home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/bazel_tools/tools/build_defs/repo/http.bzl", line 213, column 32, in _http_archive_impl
                remote_patches_info = patch(ctx)
        File "/home/philwo/.cache/bazel/_bazel_philwo/352ff19a566314e79a838237cef373aa/external/bazel_tools/tools/build_defs/repo/utils.bzl", line 251, column 22, in patch
                ctx.patch(patchfile, strip)
Error in patch: Error applying patch /home/philwo/src/repro/grpc_1_78_0_remove_invalid_python_version.patch: Cannot find file to patch (near line 3), old file name (a/MODULE.bazel) doesn't exist, new file name (b/MODULE.bazel) doesn't exist.
ERROR: no such package '@@grpc+//': Error applying patch /home/philwo/src/repro/grpc_1_78_0_remove_invalid_python_version.patch: Cannot find file to patch (near line 3), old file name (a/MODULE.bazel) doesn't exist, new file name (b/MODULE.bazel) doesn't exist.
    Fetching module extension @@rules_python+//python/extensions:pip.bzl%pip; starting