search

RCasatta / electrsd

Utility to run a regtest electrsd process connected to a given bitcoind instance, useful in integration testing environment.
MIT License
21 stars 17 forks source link

Introduce and enforce MSRV #56

Open tnull opened 1 year ago

tnull commented 1 year ago

A dependency of zip recently bumped the rustc requirement to 1.6, leading to MSRV violations in downstream projects such as LDK (cf. https://github.com/lightningdevkit/rust-lightning/pull/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

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

RCasatta commented 1 year ago

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

tnull commented 1 year ago

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 commented 9 months ago

@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.