Allow setting CFG_DISABLE_UNSTABLE_FEATURES to 0

[chbaker0]

Two locations check whether this build-time environment variable is defined. Allowing it to be explicitly disabled with a "0" value is useful, especially for integrating with external build systems.

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[Mark-Simulacrum]

Allowing it to be explicitly disabled with a "0" value is useful, especially for integrating with external build systems.

Can you say more about the motivation here? In general it seems a little suspicious to be compiling these checks in a way that something else is trying to enable feature gating and you're aiming to override that.

Can you link/name the build system in question, perhaps?

[chbaker0]

In general it seems a little suspicious to be compiling these checks in a way that something else is trying to enable feature gating and you're aiming to override that

This is not the issue at play. Rather, it's to let a build system keep track of all environment variables needed for a target. The goal is just dependency correctness.

The specific issue is integrating with GN, Chromium's build system. A GN Rust target definition can set environment variables for its rustc invocation. However, you can't explicitly unset a variable.

GN's invokes rustc asking it to produce an depfile, which will contain any build-time environment variables as well. GN notices this, but also notices the variable is unaccounted for in the target definition, so it will rebuild that target every time.

[Mark-Simulacrum]

Are you trying to compile rustc or libtest during a GN-managed build? These variables are only read at compile-time of rustc itself, not by regular invocations of rustc.

Part of my confusion is that there are many more such variables in rustc, I think, and presumably not all of them are being managed by rustc.

--

Regardless of whether this solves your particular use case, I think I'm OK merging this PR -- wanting to always set an environment variable seems largely reasonable. I would caution that in an ideal world, this would be left at 1 (i.e., you do not enable unstable features by default in GN/Chromium), and instead rely on RUSTC_BOOTSTRAP and -Zallow-features to more selectively enable features (if needed at all).

[chbaker0]

Are you trying to compile rustc or libtest during a GN-managed build? These variables are only read at compile-time of rustc itself, not by regular invocations of rustc.

Yes, building libtest in a GN-managed build. Essentially we're doing the equivalent of Cargo's build-std feature but within Chromium's GN build. The change to rustc_feature isn't even necessary for us since we build rustc the usual way: from a Rust checkout using x.py. But I figured keeping the two lines in sync was good.

Part of my confusion is that there are many more such variables in rustc, I think, and presumably not all of them are being managed by rustc.

Maybe this is an ad-hoc solution but this was the only env dependency reported by rustc that was not accounted for by GN (and could not be because setting it to anything would break the build).

I would caution that in an ideal world, this would be left at 1 (i.e., you do not enable unstable features by default in GN/Chromium), and instead rely on RUSTC_BOOTSTRAP and -Zallow-features to more selectively enable features (if needed at all).

We use a nightly or dev toolchain, so normally this would not be defined when building libtest. We have no intention to use this outside of the libtest build.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit 0a03d1c has been approved by Mark-Simulacrum

It is now in the queue for this repository.