Skip to content

Commit 0fb9e85

Browse files
committed
Auto merge of #10918 - ehuss:fix-formats_source, r=Eh2406
Fix formats_source test requiring rustfmt. The requirements added in #9892 that `rustfmt` must be present doesn't work in the `rust-lang/rust` environment. There are two issues: * Cargo is run without using rustup. If you also have rustup installed, the test will fail because the `rustfmt` binary found in `PATH` will fail to choose a toolchain because HOME points to the sandbox home which does not have a rustup configuration. * rust-lang/rust CI uninstalls rustup, and does not have rustfmt in PATH at all. It is not practical to make rustfmt available there. The solution here is to just revert the behavior back to where it was where it checks if it can run `rustfmt` in the sandbox. This should work for anyone who has a normal rustup installation (including Cargo's CI). If running the testsuite without rustup, then the test will be skipped. This also includes a small enhancement to provide better error information when rustfmt fails.
2 parents b1dc94d + f62ce1e commit 0fb9e85

File tree

2 files changed

+17
-12
lines changed

2 files changed

+17
-12
lines changed

src/cargo/ops/cargo_new.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ use std::collections::BTreeMap;
1010
use std::fmt;
1111
use std::io::{BufRead, BufReader, ErrorKind};
1212
use std::path::{Path, PathBuf};
13-
use std::process::Command;
14-
use std::str::{from_utf8, FromStr};
13+
use std::str::FromStr;
1514
use toml_edit::easy as toml;
1615

1716
#[derive(Clone, Copy, Debug, PartialEq)]
@@ -830,14 +829,12 @@ mod tests {
830829
paths::write(&path_of_source_file, default_file_content)?;
831830

832831
// Format the newly created source file
833-
match Command::new("rustfmt").arg(&path_of_source_file).output() {
834-
Err(e) => log::warn!("failed to call rustfmt: {}", e),
835-
Ok(output) => {
836-
if !output.status.success() {
837-
log::warn!("rustfmt failed: {:?}", from_utf8(&output.stdout));
838-
}
839-
}
840-
};
832+
if let Err(e) = cargo_util::ProcessBuilder::new("rustfmt")
833+
.arg(&path_of_source_file)
834+
.exec_with_output()
835+
{
836+
log::warn!("failed to call rustfmt: {:#}", e);
837+
}
841838
}
842839
}
843840

tests/testsuite/init/formats_source/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
use cargo_test_support::compare::assert_ui;
22
use cargo_test_support::prelude::*;
3-
use cargo_test_support::Project;
3+
use cargo_test_support::{process, Project};
44

55
use cargo_test_support::curr_dir;
66

7-
#[cargo_test(requires_rustfmt)]
7+
#[cargo_test]
88
fn formats_source() {
9+
// This cannot use `requires_rustfmt` because rustfmt is not available in
10+
// the rust-lang/rust environment. Additionally, if running cargo without
11+
// rustup (but with rustup installed), this test also fails due to HOME
12+
// preventing the proxy from choosing a toolchain.
13+
if let Err(e) = process("rustfmt").arg("-V").exec_with_output() {
14+
eprintln!("skipping test, rustfmt not available:\n{e:?}");
15+
return;
16+
}
917
let project = Project::from_template(curr_dir!().join("in"));
1018
let project_root = &project.root();
1119

0 commit comments

Comments
 (0)