Repository: dalek-cryptography/curve25519-dalek (language: rust)

Issue:
Default `curve25519_dalek_bits` on non-standard Rust target / build tooling
**Follow-up**

- #508 and
- #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 https://github.com/dalek-cryptography/curve25519-dalek/issues/508#issuecomment-1465049852 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


Discussion / hints:
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.
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.
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.
Is there an advantage to using these new variables vs. the `CARGO_CFG` variable which @ryankurte suggested?

Fix patch:
diff --git a/build.rs b/build.rs
index a4c6b3e65..ca28d72b4 100644
--- a/build.rs
+++ b/build.rs
@@ -4,21 +4,16 @@
 
 #[allow(non_camel_case_types)]
 enum DalekBits {
-    #[cfg_attr(curve25519_dalek_bits = "64", allow(dead_code))]
     Dalek32,
-    #[cfg_attr(curve25519_dalek_bits = "32", allow(dead_code))]
     Dalek64,
 }
 
 fn main() {
-    #[cfg(curve25519_dalek_bits = "32")]
-    let curve25519_dalek_bits = DalekBits::Dalek32;
-
-    #[cfg(curve25519_dalek_bits = "64")]
-    let curve25519_dalek_bits = DalekBits::Dalek64;
-
-    #[cfg(all(not(curve25519_dalek_bits = "64"), not(curve25519_dalek_bits = "32")))]
-    let curve25519_dalek_bits = deterministic::determine_curve25519_dalek_bits();
+    let curve25519_dalek_bits = match std::env::var("CARGO_CFG_CURVE25519_DALEK_BITS").as_deref() {
+        Ok("32") => DalekBits::Dalek32,
+        Ok("64") => DalekBits::Dalek64,
+        _ => deterministic::determine_curve25519_dalek_bits(),
+    };
 
     match curve25519_dalek_bits {
         DalekBits::Dalek64 => println!("cargo:rustc-cfg=curve25519_dalek_bits=\"64\""),
@@ -27,22 +22,19 @@ fn main() {
 }
 
 // Deterministic cfg(curve25519_dalek_bits) when this is not explicitly set.
-#[cfg(all(not(curve25519_dalek_bits = "64"), not(curve25519_dalek_bits = "32")))]
 mod deterministic {
 
     use super::*;
 
     // Standard Cargo TARGET environment variable of triplet is required
-    static ERR_MSG_NO_TARGET: &str = "Standard Cargo TARGET environment variable is not set.";
+    static ERR_MSG_NO_TARGET: &str = "Standard Cargo TARGET environment variable is not set";
 
     // Custom Non-Rust standard target platforms require explicit settings.
     static ERR_MSG_NO_PLATFORM: &str = "Unknown Rust target platform.";
 
-    // Error handling when the bits setting cannot be determined
-    fn determine_curve25519_dalek_bits_error(cause: &str) -> ! {
-        eprintln!("Error: {cause}");
-        eprintln!("Please set cfg(curve25519_dalek_bits) explicitly either as 32 or 64.");
-        std::process::exit(1)
+    // Warning when the curve25519_dalek_bits cannot be determined
+    fn determine_curve25519_dalek_bits_warning(cause: &str) {
+        println!("cargo:warning=\"Defaulting to curve25519_dalek_bits=32: {cause}\"");
     }
 
     // Determine the curve25519_dalek_bits based on Rust standard TARGET triplet
@@ -53,13 +45,19 @@ mod deterministic {
         // https://doc.rust-lang.org/cargo/reference/environment-variables.html
         let target_triplet = match std::env::var("TARGET") {
             Ok(t) => t,
-            Err(_) => determine_curve25519_dalek_bits_error(ERR_MSG_NO_TARGET),
+            Err(_) => {
+                determine_curve25519_dalek_bits_warning(ERR_MSG_NO_TARGET);
+                return DalekBits::Dalek32;
+            }
         };
 
         // platforms crate is the source of truth used to determine the platform
         let platform = match platforms::Platform::find(&target_triplet) {
             Some(p) => p,
-            None => determine_curve25519_dalek_bits_error(ERR_MSG_NO_PLATFORM),
+            None => {
+                determine_curve25519_dalek_bits_warning(ERR_MSG_NO_PLATFORM);
+                return DalekBits::Dalek32;
+            }
         };
 
         #[allow(clippy::match_single_binding)]


Test patch: