Skip to content

ci: upgrade rbmt and migrate api files to packages#5914

Merged
apoelstra merged 5 commits intorust-bitcoin:masterfrom
nyonson:drop-toolchain-files
Apr 1, 2026
Merged

ci: upgrade rbmt and migrate api files to packages#5914
apoelstra merged 5 commits intorust-bitcoin:masterfrom
nyonson:drop-toolchain-files

Conversation

@nyonson
Copy link
Copy Markdown
Contributor

@nyonson nyonson commented Mar 27, 2026 •
edited
Loading

Some more CI cleanup.

  • First patch fixes a bug in the prerelease task, making sure it works running post-merge on master as well.
  • Second patch removes usage of the old legacy toolchain files, this is the root manifest now. Had to slim down the justfile which was using these as well, but I think cargo-rbmt is good enough now for this. I didn't put much though in the githooks, not sure how much value.
  • Third bumps the cargo-rbmt version, includes some minor feature cleanups.
  • Forth dumps the old check api script for the cargo-rbmt command. Will help use a consistent version of the underlying public-api library in dev envs and CI.
  • Fifth converts the weekly format job to cargo rbmt fmt. This locks in the toolchain used, no more floating +nightly.

@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented Mar 27, 2026

In draft while I tinker on this and Andrew upgrades his CI to no longer rely on the toolchain files.

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Mar 27, 2026

71d60dd needs rebase

@nyonson nyonson force-pushed the drop-toolchain-files branch 2 times, most recently from 8981349 to 3e287b6 Compare March 27, 2026 19:20
@github-actions github-actions bot added ci C-hashes PRs modifying the hashes crate C-units PRs modifying the units crate C-consensus_encoding PRs modifying the consensus-encoding crate C-internals PRs modifying the internals crate C-primitives labels Mar 27, 2026
@nyonson nyonson force-pushed the drop-toolchain-files branch from 3e287b6 to fa20bf5 Compare March 27, 2026 19:28
@nyonson nyonson changed the title ci: drop toolchain files and migrate api files to packages ci: upgrade rbmt and migrate api files to packages Mar 27, 2026
@nyonson nyonson marked this pull request as ready for review March 27, 2026 19:32
@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented Mar 27, 2026

Undrafted because I figured I don't actually have to remove the legacy files yet, just removed all usage here.

tcharding
tcharding approved these changes Mar 27, 2026
View reviewed changes
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 fa20bf5

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 27, 2026

LOL, the line count of changes for your account just got a bump up the ladder.

@mpbagot
Copy link
Copy Markdown
Contributor

mpbagot commented Mar 28, 2026

Do the API files need to move? I find it really convenient to have them in their own directory. Makes grepping and git add/remove etc really easy.

@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented Mar 28, 2026 •
edited
Loading

Do the API files need to move? I find it really convenient to have them in their own directory. Makes grepping and git add/remove etc really easy.

It definitely doesn't have to and I appreciate the feedback. Beyond the line diff number pumping, the move simplifies things on the rbmt side, described in rust-bitcoin/rust-bitcoin-maintainer-tools#86. If it makes actually using the files more difficult though, could flip it back. I think my workflow has all been to generate the files and just eyeball the diff, so I may have missed something.

apoelstra
apoelstra approved these changes Mar 29, 2026
View reviewed changes
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 fa20bf5; successfully ran local tests

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Mar 29, 2026

fa20bf5 needs rebase

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 30, 2026

Good points. I usually grep by crate. Restoring changes is definitely going to be worse. A quick shell alias' will fix that though.

@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented Mar 30, 2026

@tcharding @mpbagot is there a justfile recipe or some functionality I can bake into cargo-rbmt to bridge the gap?

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 31, 2026

Nothing required from my end. If I find I need a funky shell alias I'll share it with y'all.

nyonson added 5 commits March 31, 2026 07:03
@nyonson
ci: add explicit git ref for prerelease
2bede00 
Allows the prerelease job to be run post-merge on master branch.
@nyonson
ci: remove legacy toolchain file usage
9fb89c5 
The legacy toolchain version pins have been migrated to the root
manifest Cargo.toml. Clean up any tooling using the old locations.
@nyonson
ci: upgrade rbmt version
19d066b
@nyonson
ci: switch to rbmt api command
752c98e 
API files have been moved into the package directories themselves instead
of the shared root level api/ directory.
@nyonson
ci: update weekly fmt job to cargo-rbmt
ccf2e22 
This locks in the compiler version for the format job.
@nyonson nyonson force-pushed the drop-toolchain-files branch from fa20bf5 to ccf2e22 Compare March 31, 2026 14:11
@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented Mar 31, 2026 •
edited
Loading

ccf2e22: rebased to pick up API updates and added commit at end to flip fmt job to cargo-rbmt.

@nyonson nyonson mentioned this pull request Mar 31, 2026
Open
tcharding
tcharding approved these changes Mar 31, 2026
View reviewed changes
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 ccf2e22

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Mar 31, 2026

This is going to cause merge conflict havoc. @apoelstra maybe you could just merge this now while the API files are broken on master anyways?

apoelstra
apoelstra approved these changes Apr 1, 2026
View reviewed changes
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 ccf2e22; successfully ran local tests

@apoelstra apoelstra merged commit e4b022e into rust-bitcoin:master Apr 1, 2026
30 checks passed
This was referenced Apr 1, 2026
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra approved these changes

@tcharding tcharding tcharding approved these changes

Assignees

No one assigned

Labels

C-consensus_encoding PRs modifying the consensus-encoding crate C-hashes PRs modifying the hashes crate C-internals PRs modifying the internals crate C-primitives C-units PRs modifying the units crate ci

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nyonson @apoelstra @tcharding @mpbagot