Skip to content
This repository was archived by the owner on Nov 30, 2022. It is now read-only.

Test in CI if the lib works in embedded #119

Closed
wants to merge 3 commits into from

Conversation

RCasatta
Copy link
Contributor

@RCasatta RCasatta commented Apr 9, 2021

on top of #118

Test in CI if the lib works in embedded envs (it only builds because running would require virtualization over virtualization which I don't think is supported in github actions at the moment, cargo run works locally with qemu for arm)

Reveals issue with the way core is exported in impl_newtype! macro https://github.com/RCasatta/bitcoin_hashes/runs/2306757898?check_suite_focus=true

When fixing $crate::core in 40e9fb7 it creates an issue with the MSRV https://github.com/RCasatta/bitcoin_hashes/actions/runs/733467719 (works from 1.31)

Any idea how to be compatible with 1.29 ?

FIXED

@elichai
Copy link
Member

elichai commented Apr 10, 2021

Thanks, that's a great idea.
How do you think your way compares to https://github.com/rust-bitcoin/rust-secp256k1/tree/master/no_std_test (this runs on x64 and uses the linker to make sure libstd doesn't link in)

@RCasatta
Copy link
Contributor Author

RCasatta commented Apr 10, 2021

Oh, I didn't saw no_std_test in rust-secp256k1...

I think this way is more a concrete embedded situation (it's taken from the https://github.com/rust-embedded/cortex-m-quickstart). Advantages may be: enforced limited memory (UPDATE, maybe too limited! I noticed 64k wouldn't be enough for the secp256k1 allocation of 70k... UPDATE2: there are other supported machine instead of lm3s6965evb with much memory like mps2-an511) and the possibility to use real implementation for print to stdout and rng. While the downside is to require arm virtualization.

I think the important thing is to have one or the other (or even both if they provide differents check) in the CI

@RCasatta
Copy link
Contributor Author

RCasatta commented Apr 11, 2021

Any idea how to be compatible with 1.29 ?

Probably the best option is to have different MSRV for std (1.29) and no_std (1.36) since I think it's hard to do without alloc

(along the lines of rust-bitcoin/rust-bech32#51)

@RCasatta RCasatta marked this pull request as ready for review April 12, 2021 12:09
@RCasatta RCasatta marked this pull request as draft April 14, 2021 21:55
@RCasatta
Copy link
Contributor Author

superseded by #122

@RCasatta RCasatta closed this Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants