Skip to content

Conversation

@hofbi
Copy link
Contributor

@hofbi hofbi commented Oct 24, 2025

Description

Another attempt to improve #3522

Current status/plan on the issue above was

  • pre-commit hook that converts the Cargo.lock file into a Cargo.bazel.lock file by removing internal deps
  • use crate.from_cargo with the Cargo.bazel.lock file and skip_cargo_lockfile_overwrite = True
  • commit MODULE.bazel.lock which should have less merge conflicts since the sha of Cargo.bazel.lock does not change that often

This works, but we need to make sure Cargo.lock is up to date. This requires another step, so we replace a slow automated step (cargo splicing) with an extra manual/non-bazel step (updating the Cargo.lock file).

New idea to iterate

  • use crate.from_cargo with the Cargo.bazel.lock file but instead of using skip_cargo_lockfile_overwrite = True
    • we let rules_rust do the cargo dependency resolution that is happening as part of splicing
    • instead of writing back the full content into the cargo lock file, we only write back the third party (making the pre-commit hook obsolete). For this, I introduced the strip_internal_dependencies_from_cargo_lockfile flag

This would save us the pre-commit hook as well as the manual step to update the Cargo.lock file. I already did a quick verification and remove_internal_dependencies_from_cargo_lockfile achieves the same results as the python implementation I listed in #3522

Open Points

This is an early draft to collect feedback. Once we aligned how exactly we want to proceed, I would like to

  • add/extend an integration test to have this tested end to end
  • add and example to demonstrate how this should be used

@hofbi hofbi force-pushed the write-lockfile-external-only branch from 6aed689 to 32e534d Compare October 29, 2025 09:56
@hofbi hofbi force-pushed the write-lockfile-external-only branch from 32e534d to 6873aac Compare October 29, 2025 17:17
@hofbi hofbi marked this pull request as ready for review October 29, 2025 17:18
Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me, thank you! Just two small testing addition suggestions :)

Comment on lines +343 to +346
assert!(!filtered_lockfile
.packages
.iter()
.any(|pkg| pkg.name.as_str() == "child"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert!(!filtered_lockfile
.packages
.iter()
.any(|pkg| pkg.name.as_str() == "child"));
assert!(!filtered_lockfile
.packages
.iter()
.any(|pkg| pkg.name.as_str() == "child"));
assert!(filtered_lockfile
.packages
.iter()
.any(|pkg| pkg.name.as_str() == "anyhow"));


# https://bazelbuild.github.io/rules_rust/crate_universe_bzlmod.html
crate_index_remove_internal_deps = use_extension("@rules_rust//crate_universe:extensions.bzl", "crate")
crate_index_remove_internal_deps.from_cargo(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm tempted to add a second from_cargo with the un-stripped Cargo.lock, and a second number_printer target pointing at the unstripped repo, so that we can see both Cargo.lock files work and are up to date - WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants