Stop pinning everything in CI

[LLFourn]

          Why do we need all these pins?

We don't have to test on MSRV we just have to check that it builds on MSRV. dev-dependencies shouldn't have to be pinned. Or at least that's the policy I'd like to adopt.

See sophisticated technology on how to do this: https://github.com/bitcoindevkit/coin-select/blob/990226a982bd6036bed5a7674fe57d6f9d48e49e/.github/workflows/test.yml#L40

cc @notmandatory

Originally posted by @LLFourn in bitcoindevkit/bdk#1397 (comment)

[tnull]

We don't have to test on MSRV we just have to check that it builds on MSRV. dev-dependencies shouldn't have to be pinned.

I disagree: MSRV means "This project supports/works with this rust version", and, everything untested may be considered broken. I regularly run into MSRV breakages for depedencies that 'check' but don't test their MSRV on all supported platforms.

[LLFourn]

We don't have to test on MSRV we just have to check that it builds on MSRV. dev-dependencies shouldn't have to be pinned.

I disagree: MSRV means "This project supports/works with this rust version", and, everything untested may be considered broken. I regularly run into MSRV breakages for depedencies that 'check' but don't test their MSRV on all supported platforms.

This "regularly" is quite concerning tbh 😅. It's a good point that if our tests don't have the same MSRV as build then there's no way to verify that the tests actually pass on the MSRV. I want to add that this situation would imply a bug in the rust compiler, either at the version in CI or at the MSRV. Either that or some weird rust version based conditional compilation in a dependency.

For me the monotonically growing list of dependency pins and using old versions of dev dependencies is a worse problem than this. I definitely don't take the attitude that "everything untested may be considered broken" when developing in rust. Logic that is not tested can be considered broken but not everything :)

Not a hill I'll die on though and if the maintainers who are really affected by this want to take the stricter interpretation of MSRV that's fine with me!

[tnull]

This "regularly" is quite concerning tbh 😅. It's a good point that if our tests don't have the same MSRV as build then there's no way to verify that the tests actually pass on the MSRV. I want to add that this situation would imply a bug in the rust compiler, either at the version in CI or at the MSRV. Either that or some weird rust version based conditional compilation in a dependency.

Yeah, at the very least building with MSRV on every supported platform should be covered by CI. I previously suggested that to other projects which deemed it unnecessary, only to introduce MSRV unnoticed violations on some of the platforms (looking at you reqwest).

For me the monotonically growing list of dependency pins and using old versions of dev dependencies is a worse problem than this.

Hopefully it's not monotonically growing. Many projects now support an MSRV compatible with BDK's, but they tend to not check it and violate it at some point in time. In my experience they are often willing to fix it, at which point the pins can be dropped again (also looking an reqwest, for example).

I definitely don't take the attitude that "everything untested may be considered broken" when developing in rust. Logic that is not tested can be considered broken but not everything :)

Yeah, fair, that was was of course hyperbolic :)

[notmandatory]

Since updating MSRV to 1.63 we were able to remove many of the originally pinned versions in CI. As 1.63 gets older this list will likely grow again and we'll need to re-verify they're still needed but otherwise I don't see any other way to reduce the pins except bumping our MSRV at least when ever a new Debian stable release comes out with an updated Rust version.

[evanlinjin]

I don't think we need to make sure tests run under MSRV, it just makes testing unnecessarily difficult. We can't use the latest tooling. There is no workaround to this because dev-dependencies cannot be optional.

Another pain-point for enforcing MSRV for tests is that our "pin for MSRV" section in the README may need two versions; one for tests, and one for building (as dependencies for each may vary).

[notmandatory]

I'm open to taking this approach but would like to know if there are any other big projects who handle CI MSRV maintenance this way?

Also if we update our CI to not use the automatic binary download feature of bitcoind and electrsd (install locally in a custom docker image instead) I think we can eliminate a bunch of the troublesome dev dependencies, ie. zstd-sys, time, home. The only other pinned dep that is only for testing is proptest.

[notmandatory]

Can we move this out of the 1.0.0 milestone since it will require some discussion and only affects CI?

[oleonardolima]

Also if we update our CI to not use the automatic binary download feature of bitcoind and electrsd (install locally in a custom docker image instead) I think we can eliminate a bunch of the troublesome dev dependencies, ie. zstd-sys, time, home. The only other pinned dep that is only for testing is proptest.

It's also valid to notice that bitcoind development moved to corepc, that probably should be taken into consideration too.

[notmandatory]

I think all we need to do to fix this is exclude bdk_testenv when doing the MSRV build and don't try to run the tests when using the MSRV. I made these changes in bitcoindevkit/bdk#1776 and only 6 non-test related dependencies needed to be pinned.

[notmandatory]

I think all we need to do to fix this is exclude bdk_testenv when doing the MSRV build and don't try to run the tests when using the MSRV. I made these changes in bitcoindevkit/bdk#1776 and only 6 non-test related dependencies needed to be pinned.

I removed this change from 1776 since we probably should always run tests with MSRV also. See also: bitcoindevkit/bdk#1776 (review)