Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion src/build_helper/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ fn get_latest_upstream_commit_that_modified_files(
/// author.
///
/// If we are in CI, we simply return our first parent.
fn get_closest_upstream_commit(
pub fn get_closest_upstream_commit(
git_dir: Option<&Path>,
config: &GitConfig<'_>,
env: CiEnv,
Expand Down
1 change: 1 addition & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub mod pal;
pub mod rustdoc_css_themes;
pub mod rustdoc_gui_tests;
pub mod rustdoc_js;
pub mod rustdoc_json;
pub mod rustdoc_templates;
pub mod style;
pub mod target_policy;
Expand Down
1 change: 1 addition & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ fn main() {
check!(rustdoc_css_themes, &librustdoc_path);
check!(rustdoc_templates, &librustdoc_path);
check!(rustdoc_js, &librustdoc_path, &tools_path, &src_path);
check!(rustdoc_json, &src_path);
check!(known_bug, &crashes_path);
check!(unknown_revision, &tests_path);

Expand Down
91 changes: 91 additions & 0 deletions src/tools/tidy/src/rustdoc_json.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
//! Tidy check to ensure that `FORMAT_VERSION` was correctly updated if `rustdoc-json-types` was
//! updated as well.

use std::ffi::OsStr;
use std::path::Path;
use std::process::Command;

use build_helper::ci::CiEnv;
use build_helper::git::{GitConfig, get_closest_upstream_commit};
use build_helper::stage0_parser::parse_stage0_file;

fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<String> {
let output = Command::new("git").arg("diff").arg(base_commit).arg(extra_arg).output().ok()?;
Some(String::from_utf8_lossy(&output.stdout).into())
}

pub fn check(src_path: &Path, bad: &mut bool) {
println!("Checking tidy rustdoc_json...");
let stage0 = parse_stage0_file();
let base_commit = match get_closest_upstream_commit(
None,
&GitConfig {
nightly_branch: &stage0.config.nightly_branch,
git_merge_commit_email: &stage0.config.git_merge_commit_email,
},
CiEnv::current(),
) {
Ok(Some(commit)) => commit,
Ok(None) => {
*bad = true;
Copy link
Member

Choose a reason for hiding this comment

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

Could you please only set *bad here and below on CI (if CiEnv::current() is CI)? For some users get_closest_upstream_commit might not return anything, e.g. if they have a shallow checkout. Yes, this is all sadly very annoying.. :D

Copy link
Member Author

Choose a reason for hiding this comment

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

I keep discovering new cases to handle 😆

eprintln!("error: no base commit found for rustdoc_json check");
return;
}
Err(error) => {
*bad = true;
eprintln!(
"error: failed to retrieve base commit for rustdoc_json check because of `{error}`"
);
return;
}
};

// First we check that `src/rustdoc-json-types` was modified.
match git_diff(&base_commit, "--name-status") {
Some(output) => {
if !output
.lines()
.any(|line| line.starts_with("M") && line.contains("src/rustdoc-json-types"))
{
// `rustdoc-json-types` was not modified so nothing more to check here.
println!("`rustdoc-json-types` was not modified.");
return;
}
}
None => {
*bad = true;
eprintln!("error: failed to run `git diff` in rustdoc_json check");
return;
}
}
// Then we check that if `FORMAT_VERSION` was updated, the `Latest feature:` was also updated.
match git_diff(&base_commit, src_path.join("rustdoc-json-types")) {
Some(output) => {
let mut format_version_updated = false;
let mut latest_feature_comment_updated = false;
for line in output.lines() {
if line.starts_with("+pub const FORMAT_VERSION: u32 =") {
format_version_updated = true;
} else if line.starts_with("+// Latest feature:") {
latest_feature_comment_updated = true;
}
}
if format_version_updated != latest_feature_comment_updated {
*bad = true;
if latest_feature_comment_updated {
eprintln!(
"error: `Latest feature` comment was updated whereas `FORMAT_VERSION` wasn't"
Copy link
Member

Choose a reason for hiding this comment

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

Although people will probably be able to infer it, I think it would be nice if the error message contained the filename that should be fixed, otherwise the error can be quite opaque.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

);
} else {
eprintln!(
"error: `Latest feature` comment was not updated whereas `FORMAT_VERSION` was"
);
}
}
}
None => {
*bad = true;
eprintln!("error: failed to run `git diff` in rustdoc_json check");
}
}
}
Loading