Skip to content

Add MSRV Cargo-minimal.lock and Cargo-recent.lock files#150

Open
alexanderwiederin wants to merge 1 commit intosedited:masterfrom
alexanderwiederin:cargo-lock
Open

Add MSRV Cargo-minimal.lock and Cargo-recent.lock files#150
alexanderwiederin wants to merge 1 commit intosedited:masterfrom
alexanderwiederin:cargo-lock

Conversation

@alexanderwiederin
Copy link
Copy Markdown
Contributor

@alexanderwiederin alexanderwiederin commented Apr 7, 2026

Fixes #147

Summary

Without commited lock files, CI dependency resolution can pull in transitive dependencies that require a newer compiler than the project's MSRV. The triggering issue was that bindgen v0.72 depends on rustc-hash v2.x, and the recent release of rustc-hash v2.1.2 introduced a rustc 1.77.0 requirement, breaking the MSRV build against rustc 1.71.0.

CI Changes

Cargo-minimal.lock pins dependencies to their oldest MSRV-compatible versions. Cargo-recent.lock pins to the latest patch versions compatible with the MSRV compiler. The CI msrv-check job now runs twice as a matrix, once with each lock file, similar to the approach used by rust-bitcoin.

Cargo Dependency Changes

libbitcoinkernel-sys

rustc-hash is pinned to below 2.1.1 to prevent bindgen from resolving to incompatible release. 2.1.0 in the minimal lockfile and 2.1.1 in the recent lockfile.

dev-dependencies

tempdir is pinned to >=0.3.6 to prevent it from resolving to an older version that depends on an incompatible rand release under minimal-versions.

As a follow-up, I would suggest we remove tempdir as a dependency (#151)

@alexanderwiederin alexanderwiederin marked this pull request as draft April 7, 2026 11:13
@alexanderwiederin alexanderwiederin force-pushed the cargo-lock branch 2 times, most recently from e0b6d70 to 1bfecb9 Compare April 7, 2026 11:24
@alexanderwiederin alexanderwiederin marked this pull request as ready for review April 7, 2026 11:31
Copy link
Copy Markdown
Contributor

@jaoleal jaoleal left a comment

Choose a reason for hiding this comment

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

ACK 1bfecb9

Just a nit about the name of the step

Comment thread .github/workflows/ci.yml Outdated
@jaoleal
Copy link
Copy Markdown
Contributor

jaoleal commented Apr 7, 2026

ACK da606fa

Comment thread libbitcoinkernel-sys/Cargo.toml Outdated
[build-dependencies]
cc = "1.2"
bindgen = "0.72"
rustc-hash = "=2.1.1"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Won't this mean that if you do not need the MSRV, for exmple if you can run stable, then this will preven rustc-hash from using the latest version when it could? If you need this version to work for the MSRV, then why not just defer to the lockfile?

Copy link
Copy Markdown
Contributor Author

@alexanderwiederin alexanderwiederin Apr 8, 2026

Choose a reason for hiding this comment

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

That is a good point.

The reason I pinned it is that a user running rust v1.71.0 that calls cargo update will have a broken build unless we have this pin in Cargo.toml or ensure that they use the committed lock files.

But thinking about it again, maybe a disclaimer for 1.71.0 users to add --locked might be the lesser evil. What do you think?

Copy link
Copy Markdown
Contributor Author

@alexanderwiederin alexanderwiederin Apr 8, 2026

Choose a reason for hiding this comment

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

In fact it's not just v1.71.0 users that are affected but all users on rustc older than 1.77.0

https://crates.io/crates/rustc-hash/2.1.2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Opened an issue for rustc-hash: rust-lang/rustc-hash#68

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We could also consider lifting the MSRV. If I remember correctly we set it to 1.71.0 for Floresta, but they seem to have bumped to 1.81.0 getfloresta/Floresta#687

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

fair. I think it would be a good idea to remove tempdir than before publishing the next release on crates.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

s/than/then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, tempdir is a dev dependency, so it doesn't matter so much.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! If you feel comfortable, could leave an ACK? That will make @sedited`s life easier when he gets back.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sure, I left a minor nit. Am happy to ACK the current commit or an updated one.

Comment thread README.md Outdated
Without commited lock files, CI dependency resolution can pull in
transitive dependencies that require a newer compiler than the project's
MSRV. The triggering issue was that bindgen v0.72 depends on rustc-hash
v2.x, and the recent release of rustc-hash v2.1.2 introduced a rustc
1.77.0 requirement, breaking the MSRV build against rustc 1.71.0.

Cargo-minimal.lock pins dependencies to their oldest MSRV-compatible
versions. Cargo-recent.lock pins to the latest patch versions compatible
with the MSRV compiler. The CI msrv-check job now runs twice as a
matrix, once with each lock file, similar to the approach used by
rust-bitcoin. Both lockfiles pin rustc-hash below 2.1.2 to avoid the
incompatible release, to 2.1.0 in the minimal lockfile and 2.1.1 in the
recent lockfile.

tempdir is bumped to >=0.3.6 in dev-dependencies to prevent it from
resolving to an older version that depends on an incompatible rand
release under minimal-versions.

See also: sedited#147
@yancyribbens
Copy link
Copy Markdown

utACK cd69fe2

Copy link
Copy Markdown
Contributor

@jaoleal jaoleal left a comment

Choose a reason for hiding this comment

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

ACK cd69fe2

@luisschwab
Copy link
Copy Markdown

FIY: cargo-rbmt is great to manage this kind of stuff. It's what rust-bitcoin uses.

@alexanderwiederin
Copy link
Copy Markdown
Contributor Author

alexanderwiederin commented Apr 13, 2026

FIY: cargo-rbmt is great to manage this kind of stuff. It's what rust-bitcoin uses.

Interesting! Will look into it. Thanks for the pointer.

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.

Add MSRV Cargo.lock file

4 participants