From 3fe0b3d4d04dd86cbe22770217f3b42a8856d382 Mon Sep 17 00:00:00 2001 From: Mu001999 Date: Sun, 23 Feb 2025 11:29:35 +0800 Subject: [PATCH 01/28] not lint break with label and unsafe block --- compiler/rustc_parse/src/parser/expr.rs | 14 ++++++++------ tests/ui/lint/break-with-label-and-unsafe-block.rs | 11 +++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lint/break-with-label-and-unsafe-block.rs diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index e0e6c2177da54..21e01ff535a8c 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1859,13 +1859,15 @@ impl<'a> Parser<'a> { let mut expr = self.parse_expr_opt()?; if let Some(expr) = &mut expr { if label.is_some() - && matches!( - expr.kind, + && match &expr.kind { ExprKind::While(_, _, None) - | ExprKind::ForLoop { label: None, .. } - | ExprKind::Loop(_, None, _) - | ExprKind::Block(_, None) - ) + | ExprKind::ForLoop { label: None, .. } + | ExprKind::Loop(_, None, _) => true, + ExprKind::Block(block, None) => { + matches!(block.rules, BlockCheckMode::Default) + } + _ => false, + } { self.psess.buffer_lint( BREAK_WITH_LABEL_AND_LOOP, diff --git a/tests/ui/lint/break-with-label-and-unsafe-block.rs b/tests/ui/lint/break-with-label-and-unsafe-block.rs new file mode 100644 index 0000000000000..a76a576147556 --- /dev/null +++ b/tests/ui/lint/break-with-label-and-unsafe-block.rs @@ -0,0 +1,11 @@ +//@ check-pass + +#![deny(break_with_label_and_loop)] + +unsafe fn foo() -> i32 { 42 } + +fn main () { + 'label: loop { + break 'label unsafe { foo() } + }; +} From 1c1febc59db038876d7fe78a1f056bf324fdff6a Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 11:19:57 +0300 Subject: [PATCH 02/28] add new config option: `include` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 25 ++++++++++++++++++++++- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 25ec64f90b53d..2e88108129010 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -701,6 +701,7 @@ pub(crate) struct TomlConfig { target: Option>, dist: Option, profile: Option, + include: Option>, } /// This enum is used for deserializing change IDs from TOML, allowing both numeric values and the string `"ignore"`. @@ -753,7 +754,7 @@ trait Merge { impl Merge for TomlConfig { fn merge( &mut self, - TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id }: Self, + TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self, replace: ReplaceOpt, ) { fn do_merge(x: &mut Option, y: Option, replace: ReplaceOpt) { @@ -766,6 +767,17 @@ impl Merge for TomlConfig { } } + for include_path in include.clone().unwrap_or_default() { + let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { + eprintln!( + "ERROR: Failed to parse default config profile at '{}': {e}", + include_path.display() + ); + exit!(2); + }); + self.merge(included_toml, ReplaceOpt::Override); + } + self.change_id.inner.merge(change_id.inner, replace); self.profile.merge(profile, replace); @@ -1600,6 +1612,17 @@ impl Config { toml.merge(included_toml, ReplaceOpt::IgnoreDuplicate); } + for include_path in toml.include.clone().unwrap_or_default() { + let included_toml = get_toml(&include_path).unwrap_or_else(|e| { + eprintln!( + "ERROR: Failed to parse default config profile at '{}': {e}", + include_path.display() + ); + exit!(2); + }); + toml.merge(included_toml, ReplaceOpt::Override); + } + let mut override_toml = TomlConfig::default(); for option in flags.set.iter() { fn get_table(option: &str) -> Result { diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 48b6f77e8a587..3f1885a425f83 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -396,4 +396,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Info, summary: "Added a new option `build.compiletest-use-stage0-libtest` to force `compiletest` to use the stage 0 libtest.", }, + ChangeInfo { + change_id: 138934, + severity: ChangeSeverity::Info, + summary: "Added new option `include` to create config extensions.", + }, ]; From 89e3befe63f35b7556614d85ce63214f62a9a771 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 25 Mar 2025 20:17:32 +0300 Subject: [PATCH 03/28] document config extensions Signed-off-by: onur-ozkan --- .../rustc-dev-guide/src/building/suggested.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/doc/rustc-dev-guide/src/building/suggested.md b/src/doc/rustc-dev-guide/src/building/suggested.md index 43ff2ba726f91..5d37a78af8855 100644 --- a/src/doc/rustc-dev-guide/src/building/suggested.md +++ b/src/doc/rustc-dev-guide/src/building/suggested.md @@ -20,6 +20,42 @@ your `.git/hooks` folder as `pre-push` (without the `.sh` extension!). You can also install the hook as a step of running `./x setup`! +## Config extensions + +When working on different tasks, you might need to switch between different bootstrap configurations. +Sometimes you may want to keep an old configuration for future use. But saving raw config values in +random files and manually copying and pasting them can quickly become messy, especially if you have a +long history of different configurations. + +To simplify managing multiple configurations, you can create config extensions. + +For example, you can create a simple config file named `cross.toml`: + +```toml +[build] +build = "x86_64-unknown-linux-gnu" +host = ["i686-unknown-linux-gnu"] +target = ["i686-unknown-linux-gnu"] + + +[llvm] +download-ci-llvm = false + +[target.x86_64-unknown-linux-gnu] +llvm-config = "/path/to/llvm-19/bin/llvm-config" +``` + +Then, include this in your `bootstrap.toml`: + +```toml +include = ["cross.toml"] +``` + +You can also include extensions within extensions recursively. + +**Note:** In the `include` field, the overriding logic follows a right-to-left order. Also, the outer +extension/config always overrides the inner ones. + ## Configuring `rust-analyzer` for `rustc` ### Project-local rust-analyzer setup From 4e80659b32a794513646b93890c9a6eeb103de17 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 11:44:33 +0300 Subject: [PATCH 04/28] implement cyclic inclusion handling Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 51 ++++++++++++++++++------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 2e88108129010..86566d0eee178 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -6,6 +6,7 @@ use std::cell::{Cell, RefCell}; use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt::{self, Display}; +use std::hash::Hash; use std::io::IsTerminal; use std::path::{Path, PathBuf, absolute}; use std::process::Command; @@ -748,19 +749,25 @@ enum ReplaceOpt { } trait Merge { - fn merge(&mut self, other: Self, replace: ReplaceOpt); + fn merge( + &mut self, + included_extensions: &mut HashSet, + other: Self, + replace: ReplaceOpt, + ); } impl Merge for TomlConfig { fn merge( &mut self, + included_extensions: &mut HashSet, TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self, replace: ReplaceOpt, ) { fn do_merge(x: &mut Option, y: Option, replace: ReplaceOpt) { if let Some(new) = y { if let Some(original) = x { - original.merge(new, replace); + original.merge(&mut Default::default(), new, replace); } else { *x = Some(new); } @@ -775,11 +782,20 @@ impl Merge for TomlConfig { ); exit!(2); }); - self.merge(included_toml, ReplaceOpt::Override); + + assert!( + included_extensions.insert(include_path.clone()), + "Cyclic inclusion detected: '{}' is being included again before its previous inclusion was fully processed.", + include_path.display() + ); + + self.merge(included_extensions, included_toml, ReplaceOpt::Override); + + included_extensions.remove(&include_path); } - self.change_id.inner.merge(change_id.inner, replace); - self.profile.merge(profile, replace); + self.change_id.inner.merge(&mut Default::default(), change_id.inner, replace); + self.profile.merge(&mut Default::default(), profile, replace); do_merge(&mut self.build, build, replace); do_merge(&mut self.install, install, replace); @@ -794,7 +810,7 @@ impl Merge for TomlConfig { (Some(original_target), Some(new_target)) => { for (triple, new) in new_target { if let Some(original) = original_target.get_mut(&triple) { - original.merge(new, replace); + original.merge(&mut Default::default(), new, replace); } else { original_target.insert(triple, new); } @@ -815,7 +831,7 @@ macro_rules! define_config { } impl Merge for $name { - fn merge(&mut self, other: Self, replace: ReplaceOpt) { + fn merge(&mut self, _included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt) { $( match replace { ReplaceOpt::IgnoreDuplicate => { @@ -915,7 +931,12 @@ macro_rules! define_config { } impl Merge for Option { - fn merge(&mut self, other: Self, replace: ReplaceOpt) { + fn merge( + &mut self, + _included_extensions: &mut HashSet, + other: Self, + replace: ReplaceOpt, + ) { match replace { ReplaceOpt::IgnoreDuplicate => { if self.is_none() { @@ -1609,7 +1630,7 @@ impl Config { ); exit!(2); }); - toml.merge(included_toml, ReplaceOpt::IgnoreDuplicate); + toml.merge(&mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate); } for include_path in toml.include.clone().unwrap_or_default() { @@ -1620,7 +1641,7 @@ impl Config { ); exit!(2); }); - toml.merge(included_toml, ReplaceOpt::Override); + toml.merge(&mut Default::default(), included_toml, ReplaceOpt::Override); } let mut override_toml = TomlConfig::default(); @@ -1631,7 +1652,7 @@ impl Config { let mut err = match get_table(option) { Ok(v) => { - override_toml.merge(v, ReplaceOpt::ErrorOnDuplicate); + override_toml.merge(&mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate); continue; } Err(e) => e, @@ -1642,7 +1663,11 @@ impl Config { if !value.contains('"') { match get_table(&format!(r#"{key}="{value}""#)) { Ok(v) => { - override_toml.merge(v, ReplaceOpt::ErrorOnDuplicate); + override_toml.merge( + &mut Default::default(), + v, + ReplaceOpt::ErrorOnDuplicate, + ); continue; } Err(e) => err = e, @@ -1652,7 +1677,7 @@ impl Config { eprintln!("failed to parse override `{option}`: `{err}"); exit!(2) } - toml.merge(override_toml, ReplaceOpt::Override); + toml.merge(&mut Default::default(), override_toml, ReplaceOpt::Override); config.change_id = toml.change_id.inner; From 78cb4538ee203b08441382b5cd64b1989ed5b3b9 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 11:53:32 +0300 Subject: [PATCH 05/28] document `include` in `bootstrap.example.toml` Signed-off-by: onur-ozkan --- bootstrap.example.toml | 5 +++++ src/doc/rustc-dev-guide/src/building/suggested.md | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/bootstrap.example.toml b/bootstrap.example.toml index 0927f648635ce..0a314f65ec229 100644 --- a/bootstrap.example.toml +++ b/bootstrap.example.toml @@ -19,6 +19,11 @@ # Note that this has no default value (x.py uses the defaults in `bootstrap.example.toml`). #profile = +# Inherits configuration values from different configuration files (a.k.a. config extensions). +# Supports absolute paths, and uses the current directory (where the bootstrap was invoked) +# as the base if the given path is not absolute. +#include = [] + # Keeps track of major changes made to this configuration. # # This value also represents ID of the PR that caused major changes. Meaning, diff --git a/src/doc/rustc-dev-guide/src/building/suggested.md b/src/doc/rustc-dev-guide/src/building/suggested.md index 5d37a78af8855..b2e258be079d5 100644 --- a/src/doc/rustc-dev-guide/src/building/suggested.md +++ b/src/doc/rustc-dev-guide/src/building/suggested.md @@ -53,8 +53,9 @@ include = ["cross.toml"] You can also include extensions within extensions recursively. -**Note:** In the `include` field, the overriding logic follows a right-to-left order. Also, the outer -extension/config always overrides the inner ones. +**Note:** In the `include` field, the overriding logic follows a right-to-left order. For example, +in `include = ["a.toml", "b.toml"]`, extension `b.toml` overrides `a.toml`. Also, parent extensions +always overrides the inner ones. ## Configuring `rust-analyzer` for `rustc` From 3f70f197f2d131a1f57912f202072b2447d3e326 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 1 Apr 2025 21:56:58 +0000 Subject: [PATCH 06/28] apply nit notes Signed-off-by: onur-ozkan --- bootstrap.example.toml | 3 + src/bootstrap/src/core/config/config.rs | 95 ++++++++++++++++++------- 2 files changed, 72 insertions(+), 26 deletions(-) diff --git a/bootstrap.example.toml b/bootstrap.example.toml index 0a314f65ec229..72c4492d465d8 100644 --- a/bootstrap.example.toml +++ b/bootstrap.example.toml @@ -22,6 +22,9 @@ # Inherits configuration values from different configuration files (a.k.a. config extensions). # Supports absolute paths, and uses the current directory (where the bootstrap was invoked) # as the base if the given path is not absolute. +# +# The overriding logic follows a right-to-left order. For example, in `include = ["a.toml", "b.toml"]`, +# extension `b.toml` overrides `a.toml`. Also, parent extensions always overrides the inner ones. #include = [] # Keeps track of major changes made to this configuration. diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 86566d0eee178..e70ac74c7dae2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -751,6 +751,7 @@ enum ReplaceOpt { trait Merge { fn merge( &mut self, + parent_config_path: Option, included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt, @@ -760,6 +761,7 @@ trait Merge { impl Merge for TomlConfig { fn merge( &mut self, + parent_config_path: Option, included_extensions: &mut HashSet, TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self, replace: ReplaceOpt, @@ -767,19 +769,27 @@ impl Merge for TomlConfig { fn do_merge(x: &mut Option, y: Option, replace: ReplaceOpt) { if let Some(new) = y { if let Some(original) = x { - original.merge(&mut Default::default(), new, replace); + original.merge(None, &mut Default::default(), new, replace); } else { *x = Some(new); } } } - for include_path in include.clone().unwrap_or_default() { + let parent_dir = parent_config_path + .as_ref() + .and_then(|p| p.parent().map(ToOwned::to_owned)) + .unwrap_or_default(); + + for include_path in include.clone().unwrap_or_default().iter().rev() { + let include_path = parent_dir.join(include_path); + let include_path = include_path.canonicalize().unwrap_or_else(|e| { + eprintln!("ERROR: Failed to canonicalize '{}' path: {e}", include_path.display()); + exit!(2); + }); + let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { - eprintln!( - "ERROR: Failed to parse default config profile at '{}': {e}", - include_path.display() - ); + eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); }); @@ -789,13 +799,20 @@ impl Merge for TomlConfig { include_path.display() ); - self.merge(included_extensions, included_toml, ReplaceOpt::Override); + self.merge( + Some(include_path.clone()), + included_extensions, + included_toml, + // Ensures that parent configuration always takes precedence + // over child configurations. + ReplaceOpt::IgnoreDuplicate, + ); included_extensions.remove(&include_path); } - self.change_id.inner.merge(&mut Default::default(), change_id.inner, replace); - self.profile.merge(&mut Default::default(), profile, replace); + self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace); + self.profile.merge(None, &mut Default::default(), profile, replace); do_merge(&mut self.build, build, replace); do_merge(&mut self.install, install, replace); @@ -810,7 +827,7 @@ impl Merge for TomlConfig { (Some(original_target), Some(new_target)) => { for (triple, new) in new_target { if let Some(original) = original_target.get_mut(&triple) { - original.merge(&mut Default::default(), new, replace); + original.merge(None, &mut Default::default(), new, replace); } else { original_target.insert(triple, new); } @@ -831,7 +848,13 @@ macro_rules! define_config { } impl Merge for $name { - fn merge(&mut self, _included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt) { + fn merge( + &mut self, + _parent_config_path: Option, + _included_extensions: &mut HashSet, + other: Self, + replace: ReplaceOpt + ) { $( match replace { ReplaceOpt::IgnoreDuplicate => { @@ -933,6 +956,7 @@ macro_rules! define_config { impl Merge for Option { fn merge( &mut self, + _parent_config_path: Option, _included_extensions: &mut HashSet, other: Self, replace: ReplaceOpt, @@ -1581,7 +1605,8 @@ impl Config { // but not if `bootstrap.toml` hasn't been created. let mut toml = if !using_default_path || toml_path.exists() { config.config = Some(if cfg!(not(test)) { - toml_path.canonicalize().unwrap() + toml_path = toml_path.canonicalize().unwrap(); + toml_path.clone() } else { toml_path.clone() }); @@ -1609,6 +1634,24 @@ impl Config { toml.profile = Some("dist".into()); } + // Reverse the list to ensure the last added config extension remains the most dominant. + // For example, given ["a.toml", "b.toml"], "b.toml" should take precedence over "a.toml". + // + // This must be handled before applying the `profile` since `include`s should always take + // precedence over `profile`s. + for include_path in toml.include.clone().unwrap_or_default().iter().rev() { + let included_toml = get_toml(include_path).unwrap_or_else(|e| { + eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); + exit!(2); + }); + toml.merge( + Some(toml_path.join(include_path)), + &mut Default::default(), + included_toml, + ReplaceOpt::IgnoreDuplicate, + ); + } + if let Some(include) = &toml.profile { // Allows creating alias for profile names, allowing // profiles to be renamed while maintaining back compatibility @@ -1630,18 +1673,12 @@ impl Config { ); exit!(2); }); - toml.merge(&mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate); - } - - for include_path in toml.include.clone().unwrap_or_default() { - let included_toml = get_toml(&include_path).unwrap_or_else(|e| { - eprintln!( - "ERROR: Failed to parse default config profile at '{}': {e}", - include_path.display() - ); - exit!(2); - }); - toml.merge(&mut Default::default(), included_toml, ReplaceOpt::Override); + toml.merge( + Some(include_path), + &mut Default::default(), + included_toml, + ReplaceOpt::IgnoreDuplicate, + ); } let mut override_toml = TomlConfig::default(); @@ -1652,7 +1689,12 @@ impl Config { let mut err = match get_table(option) { Ok(v) => { - override_toml.merge(&mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate); + override_toml.merge( + None, + &mut Default::default(), + v, + ReplaceOpt::ErrorOnDuplicate, + ); continue; } Err(e) => e, @@ -1664,6 +1706,7 @@ impl Config { match get_table(&format!(r#"{key}="{value}""#)) { Ok(v) => { override_toml.merge( + None, &mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate, @@ -1677,7 +1720,7 @@ impl Config { eprintln!("failed to parse override `{option}`: `{err}"); exit!(2) } - toml.merge(&mut Default::default(), override_toml, ReplaceOpt::Override); + toml.merge(None, &mut Default::default(), override_toml, ReplaceOpt::Override); config.change_id = toml.change_id.inner; From 8e6f50bb4d30801d04619a4cd6406b320eb8919f Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 15 Apr 2025 10:44:19 +0300 Subject: [PATCH 07/28] fix path and the ordering logic Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 54 +++++++++++++------------ 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e70ac74c7dae2..19ed442c500f9 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -776,6 +776,30 @@ impl Merge for TomlConfig { } } + self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace); + self.profile.merge(None, &mut Default::default(), profile, replace); + + do_merge(&mut self.build, build, replace); + do_merge(&mut self.install, install, replace); + do_merge(&mut self.llvm, llvm, replace); + do_merge(&mut self.gcc, gcc, replace); + do_merge(&mut self.rust, rust, replace); + do_merge(&mut self.dist, dist, replace); + + match (self.target.as_mut(), target) { + (_, None) => {} + (None, Some(target)) => self.target = Some(target), + (Some(original_target), Some(new_target)) => { + for (triple, new) in new_target { + if let Some(original) = original_target.get_mut(&triple) { + original.merge(None, &mut Default::default(), new, replace); + } else { + original_target.insert(triple, new); + } + } + } + } + let parent_dir = parent_config_path .as_ref() .and_then(|p| p.parent().map(ToOwned::to_owned)) @@ -810,30 +834,6 @@ impl Merge for TomlConfig { included_extensions.remove(&include_path); } - - self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace); - self.profile.merge(None, &mut Default::default(), profile, replace); - - do_merge(&mut self.build, build, replace); - do_merge(&mut self.install, install, replace); - do_merge(&mut self.llvm, llvm, replace); - do_merge(&mut self.gcc, gcc, replace); - do_merge(&mut self.rust, rust, replace); - do_merge(&mut self.dist, dist, replace); - - match (self.target.as_mut(), target) { - (_, None) => {} - (None, Some(target)) => self.target = Some(target), - (Some(original_target), Some(new_target)) => { - for (triple, new) in new_target { - if let Some(original) = original_target.get_mut(&triple) { - original.merge(None, &mut Default::default(), new, replace); - } else { - original_target.insert(triple, new); - } - } - } - } } } @@ -1640,12 +1640,14 @@ impl Config { // This must be handled before applying the `profile` since `include`s should always take // precedence over `profile`s. for include_path in toml.include.clone().unwrap_or_default().iter().rev() { - let included_toml = get_toml(include_path).unwrap_or_else(|e| { + let include_path = toml_path.parent().unwrap().join(include_path); + + let included_toml = get_toml(&include_path).unwrap_or_else(|e| { eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); }); toml.merge( - Some(toml_path.join(include_path)), + Some(include_path), &mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate, From 7dfb457745d7f873eb5db1c3fb39d58fa768740d Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 15 Apr 2025 11:53:21 +0300 Subject: [PATCH 08/28] add FIXME note in `TomlConfig::merge` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 19ed442c500f9..b7afd81dfdb5b 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -812,6 +812,8 @@ impl Merge for TomlConfig { exit!(2); }); + // FIXME: Similar to `Config::parse_inner`, allow passing a custom `get_toml` from the caller to + // improve testability since `Config::get_toml` does nothing when `cfg(test)` is enabled. let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); From 6d52b51d3e6321b7a1d24e5f995aa709057153e3 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 15 Apr 2025 19:10:31 +0300 Subject: [PATCH 09/28] add comment in `TomlConfig::merge` about the merge order Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index b7afd81dfdb5b..e15aab4ad506e 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -805,6 +805,8 @@ impl Merge for TomlConfig { .and_then(|p| p.parent().map(ToOwned::to_owned)) .unwrap_or_default(); + // `include` handled later since we ignore duplicates using `ReplaceOpt::IgnoreDuplicate` to + // keep the upper-level configuration to take precedence. for include_path in include.clone().unwrap_or_default().iter().rev() { let include_path = parent_dir.join(include_path); let include_path = include_path.canonicalize().unwrap_or_else(|e| { From 2024e26881ea8bdfb41f53e257464be4332abc79 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 15 Apr 2025 01:01:51 +0000 Subject: [PATCH 10/28] Don't canonicalize crate paths --- compiler/rustc_metadata/src/locator.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index 112954eca0dfe..f0a898d678cae 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -427,12 +427,21 @@ impl<'a> CrateLocator<'a> { let (rlibs, rmetas, dylibs) = candidates.entry(hash.to_string()).or_default(); - let path = - try_canonicalize(&spf.path).unwrap_or_else(|_| spf.path.to_path_buf()); - if seen_paths.contains(&path) { - continue; - }; - seen_paths.insert(path.clone()); + { + // As a perforamnce optimisation we canonicalize the path and skip + // ones we've already seeen. This allows us to ignore crates + // we know are exactual equal to ones we've already found. + // Going to the same crate through different symlinks does not change the result. + let path = try_canonicalize(&spf.path) + .unwrap_or_else(|_| spf.path.to_path_buf()); + if seen_paths.contains(&path) { + continue; + }; + seen_paths.insert(path); + } + // Use the original path (potentially with unresolved symlinks), + // filesystem code should not care, but this is nicer for diagnostics. + let path = spf.path.to_path_buf(); match kind { CrateFlavor::Rlib => rlibs.insert(path, search_path.kind), CrateFlavor::Rmeta => rmetas.insert(path, search_path.kind), From 52f35d013116559035621eb83d0e9680ecf74a49 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 15 Apr 2025 12:55:33 +0000 Subject: [PATCH 11/28] Test for relative paths in crate path diagnostics --- .../crateresolve1-1.rs | 6 ++++ .../crateresolve1-2.rs | 6 ++++ .../multiple-candidates.rs | 3 ++ .../multiple-candidates.stderr | 12 +++++++ .../rmake.rs | 34 +++++++++++++++++++ 5 files changed, 61 insertions(+) create mode 100644 tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs create mode 100644 tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs create mode 100644 tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs create mode 100644 tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr create mode 100644 tests/run-make/crate-loading-multiple-candidates/rmake.rs diff --git a/tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs new file mode 100644 index 0000000000000..fe00f041a862e --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-1.rs @@ -0,0 +1,6 @@ +#![crate_name = "crateresolve1"] +#![crate_type = "lib"] + +pub fn f() -> isize { + 10 +} diff --git a/tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs new file mode 100644 index 0000000000000..0fb8591b3a52e --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/crateresolve1-2.rs @@ -0,0 +1,6 @@ +#![crate_name = "crateresolve1"] +#![crate_type = "lib"] + +pub fn f() -> isize { + 20 +} diff --git a/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs new file mode 100644 index 0000000000000..27cd7ca5c2039 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.rs @@ -0,0 +1,3 @@ +extern crate crateresolve1; + +fn main() {} diff --git a/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr new file mode 100644 index 0000000000000..de7fc3b0feb93 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/multiple-candidates.stderr @@ -0,0 +1,12 @@ +error[E0464]: multiple candidates for `rlib` dependency `crateresolve1` found + --> multiple-candidates.rs:1:1 + | +LL | extern crate crateresolve1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: candidate #1: ./mylibs/libcrateresolve1-1.rlib + = note: candidate #2: ./mylibs/libcrateresolve1-2.rlib + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0464`. diff --git a/tests/run-make/crate-loading-multiple-candidates/rmake.rs b/tests/run-make/crate-loading-multiple-candidates/rmake.rs new file mode 100644 index 0000000000000..ce090850500b8 --- /dev/null +++ b/tests/run-make/crate-loading-multiple-candidates/rmake.rs @@ -0,0 +1,34 @@ +//@ needs-symlink +//@ ignore-cross-compile + +// Tests that the multiple candidate dependencies diagnostic prints relative +// paths if a relative library path was passed in. + +use run_make_support::{bare_rustc, diff, rfs, rustc}; + +fn main() { + // Check that relative paths are preserved in the diagnostic + rfs::create_dir("mylibs"); + rustc().input("crateresolve1-1.rs").out_dir("mylibs").extra_filename("-1").run(); + rustc().input("crateresolve1-2.rs").out_dir("mylibs").extra_filename("-2").run(); + check("./mylibs"); + + // Check that symlinks aren't followed when printing the diagnostic + rfs::rename("mylibs", "original"); + rfs::symlink_dir("original", "mylibs"); + check("./mylibs"); +} + +fn check(library_path: &str) { + let out = rustc() + .input("multiple-candidates.rs") + .library_search_path(library_path) + .ui_testing() + .run_fail() + .stderr_utf8(); + diff() + .expected_file("multiple-candidates.stderr") + .normalize(r"\\", "/") + .actual_text("(rustc)", &out) + .run(); +} From 827047895a4c036f652b7105443f6305164f0e25 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 16 Apr 2025 20:53:51 +0300 Subject: [PATCH 12/28] resolve config include FIXME Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index e15aab4ad506e..0c221abc9698d 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -814,9 +814,7 @@ impl Merge for TomlConfig { exit!(2); }); - // FIXME: Similar to `Config::parse_inner`, allow passing a custom `get_toml` from the caller to - // improve testability since `Config::get_toml` does nothing when `cfg(test)` is enabled. - let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| { + let included_toml = Config::get_toml_inner(&include_path).unwrap_or_else(|e| { eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display()); exit!(2); }); @@ -1424,13 +1422,15 @@ impl Config { Self::get_toml(&builder_config_path) } - #[cfg(test)] - pub(crate) fn get_toml(_: &Path) -> Result { - Ok(TomlConfig::default()) + pub(crate) fn get_toml(file: &Path) -> Result { + #[cfg(test)] + return Ok(TomlConfig::default()); + + #[cfg(not(test))] + Self::get_toml_inner(file) } - #[cfg(not(test))] - pub(crate) fn get_toml(file: &Path) -> Result { + fn get_toml_inner(file: &Path) -> Result { let contents = t!(fs::read_to_string(file), format!("config file {} not found", file.display())); // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of From 5a38550a39a99b1cd98f9be769e4f1face3d1256 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 2 Apr 2025 20:33:37 -0400 Subject: [PATCH 13/28] Deduplicate nix code And clean it up a little. --- src/tools/nix-dev-shell/flake.nix | 46 +++++++--------- src/tools/nix-dev-shell/shell.nix | 38 +++++++------ src/tools/nix-dev-shell/x/default.nix | 77 ++++++++++++++++++++++++--- 3 files changed, 111 insertions(+), 50 deletions(-) diff --git a/src/tools/nix-dev-shell/flake.nix b/src/tools/nix-dev-shell/flake.nix index 1b838bd2f7b37..b8287de5fcf09 100644 --- a/src/tools/nix-dev-shell/flake.nix +++ b/src/tools/nix-dev-shell/flake.nix @@ -1,32 +1,24 @@ { description = "rustc dev shell"; - inputs = { - nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; - flake-utils.url = "github:numtide/flake-utils"; - }; + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; - outputs = { self, nixpkgs, flake-utils, ... }: - flake-utils.lib.eachDefaultSystem (system: - let - pkgs = import nixpkgs { inherit system; }; - x = import ./x { inherit pkgs; }; - in - { - devShells.default = with pkgs; mkShell { - name = "rustc-dev-shell"; - nativeBuildInputs = with pkgs; [ - binutils cmake ninja pkg-config python3 git curl cacert patchelf nix - ]; - buildInputs = with pkgs; [ - openssl glibc.out glibc.static x - ]; - # Avoid creating text files for ICEs. - RUSTC_ICE = "0"; - # Provide `libstdc++.so.6` for the self-contained lld. - # Provide `libz.so.1`. - LD_LIBRARY_PATH = "${with pkgs; lib.makeLibraryPath [stdenv.cc.cc.lib zlib]}"; - }; - } - ); + outputs = + { + self, + nixpkgs, + }: + let + inherit (nixpkgs) lib; + forEachSystem = lib.genAttrs lib.systems.flakeExposed; + in + { + devShells = forEachSystem (system: { + default = nixpkgs.legacyPackages.${system}.callPackage ./shell.nix { }; + }); + + packages = forEachSystem (system: { + default = nixpkgs.legacyPackages.${system}.callPackage ./x { }; + }); + }; } diff --git a/src/tools/nix-dev-shell/shell.nix b/src/tools/nix-dev-shell/shell.nix index a3f5969bd812d..0adbacf7e8d56 100644 --- a/src/tools/nix-dev-shell/shell.nix +++ b/src/tools/nix-dev-shell/shell.nix @@ -1,18 +1,26 @@ -{ pkgs ? import {} }: -let - x = import ./x { inherit pkgs; }; +{ + pkgs ? import { }, +}: +let + inherit (pkgs.lib) lists attrsets; + + x = pkgs.callPackage ./x { }; + inherit (x.passthru) cacert env; in pkgs.mkShell { - name = "rustc"; - nativeBuildInputs = with pkgs; [ - binutils cmake ninja pkg-config python3 git curl cacert patchelf nix - ]; - buildInputs = with pkgs; [ - openssl glibc.out glibc.static x - ]; - # Avoid creating text files for ICEs. - RUSTC_ICE = "0"; - # Provide `libstdc++.so.6` for the self-contained lld. - # Provide `libz.so.1` - LD_LIBRARY_PATH = "${with pkgs; lib.makeLibraryPath [stdenv.cc.cc.lib zlib]}"; + name = "rustc-shell"; + + inputsFrom = [ x ]; + packages = [ + pkgs.git + pkgs.nix + x + # Get the runtime deps of the x wrapper + ] ++ lists.flatten (attrsets.attrValues env); + + env = { + # Avoid creating text files for ICEs. + RUSTC_ICE = 0; + SSL_CERT_FILE = cacert; + }; } diff --git a/src/tools/nix-dev-shell/x/default.nix b/src/tools/nix-dev-shell/x/default.nix index e6dfbad6f19c8..422c1c4a2aed8 100644 --- a/src/tools/nix-dev-shell/x/default.nix +++ b/src/tools/nix-dev-shell/x/default.nix @@ -1,22 +1,83 @@ { - pkgs ? import { }, + pkgs, + lib, + stdenv, + rustc, + python3, + makeBinaryWrapper, + # Bootstrap + curl, + pkg-config, + libiconv, + openssl, + patchelf, + cacert, + zlib, + # LLVM Deps + ninja, + cmake, + glibc, }: -pkgs.stdenv.mkDerivation { - name = "x"; +stdenv.mkDerivation (self: { + strictDeps = true; + name = "x-none"; + + outputs = [ + "out" + "unwrapped" + ]; src = ./x.rs; dontUnpack = true; - nativeBuildInputs = with pkgs; [ rustc ]; + nativeBuildInputs = [ + rustc + makeBinaryWrapper + ]; + env.PYTHON = python3.interpreter; buildPhase = '' - PYTHON=${pkgs.lib.getExe pkgs.python3} rustc -Copt-level=3 --crate-name x $src --out-dir $out/bin + rustc -Copt-level=3 --crate-name x $src --out-dir $unwrapped/bin ''; - meta = with pkgs.lib; { + installPhase = + let + inherit (self.passthru) cacert env; + in + '' + makeWrapper $unwrapped/bin/x $out/bin/x \ + --set-default SSL_CERT_FILE ${cacert} \ + --prefix CPATH ";" "${lib.makeSearchPath "include" env.cpath}" \ + --prefix PATH : ${lib.makeBinPath env.path} \ + --prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath env.ldLib} + ''; + + # For accessing them in the devshell + passthru = { + env = { + cpath = [ libiconv ]; + path = [ + python3 + patchelf + curl + pkg-config + cmake + ninja + stdenv.cc + ]; + ldLib = [ + openssl + zlib + stdenv.cc.cc.lib + ]; + }; + cacert = "${cacert}/etc/ssl/certs/ca-bundle.crt"; + }; + + meta = { description = "Helper for rust-lang/rust x.py"; homepage = "https://github.com/rust-lang/rust/blob/master/src/tools/x"; - license = licenses.mit; + license = lib.licenses.mit; mainProgram = "x"; }; -} +}) From d14df2652df492fe0f19c5e0ae5041b39b5a4d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 16 Apr 2025 13:24:01 +0200 Subject: [PATCH 14/28] Make `parent` in `download_auto_job_metrics` optional --- src/ci/citool/src/main.rs | 2 +- src/ci/citool/src/metrics.rs | 23 ++++++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs index a1956da352f5c..0fee862f5721b 100644 --- a/src/ci/citool/src/main.rs +++ b/src/ci/citool/src/main.rs @@ -180,7 +180,7 @@ fn postprocess_metrics( } fn post_merge_report(db: JobDatabase, current: String, parent: String) -> anyhow::Result<()> { - let metrics = download_auto_job_metrics(&db, &parent, ¤t)?; + let metrics = download_auto_job_metrics(&db, Some(&parent), ¤t)?; println!("\nComparing {parent} (parent) -> {current} (this PR)\n"); diff --git a/src/ci/citool/src/metrics.rs b/src/ci/citool/src/metrics.rs index a816fb3c4f165..3d8b1ad84cf72 100644 --- a/src/ci/citool/src/metrics.rs +++ b/src/ci/citool/src/metrics.rs @@ -46,24 +46,25 @@ pub struct JobMetrics { /// `parent` and `current` should be commit SHAs. pub fn download_auto_job_metrics( job_db: &JobDatabase, - parent: &str, + parent: Option<&str>, current: &str, ) -> anyhow::Result> { let mut jobs = HashMap::default(); for job in &job_db.auto_jobs { eprintln!("Downloading metrics of job {}", job.name); - let metrics_parent = match download_job_metrics(&job.name, parent) { - Ok(metrics) => Some(metrics), - Err(error) => { - eprintln!( - r#"Did not find metrics for job `{}` at `{parent}`: {error:?}. + let metrics_parent = + parent.and_then(|parent| match download_job_metrics(&job.name, parent) { + Ok(metrics) => Some(metrics), + Err(error) => { + eprintln!( + r#"Did not find metrics for job `{}` at `{parent}`: {error:?}. Maybe it was newly added?"#, - job.name - ); - None - } - }; + job.name + ); + None + } + }); let metrics_current = download_job_metrics(&job.name, current)?; jobs.insert( job.name.clone(), From 111c15c48e618b30a0cf11d91b135d87d73053a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 16 Apr 2025 17:20:53 +0200 Subject: [PATCH 15/28] Extract function for normalizing path delimiters to `utils` --- src/ci/citool/src/analysis.rs | 13 ++++++------- src/ci/citool/src/utils.rs | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ci/citool/src/analysis.rs b/src/ci/citool/src/analysis.rs index 9fc7c309bfbdc..62974be2dbe8c 100644 --- a/src/ci/citool/src/analysis.rs +++ b/src/ci/citool/src/analysis.rs @@ -8,9 +8,9 @@ use build_helper::metrics::{ }; use crate::github::JobInfoResolver; -use crate::metrics; use crate::metrics::{JobMetrics, JobName, get_test_suites}; use crate::utils::{output_details, pluralize}; +use crate::{metrics, utils}; /// Outputs durations of individual bootstrap steps from the gathered bootstrap invocations, /// and also a table with summarized information about executed tests. @@ -394,18 +394,17 @@ fn aggregate_tests(metrics: &JsonRoot) -> TestSuiteData { // Poor man's detection of doctests based on the "(line XYZ)" suffix let is_doctest = matches!(suite.metadata, TestSuiteMetadata::CargoPackage { .. }) && test.name.contains("(line"); - let test_entry = Test { name: generate_test_name(&test.name), stage, is_doctest }; + let test_entry = Test { + name: utils::normalize_path_delimiters(&test.name).to_string(), + stage, + is_doctest, + }; tests.insert(test_entry, test.outcome.clone()); } } TestSuiteData { tests } } -/// Normalizes Windows-style path delimiters to Unix-style paths. -fn generate_test_name(name: &str) -> String { - name.replace('\\', "/") -} - /// Prints test changes in Markdown format to stdout. fn report_test_diffs( diff: AggregatedTestDiffs, diff --git a/src/ci/citool/src/utils.rs b/src/ci/citool/src/utils.rs index a4c6ff85ef73c..0367d349a1ef4 100644 --- a/src/ci/citool/src/utils.rs +++ b/src/ci/citool/src/utils.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::path::Path; use anyhow::Context; @@ -28,3 +29,8 @@ where func(); println!("\n"); } + +/// Normalizes Windows-style path delimiters to Unix-style paths. +pub fn normalize_path_delimiters(name: &str) -> Cow { + if name.contains("\\") { name.replace('\\', "/").into() } else { name.into() } +} From c8a882b7b58a7b8f6b276ab64117b55bbe2626e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 09:41:12 +0200 Subject: [PATCH 16/28] Add command to `citool` for generating a test dashboard --- src/ci/citool/Cargo.lock | 67 ++++++ src/ci/citool/Cargo.toml | 1 + src/ci/citool/src/main.rs | 16 +- src/ci/citool/src/test_dashboard/mod.rs | 239 +++++++++++++++++++++ src/ci/citool/templates/layout.askama | 71 ++++++ src/ci/citool/templates/test_group.askama | 22 ++ src/ci/citool/templates/test_suites.askama | 18 ++ 7 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 src/ci/citool/src/test_dashboard/mod.rs create mode 100644 src/ci/citool/templates/layout.askama create mode 100644 src/ci/citool/templates/test_group.askama create mode 100644 src/ci/citool/templates/test_suites.askama diff --git a/src/ci/citool/Cargo.lock b/src/ci/citool/Cargo.lock index 2fe219f368b9c..43321d12cafcd 100644 --- a/src/ci/citool/Cargo.lock +++ b/src/ci/citool/Cargo.lock @@ -64,12 +64,63 @@ version = "1.0.95" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04" +[[package]] +name = "askama" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d4744ed2eef2645831b441d8f5459689ade2ab27c854488fbab1fbe94fce1a7" +dependencies = [ + "askama_derive", + "itoa", + "percent-encoding", + "serde", + "serde_json", +] + +[[package]] +name = "askama_derive" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d661e0f57be36a5c14c48f78d09011e67e0cb618f269cca9f2fd8d15b68c46ac" +dependencies = [ + "askama_parser", + "basic-toml", + "memchr", + "proc-macro2", + "quote", + "rustc-hash", + "serde", + "serde_derive", + "syn", +] + +[[package]] +name = "askama_parser" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf315ce6524c857bb129ff794935cf6d42c82a6cff60526fe2a63593de4d0d4f" +dependencies = [ + "memchr", + "serde", + "serde_derive", + "winnow", +] + [[package]] name = "base64" version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "basic-toml" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba62675e8242a4c4e806d12f11d136e626e6c8361d6b829310732241652a178a" +dependencies = [ + "serde", +] + [[package]] name = "build_helper" version = "0.1.0" @@ -104,6 +155,7 @@ name = "citool" version = "0.1.0" dependencies = [ "anyhow", + "askama", "build_helper", "clap", "csv", @@ -646,6 +698,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rustc-hash" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "357703d41365b4b27c590e3ed91eabb1b663f07c4c084095e60cbed4362dff0d" + [[package]] name = "rustls" version = "0.23.23" @@ -1026,6 +1084,15 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "winnow" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63d3fcd9bba44b03821e7d699eeee959f3126dcc4aa8e4ae18ec617c2a5cea10" +dependencies = [ + "memchr", +] + [[package]] name = "write16" version = "1.0.0" diff --git a/src/ci/citool/Cargo.toml b/src/ci/citool/Cargo.toml index f18436a126359..0e2aba3b9e3fc 100644 --- a/src/ci/citool/Cargo.toml +++ b/src/ci/citool/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" [dependencies] anyhow = "1" +askama = "0.13" clap = { version = "4.5", features = ["derive"] } csv = "1" diff = "0.1" diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs index 0fee862f5721b..a7a289fc3d4b1 100644 --- a/src/ci/citool/src/main.rs +++ b/src/ci/citool/src/main.rs @@ -4,6 +4,7 @@ mod datadog; mod github; mod jobs; mod metrics; +mod test_dashboard; mod utils; use std::collections::{BTreeMap, HashMap}; @@ -22,6 +23,7 @@ use crate::datadog::upload_datadog_metric; use crate::github::JobInfoResolver; use crate::jobs::RunType; use crate::metrics::{JobMetrics, download_auto_job_metrics, download_job_metrics, load_metrics}; +use crate::test_dashboard::generate_test_dashboard; use crate::utils::load_env_var; const CI_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/.."); @@ -234,6 +236,14 @@ enum Args { /// Current commit that will be compared to `parent`. current: String, }, + /// Generate a directory containing a HTML dashboard of test results from a CI run. + TestDashboard { + /// Commit SHA that was tested on CI to analyze. + current: String, + /// Output path for the HTML directory. + #[clap(long)] + output_dir: PathBuf, + }, } #[derive(clap::ValueEnum, Clone)] @@ -275,7 +285,11 @@ fn main() -> anyhow::Result<()> { postprocess_metrics(metrics_path, parent, job_name)?; } Args::PostMergeReport { current, parent } => { - post_merge_report(load_db(default_jobs_file)?, current, parent)?; + post_merge_report(load_db(&default_jobs_file)?, current, parent)?; + } + Args::TestDashboard { current, output_dir } => { + let db = load_db(&default_jobs_file)?; + generate_test_dashboard(db, ¤t, &output_dir)?; } } diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard/mod.rs new file mode 100644 index 0000000000000..ad9fe029e15df --- /dev/null +++ b/src/ci/citool/src/test_dashboard/mod.rs @@ -0,0 +1,239 @@ +use std::collections::{BTreeMap, HashMap}; +use std::fs::File; +use std::io::BufWriter; +use std::path::{Path, PathBuf}; + +use askama::Template; +use build_helper::metrics::{TestOutcome, TestSuiteMetadata}; + +use crate::jobs::JobDatabase; +use crate::metrics::{JobMetrics, JobName, download_auto_job_metrics, get_test_suites}; +use crate::utils::normalize_path_delimiters; + +pub struct TestInfo { + name: String, + jobs: Vec, +} + +struct JobTestResult { + job_name: String, + outcome: TestOutcome, +} + +#[derive(Default)] +struct TestSuiteInfo { + name: String, + tests: BTreeMap, +} + +/// Generate a set of HTML files into a directory that contain a dashboard of test results. +pub fn generate_test_dashboard( + db: JobDatabase, + current: &str, + output_dir: &Path, +) -> anyhow::Result<()> { + let metrics = download_auto_job_metrics(&db, None, current)?; + + let suites = gather_test_suites(&metrics); + + std::fs::create_dir_all(output_dir)?; + + let test_count = suites.test_count(); + write_page(output_dir, "index.html", &TestSuitesPage { suites, test_count })?; + + Ok(()) +} + +fn write_page(dir: &Path, name: &str, template: &T) -> anyhow::Result<()> { + let mut file = BufWriter::new(File::create(dir.join(name))?); + Template::write_into(template, &mut file)?; + Ok(()) +} + +fn gather_test_suites(job_metrics: &HashMap) -> TestSuites { + struct CoarseTestSuite<'a> { + kind: TestSuiteKind, + tests: BTreeMap>, + } + + let mut suites: HashMap = HashMap::new(); + + // First, gather tests from all jobs, stages and targets, and aggregate them per suite + for (job, metrics) in job_metrics { + let test_suites = get_test_suites(&metrics.current); + for suite in test_suites { + let (suite_name, stage, target, kind) = match &suite.metadata { + TestSuiteMetadata::CargoPackage { crates, stage, target, .. } => { + (crates.join(","), *stage, target, TestSuiteKind::Cargo) + } + TestSuiteMetadata::Compiletest { suite, stage, target, .. } => { + (suite.clone(), *stage, target, TestSuiteKind::Compiletest) + } + }; + let suite_entry = suites + .entry(suite_name.clone()) + .or_insert_with(|| CoarseTestSuite { kind, tests: Default::default() }); + let test_metadata = TestMetadata { job, stage, target }; + + for test in &suite.tests { + let test_name = normalize_test_name(&test.name, &suite_name); + let test_entry = suite_entry + .tests + .entry(test_name.clone()) + .or_insert_with(|| Test { name: test_name, passed: vec![], ignored: vec![] }); + match test.outcome { + TestOutcome::Passed => { + test_entry.passed.push(test_metadata); + } + TestOutcome::Ignored { ignore_reason: _ } => { + test_entry.ignored.push(test_metadata); + } + TestOutcome::Failed => { + eprintln!("Warning: failed test"); + } + } + } + } + } + + // Then, split the suites per directory + let mut suites = suites.into_iter().collect::>(); + suites.sort_by(|a, b| a.1.kind.cmp(&b.1.kind).then_with(|| a.0.cmp(&b.0))); + + let mut target_suites = vec![]; + for (suite_name, suite) in suites { + let suite = match suite.kind { + TestSuiteKind::Compiletest => TestSuite { + name: suite_name.clone(), + kind: TestSuiteKind::Compiletest, + group: build_test_group(&suite_name, suite.tests), + }, + TestSuiteKind::Cargo => { + let mut tests: Vec<_> = suite.tests.into_iter().collect(); + tests.sort_by(|a, b| a.0.cmp(&b.0)); + TestSuite { + name: format!("[cargo] {}", suite_name.clone()), + kind: TestSuiteKind::Cargo, + group: TestGroup { + name: suite_name, + root_tests: tests.into_iter().map(|t| t.1).collect(), + groups: vec![], + }, + } + } + }; + target_suites.push(suite); + } + + TestSuites { suites: target_suites } +} + +/// Recursively expand a test group based on filesystem hierarchy. +fn build_test_group<'a>(name: &str, tests: BTreeMap>) -> TestGroup<'a> { + let mut root_tests = vec![]; + let mut subdirs: BTreeMap>> = Default::default(); + + // Split tests into root tests and tests located in subdirectories + for (name, test) in tests { + let mut components = Path::new(&name).components().peekable(); + let subdir = components.next().unwrap(); + + if components.peek().is_none() { + // This is a root test + root_tests.push(test); + } else { + // This is a test in a nested directory + let subdir_tests = + subdirs.entry(subdir.as_os_str().to_str().unwrap().to_string()).or_default(); + let test_name = + components.into_iter().collect::().to_str().unwrap().to_string(); + subdir_tests.insert(test_name, test); + } + } + let dirs = subdirs + .into_iter() + .map(|(name, tests)| { + let group = build_test_group(&name, tests); + (name, group) + }) + .collect(); + + TestGroup { name: name.to_string(), root_tests, groups: dirs } +} + +/// Compiletest tests start with `[suite] tests/[suite]/a/b/c...`. +/// Remove the `[suite] tests/[suite]/` prefix so that we can find the filesystem path. +/// Also normalizes path delimiters. +fn normalize_test_name(name: &str, suite_name: &str) -> String { + let name = normalize_path_delimiters(name); + let name = name.as_ref(); + let name = name.strip_prefix(&format!("[{suite_name}]")).unwrap_or(name).trim(); + let name = name.strip_prefix("tests/").unwrap_or(name); + let name = name.strip_prefix(suite_name).unwrap_or(name); + name.trim_start_matches("/").to_string() +} + +#[derive(serde::Serialize)] +struct TestSuites<'a> { + suites: Vec>, +} + +impl<'a> TestSuites<'a> { + fn test_count(&self) -> u64 { + self.suites.iter().map(|suite| suite.group.test_count()).sum::() + } +} + +#[derive(serde::Serialize)] +struct TestSuite<'a> { + name: String, + kind: TestSuiteKind, + group: TestGroup<'a>, +} + +#[derive(Debug, serde::Serialize)] +struct Test<'a> { + name: String, + passed: Vec>, + ignored: Vec>, +} + +#[derive(Clone, Copy, Debug, serde::Serialize)] +struct TestMetadata<'a> { + job: &'a str, + stage: u32, + target: &'a str, +} + +// We have to use a template for the TestGroup instead of a macro, because +// macros cannot be recursive in askama at the moment. +#[derive(Template, serde::Serialize)] +#[template(path = "test_group.askama")] +/// Represents a group of tests +struct TestGroup<'a> { + name: String, + /// Tests located directly in this directory + root_tests: Vec>, + /// Nested directories with additional tests + groups: Vec<(String, TestGroup<'a>)>, +} + +impl<'a> TestGroup<'a> { + fn test_count(&self) -> u64 { + let root = self.root_tests.len() as u64; + self.groups.iter().map(|(_, group)| group.test_count()).sum::() + root + } +} + +#[derive(PartialEq, Eq, PartialOrd, Ord, serde::Serialize)] +enum TestSuiteKind { + Compiletest, + Cargo, +} + +#[derive(Template)] +#[template(path = "test_suites.askama")] +struct TestSuitesPage<'a> { + suites: TestSuites<'a>, + test_count: u64, +} diff --git a/src/ci/citool/templates/layout.askama b/src/ci/citool/templates/layout.askama new file mode 100644 index 0000000000000..2e830aaa9f583 --- /dev/null +++ b/src/ci/citool/templates/layout.askama @@ -0,0 +1,71 @@ + + + + Rust CI Test Dashboard + + + + +{% block content %}{% endblock %} + + diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama new file mode 100644 index 0000000000000..a0b7fa863e577 --- /dev/null +++ b/src/ci/citool/templates/test_group.askama @@ -0,0 +1,22 @@ +
  • +
    +{{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}) + +{% if !groups.is_empty() %} +
      + {% for (dir_name, subgroup) in groups %} + {{ subgroup|safe }} + {% endfor %} +
    +{% endif %} + +{% if !root_tests.is_empty() %} +
      + {% for test in root_tests %} +
    • {{ test.name }} ({{ test.passed.len() }} passed, {{ test.ignored.len() }} ignored)
      • + {% endfor %} +
    +{% endif %} + +
    +
    • diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama new file mode 100644 index 0000000000000..a6f8d0e1abe03 --- /dev/null +++ b/src/ci/citool/templates/test_suites.askama @@ -0,0 +1,18 @@ +{% extends "layout.askama" %} + +{% block content %} +

    Rust CI Test Dashboard

    +
    +
    + Total tests: {{ test_count }} +
    + +
      + {% for suite in suites.suites %} + {% if suite.kind == TestSuiteKind::Compiletest %} + {{ suite.group|safe }} + {% endif %} + {% endfor %} +
    +
    +{% endblock %} From a326afd5dd810427c72ed81e705c0d903e74edcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 16:30:23 +0200 Subject: [PATCH 17/28] Add buttons for expanding and collapsing all test suites --- src/ci/citool/templates/layout.askama | 57 ++------------- src/ci/citool/templates/test_suites.askama | 81 +++++++++++++++++++++- 2 files changed, 84 insertions(+), 54 deletions(-) diff --git a/src/ci/citool/templates/layout.askama b/src/ci/citool/templates/layout.askama index 2e830aaa9f583..3b3b6f23741d4 100644 --- a/src/ci/citool/templates/layout.askama +++ b/src/ci/citool/templates/layout.askama @@ -3,69 +3,20 @@ Rust CI Test Dashboard {% block content %}{% endblock %} +{% block scripts %}{% endblock %} diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index a6f8d0e1abe03..a8cedc65f2434 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -4,7 +4,11 @@

    Rust CI Test Dashboard

    - Total tests: {{ test_count }} + Total tests: {{ test_count }} +
    + + +
      @@ -16,3 +20,78 @@
    {% endblock %} + +{% block styles %} +h1 { + text-align: center; + color: #333333; + margin-bottom: 30px; +} + +.summary { + display: flex; + justify-content: space-between; +} + +.test-suites { + background: white; + border-radius: 8px; + box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); + padding: 20px; +} + +ul { + padding-left: 0; +} + +li { + list-style: none; + padding-left: 20px; +} +summary { + margin-bottom: 5px; + padding: 6px; + background-color: #F4F4F4; + border: 1px solid #ddd; + border-radius: 4px; + cursor: pointer; +} +summary:hover { + background-color: #CFCFCF; +} + +/* Style the disclosure triangles */ +details > summary { + list-style: none; + position: relative; +} + +details > summary::before { + content: "▶"; + position: absolute; + left: -15px; + transform: rotate(0); + transition: transform 0.2s; +} + +details[open] > summary::before { + transform: rotate(90deg); +} +{% endblock %} + +{% block scripts %} + +{% endblock %} From 4b310338f8d2a67cbc863ee799206709e95da6b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 16:35:34 +0200 Subject: [PATCH 18/28] Add a note about how to find tests that haven't been executed anywhere. --- src/ci/citool/src/test_dashboard/mod.rs | 60 ++++------------------ src/ci/citool/templates/test_suites.askama | 17 ++++-- 2 files changed, 22 insertions(+), 55 deletions(-) diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard/mod.rs index ad9fe029e15df..163e9c1acea0e 100644 --- a/src/ci/citool/src/test_dashboard/mod.rs +++ b/src/ci/citool/src/test_dashboard/mod.rs @@ -10,22 +10,6 @@ use crate::jobs::JobDatabase; use crate::metrics::{JobMetrics, JobName, download_auto_job_metrics, get_test_suites}; use crate::utils::normalize_path_delimiters; -pub struct TestInfo { - name: String, - jobs: Vec, -} - -struct JobTestResult { - job_name: String, - outcome: TestOutcome, -} - -#[derive(Default)] -struct TestSuiteInfo { - name: String, - tests: BTreeMap, -} - /// Generate a set of HTML files into a directory that contain a dashboard of test results. pub fn generate_test_dashboard( db: JobDatabase, @@ -33,7 +17,6 @@ pub fn generate_test_dashboard( output_dir: &Path, ) -> anyhow::Result<()> { let metrics = download_auto_job_metrics(&db, None, current)?; - let suites = gather_test_suites(&metrics); std::fs::create_dir_all(output_dir)?; @@ -52,27 +35,27 @@ fn write_page(dir: &Path, name: &str, template: &T) -> anyhow::Resu fn gather_test_suites(job_metrics: &HashMap) -> TestSuites { struct CoarseTestSuite<'a> { - kind: TestSuiteKind, tests: BTreeMap>, } let mut suites: HashMap = HashMap::new(); // First, gather tests from all jobs, stages and targets, and aggregate them per suite + // Only work with compiletest suites. for (job, metrics) in job_metrics { let test_suites = get_test_suites(&metrics.current); for suite in test_suites { - let (suite_name, stage, target, kind) = match &suite.metadata { - TestSuiteMetadata::CargoPackage { crates, stage, target, .. } => { - (crates.join(","), *stage, target, TestSuiteKind::Cargo) + let (suite_name, stage, target) = match &suite.metadata { + TestSuiteMetadata::CargoPackage { .. } => { + continue; } TestSuiteMetadata::Compiletest { suite, stage, target, .. } => { - (suite.clone(), *stage, target, TestSuiteKind::Compiletest) + (suite.clone(), *stage, target) } }; let suite_entry = suites .entry(suite_name.clone()) - .or_insert_with(|| CoarseTestSuite { kind, tests: Default::default() }); + .or_insert_with(|| CoarseTestSuite { tests: Default::default() }); let test_metadata = TestMetadata { job, stage, target }; for test in &suite.tests { @@ -98,29 +81,13 @@ fn gather_test_suites(job_metrics: &HashMap) -> TestSuites // Then, split the suites per directory let mut suites = suites.into_iter().collect::>(); - suites.sort_by(|a, b| a.1.kind.cmp(&b.1.kind).then_with(|| a.0.cmp(&b.0))); + suites.sort_by(|a, b| a.0.cmp(&b.0)); let mut target_suites = vec![]; for (suite_name, suite) in suites { - let suite = match suite.kind { - TestSuiteKind::Compiletest => TestSuite { - name: suite_name.clone(), - kind: TestSuiteKind::Compiletest, - group: build_test_group(&suite_name, suite.tests), - }, - TestSuiteKind::Cargo => { - let mut tests: Vec<_> = suite.tests.into_iter().collect(); - tests.sort_by(|a, b| a.0.cmp(&b.0)); - TestSuite { - name: format!("[cargo] {}", suite_name.clone()), - kind: TestSuiteKind::Cargo, - group: TestGroup { - name: suite_name, - root_tests: tests.into_iter().map(|t| t.1).collect(), - groups: vec![], - }, - } - } + let suite = TestSuite { + name: suite_name.clone(), + group: build_test_group(&suite_name, suite.tests), }; target_suites.push(suite); } @@ -187,7 +154,6 @@ impl<'a> TestSuites<'a> { #[derive(serde::Serialize)] struct TestSuite<'a> { name: String, - kind: TestSuiteKind, group: TestGroup<'a>, } @@ -225,12 +191,6 @@ impl<'a> TestGroup<'a> { } } -#[derive(PartialEq, Eq, PartialOrd, Ord, serde::Serialize)] -enum TestSuiteKind { - Compiletest, - Cargo, -} - #[derive(Template)] #[template(path = "test_suites.askama")] struct TestSuitesPage<'a> { diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index a8cedc65f2434..bb3d9e363911d 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -1,10 +1,15 @@ {% extends "layout.askama" %} {% block content %} -

    Rust CI Test Dashboard

    +

    Rust CI test dashboard

    - Total tests: {{ test_count }} +
    +
    Total tests: {{ test_count }}
    +
    + To find tests that haven't been executed anywhere, click on "Open all" and search for "(0 passed". +
    +
    @@ -13,9 +18,7 @@
      {% for suite in suites.suites %} - {% if suite.kind == TestSuiteKind::Compiletest %} - {{ suite.group|safe }} - {% endif %} + {{ suite.group|safe }} {% endfor %}
    @@ -33,6 +36,10 @@ h1 { justify-content: space-between; } +.test-count { + font-size: 1.2em; +} + .test-suites { background: white; border-radius: 8px; From 1a6e0d52e5b008cfd48f78285bb3655ecfd5d73e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:14:26 +0200 Subject: [PATCH 19/28] Render test revisions separately --- src/ci/citool/src/test_dashboard/mod.rs | 43 ++++++++++++++++++----- src/ci/citool/templates/test_group.askama | 13 ++++++- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard/mod.rs index 163e9c1acea0e..c16385baa3ba4 100644 --- a/src/ci/citool/src/test_dashboard/mod.rs +++ b/src/ci/citool/src/test_dashboard/mod.rs @@ -60,19 +60,27 @@ fn gather_test_suites(job_metrics: &HashMap) -> TestSuites for test in &suite.tests { let test_name = normalize_test_name(&test.name, &suite_name); - let test_entry = suite_entry - .tests - .entry(test_name.clone()) - .or_insert_with(|| Test { name: test_name, passed: vec![], ignored: vec![] }); + let (test_name, variant_name) = match test_name.rsplit_once('#') { + Some((name, variant)) => (name.to_string(), variant.to_string()), + None => (test_name, "".to_string()), + }; + let test_entry = suite_entry.tests.entry(test_name.clone()).or_insert_with(|| { + Test { name: test_name.clone(), revisions: Default::default() } + }); + let variant_entry = test_entry + .revisions + .entry(variant_name) + .or_insert_with(|| TestResults { passed: vec![], ignored: vec![] }); + match test.outcome { TestOutcome::Passed => { - test_entry.passed.push(test_metadata); + variant_entry.passed.push(test_metadata); } TestOutcome::Ignored { ignore_reason: _ } => { - test_entry.ignored.push(test_metadata); + variant_entry.ignored.push(test_metadata); } TestOutcome::Failed => { - eprintln!("Warning: failed test"); + eprintln!("Warning: failed test {test_name}"); } } } @@ -158,12 +166,29 @@ struct TestSuite<'a> { } #[derive(Debug, serde::Serialize)] -struct Test<'a> { - name: String, +struct TestResults<'a> { passed: Vec>, ignored: Vec>, } +#[derive(Debug, serde::Serialize)] +struct Test<'a> { + name: String, + revisions: BTreeMap>, +} + +impl<'a> Test<'a> { + /// If this is a test without revisions, it will have a single entry in `revisions` with + /// an empty string as the revision name. + fn single_test(&self) -> Option<&TestResults<'a>> { + if self.revisions.len() == 1 { + self.revisions.iter().next().take_if(|e| e.0.is_empty()).map(|e| e.1) + } else { + None + } + } +} + #[derive(Clone, Copy, Debug, serde::Serialize)] struct TestMetadata<'a> { job: &'a str, diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index a0b7fa863e577..535d98e0c247f 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -13,7 +13,18 @@ {% if !root_tests.is_empty() %}
      {% for test in root_tests %} -
    • {{ test.name }} ({{ test.passed.len() }} passed, {{ test.ignored.len() }} ignored)
      • +
    • + {% if let Some(result) = test.single_test() %} + {{ test.name }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored) + {% else %} + {{ test.name }} ({{ test.revisions.len() }} revision{{ test.revisions.len() | pluralize }}) +
        + {% for (revision, result) in test.revisions %} +
      • #{{ revision }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored)
        • + {% endfor %} +
      + {% endif %} +
      • {% endfor %}
    {% endif %} From d2c1763336080030a235dae56f6096e9deb2ec9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:18:38 +0200 Subject: [PATCH 20/28] Create a macro for rendering test results --- src/ci/citool/templates/test_group.askama | 8 ++++++-- src/ci/citool/templates/test_suites.askama | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index 535d98e0c247f..ba19d9258d8a0 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -1,3 +1,7 @@ +{% macro test_result(r) -%} +passed: {{ r.passed.len() }}, ignored: {{ r.ignored.len() }} +{%- endmacro %} +
  • {{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}) @@ -15,12 +19,12 @@ {% for test in root_tests %}
  • {% if let Some(result) = test.single_test() %} - {{ test.name }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored) + {{ test.name }} ({% call test_result(result) %}) {% else %} {{ test.name }} ({{ test.revisions.len() }} revision{{ test.revisions.len() | pluralize }})
      {% for (revision, result) in test.revisions %} -
    • #{{ revision }} ({{ result.passed.len() }} passed, {{ result.ignored.len() }} ignored)
      • +
    • #{{ revision }} ({% call test_result(result) %})
      • {% endfor %}
    {% endif %} diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index bb3d9e363911d..d36e85228e2b0 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -7,7 +7,7 @@
    Total tests: {{ test_count }}
    - To find tests that haven't been executed anywhere, click on "Open all" and search for "(0 passed". + To find tests that haven't been executed anywhere, click on "Open all" and search for "passed: 0".
    From aa9cb70190bb38e466983abf0c53c6f26afab4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:25:12 +0200 Subject: [PATCH 21/28] Print number of root tests and subdirectories --- src/ci/citool/templates/test_group.askama | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index ba19d9258d8a0..bdf32d00f4a19 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -4,7 +4,12 @@ passed: {{ r.passed.len() }}, ignored: {{ r.ignored.len() }}
  • -{{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}) +{{ name }} ({{ test_count() }} test{{ test_count() | pluralize }}{% if !root_tests.is_empty() && root_tests.len() as u64 != test_count() -%} + , {{ root_tests.len() }} root test{{ root_tests.len() | pluralize }} +{%- endif %}{% if !groups.is_empty() -%} + , {{ groups.len() }} subdir{{ groups.len() | pluralize }} +{%- endif %}) + {% if !groups.is_empty() %}
      From 08cb187d263ec91e8e6d35b4b235eee4ecad3357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:26:40 +0200 Subject: [PATCH 22/28] Turn `test_dashboard` into a file --- src/ci/citool/src/{test_dashboard/mod.rs => test_dashboard.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/ci/citool/src/{test_dashboard/mod.rs => test_dashboard.rs} (100%) diff --git a/src/ci/citool/src/test_dashboard/mod.rs b/src/ci/citool/src/test_dashboard.rs similarity index 100% rename from src/ci/citool/src/test_dashboard/mod.rs rename to src/ci/citool/src/test_dashboard.rs From cecf16785f193c247c47e42524322aeccc96c8a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 17 Apr 2025 17:38:15 +0200 Subject: [PATCH 23/28] Add a note about the test dashboard to the post-merge report --- src/ci/citool/src/main.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ci/citool/src/main.rs b/src/ci/citool/src/main.rs index a7a289fc3d4b1..f4e671b609fa6 100644 --- a/src/ci/citool/src/main.rs +++ b/src/ci/citool/src/main.rs @@ -24,7 +24,7 @@ use crate::github::JobInfoResolver; use crate::jobs::RunType; use crate::metrics::{JobMetrics, download_auto_job_metrics, download_job_metrics, load_metrics}; use crate::test_dashboard::generate_test_dashboard; -use crate::utils::load_env_var; +use crate::utils::{load_env_var, output_details}; const CI_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/.."); const DOCKER_DIRECTORY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/../docker"); @@ -188,6 +188,20 @@ fn post_merge_report(db: JobDatabase, current: String, parent: String) -> anyhow let mut job_info_resolver = JobInfoResolver::new(); output_test_diffs(&metrics, &mut job_info_resolver); + + output_details("Test dashboard", || { + println!( + r#"\nRun + +```bash +cargo run --manifest-path src/ci/citool/Cargo.toml -- \ + test-dashboard {current} --output-dir test-dashboard +``` +And then open `test-dashboard/index.html` in your browser to see an overview of all executed tests. +"# + ); + }); + output_largest_duration_changes(&metrics, &mut job_info_resolver); Ok(()) From 136171c8d8c81ff8de4446fb4bb339a1bd475a49 Mon Sep 17 00:00:00 2001 From: Manuel Drehwald Date: Fri, 18 Apr 2025 00:58:04 -0400 Subject: [PATCH 24/28] skip llvm-config in autodiff check builds, when its unavailable --- src/bootstrap/src/core/build_steps/compile.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index dab58fccf5e68..106a04de5d9a7 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1194,8 +1194,7 @@ pub fn rustc_cargo( let enzyme_dir = builder.build.out.join(arch).join("enzyme").join("lib"); cargo.rustflag("-L").rustflag(enzyme_dir.to_str().expect("Invalid path")); - if !builder.config.dry_run() { - let llvm_config = builder.llvm_config(builder.config.build).unwrap(); + if let Some(llvm_config) = builder.llvm_config(builder.config.build) { let llvm_version_major = llvm::get_llvm_version_major(builder, &llvm_config); cargo.rustflag("-l").rustflag(&format!("Enzyme-{llvm_version_major}")); } From 65ce38a4c96da2f950fb6b181b2b83b0320d103d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 18 Apr 2025 12:32:19 +0200 Subject: [PATCH 25/28] Add a note that explains the counts --- src/ci/citool/templates/test_suites.askama | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ci/citool/templates/test_suites.askama b/src/ci/citool/templates/test_suites.askama index d36e85228e2b0..4997f6a3f1c9a 100644 --- a/src/ci/citool/templates/test_suites.askama +++ b/src/ci/citool/templates/test_suites.askama @@ -2,6 +2,10 @@ {% block content %}

      Rust CI test dashboard

      +
      +Here's how to interpret the "passed" and "ignored" counts: +the count includes all combinations of "stage" x "target" x "CI job where the test was executed or ignored". +
      From b18e37305304780530224736ad55145063c7e8a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 18 Apr 2025 12:44:39 +0200 Subject: [PATCH 26/28] Reduce duplicated test prefixes in nested subdirectories `assembly/asm` contained a test named `asm/aarch64-el2vmsa.rs`, while it should have been only `arch64-el2vmsa.rs`. --- src/ci/citool/src/test_dashboard.rs | 36 +++++++++-------------- src/ci/citool/templates/test_group.askama | 6 ++-- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/ci/citool/src/test_dashboard.rs b/src/ci/citool/src/test_dashboard.rs index c16385baa3ba4..8fbd0d3f200d4 100644 --- a/src/ci/citool/src/test_dashboard.rs +++ b/src/ci/citool/src/test_dashboard.rs @@ -64,9 +64,10 @@ fn gather_test_suites(job_metrics: &HashMap) -> TestSuites Some((name, variant)) => (name.to_string(), variant.to_string()), None => (test_name, "".to_string()), }; - let test_entry = suite_entry.tests.entry(test_name.clone()).or_insert_with(|| { - Test { name: test_name.clone(), revisions: Default::default() } - }); + let test_entry = suite_entry + .tests + .entry(test_name.clone()) + .or_insert_with(|| Test { revisions: Default::default() }); let variant_entry = test_entry .revisions .entry(variant_name) @@ -91,16 +92,12 @@ fn gather_test_suites(job_metrics: &HashMap) -> TestSuites let mut suites = suites.into_iter().collect::>(); suites.sort_by(|a, b| a.0.cmp(&b.0)); - let mut target_suites = vec![]; - for (suite_name, suite) in suites { - let suite = TestSuite { - name: suite_name.clone(), - group: build_test_group(&suite_name, suite.tests), - }; - target_suites.push(suite); - } + let suites = suites + .into_iter() + .map(|(suite_name, suite)| TestSuite { group: build_test_group(&suite_name, suite.tests) }) + .collect(); - TestSuites { suites: target_suites } + TestSuites { suites } } /// Recursively expand a test group based on filesystem hierarchy. @@ -115,7 +112,7 @@ fn build_test_group<'a>(name: &str, tests: BTreeMap>) -> TestGr if components.peek().is_none() { // This is a root test - root_tests.push(test); + root_tests.push((name, test)); } else { // This is a test in a nested directory let subdir_tests = @@ -148,7 +145,6 @@ fn normalize_test_name(name: &str, suite_name: &str) -> String { name.trim_start_matches("/").to_string() } -#[derive(serde::Serialize)] struct TestSuites<'a> { suites: Vec>, } @@ -159,21 +155,16 @@ impl<'a> TestSuites<'a> { } } -#[derive(serde::Serialize)] struct TestSuite<'a> { - name: String, group: TestGroup<'a>, } -#[derive(Debug, serde::Serialize)] struct TestResults<'a> { passed: Vec>, ignored: Vec>, } -#[derive(Debug, serde::Serialize)] struct Test<'a> { - name: String, revisions: BTreeMap>, } @@ -189,7 +180,8 @@ impl<'a> Test<'a> { } } -#[derive(Clone, Copy, Debug, serde::Serialize)] +#[derive(Clone, Copy)] +#[allow(dead_code)] struct TestMetadata<'a> { job: &'a str, stage: u32, @@ -198,13 +190,13 @@ struct TestMetadata<'a> { // We have to use a template for the TestGroup instead of a macro, because // macros cannot be recursive in askama at the moment. -#[derive(Template, serde::Serialize)] +#[derive(Template)] #[template(path = "test_group.askama")] /// Represents a group of tests struct TestGroup<'a> { name: String, /// Tests located directly in this directory - root_tests: Vec>, + root_tests: Vec<(String, Test<'a>)>, /// Nested directories with additional tests groups: Vec<(String, TestGroup<'a>)>, } diff --git a/src/ci/citool/templates/test_group.askama b/src/ci/citool/templates/test_group.askama index bdf32d00f4a19..95731103f3b9d 100644 --- a/src/ci/citool/templates/test_group.askama +++ b/src/ci/citool/templates/test_group.askama @@ -21,12 +21,12 @@ passed: {{ r.passed.len() }}, ignored: {{ r.ignored.len() }} {% if !root_tests.is_empty() %}
        - {% for test in root_tests %} + {% for (name, test) in root_tests %}
      • {% if let Some(result) = test.single_test() %} - {{ test.name }} ({% call test_result(result) %}) + {{ name }} ({% call test_result(result) %}) {% else %} - {{ test.name }} ({{ test.revisions.len() }} revision{{ test.revisions.len() | pluralize }}) + {{ name }} ({{ test.revisions.len() }} revision{{ test.revisions.len() | pluralize }})
          {% for (revision, result) in test.revisions %}
        • #{{ revision }} ({% call test_result(result) %})
          • From 1b393025718ca9c75d67e82b55a390e3506e536c Mon Sep 17 00:00:00 2001 From: roblabla Date: Fri, 18 Apr 2025 13:30:26 +0200 Subject: [PATCH 27/28] Disable has_thread_local on i686-win7-windows-msvc On Windows 7 32-bit, the alignment characteristic of the TLS Directory don't appear to be respected by the PE Loader, leading to crashes. As a result, let's disable has_thread_local to make sure TLS goes through the emulation layer. --- .../rustc_target/src/spec/targets/i686_win7_windows_msvc.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/rustc_target/src/spec/targets/i686_win7_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/i686_win7_windows_msvc.rs index 233a1c4fd7a54..91ab311109787 100644 --- a/compiler/rustc_target/src/spec/targets/i686_win7_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/i686_win7_windows_msvc.rs @@ -7,6 +7,12 @@ pub(crate) fn target() -> Target { base.cpu = "pentium4".into(); base.max_atomic_width = Some(64); base.supported_sanitizers = SanitizerSet::ADDRESS; + // On Windows 7 32-bit, the alignment characteristic of the TLS Directory + // don't appear to be respected by the PE Loader, leading to crashes. As + // a result, let's disable has_thread_local to make sure TLS goes through + // the emulation layer. + // See https://github.com/rust-lang/rust/issues/138903 + base.has_thread_local = false; base.add_pre_link_args( LinkerFlavor::Msvc(Lld::No), From ac7d1be031ac08cc0910efa3ec2f4186d7943792 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 16 Apr 2025 20:54:15 +0300 Subject: [PATCH 28/28] add coverage on config include logic Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/tests.rs | 211 ++++++++++++++++++++++++- 1 file changed, 209 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index d8002ba8467bd..c8a12c9072c91 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -1,8 +1,8 @@ use std::collections::BTreeSet; -use std::env; use std::fs::{File, remove_file}; use std::io::Write; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::{env, fs}; use build_helper::ci::CiEnv; use clap::CommandFactory; @@ -23,6 +23,27 @@ pub(crate) fn parse(config: &str) -> Config { ) } +fn get_toml(file: &Path) -> Result { + let contents = std::fs::read_to_string(file).unwrap(); + toml::from_str(&contents).and_then(|table: toml::Value| TomlConfig::deserialize(table)) +} + +/// Helps with debugging by using consistent test-specific directories instead of +/// random temporary directories. +fn prepare_test_specific_dir() -> PathBuf { + let current = std::thread::current(); + // Replace "::" with "_" to make it safe for directory names on Windows systems + let test_path = current.name().unwrap().replace("::", "_"); + + let testdir = parse("").tempdir().join(test_path); + + // clean up any old test files + let _ = fs::remove_dir_all(&testdir); + let _ = fs::create_dir_all(&testdir); + + testdir +} + #[test] fn download_ci_llvm() { let config = parse("llvm.download-ci-llvm = false"); @@ -539,3 +560,189 @@ fn test_ci_flag() { let config = Config::parse_inner(Flags::parse(&["check".into()]), |&_| toml::from_str("")); assert_eq!(config.is_running_on_ci, CiEnv::is_ci()); } + +#[test] +fn test_precedence_of_includes() { + let testdir = prepare_test_specific_dir(); + + let root_config = testdir.join("config.toml"); + let root_config_content = br#" + include = ["./extension.toml"] + + [llvm] + link-jobs = 2 + "#; + File::create(&root_config).unwrap().write_all(root_config_content).unwrap(); + + let extension = testdir.join("extension.toml"); + let extension_content = br#" + change-id=543 + include = ["./extension2.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let extension = testdir.join("extension2.toml"); + let extension_content = br#" + change-id=742 + + [llvm] + link-jobs = 10 + + [build] + description = "Some creative description" + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let config = Config::parse_inner( + Flags::parse(&["check".to_owned(), format!("--config={}", root_config.to_str().unwrap())]), + get_toml, + ); + + assert_eq!(config.change_id.unwrap(), ChangeId::Id(543)); + assert_eq!(config.llvm_link_jobs.unwrap(), 2); + assert_eq!(config.description.unwrap(), "Some creative description"); +} + +#[test] +#[should_panic(expected = "Cyclic inclusion detected")] +fn test_cyclic_include_direct() { + let testdir = prepare_test_specific_dir(); + + let root_config = testdir.join("config.toml"); + let root_config_content = br#" + include = ["./extension.toml"] + "#; + File::create(&root_config).unwrap().write_all(root_config_content).unwrap(); + + let extension = testdir.join("extension.toml"); + let extension_content = br#" + include = ["./config.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let config = Config::parse_inner( + Flags::parse(&["check".to_owned(), format!("--config={}", root_config.to_str().unwrap())]), + get_toml, + ); +} + +#[test] +#[should_panic(expected = "Cyclic inclusion detected")] +fn test_cyclic_include_indirect() { + let testdir = prepare_test_specific_dir(); + + let root_config = testdir.join("config.toml"); + let root_config_content = br#" + include = ["./extension.toml"] + "#; + File::create(&root_config).unwrap().write_all(root_config_content).unwrap(); + + let extension = testdir.join("extension.toml"); + let extension_content = br#" + include = ["./extension2.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let extension = testdir.join("extension2.toml"); + let extension_content = br#" + include = ["./extension3.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let extension = testdir.join("extension3.toml"); + let extension_content = br#" + include = ["./extension.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let config = Config::parse_inner( + Flags::parse(&["check".to_owned(), format!("--config={}", root_config.to_str().unwrap())]), + get_toml, + ); +} + +#[test] +fn test_include_absolute_paths() { + let testdir = prepare_test_specific_dir(); + + let extension = testdir.join("extension.toml"); + File::create(&extension).unwrap().write_all(&[]).unwrap(); + + let root_config = testdir.join("config.toml"); + let extension_absolute_path = + extension.canonicalize().unwrap().to_str().unwrap().replace('\\', r"\\"); + let root_config_content = format!(r#"include = ["{}"]"#, extension_absolute_path); + File::create(&root_config).unwrap().write_all(root_config_content.as_bytes()).unwrap(); + + let config = Config::parse_inner( + Flags::parse(&["check".to_owned(), format!("--config={}", root_config.to_str().unwrap())]), + get_toml, + ); +} + +#[test] +fn test_include_relative_paths() { + let testdir = prepare_test_specific_dir(); + + let _ = fs::create_dir_all(&testdir.join("subdir/another_subdir")); + + let root_config = testdir.join("config.toml"); + let root_config_content = br#" + include = ["./subdir/extension.toml"] + "#; + File::create(&root_config).unwrap().write_all(root_config_content).unwrap(); + + let extension = testdir.join("subdir/extension.toml"); + let extension_content = br#" + include = ["../extension2.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let extension = testdir.join("extension2.toml"); + let extension_content = br#" + include = ["./subdir/another_subdir/extension3.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let extension = testdir.join("subdir/another_subdir/extension3.toml"); + let extension_content = br#" + include = ["../../extension4.toml"] + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let extension = testdir.join("extension4.toml"); + File::create(extension).unwrap().write_all(&[]).unwrap(); + + let config = Config::parse_inner( + Flags::parse(&["check".to_owned(), format!("--config={}", root_config.to_str().unwrap())]), + get_toml, + ); +} + +#[test] +fn test_include_precedence_over_profile() { + let testdir = prepare_test_specific_dir(); + + let root_config = testdir.join("config.toml"); + let root_config_content = br#" + profile = "dist" + include = ["./extension.toml"] + "#; + File::create(&root_config).unwrap().write_all(root_config_content).unwrap(); + + let extension = testdir.join("extension.toml"); + let extension_content = br#" + [rust] + channel = "dev" + "#; + File::create(extension).unwrap().write_all(extension_content).unwrap(); + + let config = Config::parse_inner( + Flags::parse(&["check".to_owned(), format!("--config={}", root_config.to_str().unwrap())]), + get_toml, + ); + + // "dist" profile would normally set the channel to "auto-detect", but includes should + // override profile settings, so we expect this to be "dev" here. + assert_eq!(config.channel, "dev"); +}