Skip to content

chore(ci): replace pin-msrv.sh with cargo-rbmt and committed lockfiles#494

Open
EliteCoder18 wants to merge 3 commits into
bitcoindevkit:masterfrom
EliteCoder18:feat/rbmt-ci-migration
Open

chore(ci): replace pin-msrv.sh with cargo-rbmt and committed lockfiles#494
EliteCoder18 wants to merge 3 commits into
bitcoindevkit:masterfrom
EliteCoder18:feat/rbmt-ci-migration

Conversation

@EliteCoder18

Copy link
Copy Markdown

Description

Closes #400
Closes #79
Part of #410

Replaces the fragile ci/pin-msrv.sh dependency-pinning script with a two-lockfile strategy using cargo-rbmt the same tooling used by the rust-bitcoin project. This implements the approach

Problem

ci/pin-msrv.sh pinned specific transitive dependency versions via cargo update --precise at build time. This was brittle: every time a dependency chain changed, someone had to manually discover which packages needed pinning and update the script by hand. It also couldn't be replicated locally without running the same script in the same order.

Solution

Two versioned lockfiles are now committed to the repository:

Cargo-minimal.lock :- Minimal compatible versions used for the MSRV build
Cargo-recent.lock :- Recent/latest compatible versions used for the stable build

CI copies the appropriate lockfile to Cargo.lock before building and passes --locked to cargo build/cargo test, giving fully reproducible builds that contributors can replicate locally.

A new check-lockfiles CI job runs cargo rbmt lock and fails if the committed lockfiles drift from what cargo-rbmt would generate, so staleness is caught automatically on every PR.

Files changed

  • Cargo-minimal.lock && Cargo-recent.lock: committed lockfiles generated by cargo rbmt lock
  • ci/install-rbmt.sh: installs cargo-rbmt at the version pinned in rbmt-version
  • rbmt-version: pins cargo-rbmt at 0.3.0 for auditable, cached installs
  • Cargo.toml: adds [package.metadata.rbmt.*] config with toolchain versions and feature exclusions for no-std; fixes bdk_chain (hashbrown) and miniscript (no-std) feature flags for no-std targets; pins dev-dependency versions for lock stability
  • .github/workflows/cont_integration.yml: replaces pin-msrv.sh step with lockfile copy and --locked; adds check-lockfiles job
  • CONTRIBUTING.md: documents how to regenerate lockfiles when Cargo.toml changes
  • ci/pin-msrv.sh: deleted

Notes to the reviewers

  • The [package.metadata.rbmt.test].exclude_features list in Cargo.toml tells cargo-rbmt which features to skip in its matrix — needed to avoid pulling std-only crates (anyhow, bdk_file_store, etc.) into no-std feature combos.
  • The nightly toolchain entry under [package.metadata.rbmt.toolchains] is required by cargo-rbmt's.
  • This mirrors exactly how rust-bitcoin manages their MSRV CI see their Cargo-minimal.lock for reference.
  • To regenerate the lockfiles after a Cargo.toml change:
    ./ci/install-rbmt.sh
    cargo rbmt toolchains
    cargo rbmt lock
    

Changelog notice

  • chore(deps): add Cargo-minimal.lock and Cargo-recent.lock lockfiles; fix no-std feature flags for bdk_chain (hashbrown) and miniscript (no-std)
  • chore(ci): replace ci/pin-msrv.sh with cargo-rbmt; copy lockfile before build and add --locked; add check-lockfiles job to detect stale lockfiles on every PR
  • docs: document lockfile regeneration workflow in CONTRIBUTING.md and README.md

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.96%. Comparing base (39de6ed) to head (5789c83).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
+ Coverage   80.93%   80.96%   +0.02%     
==========================================
  Files          24       24              
  Lines        5482     5489       +7     
  Branches      247      247              
==========================================
+ Hits         4437     4444       +7     
  Misses        968      968              
  Partials       77       77              
Flag Coverage Δ
rust 80.96% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@EliteCoder18 EliteCoder18 marked this pull request as draft June 2, 2026 17:29
@ValuedMammal ValuedMammal added this to the Wallet 4.0.0 milestone Jun 4, 2026
@ValuedMammal ValuedMammal added build Build system github_actions Pull requests that update GitHub Actions code labels Jun 4, 2026
@ValuedMammal ValuedMammal moved this to Todo in BDK Wallet Jun 4, 2026
@EliteCoder18 EliteCoder18 marked this pull request as ready for review June 4, 2026 20:38
@EliteCoder18 EliteCoder18 force-pushed the feat/rbmt-ci-migration branch from ea9ef2a to 5789c83 Compare June 11, 2026 13:13
apoelstra added a commit to rust-bitcoin/rust-miniscript that referenced this pull request Jun 14, 2026
…-std

c7adcfd fix(manifest): correct serde feature configuration for no-std (Rishit Modi)

Pull request description:

  ### Description
  
  This PR fixes a bug where the optional serde dependency implicitly leaks the standard library into no-std builds.
  
  Previously, serde was missing `default-features = false` in the `[dependencies]` block. When downstream crates attempted to compile miniscript for bare-metal targets (e.g., thumbv7m-none-eabi) using minimal-version dependency resolution, Cargo would resolve serde with its default "std" feature enabled, causing immediate compilation failures.
  
  ### Notes to the reviewers
  
  I encountered this feature leakage while working on integrating cargo-rbmt CI checks over in bdk_wallet (specifically in this PR: bitcoindevkit/bdk_wallet#494).
  
  When the rbmt matrix runs its minimal dependency sweep on no-std bare-metal targets, it exposes this missing feature configuration. Applying this fix locally allowed the embedded target compilation to pass cleanly.
  
  ### Changelog notice
  
  - Added default-features = false to the optional serde dependency.
  - Explicitly opted into the alloc feature for serde.
  - Updated the std feature array to use the weak dependency activation syntax (`"serde?/std"`)


ACKs for top commit:
  trevarj:
    Nice find. ACK c7adcfd
  apoelstra:
    ACK c7adcfd; successfully ran local tests


Tree-SHA512: 33c2993856cfa70cba83f4b0f95b5bd9df04d6a50db3485d13ec18ea4d7c9f613f50ad86e6b53f1ad9cf7642165cd67d736a3e81f8f9b480084bb38a67ad728d
@luisschwab luisschwab self-requested a review June 15, 2026 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build system github_actions Pull requests that update GitHub Actions code

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Investigate using cargo-rbmt for MSRV dep management discussion: should we adopt the Cargo-minimal.lock and Cargo-recent.lock approach ?

2 participants