Skip to content

ci: migrate to cargo rbmt#254

Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom
satsfy:migrate-ci-cargo-rbmt
Apr 21, 2026
Merged

ci: migrate to cargo rbmt#254
apoelstra merged 2 commits intorust-bitcoin:masterfrom
satsfy:migrate-ci-cargo-rbmt

Conversation

@satsfy
Copy link
Copy Markdown
Contributor

@satsfy satsfy commented Apr 15, 2026

This PR migrates CI from contrib/test.sh to cargo rbmt. It continues PR #234.

It also:

  • adds minimal and recent lockfiles for CI
  • declares rust-version = "1.56.1" in Cargo.toml
  • adds rbmt toolchain metadata
  • removes the old contrib/test.sh workflow
  • updates the workflow to use the pinned upstream setup-rbmt action
  • simplifies the embedded jobs into a matrix
  • drops the stale mutagen lint override

No library behavior is intended to change.

@apoelstra
Copy link
Copy Markdown
Member

In 6cab5af:

The commit description says this "Gets the MSRV from manifest instead of explicitly" but actually it removes the MSRV from Cargo.toml, ignores the MSRV in fuzz/Cargo.toml, adds it explicitly in a Github Action file which my local CI system can't read, and doesn't add it anywhere else.

It also introduces lockfiles with version 4 which I'm fairly sure Cargo 1.56 can't read, so I don't think we're actually testing the MSRV here.

@apoelstra
Copy link
Copy Markdown
Member

This is the first commit which is authored by @tcharding.

@tcharding
Copy link
Copy Markdown
Member

Excluding the fact that I just made mistakes cargo rbmt has changed a lot since I did #234. I don't think you should use my patch as it is, instead just roll it into your own work. I would have thought this PR would be a single patch.

@satsfy satsfy force-pushed the migrate-ci-cargo-rbmt branch from fb9a661 to f575fd9 Compare April 17, 2026 01:39
@satsfy
Copy link
Copy Markdown
Contributor Author

satsfy commented Apr 17, 2026

Squashed to a single commit, as suggested.

@tcharding
Copy link
Copy Markdown
Member

Can you grab the latest version of cargo rbmt mate? There shouldn't be a rbmt.toml file anymore. Just take a look at rust-bitcoin to see examples.

@satsfy satsfy force-pushed the migrate-ci-cargo-rbmt branch from f575fd9 to 916d82d Compare April 18, 2026 21:47
@satsfy
Copy link
Copy Markdown
Contributor Author

satsfy commented Apr 18, 2026

Sorry it really was not the most up-to-date version of rbmt.

Force push removed rbmt.toml and use latest rbmt commit af3c28.

Additionally, do you think I should read the version from rbmt-version file for CI, so that it does not repeat itself?

@tcharding
Copy link
Copy Markdown
Member

Additionally, do you think I should read the version from rbmt-version file for CI, so that it does not repeat itself?

hmm, @nyonson I'm a bit confused about rbmt-version. In rust-bitcoin is the plan to remove it completely or keep using it in the cron jobs and justfile/contrib scripts? Or said another way, can you advise on the best move here?

@apoelstra
Copy link
Copy Markdown
Member

(Reminder to self that when this resolves, to copy its approach into rust-elements.)

@nyonson
Copy link
Copy Markdown

nyonson commented Apr 19, 2026

Additionally, do you think I should read the version from rbmt-version file for CI, so that it does not repeat itself?

hmm, @nyonson I'm a bit confused about rbmt-version. In rust-bitcoin is the plan to remove it completely or keep using it in the cron jobs and justfile/contrib scripts? Or said another way, can you advise on the best move here?

I assume @satsfy you are talkin about the uses lines in the yaml? Like - uses: rust-bitcoin/rust-bitcoin-maintainer-tools/.github/actions/setup-rbmt@af3c2868415b17eedc808da6d0589e10b7482660 # v0.1.0? If you find a clean way to read the rbmt version into that, I'd love to see it. I have struggled with Github Actions because I don't think the uses key allows much to be run in it. So I have just been using this totally annoying strategy of copying the hash everywhere.

@tcharding I have been trying to think of a good way to merge these version definitions, but haven't found a good pattern yet. Top of the list for rbmt stuff though.

Comment thread .github/workflows/rust.yml Outdated
Stable:
name: Test - stable toolchain
runs-on: ubuntu-latest
MSRV: # 2 jobs: 1 toolchain × 2 lock files.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think you can drop this MSRV job and just extend the Test matrix above with toolchain: [stable, nightly, msrv]. Kinda related, but we have also talked about in other repos how testing on nightly isn't all that helpful.

Copy link
Copy Markdown
Contributor Author

@satsfy satsfy Apr 20, 2026

Choose a reason for hiding this comment

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

Thanks, its done now. MSRV is now part of the test matrix and the separate job is gone. Need to use a MSRV-specific command path because cargo rbmt --toolchain msrv currently breaks on Rust 1.56/1.57 metadata parsing.

About nightly, the CI already had a nightly test so I'm trying to retain it as is for this PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@satsfy satsfy force-pushed the migrate-ci-cargo-rbmt branch from 916d82d to 8a488b0 Compare April 20, 2026 19:57
@satsfy
Copy link
Copy Markdown
Contributor Author

satsfy commented Apr 20, 2026

About the yaml file version redeclarations, the best alternative solution I found would be adding a new github actions file just for reading it from file. I don't think the added complexity of this solution would be worth it, so I'm leaving that unchanged.

One possibility here is to point to master instead of point to a commit. I suppose this should be possible: rust-bitcoin/rust-bitcoin-maintainer-tools/.github/actions/setup-rbmt@master # v0.1.0 (but this wouldn't be stable) or rust-bitcoin/rust-bitcoin-maintainer-tools/.github/actions/setup-rbmt@rbmtv0.1.0 if rust-bitcoin-maintainer-tools had this custom branch.

@satsfy satsfy force-pushed the migrate-ci-cargo-rbmt branch from 8a488b0 to 72cadc1 Compare April 20, 2026 20:11
satsfy added 2 commits April 20, 2026 17:38
Use setup-rbmt directly in workflows, regenerate minimal/recent
lockfiles, and remove the stale mutagen lint override.
@satsfy satsfy force-pushed the migrate-ci-cargo-rbmt branch from 72cadc1 to 30686ad Compare April 20, 2026 21:02
@satsfy
Copy link
Copy Markdown
Contributor Author

satsfy commented Apr 20, 2026

Latest commit fixes lint errors by constraining feature flags.

Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 30686ad

@tcharding
Copy link
Copy Markdown
Member

Looks good to me! PR description is stale mate, still mentions rbmt.toml.

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 30686ad; successfully ran local tests

@apoelstra apoelstra merged commit cc44a3c into rust-bitcoin:master Apr 21, 2026
22 checks passed
@apoelstra
Copy link
Copy Markdown
Member

Merged this but FYI it does not update the weekly cronjob, so we are updating the nightly version in the wrong place. (Noticed when porting this to rust-elemnets.)

apoelstra added a commit that referenced this pull request Apr 23, 2026
b8a32c9 ci: unify nightly embedded version on Cargo.toml (satsfy (Renato Britto))
a924cdc ci: migrate nightly update to rbmt on Cargo.toml (satsfy (Renato Britto))

Pull request description:

  Follow up from #254 (comment)
  
  After the rbmt CI migration, most jobs were already consuming nightly from `[package.metadata.rbmt.toolchains]` in `Cargo.toml`, but the weekly updater still wrote `nightly-version`. This change makes `Cargo.toml` the single nightly source of truth across the updater and CI jobs.
  
  Changes:
  - update weekly nightly cron to use `cargo rbmt toolchains --update-nightly.` Replicates rust-bitcoin CI closely.
  - `nightly-version` removed.
  - migrate Embedded CI job to read nightly from `Cargo.toml` instead of `nightly-version.`
  
  Validated in personal fork CI: 
  - The CI run (didn't create the PR because it wasn't allowed): https://github.com/satsfy/rust-bech32/actions/runs/24799694669/job/72578713007
  - PR produced: https://github.com/satsfy/rust-bech32/compare/master...satsfy:rust-bech32:create-pull-request/weekly-nightly-update?expand=1


ACKs for top commit:
  apoelstra:
    ACK b8a32c9; successfully ran local tests


Tree-SHA512: f2381f1c13eaebcb6e256bf82ee110dab9bf1fe2615a0075b9953e0fd808d4b8f94f1f49568522dc99193f6e1a5ae9619a60cf3b77c084027d9236a8fa206bf1
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.

4 participants