dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
853 stars 422 forks source link

Pointer width detection in `build.rs` may break for obscure platforms #610

Closed Shnatsel closed 4 months ago

Shnatsel commented 7 months ago

The build.rs file currently uses the platforms crate to map the rustc target triples to pointer width. I am the author of the platforms crate :wave:

The platforms crate adds and removes targets following the target support in the latest release of rustc. This means that the list of targets it knows about may be different from the list of targets that your local rustc knows about.

It would be better to query the Rust compiler directly instead of relying on the platforms crate. You can do it with rustc --print=cfg --target=TRIPLE, e.g. rustc --print=cfg --target x86_64-unknown-uefi will print the pointer width, among other things. By sourcing it from the compiler that performs the build, there is no chance of the pointer width data to be missing.

pinkforest commented 5 months ago

Thanks - Yeah thought about that but platforms was just ergonomic at the time.

I also thought we could expand to rustc-metadata but then there was already rustc-cfg which we could add and could be suited for the task either with modifications or directly.

But we've used environment variables for this which even rustc-cfg recommends over that create - also saving dep as a bonus.

So I guess we can just read the environment variable CARGO_CFG_TARGET_POINTER_WIDTH instead and drop the dep indeed despite it being slightly ergonomic and giving better idea about obscure platforms as you've noted :+1:

EDIT: One downside is that we need to deal with string match when figuring out whether a platform default override is in future - and it could be helpful to turn this into type proper if / when there is decision to set any wasm/aarch64 target_pointer_width as default 64 over 32 - nonetheless ideally these should be set explciitly via curve25519_dalek_bits