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

Avoid trusting bootstrap for git-hash availability for the version-verbose-commit-hash test #137039

Closed
wants to merge 5 commits into from
Closed
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
4 changes: 0 additions & 4 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2125,10 +2125,6 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the

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("--git-repository").arg(git_config.git_repository);
cmd.arg("--nightly-branch").arg(git_config.nightly_branch);
Expand Down
8 changes: 8 additions & 0 deletions src/doc/rustc-dev-guide/src/tests/misc.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,11 @@ fn main() {
.run();
}
```

## `tests/run-make/version-verbose-commit-hash`

This is a special `run-make` test that checks if git hash info is available with
`rustc -vV` and `rustdoc -vV`. It must not trust bootstrap's reporting of
whether git hash is available, as bootstrap may be bugged. Instead, this test
directly checks for `COMPILETEST_HAS_GIT_HASH=1` being set in CI, otherwise this
test is a no-op locally.
3 changes: 0 additions & 3 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,6 @@ pub struct Config {
/// The current Rust channel
pub channel: String,

/// Whether adding git commit information such as the commit hash has been enabled for building
pub git_hash: bool,

/// The default Rust edition
pub edition: Option<String>,

Expand Down
1 change: 0 additions & 1 deletion src/tools/compiletest/src/directive-list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"needs-dynamic-linking",
"needs-enzyme",
"needs-force-clang-based-tests",
"needs-git-hash",
"needs-llvm-components",
"needs-llvm-zstd",
"needs-profiler-runtime",
Expand Down
5 changes: 0 additions & 5 deletions src/tools/compiletest/src/header/needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ pub(super) fn handle_needs(
condition: cache.dlltool,
ignore_reason: "ignored when dlltool for the current architecture is not present",
},
Need {
name: "needs-git-hash",
condition: config.git_hash,
ignore_reason: "ignored when git hashes have been omitted for building",
},
Need {
name: "needs-dynamic-linking",
condition: config.target_cfg().dynamic_linking,
Expand Down
18 changes: 0 additions & 18 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,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,
Expand Down Expand Up @@ -118,11 +117,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
Expand Down Expand Up @@ -184,9 +178,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());
}
Expand Down Expand Up @@ -427,15 +418,6 @@ fn debugger() {
assert!(check_ignore(&config, "//@ ignore-lldb"));
}

#[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();
assert!(!check_ignore(&config, "//@ needs-git-hash"));
}

#[test]
fn sanitizers() {
// Target that supports all sanitizers:
Expand Down
6 changes: 0 additions & 6 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,6 @@ pub 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("", "git-repository", "name of the git repository", "ORG/REPO")
.reqopt("", "nightly-branch", "name of the git branch for nightly", "BRANCH")
Expand Down Expand Up @@ -381,7 +376,6 @@ pub 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"),

cc: matches.opt_str("cc").unwrap(),
Expand Down
25 changes: 19 additions & 6 deletions tests/run-make/version-verbose-commit-hash/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,33 @@
// `--version --verbose` should display the git-commit hashes of rustc and rustdoc, but this
// functionality was lost due to #104184. After this feature was returned by #109981, this
// test ensures it will not be broken again.
// See https://github.com/rust-lang/rust/issues/107094

//@ needs-git-hash
//! `--version --verbose` should display the git-commit hashes of rustc and rustdoc, but this
//! functionality was lost due to #104184. After this feature was returned by #109981, this
//! test ensures it will not be broken again.
//! See <https://github.com/rust-lang/rust/issues/107094>.
//!
//! # Important note
//!
//! This test is **not** gated by compiletest, and **cannot** trust bootstrap's git-hash logic e.g.
//! if bootstrap reports git-hash is available yet the built rustc doesn't actually have a hash. It
//! must directly communicate with CI, and gate it being run on an env var expected to be set in CI
//! (or that env var being set locally), `COMPILETEST_HAS_GIT_HASH=1`.

use run_make_support::{bare_rustc, bare_rustdoc, regex};

fn main() {
if !std::env::var("COMPILETEST_HAS_GIT_HASH").is_ok_and(|v| v == "1") {
return;
}

let out_rustc =
bare_rustc().arg("--version").arg("--verbose").run().stdout_utf8().to_lowercase();
let out_rustdoc =
bare_rustdoc().arg("--version").arg("--verbose").run().stdout_utf8().to_lowercase();
let re =
regex::Regex::new(r#"commit-hash: [0-9a-f]{40}\ncommit-date: [0-9]{4}-[0-9]{2}-[0-9]{2}"#)
.unwrap();

println!("rustc:\n{}", out_rustc);
println!("rustdoc:\n{}", out_rustdoc);

assert!(re.is_match(&out_rustc));
assert!(re.is_match(&out_rustdoc));
}
Loading