Make bazel lock file cross-platform #2453

cameron-martin · 2024-02-01T23:10:42Z

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

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. Bazel has the `os_dependent` and `arch_dependent` attributes in the module extension to "key" this resolution by OS and arch. 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 key these 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 overriden 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 the CI runners that test on multiple platforms will pass. Resolves bazelbuild#2452

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 · 2024-02-02T10:03:55Z

examples/bzlmod/hello_world/MODULE.bazel.lock

+          "@@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",


These still look platform-specific to me?

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

illicitonion · 2024-02-02T10:04:56Z

examples/bzlmod/hello_world/MODULE.bazel.lock

+                  "@platforms//cpu:x86_64",
+


These look platform-specific too?

cameron-martin · 2024-02-02T11:28:30Z

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 · 2024-02-02T12:33:21Z

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 · 2024-02-24T20:04:05Z

Does 7.1.0rc1 fix this?

cameron-martin · 2024-02-24T21:26:57Z

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

ericmcbride · 2024-03-26T12:40:13Z

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

cameron-martin · 2024-03-26T12:56:45Z

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

UebelAndre · 2024-03-26T13:49:12Z

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 · 2024-04-01T19:01:11Z

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 · 2024-04-04T20:15:14Z

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

cameron-martin · 2024-04-04T20:18:24Z

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 · 2024-04-04T20:39:34Z

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

illicitonion

Amazing, thanks!

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [rules_rust](https://togithub.com/bazelbuild/rules_rust) | http_archive | minor | `0.41.1` -> `0.42.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_rust (rules_rust)</summary> ### [`v0.42.0`](https://togithub.com/bazelbuild/rules_rust/releases/tag/0.42.0) [Compare Source](https://togithub.com/bazelbuild/rules_rust/compare/0.41.1...0.42.0) ### 0.42.0 ```python load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "rules_rust", integrity = "sha256-XT1YVJ6FHJTXBr1v3px2fV37/OCS3dQk3ul+XvfIIf8=", urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.42.0/rules_rust-v0.42.0.tar.gz"], ) ``` Additional documentation can be found at: https://bazelbuild.github.io/rules_rust/#setup #### What's Changed - Fix crates.io URL by [@ericmcbride](https://togithub.com/ericmcbride) in [https://github.com/bazelbuild/rules_rust/pull/2597](https://togithub.com/bazelbuild/rules_rust/pull/2597) - Add support for `--compile_one_dependency` by [@william-smith-skydio](https://togithub.com/william-smith-skydio) in [https://github.com/bazelbuild/rules_rust/pull/2598](https://togithub.com/bazelbuild/rules_rust/pull/2598) - Update rules_apple by [@sgowroji](https://togithub.com/sgowroji) in [https://github.com/bazelbuild/rules_rust/pull/2602](https://togithub.com/bazelbuild/rules_rust/pull/2602) - Support building more things with bzlmod by [@matts1](https://togithub.com/matts1) in [https://github.com/bazelbuild/rules_rust/pull/2601](https://togithub.com/bazelbuild/rules_rust/pull/2601) - Make bazel lock file cross-platform by [@cameron-martin](https://togithub.com/cameron-martin) in [https://github.com/bazelbuild/rules_rust/pull/2453](https://togithub.com/bazelbuild/rules_rust/pull/2453) - Added Rust 1.77.1 by [@UebelAndre](https://togithub.com/UebelAndre) in [https://github.com/bazelbuild/rules_rust/pull/2591](https://togithub.com/bazelbuild/rules_rust/pull/2591) - Fix (+) In vendored crates semver metadata by [@ericmcbride](https://togithub.com/ericmcbride) in [https://github.com/bazelbuild/rules_rust/pull/2579](https://togithub.com/bazelbuild/rules_rust/pull/2579) - Keep default_features parity from bzlmod to workspace by [@Lev1ty](https://togithub.com/Lev1ty) in [https://github.com/bazelbuild/rules_rust/pull/2606](https://togithub.com/bazelbuild/rules_rust/pull/2606) - clippy: use --cap-lints=warn; apply clippy_flags when capture_output=True by [@goffrie](https://togithub.com/goffrie) in [https://github.com/bazelbuild/rules_rust/pull/2451](https://togithub.com/bazelbuild/rules_rust/pull/2451) - Added Rust 1.77.2 by [@UebelAndre](https://togithub.com/UebelAndre) in [https://github.com/bazelbuild/rules_rust/pull/2608](https://togithub.com/bazelbuild/rules_rust/pull/2608) - Re-vendor crate_universe outputs by [@UebelAndre](https://togithub.com/UebelAndre) in [https://github.com/bazelbuild/rules_rust/pull/2609](https://togithub.com/bazelbuild/rules_rust/pull/2609) - Release 0.42.0 by [@UebelAndre](https://togithub.com/UebelAndre) in [https://github.com/bazelbuild/rules_rust/pull/2610](https://togithub.com/bazelbuild/rules_rust/pull/2610) #### New Contributors - [@Lev1ty](https://togithub.com/Lev1ty) made their first contribution in [https://github.com/bazelbuild/rules_rust/pull/2606](https://togithub.com/bazelbuild/rules_rust/pull/2606) **Full Changelog**: bazelbuild/rules_rust@0.41.1...0.42.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/bazel-contrib/toolchains_llvm).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

UebelAndre · 2024-04-10T17:34:47Z

@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 · 2024-04-10T21:27:04Z

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 · 2024-04-10T23:43:01Z

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 · 2024-04-11T11:17:06Z

I put up a fix for this in #2612.

The reproducible attribute was added in Bazel 7.1, so unconditional usage of this breaks earlier versions. This uses `@bazel_features` to detect whether we can safely use this. For more context see #2453 (comment).

@fmeum

This PR is follow up to #2453 and marks the `crate` extension as `os_dependent` and `arch_dependent`. For `crate.from_cargo`, the extension specifies reproducibility via the `reproducible` attribute of `extension_metadata` which makes Bazel skip the extension when writing the lockfile. But in the case of `crate.from_specs`, the current implementation marks the extension as non reproducible since the crates will not be backed by a lockfile. And because the `crate` module extension depends on `rust_host_tools` (which are os / arch dependent), the entry in the lockfile for the module extension includes the checksum of the `rustc` and `cargo` binaries for whatever host it was resolved for. This makes the bazel lock file platform dependent in that case. This PR will result in a new os/arch specific entry in the lockfile for users of `crate.from_specs`. And will have no impact for users of `crate.from_cargo`. Thanks to @fmeum for the guidance.

cameron-martin force-pushed the cross-platform-lock-file branch from 1fd0898 to 504e521 Compare February 1, 2024 23:12

illicitonion requested changes Feb 2, 2024

View reviewed changes

cameron-martin marked this pull request as draft February 2, 2024 12:33

cameron-martin added 5 commits March 2, 2024 16:14

Merge branch 'main' into cross-platform-lock-file

0e6fe84

Set reproducible flag on host tools module extension

0888c0f

Set lock file mode to error

1feb1cd

Merge branch 'main' into cross-platform-lock-file

acea9b5

Update comment

90cbbb1

cameron-martin marked this pull request as ready for review March 12, 2024 23:12

cameron-martin marked this pull request as draft March 12, 2024 23:12

Empty commit

430e9b4

cameron-martin marked this pull request as ready for review March 16, 2024 12:36

cameron-martin added 5 commits March 28, 2024 21:53

Merge branch 'main' into cross-platform-lock-file

6336779

Fix buildifier

6c1a1c9

Use test_flags instead of build_flags

5111abb

Update lock file

53f9124

Merge branch 'main' into cross-platform-lock-file

67b5de8

This was referenced Apr 3, 2024

Bzlmod lock file breaks cross-platform builds #2452

Closed

Linux cargo binary selected on Mac OS (Darwin) with bzlmod #2549

Closed

cameron-martin added 2 commits April 4, 2024 21:24

Merge branch 'main' into cross-platform-lock-file

b1f283f

Update bzlmod lock file

7835de9

cameron-martin requested a review from illicitonion April 4, 2024 20:37

illicitonion approved these changes Apr 5, 2024

View reviewed changes

illicitonion added this pull request to the merge queue Apr 5, 2024

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 5, 2024

illicitonion added this pull request to the merge queue Apr 5, 2024

Merged via the queue into bazelbuild:main with commit fd71efb Apr 5, 2024
3 checks passed

cameron-martin deleted the cross-platform-lock-file branch April 5, 2024 10:33

aaronmondal mentioned this pull request Apr 10, 2024

bazel: 7.0.2 -> 7.1.0 NixOS/nixpkgs#295615

Merged

13 tasks

cameron-martin mentioned this pull request Apr 11, 2024

Fix rust module extension for Bazel pre-7.1 #2612

Merged

cerisier mentioned this pull request Oct 15, 2024

Make crate module extension os and arch dependent #2939

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make bazel lock file cross-platform #2453

Make bazel lock file cross-platform #2453

cameron-martin commented Feb 1, 2024 •

edited

Loading

illicitonion left a comment

illicitonion Feb 2, 2024

cameron-martin Apr 4, 2024

illicitonion Feb 2, 2024

cameron-martin commented Feb 2, 2024 •

edited

Loading

cameron-martin commented Feb 2, 2024 •

edited

Loading

yesudeep commented Feb 24, 2024

cameron-martin commented Feb 24, 2024

ericmcbride commented Mar 26, 2024

cameron-martin commented Mar 26, 2024

UebelAndre commented Mar 26, 2024

cameron-martin commented Apr 1, 2024

ericmcbride commented Apr 4, 2024 •

edited

Loading

cameron-martin commented Apr 4, 2024 •

edited

Loading

cameron-martin commented Apr 4, 2024

illicitonion left a comment

UebelAndre commented Apr 10, 2024

cameron-martin commented Apr 10, 2024

UebelAndre commented Apr 10, 2024

cameron-martin commented Apr 11, 2024

Make bazel lock file cross-platform #2453

Make bazel lock file cross-platform #2453

Conversation

cameron-martin commented Feb 1, 2024 • edited Loading

illicitonion left a comment

Choose a reason for hiding this comment

illicitonion Feb 2, 2024

Choose a reason for hiding this comment

cameron-martin Apr 4, 2024

Choose a reason for hiding this comment

illicitonion Feb 2, 2024

Choose a reason for hiding this comment

cameron-martin commented Feb 2, 2024 • edited Loading

cameron-martin commented Feb 2, 2024 • edited Loading

yesudeep commented Feb 24, 2024

cameron-martin commented Feb 24, 2024

ericmcbride commented Mar 26, 2024

cameron-martin commented Mar 26, 2024

UebelAndre commented Mar 26, 2024

cameron-martin commented Apr 1, 2024

ericmcbride commented Apr 4, 2024 • edited Loading

cameron-martin commented Apr 4, 2024 • edited Loading

cameron-martin commented Apr 4, 2024

illicitonion left a comment

Choose a reason for hiding this comment

UebelAndre commented Apr 10, 2024

cameron-martin commented Apr 10, 2024

UebelAndre commented Apr 10, 2024

cameron-martin commented Apr 11, 2024

cameron-martin commented Feb 1, 2024 •

edited

Loading

cameron-martin commented Feb 2, 2024 •

edited

Loading

cameron-martin commented Feb 2, 2024 •

edited

Loading

ericmcbride commented Apr 4, 2024 •

edited

Loading

cameron-martin commented Apr 4, 2024 •

edited

Loading