From cb2d8412aa3d82d23e4b27784cc045085aa097da Mon Sep 17 00:00:00 2001 From: Zhang Junyang Date: Sun, 9 Feb 2025 21:06:10 +0800 Subject: [PATCH] Refactor the generation of base crates --- Cargo.toml | 4 --- osdk/src/base_crate/Cargo.toml.template | 4 +++ osdk/src/base_crate/mod.rs | 43 +++++++++++++++++++------ osdk/src/commands/build/mod.rs | 28 ++++++++-------- osdk/src/commands/run.rs | 9 +++--- osdk/src/commands/test.rs | 21 ++++++------ 6 files changed, 67 insertions(+), 42 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8725b302..124328df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,10 +40,6 @@ exclude = [ "kernel/libs/comp-sys/component-macro", "kernel/libs/comp-sys/controlled", "osdk", - # The `base` and `test-base` crates are auto-generated by OSDK and ultimately built by Cargo - # during `cargo osdk run` and `cargo osdk test` commands - "target/osdk/base", - "target/osdk/test-base", ] [workspace.lints.clippy] diff --git a/osdk/src/base_crate/Cargo.toml.template b/osdk/src/base_crate/Cargo.toml.template index fd307541..12c06255 100644 --- a/osdk/src/base_crate/Cargo.toml.template +++ b/osdk/src/base_crate/Cargo.toml.template @@ -1,3 +1,7 @@ +# An empty workspace table keeps the base crate away from nesting in other +# workspaces. +[workspace] + [package] name = "#NAME#" version = "#VERSION#" diff --git a/osdk/src/base_crate/mod.rs b/osdk/src/base_crate/mod.rs index bb3b7f97..d335daa9 100644 --- a/osdk/src/base_crate/mod.rs +++ b/osdk/src/base_crate/mod.rs @@ -44,20 +44,43 @@ fn are_files_identical(file1: &PathBuf, file2: &PathBuf) -> Result { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BaseCrateType { + /// The base crate is for running the target kernel crate. + Run, + /// The base crate is for testing the target crate. + Test, + /// The base crate is for other actions using Cargo. + Other, +} + /// Create a new base crate that will be built by cargo. /// /// The dependencies of the base crate will be the target crate. If /// `link_unit_test_runner` is set to true, the base crate will also depend on /// the `ostd-test-runner` crate. +/// +/// It returns the path to the base crate. pub fn new_base_crate( - base_crate_path: impl AsRef, + base_type: BaseCrateType, + base_crate_path_stem: impl AsRef, dep_crate_name: &str, dep_crate_path: impl AsRef, link_unit_test_runner: bool, -) { - // Check if the existing crate base is reusable. Crate bases for ktest are never reusable. - if !base_crate_path.as_ref().ends_with("test-base") && base_crate_path.as_ref().exists() { - let base_crate_tmp_path = base_crate_path.as_ref().join("tmp"); +) -> PathBuf { + let base_crate_path: PathBuf = PathBuf::from( + (base_crate_path_stem.as_ref().as_os_str().to_string_lossy() + + match base_type { + BaseCrateType::Run => "-run-base", + BaseCrateType::Test => "-test-base", + BaseCrateType::Other => "-base", + }) + .to_string(), + ); + // Check if the existing crate base is reusable. + if base_type == BaseCrateType::Run && base_crate_path.exists() { + // Reuse the existing base crate if it is identical to the new one. + let base_crate_tmp_path = base_crate_path.join("tmp"); do_new_base_crate( &base_crate_tmp_path, dep_crate_name, @@ -65,25 +88,27 @@ pub fn new_base_crate( link_unit_test_runner, ); let cargo_result = are_files_identical( - &base_crate_path.as_ref().join("Cargo.toml"), + &base_crate_path.join("Cargo.toml"), &base_crate_tmp_path.join("Cargo.toml"), ); let main_rs_result = are_files_identical( - &base_crate_path.as_ref().join("src").join("main.rs"), + &base_crate_path.join("src").join("main.rs"), &base_crate_tmp_path.join("src").join("main.rs"), ); std::fs::remove_dir_all(&base_crate_tmp_path).unwrap(); if cargo_result.is_ok_and(|res| res) && main_rs_result.is_ok_and(|res| res) { info!("Reusing existing base crate"); - return; + return base_crate_path; } } do_new_base_crate( - base_crate_path, + &base_crate_path, dep_crate_name, dep_crate_path, link_unit_test_runner, ); + + base_crate_path } fn do_new_base_crate( diff --git a/osdk/src/commands/build/mod.rs b/osdk/src/commands/build/mod.rs index b3e8cc1b..22ecf014 100644 --- a/osdk/src/commands/build/mod.rs +++ b/osdk/src/commands/build/mod.rs @@ -16,7 +16,7 @@ use bin::make_elf_for_qemu; use super::util::{cargo, profile_name_adapter, COMMON_CARGO_ARGS, DEFAULT_TARGET_RELPATH}; use crate::{ arch::Arch, - base_crate::new_base_crate, + base_crate::{new_base_crate, BaseCrateType}, bundle::{ bin::{AsterBin, AsterBinType, AsterElfMeta}, file::BundleFile, @@ -29,7 +29,7 @@ use crate::{ }, error::Errno, error_msg, - util::{get_cargo_metadata, get_current_crates, get_target_directory}, + util::{get_cargo_metadata, get_current_crates, get_target_directory, CrateInfo, DirGuard}, }; pub fn execute_build_command(config: &Config, build_args: &BuildArgs) { @@ -65,6 +65,7 @@ pub fn execute_build_command(config: &Config, build_args: &BuildArgs) { }; let _bundle = create_base_and_cached_build( + target_info, bundle_path, &osdk_output_directory, &cargo_target_directory, @@ -75,6 +76,7 @@ pub fn execute_build_command(config: &Config, build_args: &BuildArgs) { } pub fn create_base_and_cached_build( + target_crate: CrateInfo, bundle_path: impl AsRef, osdk_output_directory: impl AsRef, cargo_target_directory: impl AsRef, @@ -82,25 +84,25 @@ pub fn create_base_and_cached_build( action: ActionChoice, rustflags: &[&str], ) -> Bundle { - let base_crate_path = osdk_output_directory.as_ref().join("base"); - new_base_crate( - &base_crate_path, - &get_current_crates().remove(0).name, - get_current_crates().remove(0).path, + let base_crate_path = new_base_crate( + match action { + ActionChoice::Run => BaseCrateType::Run, + ActionChoice::Test => BaseCrateType::Test, + }, + osdk_output_directory.as_ref().join(&target_crate.name), + &target_crate.name, + target_crate.path, false, ); - let original_dir = std::env::current_dir().unwrap(); - std::env::set_current_dir(&base_crate_path).unwrap(); - let bundle = do_cached_build( + let _dir_guard = DirGuard::change_dir(&base_crate_path); + do_cached_build( &bundle_path, &osdk_output_directory, &cargo_target_directory, config, action, rustflags, - ); - std::env::set_current_dir(original_dir).unwrap(); - bundle + ) } fn get_reusable_existing_bundle( diff --git a/osdk/src/commands/run.rs b/osdk/src/commands/run.rs index bc0e0536..5a2435aa 100644 --- a/osdk/src/commands/run.rs +++ b/osdk/src/commands/run.rs @@ -21,15 +21,15 @@ pub fn execute_run_command(config: &Config, gdb_server_args: Option<&str>) { let osdk_output_directory = cargo_target_directory.join(DEFAULT_TARGET_RELPATH); let targets = get_current_crates(); - let mut target_name = None; + let mut target_info = None; for target in targets { if target.is_kernel_crate { - target_name = Some(target.name); + target_info = Some(target); break; } } - let target_name = target_name.unwrap_or_else(|| { + let target_info = target_info.unwrap_or_else(|| { error_msg!("No kernel crate found in the current workspace"); exit(Errno::NoKernelCrate as _); }); @@ -42,8 +42,9 @@ pub fn execute_run_command(config: &Config, gdb_server_args: Option<&str>) { None }; - let default_bundle_directory = osdk_output_directory.join(target_name); + let default_bundle_directory = osdk_output_directory.join(&target_info.name); let bundle = create_base_and_cached_build( + target_info, default_bundle_directory, &osdk_output_directory, &cargo_target_directory, diff --git a/osdk/src/commands/test.rs b/osdk/src/commands/test.rs index 6020ca7e..3f1bd280 100644 --- a/osdk/src/commands/test.rs +++ b/osdk/src/commands/test.rs @@ -4,12 +4,12 @@ use std::fs; use super::{build::do_cached_build, util::DEFAULT_TARGET_RELPATH}; use crate::{ - base_crate::new_base_crate, + base_crate::{new_base_crate, BaseCrateType}, cli::TestArgs, config::{scheme::ActionChoice, Config}, error::Errno, error_msg, - util::{get_current_crates, get_target_directory}, + util::{get_current_crates, get_target_directory, DirGuard}, }; pub fn execute_test_command(config: &Config, args: &TestArgs) { @@ -24,8 +24,6 @@ pub fn test_current_crate(config: &Config, args: &TestArgs) { let current_crate = get_current_crates().remove(0); let cargo_target_directory = get_target_directory(); let osdk_output_directory = cargo_target_directory.join(DEFAULT_TARGET_RELPATH); - // Use a different name for better separation and reusability of `run` and `test` - let target_crate_dir = osdk_output_directory.join("test-base"); // A special case is that we use OSDK to test the OSDK test runner crate // itself. We check it by name. @@ -40,8 +38,9 @@ pub fn test_current_crate(config: &Config, args: &TestArgs) { false }; - new_base_crate( - &target_crate_dir, + let target_crate_dir = new_base_crate( + BaseCrateType::Test, + osdk_output_directory.join(¤t_crate.name), ¤t_crate.name, ¤t_crate.path, !runner_self_test, @@ -54,7 +53,7 @@ pub fn test_current_crate(config: &Config, args: &TestArgs) { None => r#"None"#.to_string(), }; - let mut ktest_crate_whitelist = vec![current_crate.name]; + let mut ktest_crate_whitelist = vec![current_crate.name.clone()]; if let Some(name) = &args.test_name { ktest_crate_whitelist.push(name.clone()); } @@ -85,10 +84,8 @@ pub static KTEST_CRATE_WHITELIST: Option<&[&str]> = Some(&{:#?}); fs::write(&main_rs_path, main_rs_content).unwrap(); // Build the kernel with the given base crate - let target_name = get_current_crates().remove(0).name; - let default_bundle_directory = osdk_output_directory.join(target_name); - let original_dir = std::env::current_dir().unwrap(); - std::env::set_current_dir(&target_crate_dir).unwrap(); + let default_bundle_directory = osdk_output_directory.join(¤t_crate.name); + let dir_guard = DirGuard::change_dir(&target_crate_dir); let bundle = do_cached_build( default_bundle_directory, &osdk_output_directory, @@ -98,7 +95,7 @@ pub static KTEST_CRATE_WHITELIST: Option<&[&str]> = Some(&{:#?}); &["--cfg ktest"], ); std::env::remove_var("RUSTFLAGS"); - std::env::set_current_dir(original_dir).unwrap(); + drop(dir_guard); bundle.run(config, ActionChoice::Test); }