Move module_ctx.watch to after lockfile repin

[patrickmscott]

In bazel 9, module_ctx.read verifies digests for files already read. If repin is requested, the content of the lockfile may change and cause bazel to hard crash.

relates to: bazelbuild/bazel#29114

[illicitonion]

Seems reasonable - thanks!

[patrickmscott]

@illicitonion what's going on with the failed build? Is that normal?

[UebelAndre]

The failure is: https://buildkite.com/bazel/rules-rust-rustlang/builds/16817/steps/canvas?sid=019d5359-0ec6-441f-a3ff-269f49377957&tab=output

Executed 2 out of 2 tests: 2 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.
+ echo 'Changing the version of the vendored crate, so that the patch no longer applies'
Changing the version of the vendored crate, so that the patch no longer applies
+ sed_i=(sed -i)
++ uname
+ [[ Linux == \D\a\r\w\i\n ]]
+ sed -i -e 's#^version = "1\.5\.0"$#version = "2.0.0"#' /tmp/tmp.xkLnPrRAA4/Cargo.toml
+ echo 'Running build which is expected to fail, as the patch no longer applies'
Running build which is expected to fail, as the patch no longer applies
+ set +e
++ bazel build //...
+ output='Computing main repo mapping:
Loading:
Loading: 0 packages loaded
Analyzing: 8 targets (0 packages loaded, 0 targets configured)
Analyzing: 8 targets (0 packages loaded, 0 targets configured)
INFO: Analyzed 8 targets (2 packages loaded, 42 targets configured).
INFO: Found 8 targets...
INFO: Elapsed time: 0.294s, Critical Path: 0.05s
INFO: 5 processes: 26 action cache hit, 1 internal, 4 linux-sandbox.
INFO: Build completed successfully, 5 total actions'
+ exit_code=0
+ set -e
+ [[ 0 -ne 1 ]]
+ echo 'Expected build failure, but exit code was 0. Output:'
Expected build failure, but exit code was 0. Output:
+ echo 'Computing main repo mapping:
Loading:
Loading: 0 packages loaded
Analyzing: 8 targets (0 packages loaded, 0 targets configured)
Analyzing: 8 targets (0 packages loaded, 0 targets configured)
INFO: Analyzed 8 targets (2 packages loaded, 42 targets configured).
INFO: Found 8 targets...
INFO: Elapsed time: 0.294s, Critical Path: 0.05s
INFO: 5 processes: 26 action cache hit, 1 internal, 4 linux-sandbox.
INFO: Build completed successfully, 5 total actions'
Computing main repo mapping:
Loading:
Loading: 0 packages loaded
Analyzing: 8 targets (0 packages loaded, 0 targets configured)
Analyzing: 8 targets (0 packages loaded, 0 targets configured)
INFO: Analyzed 8 targets (2 packages loaded, 42 targets configured).
INFO: Found 8 targets...
INFO: Elapsed time: 0.294s, Critical Path: 0.05s
INFO: 5 processes: 26 action cache hit, 1 internal, 4 linux-sandbox.
INFO: Build completed successfully, 5 total actions
+ exit 1
bazel run failed with exit code 1

I'm not familiar with this test. @illicitonion can you take a look?

[UebelAndre]

@patrickmscott does this diff fix the issue?

--- a/crate_universe/extensions.bzl
+++ b/crate_universe/extensions.bzl
@@ -1114,12 +1114,12 @@
         for cfg in mod.tags.from_cargo + mod.tags.from_specs:
-            # Preload all external repositories. Calling `module_ctx.watch` will cause restarts of the implementation
-            # function of the module extension when the file has changed.
+            # Watch inputs not modified by the generator. Calling `module_ctx.watch` will cause restarts of
+            # the implementation function of the module extension when the file has changed.
             if cfg.cargo_lockfile:
                 module_ctx.watch(cfg.cargo_lockfile)
-            if cfg.lockfile:
-                module_ctx.watch(cfg.lockfile)
             if cfg.cargo_config:
                 module_ctx.watch(cfg.cargo_config)
             if hasattr(cfg, "manifests"):
@@ -1181,6 +1179,12 @@
                 strip_internal_dependencies_from_cargo_lockfile = cfg.strip_internal_dependencies_from_cargo_lockfile,
             )
 
+            # Watch cfg.lockfile AFTER generation. The generator may modify it during
+            # repin, and in Bazel 9 module_ctx.read verifies digests for files already
+            # read, which would crash if the lockfile changed between watch and read.
+            if cfg.lockfile:
+                module_ctx.watch(cfg.lockfile)
+
     metadata_kwargs = {}
     if bazel_features.external_deps.extension_metadata_has_reproducible:
         metadata_kwargs["reproducible"] = reproducible