Allowing unstable features on any nightly Rust version is a time bomb that will break crates

[kornelski]

version_check/src/lib.rs

Lines 349 to 351 in d77ef9f

	// If there are no `RUSTFLAGS` or `CARGO_ENCODED_RUSTFLAGS` or they don't
	// contain an `allow-features` flag, assume compiler allows all features.
	Some(true)

By default Cargo does not pass any rustflags. These env vars are for users' extra flags, and don't include the usual flags used when compiling, so de-facto there's no filtering, and version_check naively allows any feature flag on any (edit: nightly) Rust version.

Rust renames, removes, or stabilizes nightly flags without warning. Crates checking for flags using version_check will sooner or later end up breaking the build by using flags that may no longer exist.

This check needs to be more robust. At least try compiling a file with #![feature(…)] in it to check if it still exists.

[SergioBenitez]

You issue misrepresents what version_check does. The comment in the snippet, "assume compiler allows all features," doesn't exist in a vacuum. It's preceded by other checks which, if they fail, cause the function to return early, meaning we never reach the comment. Namely:

version_check/src/lib.rs

Lines 325 to 329 in d77ef9f

	match is_feature_flaggable() {
	Some(true) => { /* continue */ }
	Some(false) => return Some(false),
	None => return None,
	}

So saying that version_check "naively allows any feature flag on any Rust version" is wrong.

These env vars are for users' extra flags, and don't include the usual flags used when compiling, so de-facto there's no filtering [..]

Perhaps you've misunderstood what this function is doing with respect to these environment variables. The point is that you can disable features via these flags, and here we're checking if they've done so. Given that these flags enable a blacklist, then yes, by default there's no filtering, but that's exactly how the compiler works: if it's a nightly or dev compiler, it allows every feature by default. We're encoding that behavior.

[..] on any Rust version [..]

Given all of the caveats above, this statement now becomes true. version_check doesn't tell you whether your compiler supports the feature you've asked for, it tells you whether your compiler can support the feature. This is a useful distinction to make, and I believe we should change the documentation accordingly.

Similarly, even if your specific version supports the specific feature, it may not implement the feature in the way you think (or previously observed) it implements the feature. This is also a useful distinction to make as well.

I don't believe there are any good general solutions to these problems available to us without support from rustc itself. See my comment at tkaitchuck/aHash#239 (comment) for more.

[dtolnay]

It answers true exactly when the release channel is known to be nightly or dev and the feature has not been disabled via RUSTFLAGS or CARGO_ENCODED_RUSTFLAGS.

Regarding this part, it's worth being aware that Cargo passes different flags to rustc than what it reveals to build scripts in CARGO_ENCODED_RUSTFLAGS. This means that even considering only compiler flags (i.e. disregarding the fact that a given feature name might mean a different thing to rustc from compiler to compiler), implementing reliable behavior for supports_feature("...") will require some fixes in Cargo. This is a longstanding Cargo bug but so far I'm not aware of a reason it couldn't be fixed, so I am hopeful that someone will eventually pick it up.

As an example of the consequence, a crate with [dependencies] ahash = "0.8" in Cargo.toml will fail to build in a project which has [unstable] allow-features = [] in .cargo/config.toml, since that is not included by Cargo in CARGO_ENCODED_RUSTFLAGS. Similarly, [profile.dev] rustflags = ["-Zallow-features="] in .cargo/config.toml would also not be included in CARGO_ENCODED_RUSTFLAGS and would fail to build.

[kornelski]

Let me correct that I mean on any nightly Rust version.

The is_feature_flaggable check does not check for specific individual nightly feature flags, only generally whether unstable features are allowed.

The issue is that Rust 1.50.0-nightly supports different unstable features than Rust 1.66.0-nightly. Features that exist on 1.80.1-nightly may cease to exist on 1.95.0-nightly.

It's not sufficient to check whether the compiler allows feature flags, it's necessary to check whether it allows the specific feature flag.

The assumption that all flags work unless explicitly disallowed in rustflags is unreliable.

Specifically, this weak check breaks ahash 0.7 that checks for stdsimd feature flag that no longer exists in nightly Rust version, but version_check says it's okay to use it.

This will break ahash again when Rust changes specialization feature flags.

[kornelski]

proc-macro2 used to have the same check of allow-features with fail-open fallback, and this has caused breakage too. Since then proc-macro2 has upgraded the check to a compile check, and I recommend copying that approach:

https://github.com/dtolnay/proc-macro2/blob/aa9476b27932ae1b1b50bbfe0530b3b261fc1b72/build.rs#L135C11-L192

[hkBst]

@kornelski I have my own reasons for wanting to disable autodetection of nightly sometimes, so I wrote a small utility to override version-based detection. Let me know if it is useful for you.