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

Logic to detect rustc version is incomplete #82

Closed
taiki-e opened this issue Aug 29, 2021 · 3 comments · Fixed by #83
Closed

Logic to detect rustc version is incomplete #82

taiki-e opened this issue Aug 29, 2021 · 3 comments · Fixed by #83
Labels
C-bug Category: related to a bug.

Comments

@taiki-e
Copy link
Owner

taiki-e commented Aug 29, 2021

The current way does not handle it correctly when the version is specified in a cargo or rustup specific way (e.g., cargo +nightly-..., rustup run +nightly-...).

let path = env.rustc.as_deref().unwrap_or_else(|| OsStr::new("rustc"));
let version = cmd!(path, "--version").dir(workspace_root).read()?;
let nightly = version.contains("-nightly") || version.contains("-dev");
let mut cmd = if nightly { cmd!(path) } else { cmd!("rustup", "run", "nightly", "rustc") };
cmd.args(&["--version", "--verbose"]);
let verbose_version = cmd.read()?;

Currently, the following heuristic is the only one that uses the retrieved rustc information, so it does not affect the coverage results, but it may accidentally remove dependency's build artifacts.

// - If there are old artifacts by a different version of rustc: remove all artifacts (handled in Context::new)

related(?): rust-lang/cargo#2833

@taiki-e taiki-e added the C-bug Category: related to a bug. label Aug 29, 2021
@taiki-e
Copy link
Owner Author

taiki-e commented Aug 29, 2021

In a build script, the RUSTC environment variable can be used for this purpose, but in a subcommand, the RUSTC environment variable has a different meaning.

If we need the major-minor-patch version of the toolchain (e.g., cargo-hack's --version-range), cargo --version --verbose is fine, but if we need the exact toolchain date of nightly rustc, cargo --version --verbose is not enough.

So I think the correct action here is just to remove this heuristic. (IICU, the way introduced in #79 should work well without rustc version-based heuristics.)

@taiki-e
Copy link
Owner Author

taiki-e commented Aug 29, 2021

So I think the correct action here is just to remove this heuristic. (IICU, the way introduced in #79 should work well without rustc version-based heuristics.)

In fact, it relies on information from rustc here as well, but that can be avoided.

let rustlib = sysroot.join(format!("lib/rustlib/{}/bin", rustc.host));

@taiki-e
Copy link
Owner Author

taiki-e commented Aug 29, 2021

Oh, actually I can find the rustc binary based on the sysroot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant