Auto merge of #149377 - jieyouxu:git-hash-availability, r=<try>

Do not trust bootstrap about git hash availability

try-job: aarch64-apple
try-job: test-various

Author: rust-bors[bot]

src/bootstrap/src/core/build_steps/test.rs

@@ -2346,10 +2346,6 @@ Please disable assertions with `rust.debug-assertions = false`.
 
         cmd.arg("--channel").arg(&builder.config.channel);
 
-        if !builder.config.omit_git_hash {
-            cmd.arg("--git-hash");
-        }
-
         let git_config = builder.config.git_config();
         cmd.arg("--nightly-branch").arg(git_config.nightly_branch);
         cmd.arg("--git-merge-commit-email").arg(git_config.git_merge_commit_email);

src/ci/run.sh

@@ -110,6 +110,11 @@ fi
 export RUST_RELEASE_CHANNEL=$(releaseChannel)
 RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --release-channel=$RUST_RELEASE_CHANNEL"
 
+# Always set `RUSTC_TEST_GIT_HASH` to run
+# `tests/run-make/version-verbose-commit-hash` under CI conditions (for jobs
+# that run tests).
+export RUSTC_TEST_GIT_HASH=1
+
 if [ "$DEPLOY$DEPLOY_ALT" = "1" ]; then
   if [[ "$CI_JOB_NAME" == *ohos* ]]; then
     RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --disable-llvm-static-stdcpp"

src/doc/rustc-dev-guide/src/tests/misc.md

@@ -38,3 +38,21 @@ fn main() {
         .run();
 }
 ```
+
+## `RUSTC_TEST_GIT_HASH` env var
+
+> Context:
+>
+> - <https://github.com/rust-lang/rust/issues/132875>
+> - <https://github.com/rust-lang/rust/issues/132845>
+
+`RUSTC_TEST_GIT_HASH` is a special env var used intentionally to bypass
+bootstrap to make sure that the built rustc and rustdoc binaries correctly
+report git hash information. We can't rely on information reported by bootstrap
+regarding git hash availability because the bootstrap logic itself can be wrong
+(see linked issues above).
+
+- This env var must be set in CI.
+- To run `tests/run-make/version-verbose-commit-hash` locally, set
+  `RUSTC_TEST_GIT_HASH` to any non-empty value. Otherwise, this test will be
+  skipped.

src/tools/compiletest/src/common.rs

@@ -632,10 +632,9 @@ pub struct Config {
     pub channel: String,
 
     /// Whether adding git commit information such as the commit hash has been enabled for building.
-    ///
-    /// FIXME: `compiletest` cannot trust `bootstrap` for this information, because `bootstrap` can
-    /// have bugs and had bugs on that logic. We need to figure out how to obtain this e.g. directly
-    /// from CI or via git locally.
+    /// Note that this information directly comes from an env var `RUSTC_TEST_GIT_HASH` set by
+    /// CI (or in the local environment), since we must assume bootstrap might report incorrect
+    /// git hash availability due to bugs, and thus we need to bypass bootstrap.
     pub git_hash: bool,
 
     /// The default Rust edition.

src/tools/compiletest/src/directives/tests.rs

@@ -99,7 +99,6 @@ struct ConfigBuilder {
     stage: Option<u32>,
     stage_id: Option<String>,
     llvm_version: Option<String>,
-    git_hash: bool,
     system_llvm: bool,
     profiler_runtime: bool,
     rustc_debug_assertions: bool,
@@ -147,11 +146,6 @@ impl ConfigBuilder {
         self
     }
 
-    fn git_hash(&mut self, b: bool) -> &mut Self {
-        self.git_hash = b;
-        self
-    }
-
     fn system_llvm(&mut self, s: bool) -> &mut Self {
         self.system_llvm = s;
         self
@@ -218,9 +212,6 @@ impl ConfigBuilder {
             args.push(llvm_version.clone());
         }
 
-        if self.git_hash {
-            args.push("--git-hash".to_owned());
-        }
         if self.system_llvm {
             args.push("--system-llvm".to_owned());
         }
@@ -422,11 +413,13 @@ fn debugger() {
 
 #[test]
 fn git_hash() {
-    let config: Config = cfg().git_hash(false).build();
-    assert!(check_ignore(&config, "//@ needs-git-hash"));
-
-    let config: Config = cfg().git_hash(true).build();
+    unsafe { std::env::set_var("RUSTC_TEST_GIT_HASH", "1") };
+    let config = cfg().build();
     assert!(!check_ignore(&config, "//@ needs-git-hash"));
+
+    unsafe { std::env::remove_var("RUSTC_TEST_GIT_HASH") };
+    let config = cfg().build();
+    assert!(check_ignore(&config, "//@ needs-git-hash"));
 }
 
 #[test]

src/tools/compiletest/src/lib.rs

@@ -51,6 +51,9 @@ use crate::executor::{CollectedTest, ColorConfig};
 /// some code here that inspects environment variables or even runs executables
 /// (e.g. when discovering debugger versions).
 fn parse_config(args: Vec<String>) -> Config {
+    // FIXME(jieyouxu): this says "parse_config", but in reality also in some cases read from
+    // env vars!
+
     let mut opts = Options::new();
     opts.reqopt("", "compile-lib-path", "path to host shared libraries", "PATH")
         .reqopt("", "run-lib-path", "path to target shared libraries", "PATH")
@@ -180,11 +183,6 @@ fn parse_config(args: Vec<String>) -> Config {
         .optflag("", "profiler-runtime", "is the profiler runtime enabled for this target")
         .optflag("h", "help", "show this message")
         .reqopt("", "channel", "current Rust channel", "CHANNEL")
-        .optflag(
-            "",
-            "git-hash",
-            "run tests which rely on commit version being compiled into the binaries",
-        )
         .optopt("", "edition", "default Rust edition", "EDITION")
         .reqopt("", "nightly-branch", "name of the git branch for nightly", "BRANCH")
         .reqopt(
@@ -370,6 +368,19 @@ fn parse_config(args: Vec<String>) -> Config {
     let build_test_suite_root = opt_path(matches, "build-test-suite-root");
     assert!(build_test_suite_root.starts_with(&build_root));
 
+    // NOTE: we cannot trust bootstrap for git hash availability, therefore we use an env var to
+    // communicate directly between compiletest and CI/local environment, bypassing bootstrap.
+    let git_hash = if build_helper::ci::CiEnv::is_ci() {
+        // In CI environment, we expect that the env var must be set to prevent it from being forgotten.
+        let _ = env::var_os("RUSTC_TEST_GIT_HASH")
+            .expect("`RUSTC_TEST_GIT_HASH` must be set under CI environments");
+        true
+    } else {
+        // But locally, we don't want to do so -- only run the test that requires git hash
+        // availability if the user explicitly sets the env var.
+        env::var_os("RUSTC_TEST_GIT_HASH").is_some()
+    };
+
     Config {
         bless: matches.opt_present("bless"),
         fail_fast: matches.opt_present("fail-fast")
@@ -455,9 +466,10 @@ fn parse_config(args: Vec<String>) -> Config {
         has_html_tidy,
         has_enzyme,
         channel: matches.opt_str("channel").unwrap(),
-        git_hash: matches.opt_present("git-hash"),
         edition: matches.opt_str("edition").as_deref().map(parse_edition),
 
+        git_hash,
+
         cc: matches.opt_str("cc").unwrap(),
         cxx: matches.opt_str("cxx").unwrap(),
         cflags: matches.opt_str("cflags").unwrap(),