Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo test -- -Z unstable-options is accepted by the stable libtest framework #75526

Closed
Nemo157 opened this issue Aug 14, 2020 · 1 comment · Fixed by #109044
Closed

cargo test -- -Z unstable-options is accepted by the stable libtest framework #75526

Nemo157 opened this issue Aug 14, 2020 · 1 comment · Fixed by #109044
Assignees
Labels
A-libtest Area: `#[test]` / the `test` library A-stability Area: `#[stable]`, `#[unstable]` etc. C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@Nemo157
Copy link
Member

Nemo157 commented Aug 14, 2020

I tried runnning

> cargo +stable test -- -Z unstable-options --format json

I expected to see this happen: an error message similar to

> cargo +stable test -Z unstable-options
error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel
See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.

Instead, this happened: the tests ran and printed their output in json

Meta

> rustc +stable --version
rustc 1.45.2 (d3fb005a3 2020-07-31)

(mentioned in rust-lang/rustfmt#4228 (comment), cc #49359)

@Nemo157 Nemo157 added the C-bug Category: This is a bug. label Aug 14, 2020
@ehuss
Copy link
Contributor

ehuss commented Aug 14, 2020

See also #65770. There are multiple issues with how this is handled.

@jyn514 jyn514 added A-libtest Area: `#[test]` / the `test` library A-stability Area: `#[stable]`, `#[unstable]` etc. labels Aug 18, 2020
@thomcc thomcc added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Mar 1, 2023
@thomcc thomcc self-assigned this Mar 1, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 16, 2023
…r=jyn514

Prevent stable `libtest` from supporting `-Zunstable-options`

Took a while for me to get around to this but seems trivial (unless I'm missing some reason this will break all our tests). Fixes rust-lang#75526

Basically `libtest` already tries to handle this in https://github.com/rust-lang/rust/blob/501ad021b9a4fb2cd6a39e0302d22f169f6166b0/library/test/src/cli.rs#L310-L318

But that env var was not passed. I'm guessing at one point [this code](https://github.com/rust-lang/rust/blob/501ad021b9a4fb2cd6a39e0302d22f169f6166b0/src/bootstrap/compile.rs#L842-L844) (or a common ancestor) was used to compile the standard library/libtest, but that is no longer the case (or perhaps it never worked, I don't have time to go digging).

I don't love that this is a "allow unstable by default" situation, as it means things like [`rustc-build-sysroot`](https://github.com/RalfJung/rustc-build-sysroot) could accidentally get unstable (CC `@RalfJung)` even if this is fixed here, but it's consistent with what happens in `rustc_feature`, so... yeah.

This is user-facing after all, even if it's hard to imagine the outcome of that conversation being "lets continue allowing use of `-Zunstable-features` from stable rust" (especially since a `RUSTC_BOOTSTRAP=1`-shaped loophole remains)... I think it probably should get a vibe check in the t-libs meeting (and plausibly a relnote along the lines of "hey `cargo test -- -Zunstable-options --some --unstable --stuff=here` used to work on stable, that's been fixed, sorry").

I'll nominate it for that after CI comes up green (I've done a smoke check but don't know what (if anything) will need `bootstrap` to enable `RUSTC_BOOTSTRAP=1` when running tests)

r? `@jyn514`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2023
…r=jyn514

Prevent stable `libtest` from supporting `-Zunstable-options`

Took a while for me to get around to this but seems trivial (unless I'm missing some reason this will break all our tests). Fixes rust-lang#75526

Basically `libtest` already tries to handle this in https://github.com/rust-lang/rust/blob/501ad021b9a4fb2cd6a39e0302d22f169f6166b0/library/test/src/cli.rs#L310-L318

But that env var was not passed. I'm guessing at one point [this code](https://github.com/rust-lang/rust/blob/501ad021b9a4fb2cd6a39e0302d22f169f6166b0/src/bootstrap/compile.rs#L842-L844) (or a common ancestor) was used to compile the standard library/libtest, but that is no longer the case (or perhaps it never worked, I don't have time to go digging).

I don't love that this is a "allow unstable by default" situation, as it means things like [`rustc-build-sysroot`](https://github.com/RalfJung/rustc-build-sysroot) could accidentally get unstable (CC ``@RalfJung)`` even if this is fixed here, but it's consistent with what happens in `rustc_feature`, so... yeah.

This is user-facing after all, even if it's hard to imagine the outcome of that conversation being "lets continue allowing use of `-Zunstable-features` from stable rust" (especially since a `RUSTC_BOOTSTRAP=1`-shaped loophole remains)... I think it probably should get a vibe check in the t-libs meeting (and plausibly a relnote along the lines of "hey `cargo test -- -Zunstable-options --some --unstable --stuff=here` used to work on stable, that's been fixed, sorry").

I'll nominate it for that after CI comes up green (I've done a smoke check but don't know what (if anything) will need `bootstrap` to enable `RUSTC_BOOTSTRAP=1` when running tests)

r? ``@jyn514``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2023
…r=jyn514

Prevent stable `libtest` from supporting `-Zunstable-options`

Took a while for me to get around to this but seems trivial (unless I'm missing some reason this will break all our tests). Fixes rust-lang#75526

Basically `libtest` already tries to handle this in https://github.com/rust-lang/rust/blob/501ad021b9a4fb2cd6a39e0302d22f169f6166b0/library/test/src/cli.rs#L310-L318

But that env var was not passed. I'm guessing at one point [this code](https://github.com/rust-lang/rust/blob/501ad021b9a4fb2cd6a39e0302d22f169f6166b0/src/bootstrap/compile.rs#L842-L844) (or a common ancestor) was used to compile the standard library/libtest, but that is no longer the case (or perhaps it never worked, I don't have time to go digging).

I don't love that this is a "allow unstable by default" situation, as it means things like [`rustc-build-sysroot`](https://github.com/RalfJung/rustc-build-sysroot) could accidentally get unstable (CC ```@RalfJung)``` even if this is fixed here, but it's consistent with what happens in `rustc_feature`, so... yeah.

This is user-facing after all, even if it's hard to imagine the outcome of that conversation being "lets continue allowing use of `-Zunstable-features` from stable rust" (especially since a `RUSTC_BOOTSTRAP=1`-shaped loophole remains)... I think it probably should get a vibe check in the t-libs meeting (and plausibly a relnote along the lines of "hey `cargo test -- -Zunstable-options --some --unstable --stuff=here` used to work on stable, that's been fixed, sorry").

I'll nominate it for that after CI comes up green (I've done a smoke check but don't know what (if anything) will need `bootstrap` to enable `RUSTC_BOOTSTRAP=1` when running tests)

r? ```@jyn514```
@bors bors closed this as completed in 0e7117b Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: `#[test]` / the `test` library A-stability Area: `#[stable]`, `#[unstable]` etc. C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants