diff --git a/Cargo.toml b/Cargo.toml index b0c65eead..5caa09eb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,10 @@ 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", ] # Cargo only looks at the profile settings diff --git a/Makefile b/Makefile index b70a29a6d..dfdd24c53 100644 --- a/Makefile +++ b/Makefile @@ -153,6 +153,10 @@ OSDK_CRATES := \ kernel/libs/aster-util \ kernel/libs/aster-bigtcp +# OSDK dependencies +OSDK_SRC_FILES := \ + $(shell find osdk/Cargo.toml osdk/Cargo.lock osdk/src -type f) + .PHONY: all all: build @@ -165,10 +169,9 @@ install_osdk: @# dependencies to `crates.io`. @OSDK_LOCAL_DEV=1 cargo install cargo-osdk --path osdk -# This will install OSDK if it is not already installed -# To update OSDK, we need to run `install_osdk` manually -$(CARGO_OSDK): - @make --no-print-directory install_osdk +# This will install and update OSDK automatically +$(CARGO_OSDK): $(OSDK_SRC_FILES) + @$(MAKE) --no-print-directory install_osdk .PHONY: check_osdk check_osdk: @@ -182,7 +185,7 @@ test_osdk: .PHONY: initramfs initramfs: - @make --no-print-directory -C test + @$(MAKE) --no-print-directory -C test .PHONY: build build: initramfs $(CARGO_OSDK) @@ -215,7 +218,7 @@ gdb_server: initramfs $(CARGO_OSDK) @cargo osdk run $(CARGO_OSDK_ARGS) --gdb-server wait-client,vscode,addr=:$(GDB_TCP_PORT) .PHONY: gdb_client -gdb_client: $(CARGO_OSDK) +gdb_client: initramfs $(CARGO_OSDK) @cargo osdk debug $(CARGO_OSDK_ARGS) --remote :$(GDB_TCP_PORT) .PHONY: profile_server @@ -223,7 +226,7 @@ profile_server: initramfs $(CARGO_OSDK) @cargo osdk run $(CARGO_OSDK_ARGS) --gdb-server addr=:$(GDB_TCP_PORT) .PHONY: profile_client -profile_client: $(CARGO_OSDK) +profile_client: initramfs $(CARGO_OSDK) @cargo osdk profile $(CARGO_OSDK_ARGS) --remote :$(GDB_TCP_PORT) \ --samples $(GDB_PROFILE_COUNT) --interval $(GDB_PROFILE_INTERVAL) --format $(GDB_PROFILE_FORMAT) @@ -257,7 +260,7 @@ docs: $(CARGO_OSDK) .PHONY: format format: @./tools/format_all.sh - @make --no-print-directory -C test format + @$(MAKE) --no-print-directory -C test format .PHONY: check check: initramfs $(CARGO_OSDK) @@ -279,7 +282,7 @@ check: initramfs $(CARGO_OSDK) echo "Checking $$dir"; \ (cd $$dir && cargo osdk clippy -- -- -D warnings) || exit 1; \ done - @make --no-print-directory -C test check + @$(MAKE) --no-print-directory -C test check @typos .PHONY: clean @@ -291,6 +294,6 @@ clean: @echo "Cleaning up documentation target files" @cd docs && mdbook clean @echo "Cleaning up test target files" - @make --no-print-directory -C test clean + @$(MAKE) --no-print-directory -C test clean @echo "Uninstalling OSDK" @rm -f $(CARGO_OSDK) diff --git a/osdk/src/base_crate/mod.rs b/osdk/src/base_crate/mod.rs index 32fe2ddc8..bb3b7f97f 100644 --- a/osdk/src/base_crate/mod.rs +++ b/osdk/src/base_crate/mod.rs @@ -5,12 +5,45 @@ use std::{ fs, + io::{Read, Result}, path::{Path, PathBuf}, str::FromStr, }; use crate::util::get_cargo_metadata; +/// Compares two files byte-by-byte to check if they are identical. +/// Returns `Ok(true)` if files are identical, `Ok(false)` if they are different, or `Err` if any I/O operation fails. +fn are_files_identical(file1: &PathBuf, file2: &PathBuf) -> Result { + // Check file size first + let metadata1 = fs::metadata(file1)?; + let metadata2 = fs::metadata(file2)?; + + if metadata1.len() != metadata2.len() { + return Ok(false); // Different sizes, not identical + } + + // Compare file contents byte-by-byte + let mut file1 = fs::File::open(file1)?; + let mut file2 = fs::File::open(file2)?; + + let mut buffer1 = [0u8; 4096]; + let mut buffer2 = [0u8; 4096]; + + loop { + let bytes_read1 = file1.read(&mut buffer1)?; + let bytes_read2 = file2.read(&mut buffer2)?; + + if bytes_read1 != bytes_read2 || buffer1[..bytes_read1] != buffer2[..bytes_read1] { + return Ok(false); // Files are different + } + + if bytes_read1 == 0 { + return Ok(true); // End of both files, identical + } + } +} + /// Create a new base crate that will be built by cargo. /// /// The dependencies of the base crate will be the target crate. If @@ -21,6 +54,43 @@ pub fn new_base_crate( 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"); + do_new_base_crate( + &base_crate_tmp_path, + dep_crate_name, + &dep_crate_path, + link_unit_test_runner, + ); + let cargo_result = are_files_identical( + &base_crate_path.as_ref().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_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; + } + } + do_new_base_crate( + base_crate_path, + dep_crate_name, + dep_crate_path, + link_unit_test_runner, + ); +} + +fn do_new_base_crate( + base_crate_path: impl AsRef, + dep_crate_name: &str, + dep_crate_path: impl AsRef, + link_unit_test_runner: bool, ) { let workspace_root = { let meta = get_cargo_metadata(None::<&str>, None::<&[&str]>).unwrap(); diff --git a/osdk/src/bundle/bin.rs b/osdk/src/bundle/bin.rs index 9cc6a9d24..b09532aa4 100644 --- a/osdk/src/bundle/bin.rs +++ b/osdk/src/bundle/bin.rs @@ -83,12 +83,11 @@ impl AsterBin { self.stripped } - /// Move the binary to the `base` directory and convert the path to a relative path. - pub fn move_to(self, base: impl AsRef) -> Self { + /// Copy the binary to the `base` directory and convert the path to a relative path. + pub fn copy_to(self, base: impl AsRef) -> Self { let file_name = self.path.file_name().unwrap(); let copied_path = base.as_ref().join(file_name); fs::copy(&self.path, copied_path).unwrap(); - fs::remove_file(&self.path).unwrap(); Self { path: PathBuf::from(file_name), arch: self.arch, diff --git a/osdk/src/bundle/file.rs b/osdk/src/bundle/file.rs index f95062c39..ee2c55db5 100644 --- a/osdk/src/bundle/file.rs +++ b/osdk/src/bundle/file.rs @@ -53,7 +53,7 @@ impl Initramfs { } } - /// Move the initramfs to the `base` directory and convert the path to a relative path. + /// Copy the initramfs to the `base` directory and convert the path to a relative path. pub fn copy_to(self, base: impl AsRef) -> Self { let name = self.path.file_name().unwrap(); let dest = base.as_ref().join(name); diff --git a/osdk/src/bundle/mod.rs b/osdk/src/bundle/mod.rs index 2c75c8da7..f66a7b268 100644 --- a/osdk/src/bundle/mod.rs +++ b/osdk/src/bundle/mod.rs @@ -291,7 +291,7 @@ impl Bundle { if self.manifest.vm_image.is_some() { panic!("vm_image already exists"); } - self.manifest.vm_image = Some(vm_image.move_to(&self.path)); + self.manifest.vm_image = Some(vm_image.copy_to(&self.path)); self.write_manifest_to_fs(); } @@ -300,7 +300,7 @@ impl Bundle { if self.manifest.aster_bin.is_some() { panic!("aster_bin already exists"); } - self.manifest.aster_bin = Some(aster_bin.move_to(&self.path)); + self.manifest.aster_bin = Some(aster_bin.copy_to(&self.path)); self.write_manifest_to_fs(); } diff --git a/osdk/src/bundle/vm_image.rs b/osdk/src/bundle/vm_image.rs index 5c90d2956..6ebb4cd0e 100644 --- a/osdk/src/bundle/vm_image.rs +++ b/osdk/src/bundle/vm_image.rs @@ -59,8 +59,8 @@ impl AsterVmImage { &self.typ } - /// Move the binary to the `base` directory and convert the path to a relative path. - pub fn move_to(self, base: impl AsRef) -> Self { + /// Copy the binary to the `base` directory and convert the path to a relative path. + pub fn copy_to(self, base: impl AsRef) -> Self { let file_name = self.path.file_name().unwrap(); let copied_path = base.as_ref().join(file_name); fs::copy(&self.path, copied_path).unwrap(); diff --git a/osdk/src/commands/build/mod.rs b/osdk/src/commands/build/mod.rs index 2a3176be0..6e88e7461 100644 --- a/osdk/src/commands/build/mod.rs +++ b/osdk/src/commands/build/mod.rs @@ -8,7 +8,7 @@ use std::{ ffi::OsString, path::{Path, PathBuf}, process, - time::{Duration, SystemTime}, + time::SystemTime, }; use bin::make_elf_for_qemu; @@ -19,6 +19,7 @@ use crate::{ base_crate::new_base_crate, bundle::{ bin::{AsterBin, AsterBinType, AsterElfMeta}, + file::BundleFile, Bundle, }, cli::BuildArgs, @@ -88,6 +89,31 @@ pub fn create_base_and_cached_build( bundle } +fn get_reusable_existing_bundle( + bundle_path: impl AsRef, + config: &Config, + action: ActionChoice, +) -> Option { + let existing_bundle = Bundle::load(&bundle_path); + let Some(existing_bundle) = existing_bundle else { + info!("Building a new bundle: No cached bundle found or validation of the existing bundle failed"); + return None; + }; + if let Err(e) = existing_bundle.can_run_with_config(config, action) { + info!("Building a new bundle: {}", e); + return None; + } + let workspace_root = { + let meta = get_cargo_metadata(None::<&str>, None::<&[&str]>).unwrap(); + PathBuf::from(meta.get("workspace_root").unwrap().as_str().unwrap()) + }; + if existing_bundle.last_modified_time() < get_last_modified_time(&workspace_root) { + info!("Building a new bundle: workspace_root has been updated"); + return None; + } + Some(existing_bundle) +} + /// If the source is not since modified and the last build is recent, we can reuse the existing bundle. pub fn do_cached_build( bundle_path: impl AsRef, @@ -97,54 +123,6 @@ pub fn do_cached_build( action: ActionChoice, rustflags: &[&str], ) -> Bundle { - let build_a_new_one = || { - do_build( - &bundle_path, - &osdk_output_directory, - &cargo_target_directory, - config, - action, - rustflags, - ) - }; - - let existing_bundle = Bundle::load(&bundle_path); - let Some(existing_bundle) = existing_bundle else { - return build_a_new_one(); - }; - if existing_bundle.can_run_with_config(config, action).is_err() { - return build_a_new_one(); - } - let Ok(built_since) = SystemTime::now().duration_since(existing_bundle.last_modified_time()) - else { - return build_a_new_one(); - }; - if built_since > Duration::from_secs(600) { - return build_a_new_one(); - } - let workspace_root = { - let meta = get_cargo_metadata(None::<&str>, None::<&[&str]>).unwrap(); - PathBuf::from(meta.get("workspace_root").unwrap().as_str().unwrap()) - }; - if get_last_modified_time(workspace_root) < existing_bundle.last_modified_time() { - return existing_bundle; - } - build_a_new_one() -} - -pub fn do_build( - bundle_path: impl AsRef, - osdk_output_directory: impl AsRef, - cargo_target_directory: impl AsRef, - config: &Config, - action: ActionChoice, - rustflags: &[&str], -) -> Bundle { - if bundle_path.as_ref().exists() { - std::fs::remove_dir_all(&bundle_path).unwrap(); - } - let mut bundle = Bundle::new(&bundle_path, config, action); - let (build, boot) = match action { ActionChoice::Run => (&config.run.build, &config.run.boot), ActionChoice::Test => (&config.test.build, &config.test.boot), @@ -160,6 +138,21 @@ pub fn do_build( rustflags, ); + // Check the existing bundle's reusability + if let Some(existing_bundle) = get_reusable_existing_bundle(&bundle_path, config, action) { + if get_last_modified_time(aster_elf.path()) < existing_bundle.last_modified_time() { + info!("Reusing existing bundle: aster_elf is unchanged"); + return existing_bundle; + } + } + + // Build a new bundle + info!("Building a new bundle"); + if bundle_path.as_ref().exists() { + std::fs::remove_dir_all(&bundle_path).unwrap(); + } + let mut bundle = Bundle::new(&bundle_path, config, action); + match boot.method { BootMethod::GrubRescueIso | BootMethod::GrubQcow2 => { info!("Building boot device image"); @@ -274,6 +267,9 @@ fn build_kernel_elf( } fn get_last_modified_time(path: impl AsRef) -> SystemTime { + if path.as_ref().is_file() { + return path.as_ref().metadata().unwrap().modified().unwrap(); + } let mut last_modified = SystemTime::UNIX_EPOCH; for entry in std::fs::read_dir(path).unwrap() { let entry = entry.unwrap(); diff --git a/osdk/src/commands/new/mod.rs b/osdk/src/commands/new/mod.rs index 3ea282e91..3653b6ba2 100644 --- a/osdk/src/commands/new/mod.rs +++ b/osdk/src/commands/new/mod.rs @@ -47,9 +47,10 @@ fn add_manifest_dependencies(cargo_metadata: &serde_json::Value, crate_name: &st fs::write(manifest_path, content).unwrap(); } -// Add `target/osdk/base` to `exclude` array of the workspace manifest +// Add `target/osdk/base` and `target/osdk/test-base` to `exclude` array of the workspace manifest fn exclude_osdk_base(metadata: &serde_json::Value) { - let osdk_base_path = "target/osdk/base"; + let osdk_run_base_path = "target/osdk/base"; + let osdk_test_base_path = "target/osdk/test-base"; let workspace_manifest_path = { let workspace_root = metadata.get("workspace_root").unwrap().as_str().unwrap(); @@ -64,17 +65,25 @@ fn exclude_osdk_base(metadata: &serde_json::Value) { if let Some(exclude) = workspace.get_mut("exclude") { let exclude = exclude.as_array_mut().unwrap(); - if exclude.contains(&toml::Value::String(osdk_base_path.to_string())) { + if exclude.contains(&toml::Value::String(osdk_run_base_path.to_string())) + || exclude.contains(&toml::Value::String(osdk_test_base_path.to_string())) + { return; } - exclude.push(toml::Value::String(osdk_base_path.to_string())); + exclude.push(toml::Value::String(osdk_run_base_path.to_string())); + exclude.push(toml::Value::String(osdk_test_base_path.to_string())); } else { - let exclude = vec![toml::Value::String(osdk_base_path.to_string())]; + let exclude = vec![ + toml::Value::String(osdk_run_base_path.to_string()), + toml::Value::String(osdk_test_base_path.to_string()), + ]; workspace.insert("exclude".to_string(), toml::Value::Array(exclude)); } } else { - let exclude = toml::Table::from_str(r#"exclude = ["target/osdk/base"]"#).unwrap(); + let exclude = + toml::Table::from_str(r#"exclude = ["target/osdk/base", "target/osdk/test-base"]"#) + .unwrap(); manifest_toml.insert("workspace".to_string(), toml::Value::Table(exclude)); } diff --git a/osdk/src/commands/test.rs b/osdk/src/commands/test.rs index 327ebd476..bd2cb4fa6 100644 --- a/osdk/src/commands/test.rs +++ b/osdk/src/commands/test.rs @@ -26,7 +26,8 @@ pub fn test_current_crate(config: &Config, args: &TestArgs) { let current_crate = get_current_crate_info(); let cargo_target_directory = get_target_directory(); let osdk_output_directory = cargo_target_directory.join(DEFAULT_TARGET_RELPATH); - let target_crate_dir = osdk_output_directory.join("base"); + // 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. diff --git a/osdk/src/config/scheme/qemu.rs b/osdk/src/config/scheme/qemu.rs index 3b307aa99..acf6ca110 100644 --- a/osdk/src/config/scheme/qemu.rs +++ b/osdk/src/config/scheme/qemu.rs @@ -34,7 +34,7 @@ pub struct QemuScheme { pub path: Option, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Eq, Serialize, Deserialize)] pub struct Qemu { pub args: String, /// This finalized config has a unorthodox `Option` because @@ -55,6 +55,20 @@ impl Default for Qemu { } } +// Implements `PartialEq` for `Qemu`, comparing `args` while ignoring numeric characters +// (random ports), and comparing other fields (`bootdev_append_options` and `path`) normally. +impl PartialEq for Qemu { + fn eq(&self, other: &Self) -> bool { + fn strip_numbers(input: &str) -> String { + input.chars().filter(|c| !c.is_numeric()).collect() + } + + strip_numbers(&self.args) == strip_numbers(&other.args) + && self.bootdev_append_options == other.bootdev_append_options + && self.path == other.path + } +} + impl Qemu { pub fn apply_qemu_args(&mut self, args: &Vec) { let mut joined = split_to_kv_array(&self.args); diff --git a/osdk/tests/util/mod.rs b/osdk/tests/util/mod.rs index 52403088c..a80aa9526 100644 --- a/osdk/tests/util/mod.rs +++ b/osdk/tests/util/mod.rs @@ -57,7 +57,10 @@ pub fn create_workspace(workspace_name: &str, members: &[&str]) { .map(|member| toml::Value::String(member.to_string())) .collect(); - let exclude = toml::Value::Array(vec![toml::Value::String("target/osdk/base".to_string())]); + let exclude = toml::Value::Array(vec![ + toml::Value::String("target/osdk/base".to_string()), + toml::Value::String("target/osdk/test-base".to_string()), + ]); table.insert("members".to_string(), toml::Value::Array(members)); table.insert("exclude".to_string(), exclude);