Default curve25519_dalek_bits on non-standard Rust target / build tooling

[pinkforest]

Follow-up

• Build script failure on platform detection #508 and
• Fix error for custom targets #510

Use-Case

• a) Non-standard Rust build tooling which doesn't signal cfg correctly and

• b) Custom non-Rust targets which are not known or

• c) Situations where the RUSTENV was set incorrectly but relied on

Current

We spit out an error #508 (comment) where we ask the downstream to set the desired cfg(curve25519_dalek_bits)

This ensures that the downstream is consciously selecting the correct curve25519_dalek_bits if the downstream is attempting to use the cfg to set it and errors out appropriately when the target platform is non-standard Rust target with unknown target_pointer_width

Alternative

Set the cfg(curve25519_dalek_bits) to 32 silently if either the cfg was not signalled or a custom non-standard Rust target.

This would have a downside where it would silently set something else but it would "just work" on other hand but silently.

Potential Improvements

1. Maybe we could print out a warning ? Do people read warnings ?

2. Also maybe could add some tests that simulate these error messages.

3. Document these error messages in README

[pinkforest]

It seems some of these are using a Makefile where simple thing is just to add the environment variable in there.

e.g. this is just a matter of education given we've changed this - educational blog coming up I think will suffice here.

Will just add some wordding around Makefile

Also some people may be using Cross.toml -

https://github.com/cross-rs/cross#passing-environment-variables-into-the-build-environment

There is passthrough option which people usually use with RUSTFLAGS to relay it to rustc.

[tarcieri]

I think picking some default and if possible printing a warning is the way to go.

It seems enough people have hit this even in prerelease testing we should do something about it.

[pinkforest]

I suspect this may be a cargo / rustc bug - I have a feeling it doesn't allow passing cfg with custom targets.

I want to get to bottom of this why even setting the cfg does work as that would mean we have problems setting backend.

So if we introduce the environment variables for build:

curve25519_dalek_bits and
curve25519_dalek_backend

This would work/around the rustc issue not setting cfg correctly and allows people set these.

These would be additional where the cfg if present would always win.

Even if we hack a default for bits then people would be having problems setting these that is an issue as well we need to address.

[jcape]

Is there an advantage to using these new variables vs. the CARGO_CFG variable which @ryankurte suggested?

[pinkforest]

Does it relay the CARGO_CFG correctly considering the derive cfg-predicates didn't work unlike in standard targets ?

Vary of using namespace that is affected by a cargo / rustc bug setting them.

EDIT: @jcape the PR is up #516 with @ryankurte fix - as indeed that variable works across all the rust versions we support and various configurations.

Given the complexity of setting these cfg flags we might just fallback to u32 and print out that warning.

EDIT: I just checked that there seems to be no way of doing "warnings" in build scripts 🤷‍♀️

So it's either hard error or transparently set something the end-user might not have intended to be set.

[jcape]

FWIW, you can use println!("cargo:warning=the string");, or https://docs.rs/cargo-emit/latest/cargo_emit/macro.warning.html though that's not something that will stop the world (by itself) with -D warnings.

[pinkforest]

Ah thanks! Think we'll just default to Dalek32 and put that one out then.