Check for changes to the public API

[tcharding]

We would like to get to a stage where we can commit to the public API. To help us achieve this add a script that generates the public API and checks it against three committed files, one for each feature set: no features, alloc, std.

The idea is that with this applied any PR that changes the public API should include a final patch that is just the changes to the api/*.txt files, that way reviewers can discuss the changes without even needing to look at the code, quickly giving concept ACK/NACKs. We also run the script in CI to make sure we have not accidentally changed the public API so that we can be confident that don't break semver during releases. The script can also be used to diff between two release versions to get a complete list of API changes, useful for writing release notes and for users upgrading.

There is a development burden involved if we apply this patch.

[tcharding]

Same as:

• Check for changes to the public API rust-bech32#141
• Check for changes to the public API rust-bitcoin#1880

[tcharding]

I did this so I could check that #655 was needed, the feature sets are blindly copied from other PRs without any thought, will re-visit if this gets a concept ack.

[apoelstra]

concept ACK, but yes, the feature set does not make sense for this library.

I think here we can go ahead and ACK/merge this. I'll maybe leave a week for open comments. But unlike rust-bitcoin, this library is pretty-much "single owner" (me) so we can push forward without buy-in from others.

[tcharding]

Sweet, I'll fix it up the feature sets then. Thanks

[tcharding]

I went with:

1. no-default-features
2. alloc,rand,hashes
3. all-features

Is (2) right? Perhaps no "rand"? Would it be good to have another that is just "std"?

[apoelstra]

I think we should do --no-default-features, nothing (default features), and --all-features.

In my view there are two reasons to treat a particular feature specially:

• If we are worried that a gated feature will have more gates added to it (so the original feature loses functionality)
• If we have code that is not(feature) so it's important to test with or without that feature

So with this in mind, std is probably the only feature gate that really qualifies.

In principle, we could have API breaks that affect any set of features. Especially if we are changing the feature-gates themselves. But it's not practical to do a full matrix.

When we fix up the context rerandomization logic we'll probably have to be careful and manually look for API breaks because we might add complicated not(rand) gates. But otherwise contributions to this library use features in a very simple way: if a particular function needs a particular dep, it gets a gate, otherwise it doesn't. And we rarely change feature-gates or remove functionality.

[tcharding]

Nice explanation, thanks man. I had not thought about the "(not(feature))" thing but I guess my intuition behind (2) was correct. Grepping for 'not(feature' we get

secp256k1-sys/src/lib.rs:10: #![cfg_attr(all(not(test), not(feature = "std")), no_std)]
secp256k1-sys/src/lib.rs:1102:    #[cfg(not(feature = "lowmemory"))]
src/context.rs:52:                     not(feature = "global-context-less-secure")
src/context.rs:198:        #[cfg_attr(not(feature = "rand-std"), allow(clippy::let_and_return, unused_mut))]
src/context.rs:218:                not(feature = "global-context-less-secure")
src/key.rs:1027:        #[cfg(all(not(feature = "global-context"), feature = "alloc"))]
src/key.rs:1071:                #[cfg(all(not(feature = "global-context"), feature = "alloc"))]
src/lib.rs:142:#![cfg_attr(all(not(test), not(feature = "std")), no_std)]
src/macros.rs:84:             #[cfg(not(feature = "std"))]
src/secret.rs:35:         #[cfg(all(not(feature = "std"), feature = "hashes"))]
src/secret.rs:53:         #[cfg(all(not(feature = "std"), not(feature = "hashes")))]

Do we want to do one with "alloc" on and "global-context" off (because of keys.rs)? And maybe turn on "hashes" as well because of secrets.rs?

[apoelstra]

Yeah, I like that idea. So I would suggest:

• No default features (nostd)
• No default features + alloc
• No default features + alloc + global context
• Default features (std, no global context)
• Everything

I wouldn't worry about hashes, even though we do have a not(hashes), because it's a minor thing and I don't want to blow up the matrix. Similarly I feel like we should have a "std" and "std + global context" ... but again, 4 options already seems like a lot.

[tcharding]

No default features + alloc + global context

You mean plain old "global-context", right? ("global-context" turns on "std")

So we'd then have "std" with and with "global-context".

[tcharding]

Force push makes it so we have:

all-features.txt  alloc.txt  default-features.txt  global-context.txt  no-default-features.txt

[apoelstra]

ACK e9e17a0

[apoelstra]

Let's do this!