Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curve: Fix no_std for fiat backend and add test for it #572

Merged
merged 3 commits into from
Sep 4, 2023

Conversation

pinkforest
Copy link
Contributor

@pinkforest pinkforest commented Sep 4, 2023

Fiat backend required std as we didn't call it with default-features - false - I've changed it here.

We only test the serial (default) backend in no_std that gets tested changes on any crates

This adds fiat backend specific no_std test when there is/are changes on curve25519-dalek in addition.

Given mit-plv/fiat-crypto#1646 it may be feasible to fiat is tested as well

This should be merged and re-based into the fiat-backend 0.2 bump after this has been tested with 0.1 via this PR

@pinkforest pinkforest changed the title curve: Add no_std test for fiat backend curve: Fix no_std for fiat backend and add test for it Sep 4, 2023
@tarcieri
Copy link
Contributor

tarcieri commented Sep 4, 2023

Suddenly I'm confused why there's both a no_std.yml workflow file and no_std jobs within curve25519-dalek.yml

@pinkforest
Copy link
Contributor Author

pinkforest commented Sep 4, 2023

Yeah no_std.yml gets applied to whole workspace and curve25519-dalek.yml only to curve25519 changes -

Maybe we can rename the no_std.yml to workspace-no_std.yml regardless ?

Or just merge no_std.yml into workspace.yml - EDIT: Done

I thought fiat no_std needs to be only tested when curve25519-dalek has changes - serial gets tested always in any case

Do we want to test both fiat and serial (default) in no_std context across all crates ?

@tarcieri tarcieri merged commit 5c5a320 into dalek-cryptography:main Sep 4, 2023
22 checks passed
@pinkforest pinkforest mentioned this pull request Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants