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

fix: Fix unusual errors with RUSTC_WRAPPER #5983

Merged
merged 1 commit into from
Sep 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,26 @@ pub struct Compilation<'cfg> {

impl<'cfg> Compilation<'cfg> {
pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult<Compilation<'cfg>> {
let mut rustc = bcx.rustc.process();
// If we're using cargo as a rustc wrapper then we're in a situation
// like `cargo fix`. For now just disregard the `RUSTC_WRAPPER` env var
// (which is typically set to `sccache` for now). Eventually we'll
// probably want to implement `RUSTC_WRAPPER` for `cargo fix`, but we'll
// leave that open as a bug for now.
let mut rustc = if bcx.build_config.cargo_as_rustc_wrapper {
Copy link
Member

@dwijnand dwijnand Sep 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option sounds more generic than it actually is. Before looking it up I would've assumed it was also used for at least cargo rustc, maybe cargo rustdoc too (given the chat in #5958).

But, no, it's just set in ops::fix.

let mut rustc = bcx.rustc.process_no_wrapper();
let prog = rustc.get_program().to_owned();
rustc.env("RUSTC", prog);
rustc.program(env::current_exe()?);
rustc
} else {
bcx.rustc.process()
};
for (k, v) in bcx.build_config.extra_rustc_env.iter() {
rustc.env(k, v);
}
for arg in bcx.build_config.extra_rustc_args.iter() {
rustc.arg(arg);
}
if bcx.build_config.cargo_as_rustc_wrapper {
let prog = rustc.get_program().to_owned();
rustc.env("RUSTC", prog);
rustc.program(env::current_exe()?);
}
let srv = bcx.build_config.rustfix_diagnostic_server.borrow();
if let Some(server) = &*srv {
server.configure(&mut rustc);
Expand Down
10 changes: 6 additions & 4 deletions src/cargo/util/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,17 @@ impl Rustc {
pub fn process(&self) -> ProcessBuilder {
if let Some(ref wrapper) = self.wrapper {
let mut cmd = util::process(wrapper);
{
cmd.arg(&self.path);
}
cmd.arg(&self.path);
cmd
} else {
util::process(&self.path)
self.process_no_wrapper()
}
}

pub fn process_no_wrapper(&self) -> ProcessBuilder {
util::process(&self.path)
}

pub fn cached_output(&self, cmd: &ProcessBuilder) -> CargoResult<(String, String)> {
self.cache.lock().unwrap().cached_output(cmd)
}
Expand Down
23 changes: 23 additions & 0 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1117,3 +1117,26 @@ fn doesnt_rebuild_dependencies() {
")
.run();
}

#[test]
fn does_not_crash_with_rustc_wrapper() {
// We don't have /usr/bin/env on Windows.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So? It's expressly ignored, so whatever you set RUSTC_WRAPPER doesn't even need to exist on non-windows, no?

if cfg!(windows) {
return;
}
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old habits die hard 😉

"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("fix --allow-no-vcs")
.env("RUSTC_WRAPPER", "/usr/bin/env")
.run();
}