rust-bitcoin/rust-bitcoinconsensus

v0.20.2-0.6.1 Broke rust-bitcoin 0.30.2

Closed this issue · 11 comments

The point release is still broken, it seems:

error[E0277]: can't compare `bitcoinconsensus::Error` with `bitcoinconsensus::Error`
   --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bitcoin-0.30.2/src/blockdata/script/mod.rs:743:22
    |
730 | #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
    |                         ---------- in this derive macro expansion
...
743 |     BitcoinConsensus(bitcoinconsensus::Error),
    |                      ^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `bitcoinconsensus::Error < bitcoinconsensus::Error` and `bitcoinconsensus::Error > bitcoinconsensus::Error`
    |
    = help: the trait `PartialOrd` is not implemented for `bitcoinconsensus::Error`
    = note: this error originates in the derive macro `PartialOrd` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `bitcoinconsensus::Error: Ord` is not satisfied
   --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bitcoin-0.30.2/src/blockdata/script/mod.rs:743:22
    |
730 | #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
    |                                     --- in this derive macro expansion
...
743 |     BitcoinConsensus(bitcoinconsensus::Error),
    |                      ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Ord` is not implemented for `bitcoinconsensus::Error`
    |
    = note: this error originates in the derive macro `Ord` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `bitcoinconsensus::Error: std::hash::Hash` is not satisfied
   --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bitcoin-0.30.2/src/blockdata/script/mod.rs:743:22
    |
730 | #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
    |                                          ---- in this derive macro expansion
...
743 |     BitcoinConsensus(bitcoinconsensus::Error),
    |                      ^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::hash::Hash` is not implemented for `bitcoinconsensus::Error`
    |
    = note: this error originates in the derive macro `Hash` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `bitcoin` (lib) due to 3 previous errors

Looks like #72 also needs to include PartialOrd.

I'll yank this release again. After this we really need to fix our versioning..

OMG, I only tested against the most recent release of rust-bitcoin.

I don't think we want to yank the point release, we do not want to add PartialOrd back in.

2 possible solutions I can think of:

  1. Do a point release rust-bitcoin v0.30.3 that uses = in the dependency of bitocinconsensus
  2. Add a note in the readme that to build rust-bitcoin version < 0.31.0 one needs to pin bitcoinconsensus
  1. is a huge ecosystem-wide pain and a non-starter IMO. An option 3. would be a point release of rust-bitcoin that doesn't rely on PartialOrd, which would mean manually implementing Ord on script::Error. Would be awkward, but its an option.

We can't technically remove the PartialOrd implementation in a point release though.

I don't think we want to yank the point release, we do not want to add PartialOrd back in.

I already yanked it because it removes PartialOrd in something that cargo views to be a patch version.

If we want to remove it, we should first change our versioning scheme.

Fair and fair.

I'm out of ideas on a versioning scheme that works. I'll start a discussions thread so we can talk in one place.

This mess is all cleaned up now, the latest releases use a different versioning number scheme, it even plays nicely with the crates.io color scheme!

https://crates.io/crates/bitcoinconsensus/versions