build script is wrong

[Noratrieb]

The build script in this repository is wrong in many ways.

https://github.com/ogxd/gxhash/blob/c9c9f61f202be9a5b3261913f2a4c2f8c883caa6/build.rs#L5-L6

Checking for nightly to enable features should not be done. If the feature ever changes, builds using old versions of this library on new nightlies will break, which is highly undesirable. Not everyone that uses nightly wants to use unstable features, and they should never be implicitly enabled behind the users back. Use a feature that can be enabled by the user or some global cfg instead.

https://github.com/ogxd/gxhash/blob/c9c9f61f202be9a5b3261913f2a4c2f8c883caa6/build.rs#L7-L9

This checks the feature of the build script, so the host. This makes no sense, the host bears no resemblance to the target.

https://github.com/ogxd/gxhash/blob/c9c9f61f202be9a5b3261913f2a4c2f8c883caa6/build.rs#L13-L15

While this attempts to handle cross compilation (unlike the previous check), it does not handle it correctly. By default, this works. But when using cargo cross compilation mode, even for the same target as the host (by passing --target explicitly, which is done by Miri and also required for -Zbuild-std), then RUSTFLAGS only apply to the target binaries, not programs on the host. Using cfg in the build script to attempt to detect whether the target feature is enabled does not work. Instead, check the cfg in the code and compile_error! when it is not enabled.

[ambiso]

I used

extern crate rustc_version;
use rustc_version::{version_meta, Channel};

fn main() {
    // When conditions permits, enable hybrid feature to leverage wider intrinsics for even more throughput
    let features = std::env::var("CARGO_CFG_TARGET_FEATURE").unwrap();
    let features = features.split(",").collect::<Vec<_>>();
    let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").unwrap();
    if version_meta().unwrap().channel == Channel::Nightly
    && target_arch == "x86_64"
    && features.contains(&"avx2")
    && features.contains(&"vaes") {
        println!("cargo:rustc-cfg=hybrid");
    }

    // If not cross compiling, make sure the aes feature is available
    if std::env::var("HOST").unwrap_or_default() == std::env::var("TARGET").unwrap_or_default()
    && !features.contains(&"aes") {
        panic!("| GxHash requires target-feature 'aes' to be enabled.\n\
        | Build with RUSTFLAGS=\"-C target-cpu=native\" or RUSTFLAGS=\"-C target-feature=+aes\" to enable.");
    }
}

in my testing

[ogxd]

Hello @Nilstrieb ! Thanks a lot for filling this issue, I agree with everything you've said. I did not know about compile_error!, it seems things can be made a lot more simpler this way.

I have made a branch for experimenting and added a Github action to perform cross-compilation tests. For some reason it does not build in some scenarios and needs further investigation: https://github.com/ogxd/gxhash/actions/runs/9134920082