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 v0.27 tests failed when build rust 1.26.2 on "stable" channel #5648

Closed
mnd opened this issue Jun 23, 2018 · 7 comments · Fixed by #5652
Closed

cargo v0.27 tests failed when build rust 1.26.2 on "stable" channel #5648

mnd opened this issue Jun 23, 2018 · 7 comments · Fixed by #5652

Comments

@mnd
Copy link

mnd commented Jun 23, 2018

If rust configured with

[rust]
channel = "stable"

then test test_resolving_minimum_version_with_transitive_deps from cargo/tests/testsuite/resolve.rs will fail with next error:

---- resolve::test_resolving_minimum_version_with_transitive_deps stdout ----
        thread 'resolve::test_resolving_minimum_version_with_transitive_deps' panicked at 'called `Result::unwrap()` on an `Err` value: ErrorMessage { msg: "the `-Z` flag is only accepted on the nightly channel of Cargo" }', libcore/result.rs:945:5
@mnd
Copy link
Author

mnd commented Jun 23, 2018

Root cause: config.configure call tried to set unstable_flags, but parse function for CliUnstable structure allow it only on nightly channel.

@Eh2406
Copy link
Contributor

Eh2406 commented Jun 25, 2018

Looks like that test is missing a masquerade_as_nightly_cargo or equivalent. Presumably we could just use set_var to do the equivalent.

I wonder why our ci did not complain about this. @mnd Where is that flag set?

@ehuss
Copy link
Contributor

ehuss commented Jun 25, 2018

Due to the way nightly_features_allowed is written, it treats the unit-tests as the "dev" channel when CFG_RELEASE_CHANNEL environment var is not set (which is the case in CI). When run from within the rust repo, it uses the channel defined in the env var, which in this case is "stable" which causes the test to fail.

I would be reluctant to use set_var without resetting it back to the original value afterwards. I would think it would be easier just to skip the test if not running on nightly since it is a nightly-only feature.

@ehuss
Copy link
Contributor

ehuss commented Jun 26, 2018

@mnd How are you running the tests? When using x.py test in the rust repo it should set the RUSTC_BOOTSTRAP environment variable which allows this test to pass.

@mnd
Copy link
Author

mnd commented Jun 27, 2018

@ehuss I used https://static.rust-lang.org/dist/rustc-1.26.2-src.tar.gz sources, defined channel = "stable" in config.toml file and started tests with "./x.py" "-j1" "test" "src/tools/cargo"

I have next environment variables defined:

CFG_DISABLE_CROSS_TESTS=1
SHELL=/full/path/to/sh
CONFIG_SHELL=/full/path/to/sh
CC=/full/path/to/gcc
LLVM_LINK_SHARED=1

I preparing package for GuixSD and actually full config is config.txt, full environment is
environment-variables.txt

@ehuss
Copy link
Contributor

ehuss commented Jun 28, 2018

Ah, I see. I was looking at the wrong version. The check for RUSTC_BOOSTRAP is very recent, and wasn't part of the 1.27 release.

If I understand the rust dist process correctly, tests are not run from x.py dist, so cargo is never tested with channel="stable".

@Eh2406
Copy link
Contributor

Eh2406 commented Jun 29, 2018

The PR to fix this was just approved. It insures that all currently existing tests that can't run on "stable" override the stability checks; In addition it makes it difficult to regress on this by assuming "stable" for tests unless explicitly overridden.

This fix will be in the 1.28 release of rust. I doubt they will backport these changes to past releases, but I have no say.

bors added a commit that referenced this issue Jun 29, 2018
skip test if not nightly; fix for #5648

closes #5648
The alternative was to use `set_var` to emulate `masquerade_as_nightly_cargo` but we worried that it would not get unset correctly. Although I think each test is its own process, so it should work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants