Default to no_std

[tcharding]

Default to no_std for hashes and bitcoin - all other crates are already done.

Turns out this one was a bit more involved that I expected, its a fair bit of messing around in tests but I think its worth it to give us more confidence in no_std.

Close: #2413

[tcharding]

Strange errors showing up in CI on this and other PRs?

[apoelstra]

This one looks real; it's related to imports of types which are affected by nostd.

[tcharding]

No worries, thanks. Looks like I have some more work to do. Will come back to this.

[coveralls]

Pull Request Test Coverage Report for Build 8946096346

Details

• 19 of 20 (95.0%) changed or added relevant lines in 9 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 83.174%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
hashes/src/hmac.rs	0	1	0.0%

Totals
Change from base Build 8939996688:	0.01%
Covered Lines:	19209
Relevant Lines:	23095

---

💛 - Coveralls

[tcharding]

Note to self, consider updating manifest catagories and keywords to include no-std same as in bitcoin-io.

[apoelstra]

Hashes does not build with --no-default-features --features serde 5f38652

[tcharding]

Interesting, that exposes a hole in CI also then. Will investigate, thanks. Tip your local CI bot for me, he is doing good work.

EDIT: oh, this is not a hole in CI, slipped through because we don't check every patch only the final PR state.

[tcharding]

This is getting a bit silly, one has to wonder at if there is not a better way

#[cfg(test)]
mod tests {
    #[cfg(feature = "alloc")]
    #[allow(unused_imports)] // Less maintenance if we just import these.
    use crate::alloc::{format, string::ToString, vec, vec::Vec};
    #[cfg(any(feature = "alloc", feature = "serde"))]
    use crate::{sha256d, Hash as _};

[apoelstra]

This is partially fallout from rust-lang/rust#121708 .. though I think it actually predates that issue.

We have a couple options. One is to just allow redundant imports on all our test modules. I'm not thrilled with this because imports do become stale. A better one would be (similar to what you've done) to cfg_attr allowing redundant imports unless all features are enabled. (But I'm uncertain how to specify "all features". Could be gross to do.)

[tcharding]

What if we had a test prelude module (ie., gated on #[cfg(test)]) that imported all the alloc related rubbish and also all the individual hash modules, and we allow unused imports in it. Then in each unit test can just do use crate::test_prelude::*;

I believe that wildcard imports do not trigger redundant import warnings, which is nice. We'd have to be careful that for crates with a prelude that it and the test_prelude didn't get out of sync.

[tcharding]

Could be gross to do.

"gross", what are you, five?

(Not sure if that joke translates to text, but I mean it in the most jovial of ribbing ways.)

[apoelstra]

I'm not a huge fan of prelude modules, even for tests, because it means that you can't search for the location of symbols within the file that they appear in.

[tcharding]

Yeah I've heard you say that before, maybe I'm missing something, but when do you ever need to search for alloc/core/std type e.g,. Vec?

[tcharding]

Oh, I literally just suggested putting non-std types in a prelude :(

[tcharding]

Just FTR this PR is good to merge, we are just having a discussion on the pain of imports for no_std crates.

[apoelstra]

but when do you ever need to search for alloc/core/std type e.g,. Vec?

To understand what set of feature-gates are controlling their import logic, which is important when reviewing code that should work in a no-std context.

But yes, for non-std types the problem with wildcards is even clearer.

[tcharding]

but when do you ever need to search for alloc/core/std type e.g,. Vec?

To understand what set of feature-gates are controlling their import logic, which is important when reviewing code that should work in a no-std context.

Interesting, so are you saying that when you review no-std code you are thinking "what are the feature gate requirements for this code"? And then you like to look at the imports to see if it matches your thoughts?

I always have treated prelude to mean "there are a set of feature gated imports that work with this code" and because its in one place it implies "and they are sane because we looked before and don't change them much". Where as when the feature gated imports are in each place I have to reason about them to check if they are sane.

[apoelstra]

Interesting, so are you saying that when you review no-std code you are thinking "what are the feature gate requirements for this code"? And then you like to look at the imports to see if it matches your thoughts?

No, only when they break.

they are sane because we looked before and don't change them much

If only..

[tcharding]

Needs rebase, this feature gating is boring as hell - leaving rebasing until after the one-ack thing comes in and we have some hope of merging this without more rebasing.