search

SergioBenitez / version_check

Rust library for checking the installed/running rustc's version.
https://docs.rs/version_check
Apache License 2.0
50 stars 14 forks source link

Account for env var RUSTC_BOOTSTRAP when determining if features are supported #9

Closed estebank closed 4 years ago

estebank commented 5 years ago

Fix https://github.com/SergioBenitez/Pear/issues/18.

SergioBenitez commented 5 years ago

Is there official documentation for the RUSTC_BOOTSTRAP environment variable anywhere? I'd like to know when it was introduced, for what reason, and whether it is something that will remain in the compiler. In other words, I'd like to better understand the purpose of the flag.

estebank commented 5 years ago

RUSTC_BOOTSTRAP=1 is used to be able to build any version of rustc using a compatible stable rustc. It tells a stable build of rustc to behave as if it were a nightly build, allowing unstable features to be used. It should't be encouraged, but it is a tool that is available. Supporting this makes version_check behave inline with rustc. Given that supports_features is a convenience to verify whether unstable features will be able to be used in the current compilation, I am inclined to argue that this new behavior is the correct one.

https://github.com/rust-lang/rust/issues/31847#issuecomment-280666952 hints at the unlikelihood of this var going away in the short term.

The (lengthy) discussion in https://github.com/rust-lang/cargo/pull/6608 and https://github.com/rust-lang/cargo/issues/6627 elaborates on the current uses and counter arguments against using this environment variable.

The "documentation" of RUSTC_BOOTSTRAP is only the code (😢):

https://github.com/rust-lang/rust/blob/885c7dfddcca11fa543d99d702c024c3f6bfe07b/src/libtest/lib.rs#L515-L523

https://github.com/rust-lang/rust/blob/8e948df707ea8a3c88c65bf2ffdcb2f1cf5491be/src/libsyntax/feature_gate.rs#L2534-L2553

It also seems to me that support for CFG_DISABLE_UNSTABLE_FEATURES=1 should be added too to fully map against rustc's behavior.

SergioBenitez commented 5 years ago

@estebank Is RUSTC_BOOTSTRAP intended to be used by end-users?

In any case, the use in the code snippets you refer to seems to imply that setting RUSTC_BOOTSTRAP effectively makes the stable compiler act like a nightly compiler. As such, I would argue that the correct way to support this feature is to modify Channel::read() accordingly, making a note of the behavior.

estebank commented 5 years ago

@estebank Is RUSTC_BOOTSTRAP intended to be used by end-users?

It is not necessarily, but it is being used (most notably by Firefox), but it immediately lowers the assurances that stable rustc gives to those of nightly. This is used in environments where people wishes they could be using stable, but are reliant on a subset of unstable features. https://github.com/rust-lang/cargo/issues/6627 is an attempt at formalizing this in a controlled manner, and the pushback is against making RUSTC_BOOTSTRAP part of our stable API.

In any case, the use in the code snippets you refer to seems to imply that setting RUSTC_BOOTSTRAP effectively makes the stable compiler act like a nightly compiler. As such, I would argue that the correct way to support this feature is to modify Channel::read() accordingly, making a note of the behavior.

That sounds reasonable to me, but I'm not sure. I believe that the primary intent for the flag is to affect features.

Centril commented 5 years ago

@estebank Is RUSTC_BOOTSTRAP intended to be used by end-users?

It should unequivocally not be used by end users. The flag may be renamed, removed, checked for tampering, etc. at any time.

I'm against giving RUSTC_BOOTSTRAP any more visibility than it already has. I think it should remain something obscure.

SergioBenitez commented 4 years ago

Apologies. I meant to close this nearly a year ago. I don't think we should be listening to hidden/private values that may change, especially since this can be done on a case-by-case basis.