0.6.9 fails to build on unstable-less nightly

[Nemo157]

When testing with RUSTFLAGS=-Zallow-features= cargo +nightly test to emulate the upcoming stable compiler eyre 0.6.9 attempts to use an unstable feature anyway and fails to build

error[E0725]: the feature `rustdoc_missing_doc_code_examples` is not in the list of allowed features
Error:    --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/eyre-0.6.9/src/lib.rs:320:13
    |
320 |     feature(rustdoc_missing_doc_code_examples),
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Nilstrieb]

This will also break everyone's builds using nighty if the feature ever changes. This is about a lint ans therefore purely internal, using it for everyone seems like a bad idea. Can you just use a feature flag instead that you can enable in your CI?

[thenorili]

This will also break everyone's builds using nighty if the feature ever changes. This is about a lint ans therefore purely internal, using it for everyone seems like a bad idea. Can you just use a feature flag instead that you can enable in your CI?

I don't understand how it would break everyone's build on nightly, could you explain your reasoning please?

When testing with RUSTFLAGS=-Zallow-features= cargo +nightly test to emulate the upcoming stable compiler eyre 0.6.9 attempts to use an unstable feature anyway and fails to build

As best I can understand you're saying that it's important to you that we support a nightly toolchain without unstable features, is that correct? I wasn't aware of that use case so I used the nightly toolchain to test for support of unstable features. I'll look into this request in the coming week.

[Nemo157]

As best I can understand you're saying that it's important to you that we support a nightly toolchain without unstable features, is that correct? I wasn't aware of that use case so I used the nightly toolchain to test for support of unstable features.

Yes, I use this in CI on crates that only target the stable compiler as a way to help detect accidental breaking changes in the compiler before they make it to beta. Disabling nightly features is important for that usecase to avoid false positives from changes to unstable features that I don't care about.

[thenorili]

https://github.com/rust-lang/rust/blob/master/src/bootstrap/src/bin/rustc.rs
Looks like rustc passes allow-features up for build scripts to inspect. I'll look into adding an opt-in feature flag as well. Do pardon the inconvenience!

Yes, I use this in CI on crates that only target the stable compiler as a way to help detect accidental breaking changes in the compiler before they make it to beta. Disabling nightly features is important for that usecase to avoid false positives from changes to unstable features that I don't care about.

By this do you mean that you have a crate with a dependency on eyre that only targets the stable compiler?

I hope that in the meantime you don't find any false positives from enabling nightly features!

[Nilstrieb]

I don't understand how it would break everyone's build on nightly, could you explain your reasoning please?

I guess in this case here it won't, if the feature is removed that should still just be a lint. But if this wasn't a lint but something that could change, then relying on it will break nightly builds if it does change, as we've recently seen with proc_macro2.

https://github.com/rust-lang/rust/blob/master/src/bootstrap/src/bin/rustc.rs
Looks like rustc passes allow-features up for build scripts to inspect. I'll look into adding an opt-in feature flag as well. Do pardon the inconvenience!

You're looking in the wrong place, this is just a building-rustc-itself-internal-only binary. Build scripts are a cargo thing, not a rustc thing.

As a rule of thumb, one should never look whether rustc --version outputs "nightly" in a build script, it usually ends badly. It should either be avoided entirely (like it can be here by using a cargo feature that's enabled in CI) or instead be more tagetted by "probing" for support by trying to compile something (like eyre already does for old stables or used to for backtrace on nightly).

Thank you for the quick response!

[Nemo157]

By this do you mean that you have a crate with a dependency on eyre that only targets the stable compiler?

Yes, it's unlikely to be useful but this is where I saw the failure https://github.com/FerrisCraft/fc5-tool/actions/runs/6911230820/job/18805482969

I guess in this case here it won't, if the feature is removed that should still just be a lint.

There is still one case that could break this: if the lint is abandoned the feature-flag will be removed completely and builds will start failing with

error[E0635]: unknown feature `rustdoc_missing_doc_code_examples`

[thenorili]

Hey @Nemo157, did this test pass for you before 0.6.9?

I've corrected the issue with rustdoc_missing_doc_code_examples and it looks like a probe for backtrace (a stable feature now i thought) enables the unstable feature error_generic_member_access, I'm seeing a lot of errors like the following for our dependencies.

RUSTFLAGS=-Zallow-features= cargo +nightly test

error[E0725]: the feature `error_generic_member_access` is not in the list of allowed features     
   --> C:\Users\nori\.cargo\registry\src\index.crates.io-6f17d22bba15001f\anyhow-1.0.75\src\lib.rs:214:32
    |
214 | #![cfg_attr(backtrace, feature(error_generic_member_access))]
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^

    Checking futures-channel v0.3.29
error[E0725]: the feature `error_generic_member_access` is not in the list of allowed features     
   --> C:\Users\nori\.cargo\registry\src\index.crates.io-6f17d22bba15001f\thiserror-1.0.50\src\lib.rs:238:50
    |
238 | #![cfg_attr(error_generic_member_access, feature(error_generic_member_access))]
    |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0658]: use of unstable library feature 'proc_macro_span'                                    
   --> C:\Users\nori\.cargo\registry\src\index.crates.io-6f17d22bba15001f\proc-macro2-1.0.69\src\wrapper.rs:467:72
    |
467 |             (Span::Compiler(a), Span::Compiler(b)) => Span::Compiler(a.join(b)?),
    |                                                                        ^^^^
    |
    = note: see issue #54725 <https://github.com/rust-lang/rust/issues/54725> for more information 
    = help: add `#![feature(proc_macro_span)]` to the crate attributes to enable

It appears that the anyhow, thiserror, and proc-macro crates do not support unstable-less nightly. We'd have to send quite a few fixes upstream to support this use case... unless,

Maybe I could get around this by altering our compile test?

Something like this seems to work.

#[cfg_attr(any(not(nightly), not(feature = "unstable")), ignore)]
#[cfg_attr(miri, ignore)]
#[test]
fn ui() {
    let t = trybuild::TestCases::new();
    t.compile_fail("tests/ui/*.rs");
}

I'm not confident, but I am concerned that this might not actually work in production though -- it'll satisfy tests, but my guess is our dependencies will still try to use those features if they detect nightly. Would you have the time to put the build through your CI if I sent it out?

thanks,
nori

p.s. would you be opposed to an opt-out feature flag for unstable features in nightly?

[Nemo157]

Yes, the test passed previously. proc-macro2 at least I know is using some rustflag parsing to detect -Zallow-features= explicitly. Skimming thiserror's build.rs it is doing compile-detection for error_generic_member_access. The fact that they're both still failing implies that the -Zallow-features= isn't making it into CARGO_ENCODED_RUSTFLAGS when their build scripts run for some reason.

[Nemo157]

Ok, there seems to be a bug in the build scripts rerun detection. They don't explicitly tell Cargo that they're dependent on CARGO_ENCODED_RUSTFLAGS, so it doesn't rerun them when it changes. I would have expected that variable to always force a build script rerun, so maybe it's a bug in Cargo not them. If I cargo clean between runs then RUSTFLAGS=-Zallow-features= cargo test works on your branch for me.

[thenorili]

Ok, there seems to be a bug in the build scripts rerun detection. They don't explicitly tell Cargo that they're dependent on CARGO_ENCODED_RUSTFLAGS, so it doesn't rerun them when it changes. I would have expected that variable to always force a build script rerun, so maybe it's a bug in Cargo not them. If I cargo clean between runs then RUSTFLAGS=-Zallow-features= cargo test works on your branch for me.

My branch passes for me, I didn't know about the rerun thing! The important thing I was pointing out is, if you take the 'not(unstable), ignore' gate off of compile_test.rs it fails unconditionally.

[Nemo157]

-#[cfg_attr(any(not(nightly), not(feature = "unstable")), ignore)]
+#[cfg_attr(not(nightly), ignore)]

works for me too, after cleaning to force a rebuild

[thenorili]

-#[cfg_attr(any(not(nightly), not(feature = "unstable")), ignore)]
+#[cfg_attr(not(nightly), ignore)]

works for me too, after cleaning to force a rebuild

wow, it passes for me after cargo clean too : O that's so strange, I'll have to really think about how to file this.

[Nemo157]

I've asked about it on zulip https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Build.20script.20rerun.20on.20environment.20change, I'm still not certain if it's a bug in cargo or the build scripts somehow

[Nemo157]

Update: it is a bug in cargo rust-lang/cargo#13003