Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce and enforce MSRV #56

Open
tnull opened this issue Feb 28, 2023 · 6 comments
Open

Introduce and enforce MSRV #56

tnull opened this issue Feb 28, 2023 · 6 comments

Comments

@tnull
Copy link
Contributor

tnull commented Feb 28, 2023

A dependency of zip recently bumped the rustc requirement to 1.6, leading to MSRV violations in downstream projects such as LDK (cf. lightningdevkit/rust-lightning#2055) and BDK:

error: package `base64ct v1.6.0` cannot be built because it requires rustc 1.60 or newer, while the currently active rustc version is 1.57.0

It would be great if this crate and bitcoind could pin an old version of zip and enforce a reasonable MSRV, e.g., 1.48, which would allow us to keep using them.

Happy to open corresponding PRs after a concept ACK.

@RCasatta
Copy link
Owner

Pinning could break other projects, I would like to avoid it if possible.

What do you think of the approach followed by bitcoind (without features MSRV is 1.41.1) ?

Obviously, without the feature, you need to provide bitcoind executable from env var or PATH

@tnull
Copy link
Contributor Author

tnull commented Feb 28, 2023

I think introducing this approach here might be a good first step, which at least would allow us to mitigate the issue by downloading the binary ourselves in CI. However, it is of course quite tedious to make our test run in CI as well as locally. As this is exactly the point of the download feature, it would be really nice to keep using it with a lower MSRV.

Would an alternative path maybe be to introduce a new feature that rather than relying on zip/ureq uses the system's curl/gzip to download and unpack the binaries?

@danielabrozzoni
Copy link

One thing we have in rust-hwi in order to avoid pinning dependencies for everyone (as it caused some troubles: bitcoindevkit/rust-hwi#72) is a feature with a lower msrv, in which the dependencies are pinned: bitcoindevkit/rust-hwi#73

I personally don't like the approach, but it seems to work

@RCasatta
Copy link
Owner

yeah, I think pinning via a specific feature is not great but the best option

@tnull
Copy link
Contributor Author

tnull commented Mar 8, 2023

Sorry for the delay here. I'm experimenting with the optional dependency, not entirely sure it will work as expected, since it's a bit unclear to me how to still maintain the non-pinned default dependecy.

That said, the first step towards any MSRV is probably #58, as ureq depends on once_cell, which doesn't provide any guarantees whatsoever.

@tnull
Copy link
Contributor Author

tnull commented Jan 31, 2024

@RCasatta It seems by now this crate requires ~1.67 or so just due to some downstream dependency (I think zstd-sys). It would be great if we could introduce an MSRV and enforce it in CI.

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

No branches or pull requests

3 participants