Skip to content

Commit

Permalink
cleanup now that Kind is no longer used for excludes
Browse files Browse the repository at this point in the history
  • Loading branch information
jyn514 committed Jun 5, 2023
1 parent 1205e6c commit c73c5dd
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 62 deletions.
45 changes: 9 additions & 36 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -150,29 +150,6 @@ pub struct TaskPath {
pub kind: Option<Kind>,
}

impl TaskPath {
pub fn parse(path: impl Into<PathBuf>) -> 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 {
Expand Down Expand Up @@ -216,17 +193,17 @@ impl PathSet {
PathSet::Set(set)
}

fn has(&self, needle: &Path, module: Option<Kind>) -> 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),
}
}

// internal use only
fn check(p: &TaskPath, needle: &Path, module: Option<Kind>) -> 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)
}
Expand All @@ -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<Kind>,
) -> 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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -562,7 +535,7 @@ impl<'a> ShouldRun<'a> {
) -> Vec<PathSet> {
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);
}
Expand Down Expand Up @@ -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) }),
Expand Down
37 changes: 15 additions & 22 deletions src/bootstrap/builder/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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::<test::Cargotest>());
// Ensure tests for cargotest are skipped.
config.exclude = vec![exclude.clone()];
assert!(!run_build(&[path.clone()], config).contains::<test::Cargotest>());

// Ensure builds for cargotest are not skipped.
let mut config = configure("build", &["A"], &["A"]);
config.exclude = vec![exclude];
assert!(run_build(&[path], config).contains::<tool::CargoTest>());
// Ensure our test is valid, and `test::Rustc` would be run without the exclude.
assert!(run_build(&[], config.clone()).contains::<test::CrateLibrustc>());
// Ensure tests for rustc are skipped.
config.exclude = vec![path.clone()];
assert!(!run_build(&[], config.clone()).contains::<test::CrateLibrustc>());
// Ensure builds for rustc are not skipped.
assert!(run_build(&[], config).contains::<compile::Rustc>());
}

/// Ensure that if someone passes both a single crate and `library`, all library crates get built.
Expand Down
5 changes: 2 additions & 3 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -79,7 +78,7 @@ pub struct Config {
pub sanitizers: bool,
pub profiler: bool,
pub omit_git_hash: bool,
pub exclude: Vec<TaskPath>,
pub exclude: Vec<PathBuf>,
pub include_default_paths: bool,
pub rustc_error_format: Option<String>,
pub json_output: bool,
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c73c5dd

Please sign in to comment.