Skip to content

[PM-29567] Pin binary cargo tools via cargo-run-bin#1143

Merged
coroiu merged 4 commits into
mainfrom
PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file
Jun 3, 2026
Merged

[PM-29567] Pin binary cargo tools via cargo-run-bin#1143
coroiu merged 4 commits into
mainfrom
PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file

Conversation

@coroiu
Copy link
Copy Markdown
Contributor

@coroiu coroiu commented May 27, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-29567

📔 Objective

Add a [workspace.metadata.bin] block to the root Cargo.toml listing every binary cargo tool used by CI and scripts/lint.sh. CI and developers bootstrap cargo-run-bin once and invoke any tool via cargo bin <tool>, so dev versions stay in sync with CI.

scripts/lint.sh switches from PATH lookups (with manual install hints) to cargo bin <tool> invocations; the lint matrix drops its per-check install steps in favor of one shared cargo-run-bin install. The other workflows (check-powerset, build-rust-crates, build-android, version-bump, rust-test) get the same treatment.

Renovate's regex managers now target the manifest plus the cargo-run-bin bootstrap pin, replacing the four per-tool workflow regex managers.

Follow-up fixes to get the lint matrix green

Moving the tools under cargo-run-bin surfaced two lint failures and one efficiency gap, fixed here:

  • clippy: the clippy step set RUSTFLAGS: -D warnings for the whole step, which leaked into the from-source builds of clippy-sarif/sarif-fmt that cargo bin triggers in that step — a warning in their transitive dependency serde-sarif then became a hard error. -D warnings is now passed on the clippy command line instead, so the deny level applies only to the linted crates.

  • dylint: cargo-dylint invokes dylint-link as rustc's linker, found by name on PATH. Run through cargo bin, cargo-run-bin prepends a shim directory whose dylint-link shim re-runs cargo bin dylint-link, which fails from the directories rustc links in (support/lints and each dependency's source dir, none of which declare [workspace.metadata.bin]). The dylint check now invokes the cargo-dylint binary directly with the real dylint-link on an absolute PATH, bypassing the shims. It builds only the two tools dylint needs rather than cargo bin --install, which also tries to build cross (which doesn't compile on the pinned toolchain).

  • caching: cargo-run-bin builds its tools into .bin, which wasn't cached, so every lint check rebuilt its tools from source each run. A dedicated actions/cache step now caches .bin per check, keyed on Cargo.toml + rust-toolchain.toml (the tools depend on their pinned versions and the toolchain, not on Cargo.lock) with restore-keys for graceful fallback when a version is bumped.

🚨 Breaking Changes

None.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

🔍 SDK Breaking Change Detection

SDK Version: PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file (02cc617)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client Status Details
typescript ✅ No breaking changes detected Compilation passed with new SDK version - View Details
android ✅ No breaking changes detected Compilation passed with new SDK version - View Details

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.12%. Comparing base (7bca9fc) to head (9170a1c).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1143      +/-   ##
==========================================
+ Coverage   84.10%   84.12%   +0.01%     
==========================================
  Files         446      446              
  Lines       58768    58817      +49     
==========================================
+ Hits        49428    49478      +50     
+ Misses       9340     9339       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from PM-24643-add-an-easy-to-use-formatting-linting-command-to-the-sdk to main June 2, 2026 13:32
Add a [workspace.metadata.bin] block to the root Cargo.toml listing every
binary cargo tool used by CI and scripts/lint.sh. CI and developers
bootstrap cargo-run-bin once and invoke any tool via `cargo bin <tool>`,
so dev versions stay in sync with CI.

scripts/lint.sh switches from PATH lookups (with manual install hints) to
`cargo bin <tool>` invocations; the lint matrix drops its per-check
install steps in favor of one shared cargo-run-bin install. The other
workflows (check-powerset, build-rust-crates, build-android, version-bump,
rust-test) get the same treatment.

Renovate's regex managers now target the manifest plus the cargo-run-bin
bootstrap pin, replacing the four per-tool workflow regex managers.
@coroiu coroiu force-pushed the PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file branch from 02cc617 to af6ce05 Compare June 2, 2026 13:42
@coroiu coroiu marked this pull request as ready for review June 2, 2026 13:44
@coroiu coroiu requested a review from a team as a code owner June 2, 2026 13:44
@coroiu coroiu requested a review from djsmith85 June 2, 2026 13:44
coroiu added 2 commits June 3, 2026 09:40
The clippy lint step set RUSTFLAGS=-D warnings for the whole step, which
leaked into the from-source builds of clippy-sarif/sarif-fmt triggered by
`cargo bin` in the same step. A warning in their transitive dependency
serde-sarif then became a hard error and failed the build.

Pass -D warnings to clippy directly instead, so the deny level applies only
to the linted crates and not to the tool builds.
cargo-dylint invokes dylint-link as rustc's linker, found by name on PATH.
Running it via `cargo bin cargo-dylint` fails: cargo-run-bin prepends a shim
directory to PATH, and the dylint-link shim re-runs `cargo bin dylint-link`,
which resolves the project root from the current directory. During linking the
cwd is inside support/lints or a dependency's source dir, none of which declare
[workspace.metadata.bin], so the shim fails with "No binaries configured".

Build the tools and invoke the cargo-dylint binary directly with the real
dylint-link binary on PATH (via an absolute path, so it resolves regardless of
which directory cargo-dylint links from). Build only the two tools dylint needs
rather than `cargo bin --install`, which builds every pinned tool including
some that don't compile on this toolchain (e.g. cross).
@coroiu coroiu force-pushed the PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file branch from af14a21 to c5cc1f9 Compare June 3, 2026 08:07
cargo-run-bin builds its tools into .bin, which was not cached, so every lint
check rebuilt its tools from source each run.

Cache .bin with a dedicated actions/cache step rather than via rust-cache's
cache-directories. rust-cache keys every matrix check under one shared
job-based key (so only the first check to save wins, and the others rebuild
their tools anyway) and folds in the Cargo.lock hash (which rotates every day
or two). The tools depend only on their pinned versions and the toolchain, so
key on Cargo.toml + rust-toolchain.toml per check instead, with restore-keys
for graceful fallback.
@coroiu coroiu force-pushed the PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file branch from c5cc1f9 to 9170a1c Compare June 3, 2026 09:14
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 3, 2026

Comment thread README.md
### Installation

The tools each check needs (and pinned versions) are installed in the lint workflow above. The
underlying tools are:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you, this step especially was weird to look up from the lint workflow and last time I setup my machine I was unaware of the pinned versions, as complaints about missing tools, came up while I was building the SDK locally.

@coroiu coroiu merged commit a2734ac into main Jun 3, 2026
84 checks passed
@coroiu coroiu deleted the PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file branch June 3, 2026 11:42
bw-ghapp Bot added a commit to bitwarden/sdk-swift that referenced this pull request Jun 3, 2026
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