rustpq / pqcrypto

Rust Post-Quantum cryptography
212 stars 38 forks source link

support cross-compilation by not using the cfg macro in build.rs #15

Closed noonien closed 3 years ago

noonien commented 3 years ago

Fixes #14

noonien commented 3 years ago

Opps, missed that.

It's fixed now. Also did a cargo check and cargo test in the workspace to make sure everything compiles, and runs.

There is a SIGSEGV in pqcrypto-classicmceliece, however, the error exists in the old code as well.

thomwiggers commented 3 years ago

There is a SIGSEGV in pqcrypto-classicmceliece, however, the error exists in the old code as well.

Yeah, I think there's some issue in the upstream code but I had a hard time locking it down.

noonien commented 3 years ago

I'm not exactly sure what the disable_avx2 opt-in feature is. Will it be passed using a cfg_attr macro? Because it's not defined in Config.tom. If so, are you sure that is the right place to use it like this? I'm not sure those config attrs will be available in build.rs.

thomwiggers commented 3 years ago

Some good questions :sweat_smile: I think I used it via -C disable_avx2 for testing.

noonien commented 3 years ago

Was that set in RUSTFLAGS?

thomwiggers commented 3 years ago

I think so, yeah

noonien commented 3 years ago

RUSTFLAGS also doesn't work when cross-compiling. I'll add a feature to Cargo.toml.

thomwiggers commented 3 years ago

It's still helpful to have, it allows me to test the non-AVX2 codepaths on an AVX2 machine.

noonien commented 3 years ago

Fixed.

I added a feature in Cargo.toml for packages that support avx2. To run without avx2 support use:

cargo build --no-default-features
noonien commented 3 years ago

Thanks, cheers!

:handshake:

thomwiggers commented 3 years ago

Thanks for the contribution

noonien commented 3 years ago

Any chance to get this change in a release?

thomwiggers commented 3 years ago

I've run the script.

I also should update everything with PQClean changes, but I'm not sure if I'll have time for that anytime soon...