Release tracking PR: `v0.100.0+0.20.2`

[tcharding]

Currently we use a version string of the form 0.20.2-0.6.1

While attempting to release of the 0.6 we found out that cargo pulls down this version if a crate depends transitively on 0.20.2-0.5.0 - this implies that we should not have made breaking changes in the release, which we did.

We have since yanked both the 0.6 releases.

In order to function correctly with cargo change the format to put the crate version first and put the Core version second, note we use + (build) instead of - (pre-release). This should make crates.io display the latest version correctly also.

[dpc]

While attempting to release of the 0.6 we found out that cargo ignores the stuff after the - so the newly released code is treated as a non-breaking change

Hmm... I'm not sure if this is strictly true:

Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version.

[Kixunil]

That first warning is just your local one - depend on 0.100.0, don't includethe + part.

I have no clue about missing initializers.

[apoelstra]

I would guess that the missing initializers would be fixed by updating to a more recent version of libbitcoinconsensus here.

[apoelstra]

I'll hold off on merging @tcharding but I think those warnings are fine. One is local, as Kix says, and the other is saying that we're using a multi-year-old libsecp, which we know because we have 0.20.2 in the version.

[tcharding]

Awesome. Verified the error is local, using 0.100.0 removes it. Good to merge and release please.

[tcharding]

Hmm... I'm not sure if this is strictly true:

I've updated the PR description to just say what happened not why.

[Kixunil]

Though is the release doing anything other than change the numbers? Also I think we could remove some of the traits again if it makes sense.

[tcharding]

Its the same code that is on tip of master that we released in the 0.20.2-0.6.1 version, so it has the traits correct for current rust-bitcoin.

#[allow(non_camel_case_types)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[repr(C)]
pub enum Error {
    /// Default value, passed to `libbitcoinconsensus` as a return parameter.
    ERR_SCRIPT = 0,
    /// An invalid index for `txTo`.
    ERR_TX_INDEX,
    /// `txToLen` did not match with the size of `txTo`.
    ERR_TX_SIZE_MISMATCH,
    /// An error deserializing `txTo`.
    ERR_TX_DESERIALIZE,
    /// Input amount is required if WITNESS is used.
    ERR_AMOUNT_REQUIRED,
    /// Script verification `flags` are invalid (i.e. not part of the libconsensus interface).
    ERR_INVALID_FLAGS,
}

[tcharding]

FTR I left Copy in there and did not use non_exhaustive because the error type is coupled with the Core version.

[dpc]

I've updated the PR description to just say what happened not why.

While attempting to release of the 0.6 we found out that cargo pulls down this version if a crate depends transitively on 0.20.2-0.5.0 - this implies that we should not have made breaking changes in the release, which we did.

I'm not entirely sure, but IIUC what is happening is that cargo treats all pre-releases as patch-compatible and automatically will update from one pre-release to another.

There seems to be a RFC to change it: https://github.com/rust-lang/rfcs/pull/3263

[tcharding]

Ah cool, thanks. So I wasn't mad thinking this would be how cargo would handle pre-releases.

[apoelstra]

Its the same code that is on tip of master that we released in the 0.20.2-0.6.1 version, so it has the traits correct for current rust-bitcoin.

Do we intend to reduce this set in the near future in rust-bitcoin?

[tcharding]

Since this only applies to one tye, the Error enum, I believe not. I believe it is correct and follows our current policy on error types.

[apoelstra]

Ok, great. I will merge, tag and release.

[apoelstra]

Tagged and published.

[tcharding]

Legend.

[Kixunil]

So this release is basically a no-op. The traits are correct though maybe the enum should've been non-Copy, non_exhaustive and hiding details.

[apoelstra]

It removes Ord and Hash from the list of derives, which isn't a no-op.

It also rearranges the ffi module a bit, see #71, which we forgot about amidst all this discussion since it didn't break anybody.

[Kixunil]

Ah, OK, I got confused.

[tcharding]

The traits are correct though maybe the enum should've been non-Copy, non_exhaustive and hiding details.

This error is special because it is the exact values from Core so it tracks the Core version. For that reason I intentionally did not use non_exhaustive, used Copy, and did not hide the details, i.e., it is part of the public API for that Core version - at least that's how I understand it.

[Kixunil]

Yeah, but since Core doesn't use semver it's kinda meaningless and we're just creating more breaking changes for ourselves forcing us to update both this library and bitcoin. Anyway, I think this should have -sys crate which could define it as exhaustive and safe bindings that are non_exhaustive. But maybe it's not worth it for a small one like this.

[tcharding]

but since Core doesn't use semver

Huh? What do you mean (https://bitcoincore.org/en/releases/)?

I played with adding a -sys crate but its so tiny I just added the ffi module instead.

[Kixunil]

What do you mean

Literally that there's no place where Core declares that its versioning scheme is semver.

but its so tiny

Switching between bundled and external may one day be an argument to have it.

[tcharding]

What do you mean

Literally that there's no place where Core declares that its versioning scheme is semver.

So version x.y.z does not make any claim to be semver? It seems to imply that, to me at least.

[Kixunil]

Just because versions are numbered doesn't mean they are semver. Each project must specify that it is explicitly in its documentation.

[tcharding]

ok, TIL. thanks