From ebb7aa0b8575a27e2768c42f1472d3523925357c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 1 Dec 2023 23:57:16 +0100 Subject: [PATCH] Apply suggestions --- build_system/src/build.rs | 40 +------ build_system/src/config.rs | 25 ++-- build_system/src/prepare.rs | 7 +- build_system/src/test.rs | 227 ++++++++++++++++-------------------- build_system/src/utils.rs | 47 +++++--- 5 files changed, 156 insertions(+), 190 deletions(-) diff --git a/build_system/src/build.rs b/build_system/src/build.rs index 189c393019fd..618e74be2c0c 100644 --- a/build_system/src/build.rs +++ b/build_system/src/build.rs @@ -67,14 +67,8 @@ impl BuildArg { } } -fn build_sysroot_inner( - env: &HashMap, - sysroot_panic_abort: bool, - sysroot_release_channel: bool, - config: &ConfigInfo, - start_dir: Option<&Path>, -) -> Result<(), String> { - let start_dir = start_dir.unwrap_or_else(|| Path::new(".")); +pub fn build_sysroot(env: &HashMap, config: &ConfigInfo) -> Result<(), String> { + let start_dir = Path::new("build_sysroot"); // Cleanup for previous run // Clean target dir except for build scripts and incremental cache let _ = walk_dir( @@ -121,12 +115,11 @@ fn build_sysroot_inner( // Builds libs let mut rustflags = env.get("RUSTFLAGS").cloned().unwrap_or_default(); - if sysroot_panic_abort { + if config.sysroot_panic_abort { rustflags.push_str(" -Cpanic=abort -Zpanic-abort-tests"); } - rustflags.push_str(" -Z force-unstable-if-unmarked"); let mut env = env.clone(); - let channel = if sysroot_release_channel { + let channel = if config.sysroot_release_channel { env.insert( "RUSTFLAGS".to_string(), format!("{} -Zmir-opt-level=3", rustflags), @@ -194,21 +187,6 @@ fn build_sysroot_inner( Ok(()) } -pub fn build_sysroot( - env: &HashMap, - sysroot_panic_abort: bool, - sysroot_release_channel: bool, - config: &ConfigInfo, -) -> Result<(), String> { - build_sysroot_inner( - env, - sysroot_panic_abort, - sysroot_release_channel, - config, - Some(Path::new("build_sysroot")), - ) -} - fn build_codegen(args: &mut BuildArg) -> Result<(), String> { let mut env = HashMap::new(); @@ -229,8 +207,7 @@ fn build_codegen(args: &mut BuildArg) -> Result<(), String> { } run_command_with_output_and_env(&command, None, Some(&env))?; - args.config_info - .setup(&mut env, &[], Some(&args.gcc_path))?; + args.config_info.setup(&mut env, Some(&args.gcc_path))?; // We voluntarily ignore the error. let _ = fs::remove_dir_all("target/out"); @@ -243,12 +220,7 @@ fn build_codegen(args: &mut BuildArg) -> Result<(), String> { })?; println!("[BUILD] sysroot"); - build_sysroot( - &env, - args.config_info.sysroot_panic_abort, - args.config_info.sysroot_release_channel, - &args.config_info, - )?; + build_sysroot(&env, &args.config_info)?; Ok(()) } diff --git a/build_system/src/config.rs b/build_system/src/config.rs index d602cec9f9f5..8396681b292a 100644 --- a/build_system/src/config.rs +++ b/build_system/src/config.rs @@ -1,8 +1,9 @@ use crate::utils::{get_gcc_path, get_os_name, rustc_version_info, split_args}; use std::collections::HashMap; use std::env as std_env; +use std::ffi::OsStr; -#[derive(Default)] +#[derive(Default, Debug)] pub struct ConfigInfo { pub target_triple: String, pub host_triple: String, @@ -32,7 +33,6 @@ impl ConfigInfo { }, "--out-dir" => match args.next() { Some(arg) if !arg.is_empty() => { - // env.insert("CARGO_TARGET_DIR".to_string(), arg.to_string()); self.cargo_target_dir = arg.to_string(); } _ => return Err("Expected a value after `--out-dir`, found nothing".to_string()), @@ -44,10 +44,17 @@ impl ConfigInfo { Ok(true) } + pub fn rustc_command_vec(&self) -> Vec<&dyn AsRef> { + let mut command: Vec<&dyn AsRef> = Vec::with_capacity(self.rustc_command.len()); + for arg in self.rustc_command.iter() { + command.push(arg); + } + command + } + pub fn setup( &mut self, env: &mut HashMap, - test_flags: &[String], gcc_path: Option<&str>, ) -> Result<(), String> { env.insert("CARGO_INCREMENTAL".to_string(), "0".to_string()); @@ -90,15 +97,10 @@ impl ConfigInfo { let mut linker = None; if self.host_triple != self.target_triple { - if self.target_triple == "m68k-unknown-linux-gnu" { - linker = Some("-Clinker=m68k-unknown-linux-gnu-gcc".to_string()); - } else if self.target_triple == "aarch64-unknown-linux-gnu" { - // We are cross-compiling for aarch64. Use the correct linker and run tests in qemu. - linker = Some("-Clinker=aarch64-linux-gnu-gcc".to_string()); - } else { + if self.target_triple.is_empty() { return Err("Unknown non-native platform".to_string()); } - + linker = Some(format!("-Clinker={}-gcc", self.target_triple)); self.run_in_vm = true; } @@ -145,7 +147,7 @@ impl ConfigInfo { // This environment variable is useful in case we want to change options of rustc commands. if let Some(cg_rustflags) = env.get("CG_RUSTFLAGS") { - rustflags.extend_from_slice(&split_args(&cg_rustflags)); + rustflags.extend_from_slice(&split_args(&cg_rustflags)?); } if let Some(linker) = linker { @@ -162,7 +164,6 @@ impl ConfigInfo { if !env.contains_key(&"FAT_LTO".to_string()) { rustflags.push("-Clto=off".to_string()); } - rustflags.extend_from_slice(test_flags); // FIXME(antoyo): remove once the atomic shim is gone if os_name == "Darwin" { rustflags.extend_from_slice(&[ diff --git a/build_system/src/prepare.rs b/build_system/src/prepare.rs index da9f8953ec3c..ce9b440be053 100644 --- a/build_system/src/prepare.rs +++ b/build_system/src/prepare.rs @@ -1,5 +1,7 @@ use crate::rustc_info::get_rustc_path; -use crate::utils::{cargo_install, git_clone, run_command, run_command_with_output, walk_dir}; +use crate::utils::{ + cargo_install, git_clone, remove_file, run_command, run_command_with_output, walk_dir, +}; use std::fs; use std::path::Path; @@ -137,8 +139,7 @@ fn build_raytracer(repo_dir: &Path) -> Result<(), String> { run_command(&[&"cargo", &"build"], Some(repo_dir))?; let mv_target = repo_dir.join("raytracer_cg_llvm"); if mv_target.is_file() { - std::fs::remove_file(&mv_target) - .map_err(|e| format!("Failed to remove file `{}`: {e:?}", mv_target.display()))?; + remove_file(&mv_target)?; } run_command( &[&"mv", &"target/debug/main", &"raytracer_cg_llvm"], diff --git a/build_system/src/test.rs b/build_system/src/test.rs index db3e4d9894d8..af2367e668eb 100644 --- a/build_system/src/test.rs +++ b/build_system/src/test.rs @@ -3,11 +3,13 @@ use crate::config::ConfigInfo; use crate::utils::{ get_gcc_path, get_toolchain, run_command, run_command_with_env, run_command_with_output_and_env, rustc_version_info, split_args, walk_dir, + remove_file, }; use std::collections::{BTreeSet, HashMap}; use std::ffi::OsStr; -use std::fs::remove_dir_all; +use std::fs::{File, remove_dir_all}; +use std::io::{BufRead, BufReader}; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -85,7 +87,6 @@ fn show_usage() { `test` command help: --release : Build codegen in release mode - --release-sysroot : Build sysroot in release mode --sysroot-panic-abort : Build the sysroot without unwinding support. --no-default-features : Add `--no-default-features` flag --features [arg] : Add a new feature [arg] @@ -104,7 +105,7 @@ fn show_usage() { println!(" --help : Show this help"); } -#[derive(Default, PartialEq, Eq, Clone, Copy)] +#[derive(Default, PartialEq, Eq, Clone, Copy, Debug)] enum Channel { #[default] Debug, @@ -120,7 +121,7 @@ impl Channel { } } -#[derive(Default)] +#[derive(Default, Debug)] struct TestArg { no_default_features: bool, build_only: bool, @@ -151,7 +152,6 @@ impl TestArg { test_arg.channel = Channel::Release; test_arg.config_info.sysroot_release_channel = true; } - "--release-sysroot" => test_arg.config_info.sysroot_release_channel = true, "--no-default-features" => { // To prevent adding it more than once. if !test_arg.no_default_features { @@ -210,8 +210,19 @@ impl TestArg { get_gcc_path()? }; } + match (test_arg.current_part, test_arg.nb_parts) { + (Some(_), Some(_)) | (None, None) => {} + _ => { + return Err("If either `--current-part` or `--nb-parts` is specified, the other one \ + needs to be specified as well!".to_string()); + } + } Ok(Some(test_arg)) } + + pub fn is_using_gcc_master_branch(&self) -> bool { + !self.no_default_features + } } fn build_if_no_backend(env: &Env, args: &TestArg) -> Result<(), String> { @@ -251,10 +262,7 @@ fn mini_tests(env: &Env, args: &TestArg) -> Result<(), String> { "lib,dylib" } .to_string(); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/mini_core.rs", &"--crate-name", @@ -268,10 +276,7 @@ fn mini_tests(env: &Env, args: &TestArg) -> Result<(), String> { // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[BUILD] example"); - command.clear(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/example.rs", &"--crate-type", @@ -283,10 +288,7 @@ fn mini_tests(env: &Env, args: &TestArg) -> Result<(), String> { // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] mini_core_hello_world"); - command.clear(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/mini_core_hello_world.rs", &"--crate-name", @@ -304,24 +306,19 @@ fn mini_tests(env: &Env, args: &TestArg) -> Result<(), String> { &"abc", &"bcd", ]; - run_command_in_vm(&command, env, args)?; + maybe_run_command_in_vm(&command, env, args)?; Ok(()) } fn build_sysroot(env: &Env, args: &TestArg) -> Result<(), String> { // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[BUILD] sysroot"); - build::build_sysroot( - env, - args.config_info.sysroot_panic_abort, - args.config_info.sysroot_release_channel, - &args.config_info, - )?; + build::build_sysroot(env, &args.config_info)?; Ok(()) } // TODO(GuillaumeGomez): when rewriting in Rust, refactor with the code in tests/lang_tests_common.rs if possible. -fn run_command_in_vm( +fn maybe_run_command_in_vm( command: &[&dyn AsRef], env: &Env, args: &TestArg, @@ -358,12 +355,10 @@ fn run_command_in_vm( } fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { + let cargo_target_dir = Path::new(&args.config_info.cargo_target_dir); // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] arbitrary_self_types_pointers_and_wrappers"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/arbitrary_self_types_pointers_and_wrappers.rs", &"--crate-name", @@ -374,19 +369,15 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &args.config_info.target_triple, ]); run_command_with_env(&command, None, Some(env))?; - run_command_in_vm( - &[&Path::new(&args.config_info.cargo_target_dir) - .join("arbitrary_self_types_pointers_and_wrappers")], + maybe_run_command_in_vm( + &[&cargo_target_dir.join("arbitrary_self_types_pointers_and_wrappers")], env, args, )?; // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] alloc_system"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/alloc_system.rs", &"--crate-type", @@ -394,19 +385,16 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &"--target", &args.config_info.target_triple, ]); - if !args.no_default_features { + if args.is_using_gcc_master_branch() { command.extend_from_slice(&[&"--cfg", &"feature=\"master\""]); } run_command_with_env(&command, None, Some(env))?; // FIXME: doesn't work on m68k. - if args.config_info.host_triple != args.config_info.target_triple { + if args.config_info.host_triple == args.config_info.target_triple { // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] alloc_example"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/alloc_example.rs", &"--crate-type", @@ -415,8 +403,8 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &args.config_info.target_triple, ]); run_command_with_env(&command, None, Some(env))?; - run_command_in_vm( - &[&Path::new(&args.config_info.cargo_target_dir).join("alloc_example")], + maybe_run_command_in_vm( + &[&cargo_target_dir.join("alloc_example")], env, args, )?; @@ -425,10 +413,7 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] dst_field_align"); // FIXME(antoyo): Re-add -Zmir-opt-level=2 once rust-lang/rust#67529 is fixed. - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/dst-field-align.rs", &"--crate-name", @@ -439,18 +424,15 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &args.config_info.target_triple, ]); run_command_with_env(&command, None, Some(env))?; - run_command_in_vm( - &[&Path::new(&args.config_info.cargo_target_dir).join("dst_field_align")], + maybe_run_command_in_vm( + &[&cargo_target_dir.join("dst_field_align")], env, args, )?; // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] std_example"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/std_example.rs", &"--crate-type", @@ -458,13 +440,13 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &"--target", &args.config_info.target_triple, ]); - if !args.no_default_features { + if args.is_using_gcc_master_branch() { command.extend_from_slice(&[&"--cfg", &"feature=\"master\""]); } run_command_with_env(&command, None, Some(env))?; - run_command_in_vm( + maybe_run_command_in_vm( &[ - &Path::new(&args.config_info.cargo_target_dir).join("std_example"), + &cargo_target_dir.join("std_example"), &"--target", &args.config_info.target_triple, ], @@ -472,12 +454,14 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { args, )?; + let test_flags = if let Some(test_flags) = env.get("TEST_FLAGS") { + split_args(test_flags)? + } else { + Vec::new() + }; // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] subslice-patterns-const-eval"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/subslice-patterns-const-eval.rs", &"--crate-type", @@ -485,19 +469,19 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &"--target", &args.config_info.target_triple, ]); + for test_flag in &test_flags { + command.push(test_flag); + } run_command_with_env(&command, None, Some(env))?; - run_command_in_vm( - &[&Path::new(&args.config_info.cargo_target_dir).join("subslice-patterns-const-eval")], + maybe_run_command_in_vm( + &[&cargo_target_dir.join("subslice-patterns-const-eval")], env, args, )?; // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] track-caller-attribute"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/track-caller-attribute.rs", &"--crate-type", @@ -505,19 +489,19 @@ fn std_tests(env: &Env, args: &TestArg) -> Result<(), String> { &"--target", &args.config_info.target_triple, ]); + for test_flag in &test_flags { + command.push(test_flag); + } run_command_with_env(&command, None, Some(env))?; - run_command_in_vm( - &[&Path::new(&args.config_info.cargo_target_dir).join("track-caller-attribute")], + maybe_run_command_in_vm( + &[&cargo_target_dir.join("track-caller-attribute")], env, args, )?; // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[AOT] mod_bench"); - let mut command: Vec<&dyn AsRef> = Vec::new(); - for arg in args.config_info.rustc_command.iter() { - command.push(arg); - } + let mut command = args.config_info.rustc_command_vec(); command.extend_from_slice(&[ &"example/mod_bench.rs", &"--crate-type", @@ -547,6 +531,7 @@ fn setup_rustc(env: &mut Env, args: &TestArg) -> Result<(), String> { None => return Err("Couldn't retrieve rustc commit hash".to_string()), }; run_command_with_output_and_env(&[&"git", &"checkout", &rustc_commit], rust_dir, Some(env))?; + // FIXME: Is it really needed to empty `RUSTFLAGS` here? env.insert("RUSTFLAGS".to_string(), String::new()); let cargo = String::from_utf8( run_command_with_env(&[&"rustup", &"which", &"cargo"], rust_dir, Some(env))?.stdout, @@ -661,7 +646,7 @@ fn run_cargo_command( args: &TestArg, ) -> Result<(), String> { run_cargo_command_with_callback(command, cwd, env, args, |cargo_command, cwd, env| { - run_command_with_output_and_env(&cargo_command, cwd, Some(env))?; + run_command_with_output_and_env(cargo_command, cwd, Some(env))?; Ok(()) }) } @@ -734,7 +719,7 @@ fn test_libcore(env: &Env, args: &TestArg) -> Result<(), String> { // hyperfine --runs ${RUN_RUNS:-10} $cargo_target_dir/mod_bench{,_inline} $cargo_target_dir/mod_bench_llvm_* fn extended_rand_tests(env: &Env, args: &TestArg) -> Result<(), String> { - if args.no_default_features { + if !args.is_using_gcc_master_branch() { return Ok(()); } let path = Path::new("rand"); @@ -746,7 +731,7 @@ fn extended_rand_tests(env: &Env, args: &TestArg) -> Result<(), String> { } fn extended_regex_example_tests(env: &Env, args: &TestArg) -> Result<(), String> { - if args.no_default_features { + if !args.is_using_gcc_master_branch() { return Ok(()); } let path = Path::new("regex"); @@ -800,7 +785,7 @@ fn extended_regex_example_tests(env: &Env, args: &TestArg) -> Result<(), String> } fn extended_regex_tests(env: &Env, args: &TestArg) -> Result<(), String> { - if args.no_default_features { + if !args.is_using_gcc_master_branch() { return Ok(()); } // FIXME: create a function "display_if_not_quiet" or something along the line. @@ -817,6 +802,7 @@ fn extended_regex_tests(env: &Env, args: &TestArg) -> Result<(), String> { &"test", &"--tests", &"--", + // FIXME: try removing `--exclude-should-panic` argument &"--exclude-should-panic", &"--test-threads", &"1", @@ -848,24 +834,22 @@ fn extended_sysroot_tests(env: &Env, args: &TestArg) -> Result<(), String> { Ok(()) } -fn should_remove_ui_test(content: &str) -> bool { - for line in content - .lines() - .map(|line| line.trim()) - .filter(|line| !line.is_empty()) - { - if [ - "// error-pattern:", - "// build-fail", - "// run-fail", - "-Cllvm-args", - "//~", - "// ~", - ] - .iter() - .any(|check| line.contains(check)) - { - return true; +fn should_remove_ui_test(file: File) -> bool { + for line in BufReader::new(file).lines() { + if let Ok(line) = line { + if [ + "// error-pattern:", + "// build-fail", + "// run-fail", + "-Cllvm-args", + "//~", + "// ~", + ] + .iter() + .any(|check| line.contains(check)) + { + return true; + } } } false @@ -903,7 +887,7 @@ fn should_remove_test(path: &Path, path_str: &str) -> bool { .any(|to_ignore| path_str.ends_with(to_ignore)) } -fn test_rustc_inner(env: &Env, args: &TestArg, callback: F) -> Result<(), String> +fn test_rustc_inner(env: &Env, args: &TestArg, prepare_files_callback: F) -> Result<(), String> where F: Fn() -> Result, { @@ -937,24 +921,24 @@ where |_| Ok(()), )?; + // These two functions are used to remove files that are known to not be working currently + // with the GCC backend to reduce noise. fn dir_handling(dir: &Path) -> Result<(), String> { walk_dir(dir, dir_handling, file_handling) } - fn file_handling(file: &Path) -> Result<(), String> { - let path_str = file.display().to_string().replace("\\", "/"); + fn file_handling(file_path: &Path) -> Result<(), String> { + let path_str = file_path.display().to_string().replace("\\", "/"); if !path_str.ends_with(".rs") { return Ok(()); } else if should_not_remove_test(&path_str) { return Ok(()); - } else if should_remove_test(file, &path_str) { - return std::fs::remove_file(file) - .map_err(|error| format!("Failed to remove `{}`: {:?}", file.display(), error)); + } else if should_remove_test(file_path, &path_str) { + return remove_file(&file_path); } - let file_content = std::fs::read_to_string(file) - .map_err(|error| format!("Failed to read `{}`: {:?}", file.display(), error))?; - if should_remove_ui_test(&file_content) { - std::fs::remove_file(file) - .map_err(|error| format!("Failed to remove `{}`: {:?}", file.display(), error))?; + let file = File::open(file_path) + .map_err(|error| format!("Failed to read `{}`: {:?}", file_path.display(), error))?; + if should_remove_ui_test(file) { + remove_file(&file_path)?; } Ok(()) } @@ -963,20 +947,18 @@ where walk_dir(rust_path.join("tests/ui"), dir_handling, file_handling)?; let file = rust_path.join("tests/ui/consts/const_cmp_type_id.rs"); - std::fs::remove_file(&file) - .map_err(|error| format!("Failed to remove `{}`: {:?}", file.display(), error))?; + remove_file(&file)?; let file = rust_path.join("tests/ui/consts/issue-73976-monomorphic.rs"); - std::fs::remove_file(&file) - .map_err(|error| format!("Failed to remove `{}`: {:?}", file.display(), error))?; + remove_file(&file)?; - if !callback()? { + if !prepare_files_callback()? { // FIXME: create a function "display_if_not_quiet" or something along the line. println!("Keeping all UI tests"); } let nb_parts = args.nb_parts.unwrap_or(0); if nb_parts > 0 { - let current_part = args.current_part.unwrap_or(0); + let current_part = args.current_part.unwrap(); // FIXME: create a function "display_if_not_quiet" or something along the line. println!( "Splitting ui_test into {} parts (and running part {})", @@ -1017,18 +999,19 @@ where continue; } let test_path = rust_path.join(path); - std::fs::remove_file(&test_path).map_err(|error| { - format!("Failed to remove `{}`: {:?}", test_path.display(), error) - })?; + remove_file(&test_path)?; } } // FIXME: create a function "display_if_not_quiet" or something along the line. println!("[TEST] rustc test suite"); env.insert("COMPILETEST_FORCE_STAGE0".to_string(), "1".to_string()); - let rustc_args = env - .get("RUSTFLAGS") - .expect("RUSTFLAGS should not be empty at this stage"); + let rustc_args = format!( + "{} {}", + env.get("RUSTFLAGS") + .expect("RUSTFLAGS should not be empty at this stage"), + env.get("TEST_FLAGS").unwrap_or(&String::new()), + ); run_command_with_output_and_env( &[ &"./x.py", @@ -1103,9 +1086,7 @@ fn test_successful_rustc(env: &Env, args: &TestArg) -> Result<(), String> { .filter(|line| !line.is_empty()) { let path = Path::new("rust").join(file); - std::fs::remove_file(&path).map_err(|error| { - format!("failed to remove `{}`: {:?}", path.display(), error) - })?; + remove_file(&path)?; } } else { println!( @@ -1159,9 +1140,7 @@ pub fn run() -> Result<(), String> { return Ok(()); } - let test_flags = split_args(env.get("TEST_FLAGS").unwrap_or(&String::new())); - args.config_info - .setup(&mut env, &test_flags, Some(&args.gcc_path))?; + args.config_info.setup(&mut env, Some(&args.gcc_path))?; if args.runners.is_empty() { run_all(&env, &args)?; diff --git a/build_system/src/utils.rs b/build_system/src/utils.rs index 88fce2fcbce1..59863fcfd90a 100644 --- a/build_system/src/utils.rs +++ b/build_system/src/utils.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::HashMap; use std::ffi::OsStr; use std::fmt::Debug; @@ -48,16 +49,20 @@ fn check_exit_status( let input = input.iter().map(|i| i.as_ref()).collect::>(); eprintln!("Command `{:?}` failed", input); if let Some(output) = output { - unsafe { - let stdout = std::str::from_utf8_unchecked(&output.stdout); - if !stdout.is_empty() { - error.push_str("\n==== STDOUT ====\n"); - error.push_str(stdout); + let stdout = String::from_utf8_lossy(&output.stdout); + if !stdout.is_empty() { + error.push_str("\n==== STDOUT ====\n"); + match stdout { + Cow::Owned(s) => error.push_str(&s), + Cow::Borrowed(s) => error.push_str(s), } - let stderr = std::str::from_utf8_unchecked(&output.stderr); - if !stderr.is_empty() { - error.push_str("\n==== STDERR ====\n"); - error.push_str(stderr); + } + let stderr = String::from_utf8_lossy(&output.stderr); + if !stderr.is_empty() { + error.push_str("\n==== STDERR ====\n"); + match stderr { + Cow::Owned(s) => error.push_str(&s), + Cow::Borrowed(s) => error.push_str(s), } } } @@ -295,17 +300,16 @@ where Ok(()) } -pub fn split_args(args: &str) -> Vec { +pub fn split_args(args: &str) -> Result, String> { let mut out = Vec::new(); let mut start = 0; + let args = args.trim(); let mut iter = args.char_indices().peekable(); while iter.peek().is_some() { while let Some((pos, c)) = iter.next() { if c == ' ' { - if pos != 0 { - out.push(args[start..pos].to_string()); - } + out.push(args[start..pos].to_string()); let mut found_start = false; while let Some((pos, c)) = iter.peek() { if *c != ' ' { @@ -317,7 +321,7 @@ pub fn split_args(args: &str) -> Vec { } } if !found_start { - return out; + return Ok(out); } } else if c == '"' || c == '\'' { let end = c; @@ -332,8 +336,7 @@ pub fn split_args(args: &str) -> Vec { } } if !found_end { - out.push(args[start..].to_string()); - return out; + return Err(format!("Didn't find `{}` at the end of `{}`", end, &args[start..])); } } else if c == '\\' { // We skip the escaped character. @@ -345,5 +348,15 @@ pub fn split_args(args: &str) -> Vec { if !s.is_empty() { out.push(s.to_string()); } - out + Ok(out) +} + +pub fn remove_file>(file_path: &P) -> Result<(), String> { + std::fs::remove_file(file_path).map_err(|error| { + format!( + "Failed to remove `{}`: {:?}", + file_path.as_ref().display(), + error + ) + }) }