bazelbuild
Make bazel lock file cross-platform
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 #2452
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 https://github.com/bazelbuild/bazel/issues/20272#issuecomment-1819889397 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?
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.
Yes, this should allow us to fix this: https://github.com/bazelbuild/bazel/pull/21306
There's a failing test that I need to debug. After I've done that it'll be ready.
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.
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.
Looks like https://github.com/bazelbuild/rules_rust/pull/2575 doesnt fully resolve the issue. The hosts are still locking the wrong exec
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 I think this introduced a regression for bzlmod on Bazel 6 https://github.com/bazelbuild/bazel-central-registry/pull/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?
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).
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!