test(pgo): use ignore instead of disabling test at runtime

.github/workflows/main.yml

@@ -163,6 +163,9 @@ jobs:
     - run: rustup target add ${{ matrix.other }}
     - run: rustup component add rustc-dev llvm-tools-preview rust-docs
       if: startsWith(matrix.rust, 'nightly')
+    - name: Append rustup toolchain target bins to PATH
+      run: echo "$(dirname $(rustc --print target-libdir))/bin" >> "$GITHUB_PATH"
+      if: matrix.os == 'ubuntu-latest'
     - run: sudo apt update -y && sudo apt install lldb gcc-multilib libsecret-1-0 libsecret-1-dev -y
       if: matrix.os == 'ubuntu-latest'
     - run: rustup component add rustfmt || echo "rustfmt not available"

crates/cargo-test-macro/src/lib.rs

@@ -34,8 +34,8 @@ use std::sync::Once;
 ///   This is useful for tests that use unstable options in `rustc` or `rustdoc`.
 ///   These tests are run in Cargo's CI, but are disabled in rust-lang/rust's CI due to the difficulty of updating both repos simultaneously.
 ///   A `reason` field is required to explain why it is nightly-only.
-/// * `requires_<cmd>` --- This indicates a command that is required to be installed to be run.
-///   For example, `requires_rustfmt` means the test will only run if the executable `rustfmt` is installed.
+/// * `requires = "<cmd>"` --- This indicates a command that is required to be installed to be run.
+///   For example, `requires = "rustfmt"` means the test will only run if the executable `rustfmt` is installed.
 ///   These tests are *always* run on CI.
 ///   This is mainly used to avoid requiring contributors from having every dependency installed.
 /// * `build_std_real` --- This is a "real" `-Zbuild-std` test (in the `build_std` integration test).
@@ -133,8 +133,18 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
                     "rustup or stable toolchain not installed"
                 );
             }
-            s if s.starts_with("requires_") => {
+            s if s.starts_with("requires=") => {
                 let command = &s[9..];
+                let Ok(literal) = command.parse::<Literal>() else {
+                    panic!("expect a string literal, found: {command}");
+                };
+                let literal = literal.to_string();
+                let Some(command) = literal
+                    .strip_prefix('"')
+                    .and_then(|lit| lit.strip_suffix('"'))
+                else {
+                    panic!("expect a quoted string literal, found: {literal}");
+                };
                 set_ignore!(!has_command(command), "{command} not installed");
             }
             s if s.starts_with(">=1.") => {
@@ -263,38 +273,86 @@ fn version() -> (u32, bool) {
     unsafe { VERSION }
 }
 
-fn check_command(command_path: &Path, args: &[&str]) -> bool {
+#[derive(Debug)]
+enum CheckCommandError {
+    CannotExecute {
+        program: String,
+        error: std::io::Error,
+    },
+    UnsuccessfulTermination {
+        program: String,
+        output: std::process::Output,
+    },
+}
+
+impl std::fmt::Display for CheckCommandError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            CheckCommandError::CannotExecute { program, error } => {
+                writeln!(
+                    f,
+                    "expected command `{program}` to be somewhere in PATH: {error}"
+                )
+            }
+            CheckCommandError::UnsuccessfulTermination { program, output } => {
+                writeln!(
+                    f,
+                    "expected command `{program}` to be runnable, got error {}:\n\
+                    stderr:{}\n\
+                    stdout:{}\n",
+                    output.status,
+                    String::from_utf8_lossy(&output.stderr),
+                    String::from_utf8_lossy(&output.stdout),
+                )
+            }
+        }
+    }
+}
+
+fn check_command(command_path: &Path, args: &[&str]) -> Result<(), CheckCommandError> {
     let mut command = Command::new(command_path);
-    let command_name = command.get_program().to_str().unwrap().to_owned();
+    let program = command.get_program().to_str().unwrap().to_owned();
     command.args(args);
     let output = match command.output() {
         Ok(output) => output,
-        Err(e) => {
-            // * hg is not installed on GitHub macOS or certain constrained
-            //   environments like Docker. Consider installing it if Cargo
-            //   gains more hg support, but otherwise it isn't critical.
-            // * lldb is not pre-installed on Ubuntu and Windows, so skip.
-            if is_ci() && !matches!(command_name.as_str(), "hg" | "lldb") {
-                panic!("expected command `{command_name}` to be somewhere in PATH: {e}",);
-            }
-            return false;
+        Err(error) => {
+            return Err(CheckCommandError::CannotExecute { program, error });
         }
     };
     if !output.status.success() {
-        panic!(
-            "expected command `{command_name}` to be runnable, got error {}:\n\
-            stderr:{}\n\
-            stdout:{}\n",
-            output.status,
-            String::from_utf8_lossy(&output.stderr),
-            String::from_utf8_lossy(&output.stdout)
-        );
+        return Err(CheckCommandError::UnsuccessfulTermination { program, output });
     }
-    true
+    Ok(())
 }
 
 fn has_command(command: &str) -> bool {
-    check_command(Path::new(command), &["--version"])
+    let cmd = Path::new(command);
+    let mut err = None;
+    for arg in ["--version", "-h"] {
+        match check_command(cmd, &[arg]) {
+            Ok(()) => return true,
+            Err(e) => {
+                err = Some(e);
+            }
+        }
+    }
+
+    match err.unwrap() {
+        // Always bail out if a command is not found on CI, except:
+        //
+        // * hg is not installed on GitHub macOS or certain constrained
+        //   environments like Docker. Consider installing it if Cargo
+        //   gains more hg support, but otherwise it isn't critical.
+        // * lldb is not pre-installed on Ubuntu and Windows, so skip.
+        CheckCommandError::CannotExecute { program, .. }
+            if !is_ci() || matches!(program.as_str(), "hg" | "lldb") =>
+        {
+            false
+        }
+        e => {
+            panic!("{e}");
+        }
+    }
 }
 
 fn has_rustup_stable() -> bool {
@@ -318,7 +376,12 @@ fn has_rustup_stable() -> bool {
         None => return false,
     };
     let cargo = Path::new(home).join("bin/cargo");
-    check_command(&cargo, &["+stable", "--version"])
+    if let Err(e @ CheckCommandError::UnsuccessfulTermination { .. }) =
+        check_command(&cargo, &["+stable", "--version"])
+    {
+        panic!("{e}");
+    }
+    true
 }
 
 /// Whether or not this running in a Continuous Integration environment.

tests/testsuite/cargo_init/simple_hg/mod.rs

@@ -5,7 +5,7 @@ use cargo_test_support::prelude::*;
 use cargo_test_support::str;
 use cargo_test_support::Project;
 
-#[cargo_test(requires_hg)]
+#[cargo_test(requires = "hg")]
 fn case() {
     let project = Project::from_template(current_dir!().join("in"));
     let project_root = &project.root();

tests/testsuite/git.rs

@@ -2917,7 +2917,7 @@ fn failed_submodule_checkout() {
     t.join().unwrap();
 }
 
-#[cargo_test(requires_git)]
+#[cargo_test(requires = "git")]
 fn use_the_cli() {
     let project = project();
     let git_project = git::new("dep1", |project| {
@@ -3028,7 +3028,7 @@ fn templatedir_doesnt_cause_problems() {
     p.cargo("check").run();
 }
 
-#[cargo_test(requires_git)]
+#[cargo_test(requires = "git")]
 fn git_with_cli_force() {
     // Supports a force-pushed repo.
     let git_project = git::new("dep1", |project| {
@@ -3095,7 +3095,7 @@ two
         .run();
 }
 
-#[cargo_test(requires_git)]
+#[cargo_test(requires = "git")]
 fn git_fetch_cli_env_clean() {
     // This tests that git-fetch-with-cli works when GIT_DIR environment
     // variable is set (for whatever reason).
@@ -4069,7 +4069,7 @@ src/lib.rs
         .run();
 }
 
-#[cargo_test(public_network_test, requires_git)]
+#[cargo_test(public_network_test, requires = "git")]
 fn github_fastpath_error_message() {
     let p = project()
         .file(

tests/testsuite/git_gc.rs

@@ -90,7 +90,7 @@ fn run_test(path_env: Option<&OsStr>) {
     );
 }
 
-#[cargo_test(requires_git)]
+#[cargo_test(requires = "git")]
 fn use_git_gc() {
     run_test(None);
 }

tests/testsuite/new.rs

@@ -114,7 +114,7 @@ fn simple_git() {
     cargo_process("build").cwd(&paths::root().join("foo")).run();
 }
 
-#[cargo_test(requires_hg)]
+#[cargo_test(requires = "hg")]
 fn simple_hg() {
     cargo_process("new --lib foo --edition 2015 --vcs hg").run();
 

tests/testsuite/pgo.rs

@@ -1,39 +1,14 @@
 //! Test if PGO works.
 
-use std::path::PathBuf;
-use std::process::Command;
-
 use cargo_test_support::prelude::*;
 use cargo_test_support::project;
 use cargo_test_support::str;
 
-fn llvm_profdata() -> Option<PathBuf> {
-    let output = Command::new("rustc")
-        .arg("--print=target-libdir")
-        .output()
-        .expect("rustc to run");
-    assert!(output.status.success());
-    let mut libdir = PathBuf::from(String::from_utf8(output.stdout).unwrap());
-    assert!(libdir.pop());
-    let mut bin = libdir.join("bin").join("llvm-profdata");
-    bin.exists().then(|| bin.clone()).or_else(|| {
-        bin.set_extension("exe");
-        bin.exists().then_some(bin)
-    })
-}
-
-#[cargo_test]
+// macOS may emit different LLVM PGO warnings.
+// Windows LLVM has different requirements.
+#[cfg_attr(not(target_os = "linux"), cargo_test, ignore = "linux only")]
+#[cfg_attr(target_os = "linux", cargo_test(requires = "llvm-profdata", nightly, reason = "don't run on rust-lang/rust CI"))]
 fn pgo_works() {
-    if cfg!(not(target_os = "linux")) {
-        // macOS may emit different LLVM PGO warnings.
-        // Windows LLVM has different requirements.
-        return;
-    }
-
-    let Some(llvm_profdata) = llvm_profdata() else {
-        return;
-    };
-
     let p = project()
         .file(
             "Cargo.toml",
@@ -80,7 +55,7 @@ fn pgo_works() {
         .with_process_builder(cargo_test_support::process(release_bin))
         .run();
 
-    cargo_test_support::process(llvm_profdata)
+    cargo_test_support::process("llvm-profdata")
         .arg("merge")
         .arg("-o")
         .arg(&profdata_path)

tests/testsuite/profile_trim_paths.rs

@@ -445,17 +445,17 @@ mod object_works {
             .stdout
     }
 
-    #[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")]
+    #[cargo_test(requires = "nm", nightly, reason = "-Zremap-path-scope is unstable")]
     fn with_split_debuginfo_off() {
         object_works_helper("off", inspect_debuginfo);
     }
 
-    #[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")]
+    #[cargo_test(requires = "nm", nightly, reason = "-Zremap-path-scope is unstable")]
     fn with_split_debuginfo_packed() {
         object_works_helper("packed", inspect_debuginfo);
     }
 
-    #[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")]
+    #[cargo_test(requires = "nm", nightly, reason = "-Zremap-path-scope is unstable")]
     fn with_split_debuginfo_unpacked() {
         object_works_helper("unpacked", inspect_debuginfo);
     }
@@ -475,17 +475,29 @@ mod object_works {
             .stdout
     }
 
-    #[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")]
+    #[cargo_test(
+        requires = "readelf",
+        nightly,
+        reason = "-Zremap-path-scope is unstable"
+    )]
     fn with_split_debuginfo_off() {
         object_works_helper("off", inspect_debuginfo);
     }
 
-    #[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")]
+    #[cargo_test(
+        requires = "readelf",
+        nightly,
+        reason = "-Zremap-path-scope is unstable"
+    )]
     fn with_split_debuginfo_packed() {
         object_works_helper("packed", inspect_debuginfo);
     }
 
-    #[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")]
+    #[cargo_test(
+        requires = "readelf",
+        nightly,
+        reason = "-Zremap-path-scope is unstable"
+    )]
     fn with_split_debuginfo_unpacked() {
         object_works_helper("unpacked", inspect_debuginfo);
     }
@@ -676,7 +688,7 @@ fn custom_build_env_var_trim_paths() {
 }
 
 #[cfg(unix)]
-#[cargo_test(requires_lldb, nightly, reason = "-Zremap-path-scope is unstable")]
+#[cargo_test(requires = "lldb", nightly, reason = "-Zremap-path-scope is unstable")]
 fn lldb_works_after_trimmed() {
     use cargo_test_support::compare::assert_e2e;
     use cargo_util::is_ci;