From 41ef6556c88bd0a65648f632eca429015fe04a80 Mon Sep 17 00:00:00 2001 From: SFG545 Date: Fri, 10 Jul 2026 18:38:21 -0500 Subject: [PATCH] feat: implement parallel verification for package installation and auto-select tool providers --- src/commands.rs | 81 +++++++++++++++++---- src/commands/set.rs | 161 ++++++++++++++++++++++++++++++++++++++++++ src/commands/tests.rs | 52 ++++++++++++-- src/db/repo.rs | 130 ++++++++++++++-------------------- src/install/dkms.rs | 13 ++++ src/planner.rs | 1 - 6 files changed, 342 insertions(+), 96 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index da7d4e8..6539239 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -15,6 +15,10 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fs; use std::io::IsTerminal; use std::path::{Path, PathBuf}; +use std::sync::{ + atomic::{AtomicUsize, Ordering as AtomicOrdering}, + mpsc, +}; use std::time::Duration; use url::Url; use walkdir::WalkDir; @@ -1406,6 +1410,9 @@ fn install_planned_packages_to_rootfs_with_pre_removed( pending_post_hooks.push((plan.spec.package.name.clone(), hook)); } } + // Lifecycle hooks may invoke sh, cc, or ld. Select a sole provider before + // any post-install hook runs so the aliases are usable within this transaction. + set::auto_select_sole_tool_providers(rootfs, config)?; for (pkg_name, pending_hook) in pending_post_hooks { let installed_scripts_dir = install::scripts::installed_scripts_dir(rootfs, &pkg_name); let _ = install::scripts::run_hook_if_present_or_defer( @@ -1419,6 +1426,58 @@ fn install_planned_packages_to_rootfs_with_pre_removed( Ok(()) } +fn run_parallel_verification(items: &[T], progress: &ProgressBar, verify: F) -> Result<()> +where + T: Sync, + F: Fn(&T) -> Result<()> + Sync, +{ + if items.is_empty() { + return Ok(()); + } + + let worker_count = std::thread::available_parallelism() + .map(|count| count.get()) + .unwrap_or(1) + .min(items.len()); + let next_index = AtomicUsize::new(0); + let (sender, receiver) = mpsc::channel(); + + std::thread::scope(|scope| -> Result<()> { + for _ in 0..worker_count { + let sender = sender.clone(); + let verify = &verify; + let next_index = &next_index; + scope.spawn(move || { + loop { + let index = next_index.fetch_add(1, AtomicOrdering::Relaxed); + if index >= items.len() { + break; + } + let result = verify(&items[index]); + if sender.send((index, result)).is_err() { + break; + } + } + }); + } + drop(sender); + + let mut results: Vec>> = (0..items.len()).map(|_| None).collect(); + for _ in 0..items.len() { + let (index, result) = receiver + .recv() + .context("Verification worker exited before reporting a result")?; + results[index] = Some(result); + progress.inc(1); + } + + for result in results { + result.expect("every verification item must report a result")?; + } + Ok(()) + }) +} + #[cfg(test)] fn install_package_outputs_to_rootfs( pkg_spec: &package::PackageSpec, @@ -1801,7 +1860,7 @@ fn execute_install_plan_with_child_commands( .progress_chars("#>-"), ); checksum_pb.set_prefix("checksums"); - for item in &binary_phase_items { + run_parallel_verification(&binary_phase_items, &checksum_pb, |item| { let cached = binary_archives .get(&(item.repo_name.clone(), item.record.filename.clone())) .with_context(|| { @@ -1812,13 +1871,12 @@ fn execute_install_plan_with_child_commands( })?; db::repo::verify_binary_package_archive_checksums(&cached.package_path, &item.record) .with_context(|| { - format!( - "Checksum verification failed for {} from repo '{}'", - item.record.filename, item.repo_name - ) - })?; - checksum_pb.inc(1); - } + format!( + "Checksum verification failed for {} from repo '{}'", + item.record.filename, item.repo_name + ) + }) + })?; checksum_pb.finish_and_clear(); ui::info(format!( @@ -1838,7 +1896,7 @@ fn execute_install_plan_with_child_commands( .progress_chars("#>-"), ); signature_pb.set_prefix("signatures"); - for item in &binary_phase_items { + run_parallel_verification(&binary_phase_items, &signature_pb, |item| { let repo_cfg = config .binary_repos .get(&item.repo_name) @@ -1863,9 +1921,8 @@ fn execute_install_plan_with_child_commands( "Detached signature verification failed for {} from repo '{}'", item.record.filename, item.repo_name ) - })?; - signature_pb.inc(1); - } + }) + })?; signature_pb.finish_and_clear(); } diff --git a/src/commands/set.rs b/src/commands/set.rs index bd81ec5..35698a4 100644 --- a/src/commands/set.rs +++ b/src/commands/set.rs @@ -1,5 +1,6 @@ use super::*; use crate::cli::ToolRoleArg; +use std::collections::HashSet; use std::io::ErrorKind; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -8,6 +9,48 @@ struct ToolAlias { target: &'static str, } +struct AutomaticToolProvider { + implementation: &'static str, + package_names: &'static [&'static str], +} + +const COMPILER_PROVIDERS: &[AutomaticToolProvider] = &[ + AutomaticToolProvider { + implementation: "clang", + package_names: &["clang"], + }, + AutomaticToolProvider { + implementation: "gcc", + package_names: &["gcc"], + }, +]; + +const LINKER_PROVIDERS: &[AutomaticToolProvider] = &[ + AutomaticToolProvider { + implementation: "lld", + package_names: &["lld", "llvm"], + }, + AutomaticToolProvider { + implementation: "mold", + package_names: &["mold"], + }, +]; + +const SHELL_PROVIDERS: &[AutomaticToolProvider] = &[ + AutomaticToolProvider { + implementation: "bash", + package_names: &["bash"], + }, + AutomaticToolProvider { + implementation: "dash", + package_names: &["dash"], + }, + AutomaticToolProvider { + implementation: "zsh", + package_names: &["zsh"], + }, +]; + pub(super) fn run_set(args: SetArgs) -> Result<()> { let SetArgs { rootfs_args, @@ -48,6 +91,46 @@ pub(super) fn run_set(args: SetArgs) -> Result<()> { Ok(()) } +/// Configure tool aliases automatically when exactly one supported provider is installed. +pub(crate) fn auto_select_sole_tool_providers( + rootfs: &Path, + config: &config::Config, +) -> Result<()> { + let installed = db::get_installed_packages(&config.installed_db_path(rootfs))?; + if installed.is_empty() { + return Ok(()); + } + + let alias_dir = dir_in_rootfs(rootfs, &configured_alias_dir(config)); + for role in [ + ToolRoleArg::Compiler, + ToolRoleArg::Linker, + ToolRoleArg::Shell, + ] { + let Some(provider) = sole_available_provider(role, &installed, &alias_dir)? else { + continue; + }; + let aliases = aliases_for_selection(role, provider.implementation)?; + if let Err(err) = configure_tool_aliases(&alias_dir, &aliases) { + crate::log_warn!( + "Could not automatically set {} to {} in {}: {}", + role_name(role), + provider.implementation, + alias_dir.display(), + err + ); + continue; + } + ui::info(format!( + "Automatically set {} to {} in {}", + role_name(role), + provider.implementation, + alias_dir.display() + )); + } + Ok(()) +} + fn role_name(role: ToolRoleArg) -> &'static str { match role { ToolRoleArg::Compiler => "compiler", @@ -153,6 +236,43 @@ fn aliases_for_selection(role: ToolRoleArg, implementation: &str) -> Result, + alias_dir: &Path, +) -> Result> { + let providers = match role { + ToolRoleArg::Compiler => COMPILER_PROVIDERS, + ToolRoleArg::Linker => LINKER_PROVIDERS, + ToolRoleArg::Shell => SHELL_PROVIDERS, + }; + + let available: Vec<_> = providers + .iter() + .filter(|provider| { + provider + .package_names + .iter() + .any(|name| installed.contains(*name)) + }) + .filter(|provider| { + aliases_for_selection(role, provider.implementation) + .map(|aliases| { + aliases + .iter() + .all(|alias| alias_dir.join(alias.target).exists()) + }) + .unwrap_or(false) + }) + .collect(); + + if available.len() == 1 { + Ok(Some(available[0])) + } else { + Ok(None) + } +} + #[cfg(unix)] fn configure_tool_aliases(tool_dir: &Path, aliases: &[ToolAlias]) -> Result<()> { use std::os::unix::fs as unix_fs; @@ -428,4 +548,45 @@ mod tests { .contains("Refusing to replace non-symlink") })); } + + #[cfg(unix)] + #[test] + fn sole_available_provider_selects_dash_when_it_is_the_only_shell() { + let tmp = tempfile::tempdir().unwrap(); + let tool_dir = make_tool_dir(tmp.path()); + fs::write(tool_dir.join("dash"), "").unwrap(); + let installed = HashSet::from(["dash".to_string()]); + + let provider = sole_available_provider(ToolRoleArg::Shell, &installed, &tool_dir) + .unwrap() + .unwrap(); + assert_eq!(provider.implementation, "dash"); + + configure_tool_aliases( + &tool_dir, + &aliases_for_selection(ToolRoleArg::Shell, provider.implementation).unwrap(), + ) + .unwrap(); + assert_eq!( + fs::read_link(tool_dir.join("sh")).unwrap(), + PathBuf::from("dash") + ); + } + + #[cfg(unix)] + #[test] + fn sole_available_provider_leaves_ambiguous_compilers_unselected() { + let tmp = tempfile::tempdir().unwrap(); + let tool_dir = make_tool_dir(tmp.path()); + for tool in ["clang", "clang++", "gcc", "g++"] { + fs::write(tool_dir.join(tool), "").unwrap(); + } + let installed = HashSet::from(["clang".to_string(), "gcc".to_string()]); + + assert!( + sole_available_provider(ToolRoleArg::Compiler, &installed, &tool_dir) + .unwrap() + .is_none() + ); + } } diff --git a/src/commands/tests.rs b/src/commands/tests.rs index 565ff6e..b18ff1d 100644 --- a/src/commands/tests.rs +++ b/src/commands/tests.rs @@ -6,7 +6,10 @@ use crate::cli::{ use crate::test_support::TestEnv; use git2::{Oid, Repository}; use std::path::Path; -use std::sync::{Mutex, MutexGuard}; +use std::sync::{ + Mutex, MutexGuard, + atomic::{AtomicUsize, Ordering as AtomicOrdering}, +}; static ASSUME_YES_TEST_LOCK: Mutex<()> = Mutex::new(()); @@ -89,6 +92,21 @@ fn build_env_rootfs_uses_selected_non_live_rootfs() { assert_eq!(build_cmd::build_env_rootfs(Path::new("/")), "/"); } +#[test] +fn parallel_verification_processes_every_item() -> Result<()> { + let items = vec![0_u8; 32]; + let completed = AtomicUsize::new(0); + let progress = ProgressBar::hidden(); + + run_parallel_verification(&items, &progress, |_| { + completed.fetch_add(1, AtomicOrdering::Relaxed); + Ok(()) + })?; + + assert_eq!(completed.load(AtomicOrdering::Relaxed), items.len()); + Ok(()) +} + #[test] fn install_post_extract_env_uses_selected_non_live_rootfs() -> Result<()> { let _guard = assume_yes_test_lock(); @@ -173,7 +191,6 @@ fn test_binary_repo_record(name: &str, filename: &str) -> db::repo::BinaryRepoPa completed_at: None, filename: filename.into(), size: 1, - sha256: "sha256".into(), sha512: "sha512".into(), description: None, homepage: None, @@ -499,7 +516,6 @@ fn binary_install_path_uses_repo_record_metadata_without_archive_metadata() -> R .unwrap_or_default() .to_string(), size: payload.len() as u64, - sha256: String::new(), sha512: String::new(), description: Some("test package".into()), homepage: Some("https://example.test".into()), @@ -887,7 +903,6 @@ fn binary_archive_install_preserves_setuid_permissions() -> Result<()> { .unwrap_or_default() .to_string(), size: payload.len() as u64, - sha256: String::new(), sha512: String::new(), description: Some("sudo".into()), homepage: Some("https://example.test".into()), @@ -1431,7 +1446,6 @@ fn update_candidate_prefers_binary_when_versions_match_and_config_does() { completed_at: None, filename: "pkg-1.1.0-1-x86_64.depot.pkg.tar.zst".into(), size: 1, - sha256: String::new(), sha512: String::new(), description: None, homepage: None, @@ -1762,6 +1776,34 @@ fn install_planned_packages_to_rootfs_runs_post_hooks_after_batch_install() -> R Ok(()) } +#[test] +fn install_planned_packages_sets_sole_tool_provider_before_post_hooks() -> Result<()> { + let rootfs = tempfile::tempdir().context("Failed to create temp rootfs")?; + let mut config = config::Config::for_rootfs(rootfs.path()); + config.db_dir = rootfs.path().join("var/lib/depot"); + config.build_dir = rootfs.path().join("var/cache/depot/build"); + + let mut dash_spec = test_package_spec(package::BuildType::Bin, None, &[]); + dash_spec.package.name = "dash".into(); + let dash_dest = rootfs.path().join("dash-dest"); + fs::create_dir_all(dash_dest.join("usr/bin"))?; + fs::create_dir_all(dash_dest.join("scripts"))?; + fs::write(dash_dest.join("usr/bin/dash"), "dash")?; + fs::write( + dash_dest.join("scripts/post_install"), + "[ -L \"$DEPOT_ROOTFS/usr/bin/sh\" ] && [ \"$(readlink \"$DEPOT_ROOTFS/usr/bin/sh\")\" = dash ]\n", + )?; + + let plans = plan_package_outputs_for_install(&dash_spec, &dash_dest, rootfs.path(), &config)?; + install_planned_packages_to_rootfs(&plans, rootfs.path(), &config)?; + + assert_eq!( + fs::read_link(rootfs.path().join("usr/bin/sh"))?, + PathBuf::from("dash") + ); + Ok(()) +} + #[test] fn collect_update_candidates_skips_source_when_prefer_binary_is_enabled() -> Result<()> { let temp = tempfile::tempdir().context("Failed to create temp dir")?; diff --git a/src/db/repo.rs b/src/db/repo.rs index 800d786..186e8db 100644 --- a/src/db/repo.rs +++ b/src/db/repo.rs @@ -146,7 +146,6 @@ pub struct BinaryRepoPackageRecord { pub completed_at: Option, pub filename: String, pub size: u64, - pub sha256: String, pub sha512: String, pub description: Option, pub homepage: Option, @@ -1803,7 +1802,6 @@ fn find_cached_binary_repo_packages( {completed_at_expr}, p.filename, p.size, - p.sha256, p.sha512, p.description, p.homepage, @@ -1851,11 +1849,10 @@ fn find_cached_binary_repo_packages( completed_at: row.get(7)?, filename: row.get(8)?, size: row.get::<_, i64>(9)? as u64, - sha256: row.get(10)?, - sha512: row.get(11)?, - description: row.get(12)?, - homepage: row.get(13)?, - license: row.get(14)?, + sha512: row.get(10)?, + description: row.get(11)?, + homepage: row.get(12)?, + license: row.get(13)?, provides: Vec::new(), conflicts: Vec::new(), replaces: Vec::new(), @@ -1935,7 +1932,6 @@ fn find_cached_binary_repo_packages_by_group( {completed_at_expr}, p.filename, p.size, - p.sha256, p.sha512, p.description, p.homepage, @@ -1965,11 +1961,10 @@ fn find_cached_binary_repo_packages_by_group( completed_at: row.get(7)?, filename: row.get(8)?, size: row.get::<_, i64>(9)? as u64, - sha256: row.get(10)?, - sha512: row.get(11)?, - description: row.get(12)?, - homepage: row.get(13)?, - license: row.get(14)?, + sha512: row.get(10)?, + description: row.get(11)?, + homepage: row.get(12)?, + license: row.get(13)?, provides: Vec::new(), conflicts: Vec::new(), replaces: Vec::new(), @@ -2034,7 +2029,6 @@ fn list_cached_binary_repo_packages( {completed_at_expr}, p.filename, p.size, - p.sha256, p.sha512, p.description, p.homepage, @@ -2059,11 +2053,10 @@ fn list_cached_binary_repo_packages( completed_at: row.get(7)?, filename: row.get(8)?, size: row.get::<_, i64>(9)? as u64, - sha256: row.get(10)?, - sha512: row.get(11)?, - description: row.get(12)?, - homepage: row.get(13)?, - license: row.get(14)?, + sha512: row.get(10)?, + description: row.get(11)?, + homepage: row.get(12)?, + license: row.get(13)?, provides: Vec::new(), conflicts: Vec::new(), replaces: Vec::new(), @@ -2202,59 +2195,34 @@ pub fn list_binary_repo_packages( list_cached_binary_repo_packages(repo_name, &db_path) } -fn verify_hex_digest(path: &Path, algorithm: &str, expected_hex: &str) -> Result { - let expected = expected_hex.trim().to_ascii_lowercase(); - if expected.is_empty() { - return Ok(false); - } - - let mut file = - fs::File::open(path).with_context(|| format!("Failed to open {}", path.display()))?; - let mut buf = [0u8; 64 * 1024]; - - let actual = match algorithm { - "sha256" => { - use sha2::{Digest, Sha256}; - let mut h = Sha256::new(); - loop { - let n = file.read(&mut buf)?; - if n == 0 { - break; - } - h.update(&buf[..n]); - } - crate::hex::encode_lower(h.finalize()) - } - "sha512" => { - use sha2::{Digest, Sha512}; - let mut h = Sha512::new(); - loop { - let n = file.read(&mut buf)?; - if n == 0 { - break; - } - h.update(&buf[..n]); - } - crate::hex::encode_lower(h.finalize()) - } - _ => anyhow::bail!("Unsupported checksum algorithm: {}", algorithm), - }; - - Ok(actual == expected) -} - fn verify_binary_package_record_checksums( path: &Path, rec: &BinaryRepoPackageRecord, ) -> Result<()> { - if !verify_hex_digest(path, "sha256", &rec.sha256)? { + use sha2::{Digest, Sha512}; + + let expected = rec.sha512.trim().to_ascii_lowercase(); + if expected.is_empty() { anyhow::bail!( - "SHA-256 mismatch for {} from repo '{}'", + "Missing SHA-512 checksum for {} from repo '{}'", path.display(), rec.repo_name ); } - if !verify_hex_digest(path, "sha512", &rec.sha512)? { + + let mut file = + fs::File::open(path).with_context(|| format!("Failed to open {}", path.display()))?; + let mut hasher = Sha512::new(); + let mut buf = [0u8; 64 * 1024]; + loop { + let n = file.read(&mut buf)?; + if n == 0 { + break; + } + hasher.update(&buf[..n]); + } + + if crate::hex::encode_lower(hasher.finalize()) != expected { anyhow::bail!( "SHA-512 mismatch for {} from repo '{}'", path.display(), @@ -3107,17 +3075,12 @@ revision = 1 #[test] fn test_verify_binary_package_record_checksums_accepts_valid_hashes() { - use sha2::{Digest, Sha256, Sha512}; + use sha2::{Digest, Sha512}; let tmp = tempfile::tempdir().unwrap(); let pkg = tmp.path().join("pkg.depot.pkg.tar.zst"); fs::write(&pkg, b"payload").unwrap(); - let sha256 = { - let mut h = Sha256::new(); - h.update(b"payload"); - crate::hex::encode_lower(h.finalize()) - }; let sha512 = { let mut h = Sha512::new(); h.update(b"payload"); @@ -3135,7 +3098,6 @@ revision = 1 completed_at: None, filename: "pkg.depot.pkg.tar.zst".into(), size: 7, - sha256, sha512, description: None, homepage: None, @@ -3151,14 +3113,27 @@ revision = 1 verify_binary_package_record_checksums(&pkg, &rec).unwrap(); } - fn test_record_for_payload(filename: &str, payload: &[u8]) -> BinaryRepoPackageRecord { - use sha2::{Digest, Sha256, Sha512}; + #[test] + fn test_verify_binary_package_record_checksums_requires_valid_sha512() { + use sha2::{Digest, Sha512}; + + let tmp = tempfile::tempdir().unwrap(); + let pkg = tmp.path().join("pkg.depot.pkg.tar.zst"); + fs::write(&pkg, b"payload").unwrap(); + + let mut rec = test_record_for_payload("pkg.depot.pkg.tar.zst", b"payload"); + verify_binary_package_record_checksums(&pkg, &rec).unwrap(); + + let mut wrong_sha512 = Sha512::new(); + wrong_sha512.update(b"different payload"); + rec.sha512 = crate::hex::encode_lower(wrong_sha512.finalize()); + let err = verify_binary_package_record_checksums(&pkg, &rec).unwrap_err(); + assert!(err.to_string().contains("SHA-512 mismatch")); + } + + fn test_record_for_payload(filename: &str, payload: &[u8]) -> BinaryRepoPackageRecord { + use sha2::{Digest, Sha512}; - let sha256 = { - let mut h = Sha256::new(); - h.update(payload); - crate::hex::encode_lower(h.finalize()) - }; let sha512 = { let mut h = Sha512::new(); h.update(payload); @@ -3176,7 +3151,6 @@ revision = 1 completed_at: None, filename: filename.to_string(), size: payload.len() as u64, - sha256, sha512, description: None, homepage: None, diff --git a/src/install/dkms.rs b/src/install/dkms.rs index 7869ff1..ccf61f2 100644 --- a/src/install/dkms.rs +++ b/src/install/dkms.rs @@ -210,6 +210,8 @@ fn build_for_kernel( cmd.arg("-C") .arg(&kernel.build_dir) .arg(format!("M={}", build_dir.display())) + .arg("-j") + .arg(num_cpus().to_string()) .arg("modules") .args(&manifest.make_args); crate::builder::prepare_tool_command(&mut cmd, &Vec::new()); @@ -231,6 +233,12 @@ fn build_for_kernel( Ok(()) } +fn num_cpus() -> usize { + std::thread::available_parallelism() + .map(|count| count.get()) + .unwrap_or(1) +} + fn run_pre_build_commands( rootfs: &Path, source_dir: &Path, @@ -544,4 +552,9 @@ mod tests { assert!(safe_rel_path("../x").is_err()); assert!(safe_rel_path("updates/depot").is_ok()); } + + #[test] + fn num_cpus_is_at_least_one() { + assert!(num_cpus() >= 1); + } } diff --git a/src/planner.rs b/src/planner.rs index 0359cba..e302d50 100644 --- a/src/planner.rs +++ b/src/planner.rs @@ -1032,7 +1032,6 @@ mod tests { completed_at: None, filename: format!("{name}-1.0.0-1-x86_64.depot.pkg.tar.zst"), size: 1024, - sha256: "sha256".to_string(), sha512: "sha512".to_string(), description: None, homepage: None,