Skip to content

Commit b0010dd

Browse files
KobzolKobzol
committed
Add diagnostics context and migrate the style check to it
1 parent 1d23da6 commit b0010dd

File tree

4 files changed

+187
-89
lines changed

4 files changed

+187
-89
lines changed

‎src/tools/tidy/src/diagnostics.rs‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use std::collections::HashSet;
2+
use std::fmt::Display;
3+
use std::sync::{Arc, Mutex};
4+
5+
use crate::tidy_error;
6+
7+
/// Collects diagnostics from all tidy steps, and contains shared information
8+
/// that determines how should message and logs be presented.
9+
///
10+
/// Since checks are executed in parallel, the context is internally synchronized, to avoid
11+
/// all checks to lock it explicitly.
12+
#[derive(Clone)]
13+
pub struct DiagCtx(Arc<Mutex<DiagCtxInner>>);
14+
15+
impl DiagCtx {
16+
pub fn new(verbose: bool) -> Self {
17+
Self(Arc::new(Mutex::new(DiagCtxInner {
18+
running_checks: Default::default(),
19+
finished_checks: Default::default(),
20+
verbose,
21+
})))
22+
}
23+
24+
pub fn start_check<T: Display>(&self, name: T) -> RunningCheck {
25+
let name = name.to_string();
26+
27+
let mut ctx = self.0.lock().unwrap();
28+
ctx.start_check(&name);
29+
RunningCheck { name, bad: false, ctx: self.0.clone() }
30+
}
31+
32+
pub fn into_conclusion(self) -> bool {
33+
let ctx = self.0.lock().unwrap();
34+
assert!(ctx.running_checks.is_empty(), "Some checks are still running");
35+
ctx.finished_checks.iter().any(|c| c.bad)
36+
}
37+
}
38+
39+
struct DiagCtxInner {
40+
running_checks: HashSet<String>,
41+
finished_checks: HashSet<FinishedCheck>,
42+
verbose: bool,
43+
}
44+
45+
impl DiagCtxInner {
46+
fn start_check(&mut self, name: &str) {
47+
if self.has_check(name) {
48+
panic!("Starting a check named {name} for the second time");
49+
}
50+
self.running_checks.insert(name.to_string());
51+
}
52+
53+
fn finish_check(&mut self, check: FinishedCheck) {
54+
assert!(
55+
self.running_checks.remove(&check.name),
56+
"Finishing check {} that was not started",
57+
check.name
58+
);
59+
self.finished_checks.insert(check);
60+
}
61+
62+
fn has_check(&self, name: &str) -> bool {
63+
self.running_checks
64+
.iter()
65+
.chain(self.finished_checks.iter().map(|c| &c.name))
66+
.any(|c| c == name)
67+
}
68+
}
69+
70+
#[derive(PartialEq, Eq, Hash, Debug)]
71+
struct FinishedCheck {
72+
name: String,
73+
bad: bool,
74+
}
75+
76+
/// Represents a single tidy check, identified by its `name`, running.
77+
pub struct RunningCheck {
78+
name: String,
79+
bad: bool,
80+
ctx: Arc<Mutex<DiagCtxInner>>,
81+
}
82+
83+
impl RunningCheck {
84+
/// Immediately output an error and mark the check as failed.
85+
pub fn error<T: Display>(&mut self, t: T) {
86+
self.mark_as_bad();
87+
tidy_error(&t.to_string()).expect("failed to output error");
88+
}
89+
90+
fn mark_as_bad(&mut self) {
91+
self.bad = true;
92+
}
93+
}
94+
95+
impl Drop for RunningCheck {
96+
fn drop(&mut self) {
97+
self.ctx
98+
.lock()
99+
.unwrap()
100+
.finish_check(FinishedCheck { name: self.name.clone(), bad: self.bad })
101+
}
102+
}

‎src/tools/tidy/src/lib.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,6 @@ macro_rules! tidy_error {
5050
});
5151
}
5252

53-
macro_rules! tidy_error_ext {
54-
($tidy_error:path, $bad:expr, $($fmt:tt)*) => ({
55-
$tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error");
56-
*$bad = true;
57-
});
58-
}
59-
6053
fn tidy_error(args: &str) -> std::io::Result<()> {
6154
use std::io::Write;
6255

@@ -250,6 +243,7 @@ pub mod alphabetical;
250243
pub mod bins;
251244
pub mod debug_artifacts;
252245
pub mod deps;
246+
pub mod diagnostics;
253247
pub mod edition;
254248
pub mod error_codes;
255249
pub mod extdeps;

‎src/tools/tidy/src/main.rs‎

Lines changed: 65 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ use std::num::NonZeroUsize;
99
use std::path::PathBuf;
1010
use std::str::FromStr;
1111
use std::sync::atomic::{AtomicBool, Ordering};
12+
use std::sync::{Arc, Mutex};
1213
use std::thread::{self, ScopedJoinHandle, scope};
1314
use std::{env, process};
1415

16+
use tidy::diagnostics::DiagCtx;
1517
use tidy::*;
1618

1719
fn main() {
@@ -54,6 +56,8 @@ fn main() {
5456
let ci_info = CiInfo::new(&mut bad);
5557
let bad = std::sync::Arc::new(AtomicBool::new(bad));
5658

59+
let mut diag_ctx = DiagCtx::new(verbose);
60+
5761
let drain_handles = |handles: &mut VecDeque<ScopedJoinHandle<'_, ()>>| {
5862
// poll all threads for completion before awaiting the oldest one
5963
for i in (0..handles.len()).rev() {
@@ -87,76 +91,73 @@ fn main() {
8791
(@ $p:ident, name=$name:expr $(, $args:expr)* ) => {
8892
drain_handles(&mut handles);
8993

94+
let diag_ctx = diag_ctx.clone();
9095
let handle = thread::Builder::new().name($name).spawn_scoped(s, || {
91-
let mut flag = false;
92-
$p::check($($args, )* &mut flag);
93-
if (flag) {
94-
bad.store(true, Ordering::Relaxed);
95-
}
96+
$p::check($($args, )* diag_ctx);
9697
}).unwrap();
9798
handles.push_back(handle);
9899
}
99100
}
100101

101-
check!(target_specific_tests, &tests_path);
102+
// check!(target_specific_tests, &tests_path);
102103

103104
// Checks that are done on the cargo workspace.
104-
check!(deps, &root_path, &cargo, bless);
105-
check!(extdeps, &root_path);
105+
// check!(deps, &root_path, &cargo, bless);
106+
// check!(extdeps, &root_path);
106107

107108
// Checks over tests.
108-
check!(tests_placement, &root_path);
109-
check!(tests_revision_unpaired_stdout_stderr, &tests_path);
110-
check!(debug_artifacts, &tests_path);
111-
check!(ui_tests, &root_path, bless);
112-
check!(mir_opt_tests, &tests_path, bless);
113-
check!(rustdoc_gui_tests, &tests_path);
114-
check!(rustdoc_css_themes, &librustdoc_path);
115-
check!(rustdoc_templates, &librustdoc_path);
116-
check!(rustdoc_json, &src_path, &ci_info);
117-
check!(known_bug, &crashes_path);
118-
check!(unknown_revision, &tests_path);
109+
// check!(tests_placement, &root_path);
110+
// check!(tests_revision_unpaired_stdout_stderr, &tests_path);
111+
// check!(debug_artifacts, &tests_path);
112+
// check!(ui_tests, &root_path, bless);
113+
// check!(mir_opt_tests, &tests_path, bless);
114+
// check!(rustdoc_gui_tests, &tests_path);
115+
// check!(rustdoc_css_themes, &librustdoc_path);
116+
// check!(rustdoc_templates, &librustdoc_path);
117+
// check!(rustdoc_json, &src_path, &ci_info);
118+
// check!(known_bug, &crashes_path);
119+
// check!(unknown_revision, &tests_path);
119120

120121
// Checks that only make sense for the compiler.
121-
check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose, &ci_info);
122-
check!(fluent_alphabetical, &compiler_path, bless);
123-
check!(fluent_period, &compiler_path);
124-
check!(fluent_lowercase, &compiler_path);
125-
check!(target_policy, &root_path);
126-
check!(gcc_submodule, &root_path, &compiler_path);
122+
// check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose, &ci_info);
123+
// check!(fluent_alphabetical, &compiler_path, bless);
124+
// check!(fluent_period, &compiler_path);
125+
// check!(fluent_lowercase, &compiler_path);
126+
// check!(target_policy, &root_path);
127+
// check!(gcc_submodule, &root_path, &compiler_path);
127128

128129
// Checks that only make sense for the std libs.
129-
check!(pal, &library_path);
130+
// check!(pal, &library_path);
130131

131132
// Checks that need to be done for both the compiler and std libraries.
132-
check!(unit_tests, &src_path, false);
133-
check!(unit_tests, &compiler_path, false);
134-
check!(unit_tests, &library_path, true);
135-
136-
if bins::check_filesystem_support(&[&root_path], &output_directory) {
137-
check!(bins, &root_path);
138-
}
133+
// check!(unit_tests, &src_path, false);
134+
// check!(unit_tests, &compiler_path, false);
135+
// check!(unit_tests, &library_path, true);
136+
//
137+
// if bins::check_filesystem_support(&[&root_path], &output_directory) {
138+
// check!(bins, &root_path);
139+
// }
139140

140141
check!(style, &src_path);
141142
check!(style, &tests_path);
142143
check!(style, &compiler_path);
143144
check!(style, &library_path);
144145

145-
check!(edition, &src_path);
146-
check!(edition, &compiler_path);
147-
check!(edition, &library_path);
148-
149-
check!(alphabetical, &root_manifest);
150-
check!(alphabetical, &src_path);
151-
check!(alphabetical, &tests_path);
152-
check!(alphabetical, &compiler_path);
153-
check!(alphabetical, &library_path);
154-
155-
check!(x_version, &root_path, &cargo);
156-
157-
check!(triagebot, &root_path);
158-
159-
check!(filenames, &root_path);
146+
// check!(edition, &src_path);
147+
// check!(edition, &compiler_path);
148+
// check!(edition, &library_path);
149+
//
150+
// check!(alphabetical, &root_manifest);
151+
// check!(alphabetical, &src_path);
152+
// check!(alphabetical, &tests_path);
153+
// check!(alphabetical, &compiler_path);
154+
// check!(alphabetical, &library_path);
155+
//
156+
// check!(x_version, &root_path, &cargo);
157+
//
158+
// check!(triagebot, &root_path);
159+
//
160+
// check!(filenames, &root_path);
160161

161162
let collected = {
162163
drain_handles(&mut handles);
@@ -175,24 +176,24 @@ fn main() {
175176
}
176177
r
177178
};
178-
check!(unstable_book, &src_path, collected);
179-
180-
check!(
181-
extra_checks,
182-
&root_path,
183-
&output_directory,
184-
&ci_info,
185-
&librustdoc_path,
186-
&tools_path,
187-
&npm,
188-
&cargo,
189-
bless,
190-
extra_checks,
191-
pos_args
192-
);
179+
// check!(unstable_book, &src_path, collected);
180+
//
181+
// check!(
182+
// extra_checks,
183+
// &root_path,
184+
// &output_directory,
185+
// &ci_info,
186+
// &librustdoc_path,
187+
// &tools_path,
188+
// &npm,
189+
// &cargo,
190+
// bless,
191+
// extra_checks,
192+
// pos_args
193+
// );
193194
});
194195

195-
if bad.load(Ordering::Relaxed) {
196+
if diag_ctx.into_conclusion() {
196197
eprintln!("some tidy checks failed");
197198
process::exit(1);
198199
}

0 commit comments

Comments
 (0)