Make bazel lock file cross-platform

[cameron-martin]

Since the host tools are os and arch specific, previously bazel would cache the resolution of these in the lock file, causing the repo for the wrong OS or arch to be used when moving between machines.

Since bazel 7.1.0, module extensions can be marked as reproducible to exclude these from the lock file. However, the rust module extension handles fetching of all the other toolchains as well as the host tools, and we don't really want to exclude those too. Therefore the host tools are moved to their own module extension. This means we can no longer match the host toolchain's version, edition, etc with the toolchain registered via rust.toolchain by default, and instead default to a fixed version. This can still be overridden separately in the root module. I think this is okay, because the host tools are only used for bootstrapping and I don't think there's much need to have them match.

This is tested by now checking in the MODULE.bazel.lock file of the bzlmod example, and running the bzlmod examples on multiple platforms with --lockfile_mode set to error.

Resolves #2452
Resolves #2549

[illicitonion]

Thanks for looking into this!

I think this still leads to per-platform lockfile churn - I checked our your PR and did a build on macOS and ended up with this diff:

Click to expand

b/examples/bzlmod/hello_world/MODULE.bazel.lock
index f6d55bcc..b0cd6cbf 100644
--- a/examples/bzlmod/hello_world/MODULE.bazel.lock
+++ b/examples/bzlmod/hello_world/MODULE.bazel.lock
@@ -1911,9 +1911,9 @@
         "bzlTransitiveDigest": "zLZB4ZjhD3DR2JdzkUVxFohElI05/9Q34vg+hpP9PHg=",
         "accumulatedFileDigests": {
           "@@//:anyhow.BUILD.bazel": "f856a5a378cf3c3ed1e749d9860fbfd79188fb72a7753d590d86ffbe62cc773a",
-          "@@rules_rust~override~rust_host_tools~rust_host_tools//:bin/rustc": "7cd1c64771117a00efd8eb5113e2aed512545441c23436f6923e5deb8c97016c",
-          "@@rules_rust~override~rust_host_tools~rust_host_tools//:bin/cargo": "b30a9fc475de8b03077f5947e0a20ea59af47733724af215cabd0a72e3722446",
-          "@@rules_rust~override~cargo_bazel_bootstrap~cargo_bazel_bootstrap//:cargo-bazel": "cfc88fee81b5189ded0dbd8c0df64ebbd55f983622257981cb97aefb3d8d0637",
+          "@@rules_rust~override~rust_host_tools~rust_host_tools//:bin/rustc": "2e5d8100af1c46dc9b9f2f8b644f085b7c099dce9f4237ecf304adc3e110c294",
+          "@@rules_rust~override~rust_host_tools~rust_host_tools//:bin/cargo": "d92fdab3cc38e1952e00b04d6cf4c725a08fc8519399a3ba76b93932e4985803",
+          "@@rules_rust~override~cargo_bazel_bootstrap~cargo_bazel_bootstrap//:cargo-bazel": "0025ab0604b2b3a261ef50a4aaa3667c634c8270bd3a880ea690430cd34a969d",
           "@@//third-party:Cargo.lock": "2e5a5bf001781b3e3fc7b89c42ee2a6bf1a91e4b6afa0d0b6b124b3fe4c850e6",
           "@@//third-party:Cargo.toml": "5a6a67869179924666139ca5ff6d7ec135ca0c729ec6fbb18c04e198c68a9a42"
         },
@@ -2066,7 +2066,7 @@
     },
     "@@rules_rust~override//rust:extensions.bzl%rust": {
       "general": {
-        "bzlTransitiveDigest": "xwnna/sOxCeNCnIXLvTgMmIIra9QIqStBhfyUPBRJes=",
+        "bzlTransitiveDigest": "CZR8o59wGKoAL+DRAzZT6Rw1lenon2iMorz7oeDHJqM=",
         "accumulatedFileDigests": {},
         "envVariables": {},
         "generatedRepoSpecs": {
@@ -4852,6 +4852,40 @@
             "rules_rust~override"
           ]
         ]
+      },
+      "os:osx,arch:aarch64": {
+        "bzlTransitiveDigest": "CZR8o59wGKoAL+DRAzZT6Rw1lenon2iMorz7oeDHJqM=",
+        "accumulatedFileDigests": {},
+        "envVariables": {},
+        "generatedRepoSpecs": {
+          "rust_host_tools": {
+            "bzlFile": "@@rules_rust~override//rust:repositories.bzl",
+            "ruleClassName": "rust_toolchain_tools_repository",
+            "attributes": {
+              "name": "rules_rust~override~rust_host_tools~rust_host_tools",
+              "exec_triple": "aarch64-apple-darwin",
+              "target_triple": "aarch64-apple-darwin",
+              "version": "1.75.0"
+            }
+          }
+        },
+        "recordedRepoMappingEntries": [
+          [
+            "rules_rust~override",
+            "bazel_skylib",
+            "bazel_skylib~1.5.0"
+          ],
+          [
+            "rules_rust~override",
+            "bazel_tools",
+            "bazel_tools"
+          ],
+          [
+            "rules_rust~override",
+            "rules_rust",
+            "rules_rust~override"
+          ]
+        ]
       }
     },
     "@@rules_rust~override//rust/private:extensions.bzl%internal_deps": {

See also this diiscussion on slack where the conclusion was to ignore the lockfile until bazelbuild/bazel#20272 (comment) is solved

I'm open to other solutions, but would pretty strongly prefer if doing a build on a different platform than the last person doesn't change files in the tree...

[illicitonion]

These still look platform-specific to me?

[cameron-martin]

I'm now running the bzlmod examples with --lockfile_mode=error, so I guess these aren't an issue any more?

[illicitonion]

These look platform-specific too?

[cameron-martin]

Yeah that is unfortunate. I do wonder, are the changes to the lockfile only additive? As in, if we were to run this on all OSes and archs, and commit the changes after each, would this result in the lockfile no longer being churned? I would hope so.

See also this diiscussion on slack where the conclusion was to ignore the lockfile until bazelbuild/bazel#20272 (comment) is solved

Interesting thread, thanks for the link. The problem is, this doesn't just affect rules_rust itself. It means any repository that uses rules_rust can't have a lock file if they're doing multi-platform builds.

Maybe a compromise is to have this fix, but not check in the lock file in the rules_rust examples, and instead create a different test for it?

[cameron-martin]

I guess this'll be fixed in 7.1.0, which comes out in a few weeks, so happy to wait until then and see what the situation is.

We may still have to split the host tools out, depending on which things we want to be in the checked-in lock file, so I'll leave this PR in draft.

[yesudeep]

Does 7.1.0rc1 fix this?

[cameron-martin]

Yes, this should allow us to fix this: bazelbuild/bazel#21306

[ericmcbride]

Is this PR ready to be reviewed since 7.1 release of bazel?

[cameron-martin]

There's a failing test that I need to debug. After I've done that it'll be ready.

[UebelAndre]

There's a failing test that I need to debug. After I've done that it'll be ready.

I think a rebase will fix the issue you have.

[cameron-martin]

I guess this is mostly covered by #2575, although we may want to still split out the host tools for when the rest is not reproducible.

[ericmcbride]

Looks like #2575 doesnt fully resolve the issue. The hosts are still locking the wrong exec

[cameron-martin]

Yeah that changes a totally different module extension, I misread that PR. I'll update this PR and hopefully get it in a working state.

[cameron-martin]

@illicitonion @UebelAndre This should be ready to review again now.

[illicitonion]

Amazing, thanks!

[UebelAndre]

@cameron-martin I think this introduced a regression for bzlmod on Bazel 6
bazelbuild/bazel-central-registry#1799

(14:49:14) INFO: Running command line: bazel-bin/external/rules_rust~override/tools/rust_analyzer/gen_rust_project
Error: bazel build failed:(exit status: 1)
Loading:
Loading:
Loading: 0 packages loaded
Analyzing: 9 targets (2 packages loaded, 0 targets configured)
ERROR: Traceback (most recent call last):
	File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/d3816408fbd2939714a82a2e37f9202c/external/rules_rust~override/rust/extensions.bzl", line 173, column 41, in _rust_host_tools_impl
		return module_ctx.extension_metadata(reproducible = True)
Error in extension_metadata: extension_metadata() got unexpected keyword argument 'reproducible'
ERROR: Analysis of target '//:hello_world_transient' failed; build aborted: error evaluating module extension rust_host_tools in @rules_rust~override//rust:extensions.bzl
INFO: Elapsed time: 1.110s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (12 packages loaded, 246 targets configured)
bazel run failed with exit code 1

Is that your read of this as well?

[cameron-martin]

Right, I guess we need to do some feature checking. I can submit a PR, but realistically the earliest I can do it is Friday (sorry).

[UebelAndre]

Right, I guess we need to do some feature checking. I can submit a PR, but realistically the earliest I can do it is Friday (sorry).

No need to apologize! I don't really understand the error so am happy to get confirmation I've found the root cause. I would assert the fault lies on maintainers (like myself) for not having sufficient regression testing to catch this issue pre-merge. If you have bandwidth on Friday to fix this then that certainly helps us out!

[cameron-martin]

I put up a fix for this in #2612.