From 1205e6cff3b2df4ed8465aa1f4f4b3f8e230b7a3 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 4 Jun 2023 21:37:09 -0500 Subject: [PATCH 1/2] Use the top-level Kind to determine whether Steps are excluded Previously, this would use the `Kind` passed to `--exclude` (and not do any filtering at all if no kind was passed). That meant that `x test linkchecker --exclude std` would fail - you had to explicitly say `--exclude test::std`. Change bootstrap to use the top-level Kind instead, which does the right thing automatically. Note that this breaks things like `x test --exclude doc::std`, but I'm not sure why you'd ever want to do that. --- src/bootstrap/CHANGELOG.md | 1 + src/bootstrap/builder.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/CHANGELOG.md b/src/bootstrap/CHANGELOG.md index d6924cf2cfb23..1aba0713850ae 100644 --- a/src/bootstrap/CHANGELOG.md +++ b/src/bootstrap/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `x.py fmt` now formats only files modified between the merge-base of HEAD and the last commit in the master branch of the rust-lang repository and the current working directory. To restore old behaviour, use `x.py fmt .`. The check mode is not affected by this change. [#105702](https://github.com/rust-lang/rust/pull/105702) - The `llvm.version-check` config option has been removed. Older versions were never supported. If you still need to support older versions (e.g. you are applying custom patches), patch `check_llvm_version` in bootstrap to change the minimum version. [#108619](https://github.com/rust-lang/rust/pull/108619) - The `rust.ignore-git` option has been renamed to `rust.omit-git-hash`. [#110059](https://github.com/rust-lang/rust/pull/110059) +- `--exclude` no longer accepts a `Kind` as part of a Step; instead it uses the top-level Kind of the subcommand. If this matches how you were already using --exclude (e.g. `x test --exclude test::std`), simply remove the kind: `--exclude std`. If you were using a kind that did not match the top-level subcommand, please open an issue explaining why you wanted this feature. ### Non-breaking changes diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 848fb9eade917..5bdba28158abd 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -307,7 +307,7 @@ impl StepDescription { } fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet) -> bool { - if builder.config.exclude.iter().any(|e| pathset.has(&e.path, e.kind)) { + if builder.config.exclude.iter().any(|e| pathset.has(&e.path, Some(builder.kind))) { println!("Skipping {:?} because it is excluded", pathset); return true; } From c73c5dd35fa8171ba1634bf9111073706ccf993a Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 4 Jun 2023 21:40:05 -0500 Subject: [PATCH 2/2] cleanup now that Kind is no longer used for excludes --- src/bootstrap/builder.rs | 45 +++++++--------------------------- src/bootstrap/builder/tests.rs | 37 ++++++++++++---------------- src/bootstrap/config.rs | 5 ++-- src/bootstrap/test.rs | 2 +- 4 files changed, 27 insertions(+), 62 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 5bdba28158abd..1acf522eeab6b 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -8,7 +8,7 @@ use std::fs::{self, File}; use std::hash::Hash; use std::io::{BufRead, BufReader}; use std::ops::Deref; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use std::process::Command; use std::time::{Duration, Instant}; @@ -150,29 +150,6 @@ pub struct TaskPath { pub kind: Option, } -impl TaskPath { - pub fn parse(path: impl Into) -> TaskPath { - let mut kind = None; - let mut path = path.into(); - - let mut components = path.components(); - if let Some(Component::Normal(os_str)) = components.next() { - if let Some(str) = os_str.to_str() { - if let Some((found_kind, found_prefix)) = str.split_once("::") { - if found_kind.is_empty() { - panic!("empty kind in task path {}", path.display()); - } - kind = Kind::parse(found_kind); - assert!(kind.is_some()); - path = Path::new(found_prefix).join(components.as_path()); - } - } - } - - TaskPath { path, kind } - } -} - impl Debug for TaskPath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Some(kind) = &self.kind { @@ -216,7 +193,7 @@ impl PathSet { PathSet::Set(set) } - fn has(&self, needle: &Path, module: Option) -> bool { + fn has(&self, needle: &Path, module: Kind) -> bool { match self { PathSet::Set(set) => set.iter().any(|p| Self::check(p, needle, module)), PathSet::Suite(suite) => Self::check(suite, needle, module), @@ -224,9 +201,9 @@ impl PathSet { } // internal use only - fn check(p: &TaskPath, needle: &Path, module: Option) -> bool { - if let (Some(p_kind), Some(kind)) = (&p.kind, module) { - p.path.ends_with(needle) && *p_kind == kind + fn check(p: &TaskPath, needle: &Path, module: Kind) -> bool { + if let Some(p_kind) = &p.kind { + p.path.ends_with(needle) && *p_kind == module } else { p.path.ends_with(needle) } @@ -238,11 +215,7 @@ impl PathSet { /// This is used for `StepDescription::krate`, which passes all matching crates at once to /// `Step::make_run`, rather than calling it many times with a single crate. /// See `tests.rs` for examples. - fn intersection_removing_matches( - &self, - needles: &mut Vec<&Path>, - module: Option, - ) -> PathSet { + fn intersection_removing_matches(&self, needles: &mut Vec<&Path>, module: Kind) -> PathSet { let mut check = |p| { for (i, n) in needles.iter().enumerate() { let matched = Self::check(p, n, module); @@ -307,7 +280,7 @@ impl StepDescription { } fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet) -> bool { - if builder.config.exclude.iter().any(|e| pathset.has(&e.path, Some(builder.kind))) { + if builder.config.exclude.iter().any(|e| pathset.has(&e, builder.kind)) { println!("Skipping {:?} because it is excluded", pathset); return true; } @@ -562,7 +535,7 @@ impl<'a> ShouldRun<'a> { ) -> Vec { let mut sets = vec![]; for pathset in &self.paths { - let subset = pathset.intersection_removing_matches(paths, Some(kind)); + let subset = pathset.intersection_removing_matches(paths, kind); if subset != PathSet::empty() { sets.push(subset); } @@ -2130,7 +2103,7 @@ impl<'a> Builder<'a> { let should_run = (desc.should_run)(ShouldRun::new(self, desc.kind)); for path in &self.paths { - if should_run.paths.iter().any(|s| s.has(path, Some(desc.kind))) + if should_run.paths.iter().any(|s| s.has(path, desc.kind)) && !desc.is_excluded( self, &PathSet::Suite(TaskPath { path: path.clone(), kind: Some(desc.kind) }), diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index d76b830b0e530..31dcee58216ae 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -101,23 +101,21 @@ fn test_invalid() { #[test] fn test_intersection() { - let set = PathSet::Set( - ["library/core", "library/alloc", "library/std"].into_iter().map(TaskPath::parse).collect(), - ); + let set = |paths: &[&str]| { + PathSet::Set(paths.into_iter().map(|p| TaskPath { path: p.into(), kind: None }).collect()) + }; + let library_set = set(&["library/core", "library/alloc", "library/std"]); let mut command_paths = vec![Path::new("library/core"), Path::new("library/alloc"), Path::new("library/stdarch")]; - let subset = set.intersection_removing_matches(&mut command_paths, None); - assert_eq!( - subset, - PathSet::Set(["library/core", "library/alloc"].into_iter().map(TaskPath::parse).collect()) - ); + let subset = library_set.intersection_removing_matches(&mut command_paths, Kind::Build); + assert_eq!(subset, set(&["library/core", "library/alloc"]),); assert_eq!(command_paths, vec![Path::new("library/stdarch")]); } #[test] fn test_exclude() { let mut config = configure("test", &["A"], &["A"]); - config.exclude = vec![TaskPath::parse("src/tools/tidy")]; + config.exclude = vec!["src/tools/tidy".into()]; let cache = run_build(&[], config); // Ensure we have really excluded tidy @@ -129,21 +127,16 @@ fn test_exclude() { #[test] fn test_exclude_kind() { - let path = PathBuf::from("src/tools/cargotest"); - let exclude = TaskPath::parse("test::src/tools/cargotest"); - assert_eq!(exclude, TaskPath { kind: Some(Kind::Test), path: path.clone() }); + let path = PathBuf::from("compiler/rustc_data_structures"); let mut config = configure("test", &["A"], &["A"]); - // Ensure our test is valid, and `test::Cargotest` would be run without the exclude. - assert!(run_build(&[path.clone()], config.clone()).contains::()); - // Ensure tests for cargotest are skipped. - config.exclude = vec![exclude.clone()]; - assert!(!run_build(&[path.clone()], config).contains::()); - - // Ensure builds for cargotest are not skipped. - let mut config = configure("build", &["A"], &["A"]); - config.exclude = vec![exclude]; - assert!(run_build(&[path], config).contains::()); + // Ensure our test is valid, and `test::Rustc` would be run without the exclude. + assert!(run_build(&[], config.clone()).contains::()); + // Ensure tests for rustc are skipped. + config.exclude = vec![path.clone()]; + assert!(!run_build(&[], config.clone()).contains::()); + // Ensure builds for rustc are not skipped. + assert!(run_build(&[], config).contains::()); } /// Ensure that if someone passes both a single crate and `library`, all library crates get built. diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 8ea7e83637596..42cd7922be6b5 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -16,7 +16,6 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::str::FromStr; -use crate::builder::TaskPath; use crate::cache::{Interned, INTERNER}; use crate::cc_detect::{ndk_compiler, Language}; use crate::channel::{self, GitInfo}; @@ -79,7 +78,7 @@ pub struct Config { pub sanitizers: bool, pub profiler: bool, pub omit_git_hash: bool, - pub exclude: Vec, + pub exclude: Vec, pub include_default_paths: bool, pub rustc_error_format: Option, pub json_output: bool, @@ -958,7 +957,7 @@ impl Config { // Set flags. config.paths = std::mem::take(&mut flags.paths); - config.exclude = flags.exclude.into_iter().map(|path| TaskPath::parse(path)).collect(); + config.exclude = flags.exclude; config.include_default_paths = flags.include_default_paths; config.rustc_error_format = flags.rustc_error_format; config.json_output = flags.json_output; diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 29e48481f0f84..31a2a33ebc259 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1529,7 +1529,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the for exclude in &builder.config.exclude { cmd.arg("--skip"); - cmd.arg(&exclude.path); + cmd.arg(&exclude); } // Get paths from cmd args