diff --git a/Cargo.lock b/Cargo.lock index 5db1aafe6..34fdb7767 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -412,9 +412,9 @@ checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" [[package]] name = "compak" -version = "0.1.2" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b129e47c79a2208973c9ea9af66840f1ffe533fa0aa8ad5d478d58cb4d9a898d" +checksum = "c830a2769ae55bf48f5be48a24718c9533af40e185d2fe203ade59d6802f49fe" dependencies = [ "bzip2", "flate2", diff --git a/Cargo.toml b/Cargo.toml index 5f6310e8a..8ce41e30e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ blake3 = { version = "1.8.5", features = ["mmap"] } clap = { version = "4.6.1", features = ["cargo", "derive"] } clap_complete = "4.6.5" chrono = "0.4" -compak = "0.1.2" +compak = "0.2.0" diesel = { version = "2.3.10", features = [ "64-column-tables", "returning_clauses_for_sqlite_3_35", diff --git a/crates/soar-cli/src/apply.rs b/crates/soar-cli/src/apply.rs index 971c19bbd..2528a649d 100644 --- a/crates/soar-cli/src/apply.rs +++ b/crates/soar-cli/src/apply.rs @@ -87,11 +87,7 @@ fn display_diff(diff: &ApplyDiff, prune: bool) { let pkg = &target.package; builder.push_record([ format!("{}", Colored(Green, icon_or("+", "+"))), - format!( - "{}#{}", - Colored(Blue, &pkg.pkg_name), - Colored(Cyan, &pkg.pkg_id) - ), + format!("{}", Colored(Blue, &pkg.pkg_name),), format!("{}", Colored(Green, &pkg.version)), format!("{}", Colored(Magenta, &pkg.repo_name)), ]); @@ -105,11 +101,7 @@ fn display_diff(diff: &ApplyDiff, prune: bool) { .map_or("?".to_string(), |e| e.version.clone()); builder.push_record([ format!("{}", Colored(Yellow, icon_or("~", "~"))), - format!( - "{}#{}", - Colored(Blue, &pkg.pkg_name), - Colored(Cyan, &pkg.pkg_id) - ), + format!("{}", Colored(Blue, &pkg.pkg_name),), format!( "{} -> {}", Colored(Red, &old_version), @@ -123,11 +115,7 @@ fn display_diff(diff: &ApplyDiff, prune: bool) { for pkg in &diff.to_remove { builder.push_record([ format!("{}", Colored(Red, icon_or("-", "-"))), - format!( - "{}#{}", - Colored(Blue, &pkg.pkg_name), - Colored(Cyan, &pkg.pkg_id) - ), + format!("{}", Colored(Blue, &pkg.pkg_name),), format!("{}", Colored(Yellow, &pkg.version)), format!("{}", Colored(Magenta, &pkg.repo_name)), ]); diff --git a/crates/soar-cli/src/download.rs b/crates/soar-cli/src/download.rs index 57eb51921..870e2fe66 100644 --- a/crates/soar-cli/src/download.rs +++ b/crates/soar-cli/src/download.rs @@ -154,6 +154,7 @@ pub async fn handle_direct_downloads( conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -173,6 +174,7 @@ pub async fn handle_direct_downloads( conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -210,10 +212,7 @@ pub async fn handle_direct_downloads( let package = package.resolve(query.version.as_deref()); - info!( - "Downloading package: {}#{}", - package.pkg_name, package.pkg_id - ); + info!("Downloading package: {}", package.pkg_name); if let Some(ref url) = package.ghcr_blob { let mut dl = OciDownload::new(url.as_str()).overwrite(ctx.get_overwrite_mode()); diff --git a/crates/soar-cli/src/health.rs b/crates/soar-cli/src/health.rs index 68b8d2008..a74584f95 100644 --- a/crates/soar-cli/src/health.rs +++ b/crates/soar-cli/src/health.rs @@ -1,4 +1,4 @@ -use nu_ansi_term::Color::{Blue, Cyan, Green, Red, Yellow}; +use nu_ansi_term::Color::{Blue, Green, Red, Yellow}; use soar_core::SoarResult; use soar_operations::{health, SoarContext}; use tabled::{ @@ -25,6 +25,21 @@ pub async fn display_health(ctx: &SoarContext) -> SoarResult<()> { }; builder.push_record(["PATH".to_string(), path_status]); + // Only shown once something has installed a manual page, so a user with no + // such package is not told to configure something they do not need. + if let Some(man_dir) = &report.man_path { + let man_status = if report.man_path_configured { + format!("{} Configured", Colored(Green, icon_or(Icons::CHECK, "OK"))) + } else { + format!( + "{} {} not searched by man", + Colored(Yellow, icon_or(Icons::WARNING, "!")), + Colored(Blue, man_dir.display()) + ) + }; + builder.push_record(["MANPATH".to_string(), man_status]); + } + let pkg_status = if report.broken_packages.is_empty() { format!("{} None", Colored(Green, icon_or(Icons::CHECK, "OK"))) } else { @@ -61,10 +76,9 @@ pub async fn display_health(ctx: &SoarContext) -> SoarResult<()> { info!("\nBroken packages:"); for pkg in &report.broken_packages { info!( - " {} {}#{}: {}", + " {} {}: {}", Icons::ARROW, Colored(Blue, &pkg.pkg_name), - Colored(Cyan, &pkg.pkg_id), Colored(Yellow, &pkg.installed_path) ); } @@ -94,16 +108,11 @@ pub async fn remove_broken_packages(ctx: &SoarContext) -> SoarResult<()> { } for removed in &report.removed { - info!("Removed {}#{}", removed.pkg_name, removed.pkg_id); + info!("Removed {}", removed.pkg_name); } for failed in &report.failed { - tracing::error!( - "Failed to remove {}#{}: {}", - failed.pkg_name, - failed.pkg_id, - failed.error - ); + tracing::error!("Failed to remove {}: {}", failed.pkg_name, failed.error); } if !report.removed.is_empty() && report.failed.is_empty() { @@ -114,7 +123,7 @@ pub async fn remove_broken_packages(ctx: &SoarContext) -> SoarResult<()> { report .failed .iter() - .map(|f| format!("{}#{}", f.pkg_name, f.pkg_id)) + .map(|f| f.pkg_name.to_string()) .collect::>() .join(", ") ); diff --git a/crates/soar-cli/src/inspect.rs b/crates/soar-cli/src/inspect.rs index 8baefb386..d48d92c17 100644 --- a/crates/soar-cli/src/inspect.rs +++ b/crates/soar-cli/src/inspect.rs @@ -44,7 +44,8 @@ fn get_installed_path( conn, &package.repo_name, &package.pkg_name, - &package.pkg_id, + package.pkg_id.as_deref(), + package.pkg_family.as_deref(), &package.version, ) })?; @@ -71,6 +72,7 @@ pub async fn inspect_log(package: &str, inspect_type: InspectType) -> SoarResult conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, Some(SortDirection::Asc), @@ -90,6 +92,7 @@ pub async fn inspect_log(package: &str, inspect_type: InspectType) -> SoarResult conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, Some(SortDirection::Asc), diff --git a/crates/soar-cli/src/install.rs b/crates/soar-cli/src/install.rs index daa8e7674..88ca50d41 100644 --- a/crates/soar-cli/src/install.rs +++ b/crates/soar-cli/src/install.rs @@ -75,16 +75,10 @@ pub async fn install_packages( }; if let Some(pkg) = pkg { - // Re-resolve with the specific selected package - let specific_query = - format!("{}#{}:{}", pkg.pkg_name, pkg.pkg_id, pkg.repo_name); - let re_results = - install::resolve_packages(ctx, &[specific_query], &options).await?; - for r in re_results { - if let ResolveResult::Resolved(targets) = r { - install_targets.extend(targets); - } - } + // Install the package that was chosen. Re-resolving it by + // name would ask the same ambiguous question again and + // answer it with nothing. + install_targets.push(install::target_for(ctx, pkg, &options)?); } } ResolveResult::NotFound(name) => { @@ -97,13 +91,13 @@ pub async fn install_packages( } ResolveResult::AlreadyInstalled { pkg_name, - pkg_id, repo_name, version, + .. } => { warn!( - "{}#{}:{} ({}) is already installed - skipping", - pkg_name, pkg_id, repo_name, version, + "{}:{} ({}) is already installed - skipping", + pkg_name, repo_name, version, ); if !force { info!("Hint: Use --force to reinstall, or --show to see other variants"); @@ -161,13 +155,13 @@ async fn install_with_show( ResolveResult::Resolved(targets) => install_targets.extend(targets), ResolveResult::AlreadyInstalled { pkg_name, - pkg_id, repo_name, version, + .. } => { warn!( - "{}#{}:{} ({}) is already installed - skipping", - pkg_name, pkg_id, repo_name, version, + "{}:{} ({}) is already installed - skipping", + pkg_name, repo_name, version, ); if !force { info!("Hint: Use --force to reinstall"); @@ -200,15 +194,7 @@ async fn install_with_show( }; if let Some(pkg) = pkg { - let specific_query = - format!("{}#{}:{}", pkg.pkg_name, pkg.pkg_id, pkg.repo_name); - let re_results = - install::resolve_packages(ctx, &[specific_query], options).await?; - for r in re_results { - if let ResolveResult::Resolved(targets) = r { - install_targets.extend(targets); - } - } + install_targets.push(install::target_for(ctx, pkg, options)?); } } ResolveResult::NotFound(name) => { @@ -221,13 +207,13 @@ async fn install_with_show( } ResolveResult::AlreadyInstalled { pkg_name, - pkg_id, repo_name, version, + .. } => { warn!( - "{}#{}:{} ({}) is already installed - skipping", - pkg_name, pkg_id, repo_name, version, + "{}:{} ({}) is already installed - skipping", + pkg_name, repo_name, version, ); if !force { info!( @@ -247,6 +233,7 @@ async fn install_with_show( conn, query.name.as_deref(), None, + query.family.as_deref(), None, None, Some(SortDirection::Asc), @@ -266,6 +253,7 @@ async fn install_with_show( conn, query.name.as_deref(), None, + query.family.as_deref(), None, None, Some(SortDirection::Asc), @@ -302,7 +290,7 @@ async fn install_with_show( } // Get installed packages to show [installed] marker - let installed_packages: Vec<(String, String, String)> = diesel_db + let installed_packages: Vec<(String, Option, String)> = diesel_db .with_conn(|conn| { CoreRepository::list_filtered( conn, @@ -317,7 +305,7 @@ async fn install_with_show( ) })? .into_iter() - .map(|p| (p.pkg_id, p.repo_name, p.version)) + .map(|p| (p.pkg_name, p.pkg_family, p.repo_name)) .collect(); let pkg = select_package_interactively_with_installed( @@ -337,7 +325,7 @@ async fn install_with_show( conn, Some(&pkg.repo_name), Some(&pkg.pkg_name), - Some(&pkg.pkg_id), + pkg.pkg_id.as_deref(), None, None, None, @@ -346,15 +334,17 @@ async fn install_with_show( ) })? .into_iter() - .map(Into::into) - .next(); + // The query cannot narrow by family, and an uninstalled row of the + // same name would otherwise stand in for the installed one. + .filter(|ip| ip.pkg_family.as_deref() == pkg.pkg_family.as_deref()) + .find(|ip| ip.is_installed) + .map(Into::into); if let Some(ref existing) = existing_install { if existing.is_installed { warn!( - "{}#{}:{} ({}) is already installed - {}", + "{}:{} ({}) is already installed - {}", existing.pkg_name, - existing.pkg_id, existing.repo_name, existing.version, if force { "reinstalling" } else { "skipping" } @@ -402,14 +392,39 @@ fn display_install_report(report: &InstallReport, no_notes: bool) { for info in &report.installed { info!( - "\n{} {}#{}:{} [{}]", + "\n{} {}:{} [{}]", icon_or(Icons::CHECK, "*"), Colored(Blue, &info.pkg_name), - Colored(Cyan, &info.pkg_id), Colored(Green, &info.repo_name), Colored(Magenta, info.install_dir.display()) ); + if !info.shared.is_empty() { + // Listing these would bury the binaries: gh alone ships over a + // hundred manual pages. + let mut man = 0; + let mut completions = 0; + for (_, link) in &info.shared { + let path = link.to_string_lossy(); + if path.contains("/man/") { + man += 1; + } else { + completions += 1; + } + } + let mut parts = Vec::new(); + if man > 0 { + parts.push(format!("{man} man page{}", if man == 1 { "" } else { "s" })); + } + if completions > 0 { + parts.push(format!( + "{completions} completion{}", + if completions == 1 { "" } else { "s" } + )); + } + info!(" {} Linked {}", icon_or("📖", "-"), parts.join(", ")); + } + if !info.symlinks.is_empty() { info!(" {} Binaries:", icon_or("📂", "-")); for (target, link) in &info.symlinks { @@ -424,7 +439,9 @@ fn display_install_report(report: &InstallReport, no_notes: bool) { } if !no_notes { - if let Some(ref notes) = info.notes { + // Most packages have nothing to say, and an empty list would + // otherwise print a heading with no content under it. + if let Some(notes) = info.notes.as_ref().filter(|n| !n.is_empty()) { info!( " {} Notes:\n {}", icon_or("📝", "-"), @@ -436,8 +453,8 @@ fn display_install_report(report: &InstallReport, no_notes: bool) { for err_info in &report.failed { error!( - "Failed to install {}#{}: {}", - err_info.pkg_name, err_info.pkg_id, err_info.error + "Failed to install {}: {}", + err_info.pkg_name, err_info.error ); } diff --git a/crates/soar-cli/src/json2db.rs b/crates/soar-cli/src/json2db.rs index f94460ab7..ec2d6fecf 100644 --- a/crates/soar-cli/src/json2db.rs +++ b/crates/soar-cli/src/json2db.rs @@ -8,7 +8,7 @@ use soar_db::{ connection::DbConnection, migration::DbType, repository::metadata::MetadataRepository, }; use soar_registry::RemotePackage; -use tracing::info; +use tracing::{info, warn}; /// Converts JSON metadata file to SQLite database. pub fn json_to_db(input_path: &str, output_path: &str, repo_name: Option<&str>) -> SoarResult<()> { @@ -23,7 +23,7 @@ pub fn json_to_db(input_path: &str, output_path: &str, repo_name: Option<&str>) let json_content = fs::read_to_string(input_path) .with_context(|| format!("reading JSON metadata from {}", input_path))?; - let packages: Vec = serde_json::from_str(&json_content) + let packages: Vec = soar_registry::parse_index(json_content.as_bytes()) .map_err(|e| SoarError::Custom(format!("parsing JSON from {}: {}", input_path, e)))?; info!(count = packages.len(), "Parsed JSON metadata"); @@ -41,19 +41,63 @@ pub fn json_to_db(input_path: &str, output_path: &str, repo_name: Option<&str>) } } - if output_path.exists() { - fs::remove_file(output_path) - .with_context(|| format!("removing existing database {}", output_path.display()))?; + // Built beside the target and swapped in only once it holds something. + // Writing in place would destroy a working database whenever an import + // turned out to be entirely rejected. + // Named for this process, so a second import running at the same time + // cannot delete the database this one is still building. + let mut tmp_name = output_path.file_name().unwrap_or_default().to_os_string(); + tmp_name.push(format!(".{}.tmp", std::process::id())); + let tmp_path = output_path.with_file_name(tmp_name); + for stale in [ + &tmp_path, + &tmp_path.with_extension("tmp-wal"), + &tmp_path.with_extension("tmp-shm"), + ] { + fs::remove_file(stale).ok(); } - let mut conn = DbConnection::open(output_path, DbType::Metadata) + let mut conn = DbConnection::open(&tmp_path, DbType::Metadata) .map_err(|e| SoarError::Custom(format!("opening database: {}", e)))?; - MetadataRepository::import_packages(conn.conn(), &packages, repo_name) - .map_err(|e| SoarError::Custom(format!("importing packages: {}", e)))?; + // Packages with an unsafe pkg_name/pkg_id are skipped during import. + // Reporting success while writing nothing hides that entirely, which is + // how an empty pkg_id silently produced an empty database. + let result = MetadataRepository::import_packages(conn.conn(), &packages, repo_name) + .map_err(|e| SoarError::Custom(format!("importing packages: {}", e))); + let imported = match result { + Ok(n) if n > 0 => n, + other => { + drop(conn); + fs::remove_file(&tmp_path).ok(); + return match other { + Err(e) => Err(e), + Ok(_) => { + Err(SoarError::Custom(format!( + "imported 0 of {} packages; every entry was rejected, most likely \ + an unsafe or empty pkg_name/pkg_id", + packages.len() + ))) + } + }; + } + }; + + let skipped = packages.len() - imported; + if skipped > 0 { + warn!(skipped, "some packages were rejected during import"); + } + + drop(conn); + fs::rename(&tmp_path, output_path).with_context(|| { + format!( + "replacing {} with the imported database", + output_path.display() + ) + })?; info!( - count = packages.len(), + count = imported, output = %output_path.display(), "Successfully converted JSON to SQLite database" ); diff --git a/crates/soar-cli/src/list.rs b/crates/soar-cli/src/list.rs index ed8181a71..e6c3aef74 100644 --- a/crates/soar-cli/src/list.rs +++ b/crates/soar-cli/src/list.rs @@ -53,12 +53,24 @@ pub async fn search_packages( version = package.version, description = package.description, size = package.ghcr_size.or(package.size), - "[{}] {}#{}:{} | {} | {} - {} ({})", + "[{}] {}{}:{} | {}{} | {} - {} ({})", state_icon, + package + .pkg_family + .as_ref() + .map(|f| format!("{}/", Colored(Green, f))) + .unwrap_or_default(), Colored(Blue, &package.pkg_name), - Colored(Cyan, &package.pkg_id), Colored(Green, &package.repo_name), Colored(LightRed, &package.version), + if entry.other_versions.is_empty() { + String::new() + } else { + format!( + " {}", + Colored(Yellow, format!("({})", entry.other_versions.join(", "))) + ) + }, package .pkg_type .as_ref() @@ -124,9 +136,8 @@ pub async fn query_package(ctx: &SoarContext, query_str: String) -> SoarResult<( builder.push_record([ format!("{} Name", Icons::PACKAGE), format!( - "{}#{}:{}", + "{}:{}", Colored(Blue, &package.pkg_name), - Colored(Cyan, &package.pkg_id), Colored(Green, &package.repo_name) ), ]); @@ -237,13 +248,6 @@ pub async fn query_package(ctx: &SoarContext, query_str: String) -> SoarResult<( ]); } - if let Some(ref webindex) = package.pkg_webpage { - builder.push_record([ - format!("{} Index", Icons::LINK), - Colored(Blue, webindex).to_string(), - ]); - } - let table = builder .build() .with(Style::rounded()) @@ -273,7 +277,6 @@ pub async fn query_package(ctx: &SoarContext, query_str: String) -> SoarResult<( build_script = package.build_script, ghcr_blob = package.ghcr_blob, ghcr_pkg = package.ghcr_pkg, - pkg_webpage = package.pkg_webpage, "\n{table}" ); } @@ -306,12 +309,27 @@ pub async fn list_packages(ctx: &SoarContext, repo_name: Option) -> Soar repo_name = package.repo_name, pkg_type = package.pkg_type, version = package.version, - "[{}] {}#{}:{} | {} | {}", + "[{}] {}{}:{} | {}{} | {}", state_icon, + // shown the way it is typed, so two packages sharing a name are + // told apart and can be asked for + package + .pkg_family + .as_ref() + .map(|f| format!("{}/", Colored(Cyan, f))) + .unwrap_or_default(), Colored(Blue, &package.pkg_name), - Colored(Cyan, &package.pkg_id), Colored(Cyan, &package.repo_name), Colored(LightRed, &package.version), + // only the newest is listed; name the others rather than counting them + if entry.other_versions.is_empty() { + String::new() + } else { + format!( + " {}", + Colored(Yellow, format!("({})", entry.other_versions.join(", "))) + ) + }, package .pkg_type .as_ref() @@ -408,9 +426,13 @@ pub async fn list_installed_packages( ); if entry.is_healthy { - let unique_count = unique_pkgs - .insert(format!("{}-{}", package.pkg_id, package.pkg_name)) - as u32 + let unique_count = unique_pkgs.insert(format!( + "{}-{}-{}-{}", + package.repo_name, + package.pkg_id.as_deref().unwrap_or_default(), + package.pkg_family.as_deref().unwrap_or_default(), + package.pkg_name + )) as u32 + unique_count; ( installed_count + 1, diff --git a/crates/soar-cli/src/progress.rs b/crates/soar-cli/src/progress.rs index 38c3afe60..e9c7a3eb8 100644 --- a/crates/soar-cli/src/progress.rs +++ b/crates/soar-cli/src/progress.rs @@ -5,11 +5,11 @@ use std::{ }; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; -use nu_ansi_term::Color::{Cyan, Green, Red}; +use nu_ansi_term::Color::{Cyan, Green, Red, Yellow}; use soar_dl::types::Progress; use soar_events::{ - BuildStage, InstallStage, OperationId, RemoveStage, SoarEvent, SyncStage, UpdateCleanupStage, - VerifyStage, + BuildStage, InstallStage, LogLevel, OperationId, RemoveStage, SoarEvent, SyncStage, + UpdateCleanupStage, VerifyStage, }; use crate::utils::{display_settings, progress_enabled}; @@ -56,14 +56,8 @@ fn download_style() -> ProgressStyle { } /// Format a colored prefix: pkg_name in cyan, #pkg_id in dim. -fn colored_prefix(pkg_name: &str, pkg_id: &str) -> String { - format!( - "{}{}", - Cyan.paint(pkg_name), - nu_ansi_term::Style::new() - .dimmed() - .paint(format!("#{pkg_id}")) - ) +fn colored_prefix(pkg_name: &str) -> String { + Cyan.paint(pkg_name).to_string() } fn spinner_style() -> ProgressStyle { @@ -174,12 +168,12 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::DownloadStarting { op_id, pkg_name, - pkg_id, total, + .. } => { let pb = MULTI.add(ProgressBar::new(total)); pb.set_style(download_style()); - pb.set_prefix(colored_prefix(&pkg_name, &pkg_id)); + pb.set_prefix(colored_prefix(&pkg_name)); pb.enable_steady_tick(Duration::from_millis(100)); jobs.insert(op_id, pb); reposition_batch!(batch_job, batch_msg); @@ -187,15 +181,15 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::DownloadResuming { op_id, pkg_name, - pkg_id, current, total, + .. } => { let is_new = !jobs.contains_key(&op_id); let pb = jobs.entry(op_id).or_insert_with(|| { let pb = MULTI.add(ProgressBar::new(0)); pb.set_style(download_style()); - pb.set_prefix(colored_prefix(&pkg_name, &pkg_id)); + pb.set_prefix(colored_prefix(&pkg_name)); pb.enable_steady_tick(Duration::from_millis(100)); pb }); @@ -217,12 +211,11 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::DownloadComplete { op_id, pkg_name, - pkg_id, .. } => { if let Some(pb) = jobs.get(&op_id) { pb.set_style(spinner_style()); - pb.set_message(format!("{pkg_name}#{pkg_id}: downloaded")); + pb.set_message(format!("{pkg_name}: downloaded")); } } SoarEvent::DownloadRetry { @@ -242,13 +235,13 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::DownloadRecovered { op_id, pkg_name, - pkg_id, + .. } => { let is_new = !jobs.contains_key(&op_id); jobs.entry(op_id).or_insert_with(|| { let pb = MULTI.add(ProgressBar::new(0)); pb.set_style(download_style()); - pb.set_prefix(colored_prefix(&pkg_name, &pkg_id)); + pb.set_prefix(colored_prefix(&pkg_name)); pb.enable_steady_tick(Duration::from_millis(100)); pb }); @@ -261,17 +254,17 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::Verifying { op_id, pkg_name, - pkg_id, stage, + .. } => { match stage { VerifyStage::Checksum | VerifyStage::Signature => { let msg = match stage { VerifyStage::Checksum => { - format!("{pkg_name}#{pkg_id}: verifying checksum") + format!("{pkg_name}: verifying checksum") } VerifyStage::Signature => { - format!("{pkg_name}#{pkg_id}: verifying signature") + format!("{pkg_name}: verifying signature") } _ => unreachable!(), }; @@ -292,30 +285,30 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::Installing { op_id, pkg_name, - pkg_id, stage, + .. } if stage != InstallStage::Complete => { let msg = match &stage { InstallStage::Extracting => { - format!("{pkg_name}#{pkg_id}: extracting") + format!("{pkg_name}: extracting") } InstallStage::ExtractingNested => { - format!("{pkg_name}#{pkg_id}: extracting nested") + format!("{pkg_name}: extracting nested") } InstallStage::LinkingBinaries => { - format!("{pkg_name}#{pkg_id}: linking binaries") + format!("{pkg_name}: linking binaries") } InstallStage::DesktopIntegration => { - format!("{pkg_name}#{pkg_id}: desktop integration") + format!("{pkg_name}: desktop integration") } InstallStage::SetupPortable => { - format!("{pkg_name}#{pkg_id}: setting up portable") + format!("{pkg_name}: setting up portable") } InstallStage::RecordingDatabase => { - format!("{pkg_name}#{pkg_id}: recording to db") + format!("{pkg_name}: recording to db") } InstallStage::RunningHook(hook) => { - format!("{pkg_name}#{pkg_id}: running {hook}") + format!("{pkg_name}: running {hook}") } InstallStage::Complete => unreachable!(), }; @@ -332,8 +325,8 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::Building { op_id, pkg_name, - pkg_id, stage, + .. } => { match stage { BuildStage::Sandboxing => { @@ -350,10 +343,9 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { } MULTI.suspend(|| { eprintln!( - " {} {}#{}: {}", + " {} {}: {}", Cyan.paint("⚙"), Cyan.paint(&pkg_name), - Cyan.paint(&pkg_id), nu_ansi_term::Style::new().dimmed().paint(format!( "build ({}/{})", command_index + 1, @@ -372,29 +364,29 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::Removing { op_id, pkg_name, - pkg_id, stage, + .. } => { remove_ops.insert(op_id); if !matches!(stage, RemoveStage::Complete { .. }) { let msg = match &stage { RemoveStage::RunningHook(hook) => { - format!("{pkg_name}#{pkg_id}: running {hook}") + format!("{pkg_name}: running {hook}") } RemoveStage::UnlinkingBinaries => { - format!("{pkg_name}#{pkg_id}: unlinking binaries") + format!("{pkg_name}: unlinking binaries") } RemoveStage::UnlinkingDesktop => { - format!("{pkg_name}#{pkg_id}: unlinking desktop") + format!("{pkg_name}: unlinking desktop") } RemoveStage::UnlinkingIcons => { - format!("{pkg_name}#{pkg_id}: unlinking icons") + format!("{pkg_name}: unlinking icons") } RemoveStage::RemovingDirectory => { - format!("{pkg_name}#{pkg_id}: removing files") + format!("{pkg_name}: removing files") } RemoveStage::CleaningDatabase => { - format!("{pkg_name}#{pkg_id}: cleaning db") + format!("{pkg_name}: cleaning db") } RemoveStage::Complete { .. @@ -409,7 +401,6 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::UpdateCleanup { op_id, pkg_name, - pkg_id, stage, .. } => { @@ -421,7 +412,7 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { pb.finish_and_clear(); } } else { - let msg = format!("{pkg_name}#{pkg_id}: cleaning old version"); + let msg = format!("{pkg_name}: cleaning old version"); let pb = jobs.entry(op_id).or_insert_with(|| create_op_spinner(&msg)); pb.set_message(msg); } @@ -496,15 +487,14 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::OperationComplete { op_id, pkg_name, - pkg_id, + .. } => { if !remove_ops.remove(&op_id) { MULTI.suspend(|| { eprintln!( - " {} {}#{}: {}", + " {} {}: {}", Green.paint("✓"), Cyan.paint(&pkg_name), - Cyan.paint(&pkg_id), Green.paint("installed") ); }); @@ -516,16 +506,15 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { SoarEvent::OperationFailed { op_id, pkg_name, - pkg_id, error, + .. } => { remove_ops.remove(&op_id); MULTI.suspend(|| { eprintln!( - " {} {}#{}: {}", + " {} {}: {}", Red.paint("✗"), Cyan.paint(&pkg_name), - Cyan.paint(&pkg_id), Red.paint(&error) ); }); @@ -534,6 +523,29 @@ pub fn spawn_event_handler(receiver: Receiver) -> ProgressGuard { } } + // Emitted for things that fail without aborting the run, a + // repository that could not be synced most of all. Without a + // handler these were dropped and the failure looked like + // nothing happening. + SoarEvent::Log { + level, + message, + } => { + // Printed directly rather than through tracing: the + // subscriber writes via this same progress handle, so + // logging from inside suspend() deadlocks. + MULTI.suspend(|| { + match level { + LogLevel::Error => { + eprintln!(" {} {}", Red.paint("✗"), Red.paint(&message)) + } + LogLevel::Warning => eprintln!(" {} {}", Yellow.paint("!"), message), + LogLevel::Info => eprintln!(" {message}"), + LogLevel::Debug => {} + } + }); + } + _ => {} } } diff --git a/crates/soar-cli/src/remove.rs b/crates/soar-cli/src/remove.rs index 39d06f23e..d108191b9 100644 --- a/crates/soar-cli/src/remove.rs +++ b/crates/soar-cli/src/remove.rs @@ -30,9 +30,8 @@ pub async fn remove_packages( ); for pkg in &pkgs { info!( - " - {}#{}:{} ({})", + " - {}:{} ({})", Colored(Blue, &pkg.pkg_name), - Colored(Cyan, &pkg.pkg_id), Colored(Green, &pkg.repo_name), Colored(LightRed, &pkg.version) ); @@ -73,16 +72,13 @@ pub async fn remove_packages( for removed in &report.removed { info!( - "Removed {}#{}:{} ({})", - removed.pkg_name, removed.pkg_id, removed.repo_name, removed.version + "Removed {}:{} ({})", + removed.pkg_name, removed.repo_name, removed.version ); } for failed in &report.failed { - error!( - "Failed to remove {}#{}: {}", - failed.pkg_name, failed.pkg_id, failed.error - ); + error!("Failed to remove {}: {}", failed.pkg_name, failed.error); } debug!("package removal completed"); diff --git a/crates/soar-cli/src/run.rs b/crates/soar-cli/src/run.rs index 93a402e68..16247deab 100644 --- a/crates/soar-cli/src/run.rs +++ b/crates/soar-cli/src/run.rs @@ -20,8 +20,15 @@ pub async fn run_package( let result = run::prepare_run(ctx, package_name, repo_name, pkg_id, no_verify).await?; + let downloaded; let output_path = match result { - PrepareRunResult::Ready(path) => path, + PrepareRunResult::Ready { + path, + downloaded: d, + } => { + downloaded = d; + path + } PrepareRunResult::Ambiguous(amb) => { let pkg = if yes { amb.candidates.into_iter().next() @@ -33,23 +40,45 @@ pub async fn run_package( return Ok(0); }; - // Re-run with selected package + // Run what was chosen. Resolving it by name again would pose + // the same ambiguous question the choice just answered. + let query = match pkg.pkg_family { + Some(ref family) => { + format!( + "{}/{}@{}:{}", + family, pkg.pkg_name, pkg.version, pkg.repo_name + ) + } + None => format!("{}@{}:{}", pkg.pkg_name, pkg.version, pkg.repo_name), + }; let result = run::prepare_run( ctx, - package_name, + &query, Some(&pkg.repo_name), - Some(&pkg.pkg_id), + pkg.pkg_id.as_deref(), no_verify, ) .await?; match result { - PrepareRunResult::Ready(path) => path, + PrepareRunResult::Ready { + path, + downloaded: d, + } => { + downloaded = d; + path + } _ => return Ok(0), } } }; + // The progress bar leaves the cursor mid-line, so a program that writes + // straight to stdout would start where the bar stopped. + if downloaded { + eprintln!(); + } + let run_result = run::execute_binary(&output_path, args)?; Ok(run_result.exit_code) diff --git a/crates/soar-cli/src/update.rs b/crates/soar-cli/src/update.rs index 9a81e18a2..6384e681a 100644 --- a/crates/soar-cli/src/update.rs +++ b/crates/soar-cli/src/update.rs @@ -26,9 +26,8 @@ pub async fn update_packages( // Display update info for update_info in &updates { info!( - "{}#{}: {} -> {}", + "{}: {} -> {}", Colored(Blue, &update_info.pkg_name), - Colored(Cyan, &update_info.pkg_id), Colored(Red, &update_info.current_version), Colored(Green, &update_info.new_version), ); @@ -50,10 +49,7 @@ fn display_update_report(report: &UpdateReport) { let use_icons = settings.icons(); for err_info in &report.failed { - error!( - "Failed to update {}#{}: {}", - err_info.pkg_name, err_info.pkg_id, err_info.error - ); + error!("Failed to update {}: {}", err_info.pkg_name, err_info.error); } let updated_count = report.updated.len(); diff --git a/crates/soar-cli/src/use.rs b/crates/soar-cli/src/use.rs index ace41d6b3..8013f18f7 100644 --- a/crates/soar-cli/src/use.rs +++ b/crates/soar-cli/src/use.rs @@ -19,20 +19,26 @@ pub async fn use_alternate_package(ctx: &SoarContext, name: &str) -> SoarResult< info!( active = variant.is_active, pkg_name = package.pkg_name, - pkg_id = package.pkg_id, + pkg_family = package.pkg_family, repo_name = package.repo_name, pkg_type = package.pkg_type, version = package.version, size = package.size, - "[{}] {}#{}:{} ({}-{}) ({}){}", + "[{}] {}{}:{} ({}-{}) ({}){}", idx + 1, + // Two projects can publish the same name, so the family is what + // tells their variants apart when there is one. + package + .pkg_family + .as_ref() + .map(|f| format!("{}/", Colored(Magenta, f))) + .unwrap_or_default(), Colored(Blue, &package.pkg_name), - Colored(Cyan, &package.pkg_id), Colored(Cyan, &package.repo_name), package .pkg_type .as_ref() - .map(|pkg_type| format!(":{}", Colored(Magenta, &pkg_type))) + .map(|pkg_type| format!("{}", Colored(Magenta, &pkg_type))) .unwrap_or_default(), Colored(Magenta, &package.version), Colored(Magenta, format_bytes(package.size, 2)), @@ -51,10 +57,7 @@ pub async fn use_alternate_package(ctx: &SoarContext, name: &str) -> SoarResult< let selection = get_valid_selection(variants.len())?; switch::switch_variant(ctx, name, selection).await?; - info!( - "Switched to {}#{}", - variants[selection].package.pkg_name, variants[selection].package.pkg_id - ); + info!("Switched to {}", variants[selection].package.pkg_name); Ok(()) } diff --git a/crates/soar-cli/src/utils.rs b/crates/soar-cli/src/utils.rs index 1e6ce625a..f50ea67b2 100644 --- a/crates/soar-cli/src/utils.rs +++ b/crates/soar-cli/src/utils.rs @@ -4,7 +4,7 @@ use std::{ sync::{LazyLock, RwLock}, }; -use nu_ansi_term::Color::{self, Blue, Cyan, Green, LightRed, Magenta, Red}; +use nu_ansi_term::Color::{self, Blue, Green, LightRed, Magenta, Red}; use serde::Serialize; use soar_config::{ config::get_config, display::DisplaySettings, repository::get_platform_repositories, @@ -129,23 +129,34 @@ pub fn select_package_interactively( pub fn select_package_interactively_with_installed( pkgs: Vec, package_name: &str, - installed: &[(String, String, String)], // (pkg_id, repo_name, version) + installed: &[(String, Option, String)], // (pkg_name, pkg_family, repo_name) ) -> SoarResult> { info!("Showing available packages for {package_name}"); for (idx, pkg) in pkgs.iter().enumerate() { - let is_installed = installed.iter().any(|(pkg_id, repo_name, _version)| { - pkg.pkg_id() == pkg_id && pkg.repo_name() == repo_name + // Matching on the id would treat every id-less package as identical, + // since they all read as the same empty identity. + let is_installed = installed.iter().any(|(pkg_name, pkg_family, repo_name)| { + pkg.pkg_name() == pkg_name + && pkg.pkg_family() == pkg_family.as_deref() + && pkg.repo_name() == repo_name }); let installed_marker = if is_installed { format!(" {}", Colored(Color::Yellow, "[installed]")) } else { String::new() }; + // The family is what tells two candidates of the same name apart, so + // this is the one listing that cannot leave it out. + let name = match pkg.pkg_family() { + Some(family) if family != pkg.pkg_name() => { + format!("{}/{}", family, pkg.pkg_name()) + } + _ => pkg.pkg_name().to_string(), + }; info!( - "[{}] {}#{}:{} | {}{}", + "[{}] {}:{} | {}{}", idx + 1, - Colored(Blue, &pkg.pkg_name()), - Colored(Cyan, &pkg.pkg_id()), + Colored(Blue, &name), Colored(Green, pkg.repo_name()), Colored(LightRed, pkg.version()), installed_marker @@ -180,9 +191,8 @@ pub fn ask_target_action(targets: &[InstallTarget], action: &str) -> SoarResult< ); for target in targets { info!( - "{}#{}:{} ({})", + "{}:{} ({})", Colored(Blue, &target.package.pkg_name), - Colored(Cyan, &target.package.pkg_id), Colored(Green, &target.package.repo_name), Colored(LightRed, &target.package.version) ) diff --git a/crates/soar-config/src/config.rs b/crates/soar-config/src/config.rs index 3fe6decac..97ca20391 100644 --- a/crates/soar-config/src/config.rs +++ b/crates/soar-config/src/config.rs @@ -79,8 +79,18 @@ pub struct Config { /// NOTE: This is not yet implemented pub cross_repo_updates: Option, - /// Glob patterns for package files that should be included during install. + /// Shells to link package completions for. Defaults to those whose + /// completion directory already exists, so soar does not create one for a + /// shell nobody uses. + pub completions: Option>, + + /// Glob patterns filtering which files an install keeps. + /// /// Default: ["!*.log", "!SBUILD", "!*.json", "!*.version"] + #[deprecated( + since = "0.13.0", + note = "only the OCI download path applies these; the declarative format does not use it" + )] pub install_patterns: Option>, /// Global override for signature verification @@ -189,6 +199,8 @@ impl Config { soar_utils::path::icons_dir(self.system_mode) } + // Still populated while the OCI path exists; see the field's deprecation. + #[allow(deprecated)] pub fn default_config>(selected_repos: &[T]) -> Self { trace!("creating default configuration"); let soar_root = if is_system_mode() { @@ -270,6 +282,7 @@ impl Config { ghcr_concurrency: Some(8), cross_repo_updates: Some(false), install_patterns: Some(default_install_patterns()), + completions: None, signature_verification: None, desktop_integration: None, @@ -280,6 +293,8 @@ impl Config { } /// Creates a default configuration for the given system mode. + // Still populated while the OCI path exists; see the field's deprecation. + #[allow(deprecated)] pub fn default_config_for_mode>(selected_repos: &[T], system_mode: bool) -> Self { trace!( "creating default configuration for system_mode={}", @@ -361,6 +376,7 @@ impl Config { ghcr_concurrency: Some(8), cross_repo_updates: Some(false), install_patterns: Some(default_install_patterns()), + completions: None, signature_verification: None, desktop_integration: None, @@ -405,6 +421,8 @@ impl Config { Ok(config) } + // Still populated while the OCI path exists; see the field's deprecation. + #[allow(deprecated)] pub fn resolve(&mut self) -> Result<()> { trace!("resolving configuration"); if !self.profile.contains_key(&self.default_profile) { @@ -478,6 +496,24 @@ impl Config { self.default_profile()?.get_bin_path() } + /// Shells whose completions should be linked. + pub fn completion_shells(&self) -> Vec { + if let Some(shells) = &self.completions { + return shells.clone(); + } + let data = soar_utils::path::xdg_data_home(); + let config = soar_utils::path::xdg_config_home(); + [ + ("bash", data.join("bash-completion/completions")), + ("zsh", data.join("zsh/site-functions")), + ("fish", config.join("fish/completions")), + ] + .into_iter() + .filter(|(_, dir)| dir.is_dir()) + .map(|(name, _)| name.to_string()) + .collect() + } + pub fn get_desktop_path(&self) -> Result { if let Ok(env_path) = std::env::var("SOAR_DESKTOP") { return Ok(resolve_path(&env_path)?); @@ -777,6 +813,8 @@ mod tests { } #[test] + // Still populated while the OCI path exists; see the field's deprecation. + #[allow(deprecated)] fn test_config_resolve_sets_defaults() { let mut config = Config::default_config::<&str>(&[]); config.ghcr_concurrency = None; diff --git a/crates/soar-config/src/packages.rs b/crates/soar-config/src/packages.rs index de0b30a1c..ca4dc0cd9 100644 --- a/crates/soar-config/src/packages.rs +++ b/crates/soar-config/src/packages.rs @@ -46,7 +46,11 @@ pub struct PackageDefaults { /// Whether to install binary only (exclude logs, desktop files, etc). pub binary_only: Option, - /// Default install patterns. + /// Glob patterns filtering which files an install keeps. + #[deprecated( + since = "0.13.0", + note = "only the OCI download path applies these; the declarative format does not use it" + )] pub install_patterns: Option>, /// Default sandbox configuration applied to all packages. @@ -187,7 +191,17 @@ impl SandboxConfig { /// Full package options for detailed specification. #[derive(Clone, Debug, Default, Deserialize, Serialize, Documented, DocumentedFields)] pub struct PackageOptions { + /// Package family, which tells apart projects sharing a name. + /// + /// The declarative format publishes this rather than a package id, so it + /// is what disambiguates `bat` the pager from `bat` the batch runner. + pub family: Option, + /// Specific package ID (for disambiguation when multiple packages share the same name). + #[deprecated( + since = "0.13.0", + note = "repositories publishing the declarative format have no package id; use `family`" + )] pub pkg_id: Option, /// Specific version to install. @@ -269,7 +283,11 @@ pub struct PackageOptions { /// Portable directory configuration. pub portable: Option, - /// Custom install patterns (overrides default). + /// Glob patterns filtering which files an install keeps, overriding the default. + #[deprecated( + since = "0.13.0", + note = "only the OCI download path applies these; the declarative format does not use it" + )] pub install_patterns: Option>, /// Whether to install binary only. @@ -304,6 +322,11 @@ pub struct PortableConfig { #[derive(Clone, Debug, Default)] pub struct ResolvedPackage { pub name: String, + pub family: Option, + #[deprecated( + since = "0.13.0", + note = "repositories publishing the declarative format have no package id; use `family`" + )] pub pkg_id: Option, pub version: Option, pub repo: Option, @@ -326,6 +349,10 @@ pub struct ResolvedPackage { pub pinned: bool, pub profile: Option, pub portable: Option, + #[deprecated( + since = "0.13.0", + note = "only the OCI download path applies these; the declarative format does not use it" + )] pub install_patterns: Option>, pub binary_only: bool, pub arch_map: Option>, @@ -333,6 +360,8 @@ pub struct ResolvedPackage { impl PackageSpec { /// Resolve the package specification with defaults applied. + // Still populated while the OCI path exists; see the field's deprecation. + #[allow(deprecated)] pub fn resolve(&self, name: &str, defaults: Option<&PackageDefaults>) -> ResolvedPackage { match self { PackageSpec::Simple(version_str) => { @@ -344,6 +373,7 @@ impl PackageSpec { let pinned = version.is_some(); ResolvedPackage { name: name.to_string(), + family: None, pkg_id: None, version, repo: None, @@ -381,6 +411,7 @@ impl PackageSpec { let pinned = opts.pinned || (version.is_some() && !is_remote); ResolvedPackage { name: name.to_string(), + family: opts.family.clone(), pkg_id: opts.pkg_id.clone(), version, repo: opts.repo.clone(), @@ -452,6 +483,8 @@ impl PackagesConfig { } /// Create a default configuration. + // Still populated while the OCI path exists; see the field's deprecation. + #[allow(deprecated)] pub fn default_config() -> Self { Self { defaults: Some(PackageDefaults { @@ -479,7 +512,7 @@ impl PackagesConfig { # package_name = "*" # Latest version # package_name = "1.2.3" # Specific version (pinned) # package_name = { version = "1.2" } # Same as above -# package_name = { pkg_id = "pkg-bin", repo = "bincache" } +# package_name = { family = "pkg", repo = "bincache" } # package_name = { pinned = true, portable = { home = "~/.pkg" } } "#; @@ -630,17 +663,32 @@ jq = "1.8.1" fn test_detailed_package_spec() { let toml_str = r#" [packages] -neovim = { pkg_id = "neovim-appimage", repo = "soarpkgs", pinned = true } +neovim = { family = "neovim", repo = "soarpkgs", pinned = true } "#; let config: PackagesConfig = toml::from_str(toml_str).unwrap(); let resolved = config.resolved_packages(); assert_eq!(resolved[0].name, "neovim"); - assert_eq!(resolved[0].pkg_id, Some("neovim-appimage".to_string())); + assert_eq!(resolved[0].family, Some("neovim".to_string())); assert_eq!(resolved[0].repo, Some("soarpkgs".to_string())); assert!(resolved[0].pinned); } + // Still read while the field exists, so it stays covered. + #[allow(deprecated)] + #[test] + fn deprecated_pkg_id_is_still_accepted() { + let toml_str = r#" +[packages] +neovim = { pkg_id = "neovim-appimage", repo = "soarpkgs" } +"#; + let config: PackagesConfig = toml::from_str(toml_str).unwrap(); + let resolved = config.resolved_packages(); + + assert_eq!(resolved[0].pkg_id, Some("neovim-appimage".to_string())); + assert_eq!(resolved[0].family, None); + } + #[test] fn test_defaults_applied() { let toml_str = r#" diff --git a/crates/soar-core/src/database/models.rs b/crates/soar-core/src/database/models.rs index e31edda4d..24db6eed7 100644 --- a/crates/soar-core/src/database/models.rs +++ b/crates/soar-core/src/database/models.rs @@ -3,7 +3,10 @@ use std::fmt::Display; use serde::{Deserialize, Serialize}; -use soar_db::{models::types::PackageProvide, repository::core::InstalledPackageWithPortable}; +use soar_db::{ + models::types::{PackageExtra, PackageFile, PackageProvide}, + repository::core::InstalledPackageWithPortable, +}; use soar_package::PackageExt; /// Package maintainer information. @@ -26,11 +29,10 @@ pub struct Package { pub repo_name: String, pub disabled: Option, pub disabled_reason: Option, - pub pkg_id: String, + pub pkg_id: Option, pub pkg_name: String, pub pkg_family: Option, pub pkg_type: Option, - pub pkg_webpage: Option, pub app_id: Option, pub description: String, pub version: String, @@ -46,7 +48,6 @@ pub struct Package { pub homepages: Option>, pub notes: Option>, pub source_urls: Option>, - pub tags: Option>, pub categories: Option>, pub icon: Option, pub desktop: Option, @@ -65,6 +66,11 @@ pub struct Package { pub deprecated: bool, pub desktop_integration: Option, pub portable: Option, + /// Executables inside the artifact, as source path -> installed name. + /// Pinned side files installed alongside the artifact. + pub extra: Option>, + /// What the package takes out of its artifact. Absent means all of it. + pub files: Option>, } impl PackageExt for Package { @@ -72,8 +78,12 @@ impl PackageExt for Package { &self.pkg_name } - fn pkg_id(&self) -> &str { - &self.pkg_id + fn pkg_id(&self) -> Option<&str> { + self.pkg_id.as_deref() + } + + fn pkg_family(&self) -> Option<&str> { + self.pkg_family.as_deref() } fn version(&self) -> &str { @@ -132,8 +142,9 @@ impl Package { pub struct InstalledPackage { pub id: u64, pub repo_name: String, - pub pkg_id: String, + pub pkg_id: Option, pub pkg_name: String, + pub pkg_family: Option, pub pkg_type: Option, pub version: String, pub size: u64, @@ -159,8 +170,12 @@ impl PackageExt for InstalledPackage { &self.pkg_name } - fn pkg_id(&self) -> &str { - &self.pkg_id + fn pkg_id(&self) -> Option<&str> { + self.pkg_id.as_deref() + } + + fn pkg_family(&self) -> Option<&str> { + self.pkg_family.as_deref() } fn version(&self) -> &str { @@ -180,6 +195,7 @@ impl From for InstalledPackage { repo_name: pkg.repo_name, pkg_id: pkg.pkg_id, pkg_name: pkg.pkg_name, + pkg_family: pkg.pkg_family, pkg_type: pkg.pkg_type, version: pkg.version, size: pkg.size as u64, @@ -210,6 +226,7 @@ impl From for InstalledPackage { repo_name: pkg.repo_name, pkg_id: pkg.pkg_id, pkg_name: pkg.pkg_name, + pkg_family: pkg.pkg_family, pkg_type: pkg.pkg_type, version: pkg.version, size: pkg.size as u64, @@ -244,7 +261,6 @@ impl From for Package { pkg_name: pkg.pkg_name, pkg_family: pkg.pkg_family, pkg_type: pkg.pkg_type, - pkg_webpage: pkg.pkg_webpage, app_id: pkg.app_id, description: pkg.description.unwrap_or_default(), version: pkg.version, @@ -260,7 +276,6 @@ impl From for Package { homepages: pkg.homepages, notes: pkg.notes, source_urls: pkg.source_urls, - tags: pkg.tags, categories: pkg.categories, icon: pkg.icon, desktop: pkg.desktop, @@ -279,6 +294,8 @@ impl From for Package { deprecated: false, desktop_integration: pkg.desktop_integration, portable: pkg.portable, + extra: pkg.extra, + files: pkg.files, } } } diff --git a/crates/soar-core/src/package/install.rs b/crates/soar-core/src/package/install.rs index 45cf3e38a..5b7aaac3a 100644 --- a/crates/soar-core/src/package/install.rs +++ b/crates/soar-core/src/package/install.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashMap, env, fs, io::{Read, Write}, os::unix::fs::PermissionsExt, @@ -14,8 +15,9 @@ use soar_config::{ config::Config, packages::{BinaryMapping, BuildConfig, PackageHooks, SandboxConfig}, }; -use soar_db::repository::core::{ - CoreRepository, InstalledPackageWithPortable, NewInstalledPackage, +use soar_db::{ + models::types::PackageFile, + repository::core::{CoreRepository, InstalledPackageWithPortable, NewInstalledPackage}, }; use soar_dl::{ download::Download, @@ -29,6 +31,7 @@ use soar_utils::{ error::FileSystemResult, fs::{safe_remove, walk_dir}, hash::calculate_checksum, + path::is_safe_component, }; use tracing::{debug, trace, warn}; @@ -41,6 +44,309 @@ use crate::{ SoarResult, }; +/// Fetch the side files an artifact does not carry itself. +/// +/// One that published a hash is verified against it, on the same footing as +/// the artifact. A licence publishes none, because it is served from a branch +/// and is documentation rather than something that runs: pinning it would turn +/// an upstream copyright-year edit into a failed download. +async fn install_extras(package: &Package, install_dir: &Path) -> SoarResult<()> { + let Some(extras) = &package.extra else { + return Ok(()); + }; + for e in extras { + if !is_safe_component(&e.to) { + warn!(to = e.to, "skipping side file with unsafe name"); + continue; + } + let dest = install_dir.join(&e.to); + if dest.exists() { + continue; + } + let mut dl = Download::new(&e.url) + .output(dest.to_string_lossy()) + .overwrite(OverwriteMode::Skip); + if let Some(sum) = e.blake3.as_ref() { + dl = dl.checksum(sum); + } + match dl.execute() { + Ok(_) => { + // A side file is usually a licence, but it can be a binary an + // upstream ships separately, and that has to be runnable to be + // worth linking. + if is_elf(&dest) { + fs::set_permissions(&dest, std::fs::Permissions::from_mode(0o755)).ok(); + } + debug!(file = %dest.display(), "installed side file") + } + Err(err) => { + // A missing licence should not abandon a working install, but + // it must not pass unnoticed either. + fs::remove_file(&dest).ok(); + warn!(url = e.url, error = %err, "could not install side file"); + } + } + } + Ok(()) +} + +/// Lay the package out as its recipe describes: each listed file at its own +/// path, aliases beside it, and nothing else kept. +/// +/// Built in a staging directory and swapped in at the end. Resolving a source +/// can fail, and pruning first would leave a package with its binary deleted +/// and nothing to put back. +pub fn apply_file_layout( + files: &[PackageFile], + install_dir: &Path, + artifact: &Path, +) -> SoarResult<()> { + let staging = install_dir.join(".soar-layout"); + fs::remove_dir_all(&staging).ok(); + fs::create_dir_all(&staging) + .with_context(|| format!("creating staging directory {}", staging.display()))?; + + // An archive may ship its directories read-only, and moving a file out of + // one needs write permission on the directory itself. + crate::package::remove::make_tree_writable(install_dir); + + // What each file was moved from, so a failure part-way can put it back. + let mut moved: Vec<(PathBuf, PathBuf)> = Vec::new(); + let placed = match place_files(files, install_dir, artifact, &staging, &mut moved) { + Ok(placed) => placed, + Err(err) => { + // Leave the package as it was found rather than half emptied: the + // caller may retry, and the sources are only in the staging + // directory this call is about to remove. + for (source, dest) in moved { + fs::rename(&dest, &source).ok(); + } + fs::remove_dir_all(&staging).ok(); + return Err(err); + } + }; + + // Nothing resolved means the recipe and the artifact disagree, which most + // often means the download never extracted. Keeping what is there would + // record a package with no commands in it and still report success, so the + // install fails here instead. + if placed == 0 { + fs::remove_dir_all(&staging).ok(); + return Err(SoarError::Custom(format!( + "none of the {} files listed by {} were found in the artifact", + files.len(), + install_dir.display() + ))); + } + + for entry in fs::read_dir(install_dir) + .with_context(|| format!("reading {}", install_dir.display()))? + .flatten() + { + let path = entry.path(); + // The install marker is soar's own bookkeeping, not package content: + // removing it here loses the record an interrupted install resumes from. + if path == staging || entry.file_name() == INSTALL_MARKER_FILE { + continue; + } + if path.is_dir() && !path.is_symlink() { + fs::remove_dir_all(&path).ok(); + } else { + fs::remove_file(&path).ok(); + } + } + for entry in fs::read_dir(&staging) + .with_context(|| format!("reading {}", staging.display()))? + .flatten() + { + let to = install_dir.join(entry.file_name()); + fs::rename(entry.path(), &to) + .with_context(|| format!("moving {} into place", to.display()))?; + } + fs::remove_dir_all(&staging).ok(); + Ok(()) +} + +/// Move each listed file into the staging directory and record where it came +/// from. Returns how many were placed. +fn place_files( + files: &[PackageFile], + install_dir: &Path, + artifact: &Path, + staging: &Path, + moved: &mut Vec<(PathBuf, PathBuf)>, +) -> SoarResult { + let present = walk_dir_files(install_dir, staging); + // Where a source ended up, for the second entry that names the same file. + let mut taken: HashMap = HashMap::new(); + let mut placed = 0usize; + for file in files { + if !is_safe_relative(&file.to) { + warn!(to = file.to, "skipping file with an unsafe target"); + continue; + } + let Some(source) = resolve_source(&file.source, install_dir, &present, artifact) else { + warn!( + source = file.source, + to = file.to, + "file not found in the artifact" + ); + continue; + }; + let dest = staging.join(&file.to); + if let Some(parent) = dest.parent() { + fs::create_dir_all(parent).with_context(|| format!("creating {}", parent.display()))?; + } + // A file listed twice under different names was consumed by the first + // move, so the second is a copy of where it landed. + match taken.get(&source) { + Some(already) => { + fs::copy(already, &dest).with_context(|| format!("copying to {}", file.to))?; + } + None => { + fs::rename(&source, &dest).with_context(|| format!("placing {}", file.to))?; + moved.push((source.clone(), dest.clone())); + taken.insert(source, dest.clone()); + } + } + placed += 1; + + for alias in &file.alias { + if !is_safe_relative(alias) { + warn!(alias, "skipping alias with an unsafe target"); + continue; + } + let link = staging.join(alias); + if let Some(parent) = link.parent() { + fs::create_dir_all(parent).ok(); + } + // relative, so the package directory stays movable + if let Some(target) = relative_to(alias, &file.to) { + std::os::unix::fs::symlink(&target, &link).ok(); + } + } + } + Ok(placed) +} + +/// The path `target` has when read from the directory holding `link`. +/// +/// Nearly every alias is a sibling of what it points at, which is just the +/// file's own name, but nothing in the format requires that. +fn relative_to(link: &str, target: &str) -> Option { + let link_dir: Vec<&str> = link.split('/').collect(); + let link_dir = &link_dir[..link_dir.len().saturating_sub(1)]; + let target_parts: Vec<&str> = target.split('/').collect(); + let shared = link_dir + .iter() + .zip(&target_parts) + .take_while(|(a, b)| a == b) + .count(); + let mut out = PathBuf::new(); + for _ in shared..link_dir.len() { + out.push(".."); + } + for part in &target_parts[shared..] { + out.push(part); + } + (!out.as_os_str().is_empty()).then_some(out) +} + +/// Whether a path stays inside the directory it is joined to. +fn is_safe_relative(path: &str) -> bool { + !path.is_empty() + && !Path::new(path).is_absolute() + && Path::new(path) + .components() + .all(|c| matches!(c, std::path::Component::Normal(_))) +} + +/// Every regular file under `dir`, skipping the staging directory. +fn walk_dir_files(dir: &Path, skip: &Path) -> Vec { + let mut out = Vec::new(); + let Ok(entries) = fs::read_dir(dir) else { + return out; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path == skip || path.is_symlink() { + continue; + } + if path.is_dir() { + out.extend(walk_dir_files(&path, skip)); + } else { + out.push(path); + } + } + out +} + +/// Find a listed source among the extracted files. +/// +/// An empty source means the artifact is itself the file, which is the case for +/// a bare binary. Otherwise the recorded path is tried as written, then without +/// its leading component, since an archive with a single top-level directory +/// has it promoted away, and finally by file name alone. +fn resolve_source( + source: &str, + install_dir: &Path, + present: &[PathBuf], + artifact: &Path, +) -> Option { + // An empty source means the download itself, which is how a bare binary or + // an AppImage says "the artifact is the file". + if source.is_empty() { + return artifact.exists().then(|| artifact.to_path_buf()); + } + let rel_of = |p: &PathBuf| { + p.strip_prefix(install_dir) + .unwrap_or(p) + .to_string_lossy() + .to_string() + }; + // As written first, then without the leading component, since an archive + // with a single top-level directory has it promoted away. + let mut patterns = vec![source.to_string()]; + if let Some((_, rest)) = source.split_once('/') { + patterns.push(rest.to_string()); + } + for pattern in patterns { + if let Some(hit) = present.iter().find(|p| rel_of(p) == pattern) { + return Some(hit.clone()); + } + } + // Last resort, the file name alone: an archive that was extracted but not + // promoted still has everything under the extraction directory. + let name = source.rsplit('/').next().unwrap_or(source); + present + .iter() + .find(|p| p.file_name().is_some_and(|n| n == name)) + .cloned() +} + +/// Give every ELF under `dir` the executable bit. +/// +/// Archive members carry whatever permissions the upstream tarball recorded, +/// and some ship binaries as 0644. The downloaded file is chmodded on the way +/// in, but files that only appear after extraction were being left +/// unexecutable. +fn mark_elfs_executable(dir: &Path) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_symlink() { + continue; + } + if path.is_dir() { + mark_elfs_executable(&path); + } else if is_elf(&path) { + fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).ok(); + } + } +} + /// Returns `true` if the file at `path` starts with the ELF magic bytes. /// /// AppImages and plain binaries are ELF and need the executable bit; archives @@ -109,7 +415,8 @@ use crate::utils::substitute_placeholders; /// Marker content to verify partial install matches current package #[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq, Eq)] pub struct InstallMarker { - pub pkg_id: String, + #[serde(default)] + pub pkg_id: Option, pub version: String, pub bsum: Option, } @@ -200,7 +507,7 @@ impl PackageInstaller { let has_pending = db.with_conn(|conn| { CoreRepository::has_pending_install( conn, - &package.pkg_id, + package.pkg_id.as_deref(), &package.pkg_name, &package.repo_name, &package.version, @@ -232,7 +539,7 @@ impl PackageInstaller { package.version ); let repo_name = &package.repo_name; - let pkg_id = &package.pkg_id; + let pkg_id = package.pkg_id.as_deref(); let pkg_name = &package.pkg_name; let pkg_type = package.pkg_type.as_deref(); let version = &package.version; @@ -255,6 +562,7 @@ impl PackageInstaller { repo_name, pkg_id, pkg_name, + pkg_family: package.pkg_family.as_deref(), pkg_type, version, size, @@ -296,16 +604,15 @@ impl PackageInstaller { use super::hooks::{run_hook, HookEnv}; let env = HookEnv { + pkg_id: self.package.pkg_id.as_deref().unwrap_or_default(), install_dir: &self.install_dir, pkg_name: &self.package.pkg_name, - pkg_id: &self.package.pkg_id, pkg_version: &self.package.version, }; self.events.emit(SoarEvent::Installing { op_id: self.op_id, pkg_name: self.package.pkg_name.clone(), - pkg_id: self.package.pkg_id.clone(), stage: InstallStage::RunningHook(hook_name.to_string()), }); @@ -416,7 +723,6 @@ impl PackageInstaller { self.events.emit(SoarEvent::Building { op_id: self.op_id, pkg_name: self.package.pkg_name.clone(), - pkg_id: self.package.pkg_id.clone(), stage: BuildStage::Sandboxing, }); } @@ -432,7 +738,6 @@ impl PackageInstaller { self.events.emit(SoarEvent::Building { op_id: self.op_id, pkg_name: self.package.pkg_name.clone(), - pkg_id: self.package.pkg_id.clone(), stage: BuildStage::Running { command_index: i, total_commands, @@ -447,7 +752,7 @@ impl PackageInstaller { ), ("BIN_DIR", bin_dir.to_string_lossy().to_string()), ("PKG_NAME", self.package.pkg_name.clone()), - ("PKG_ID", self.package.pkg_id.clone()), + ("PKG_ID", self.package.pkg_id.clone().unwrap_or_default()), ("PKG_VERSION", self.package.version.clone()), ("NPROC", nproc.clone()), ]; @@ -480,7 +785,7 @@ impl PackageInstaller { .env("INSTALL_DIR", &self.install_dir) .env("BIN_DIR", &bin_dir) .env("PKG_NAME", &self.package.pkg_name) - .env("PKG_ID", &self.package.pkg_id) + .env("PKG_ID", self.package.pkg_id.as_deref().unwrap_or_default()) .env("PKG_VERSION", &self.package.version) .env("NPROC", &nproc) .current_dir(&self.install_dir) @@ -499,7 +804,6 @@ impl PackageInstaller { self.events.emit(SoarEvent::Building { op_id: self.op_id, pkg_name: self.package.pkg_name.clone(), - pkg_id: self.package.pkg_id.clone(), stage: BuildStage::CommandComplete { command_index: i, }, @@ -587,7 +891,10 @@ impl PackageInstaller { .with_context(|| format!("setting permissions on {}", dest.display()))?; } - if extract { + // Extraction is offered for every install, so what the file actually is + // decides: a local AppImage or bare binary is not an archive and is + // left alone rather than failing. + if extract && compak::detect_from_file(dest).is_ok() { debug!(archive = %dest.display(), dest = %extract_dir.display(), "extracting local archive"); compak::extract_archive(dest, extract_dir).map_err(|e| { SoarError::Custom(format!( @@ -697,11 +1004,11 @@ impl PackageInstaller { } else { let extract_dir = get_extract_dir(&self.install_dir); - let should_extract = self - .package - .pkg_type - .as_deref() - .is_some_and(|t| t == "archive"); + // Offer extraction unconditionally: the downloader detects the + // format by magic number and leaves non-archives alone. Relying + // on pkg_type meant an archive published as "static" installed + // as an unusable compressed file. + let should_extract = true; let file_path = if let Some(local_src) = local_path_from_url(url) { trace!(source = %local_src.display(), "installing from local file"); @@ -737,7 +1044,8 @@ impl PackageInstaller { }; let extract_path = PathBuf::from(&extract_dir); - if extract_path.exists() { + let extracted = extract_path.exists(); + if extracted { fs::remove_file(file_path).ok(); for entry in fs::read_dir(&extract_path) @@ -748,6 +1056,28 @@ impl PackageInstaller { })?; let from = entry.path(); let to = self.install_dir.join(entry.file_name()); + // Renaming a directory rewrites its `..`, so the directory + // itself needs the write bit; archives shipping 0555 dirs + // would otherwise fail to promote. + if let Ok(meta) = fs::metadata(&from) { + let mode = meta.permissions().mode(); + if meta.is_dir() && mode & 0o200 == 0 { + fs::set_permissions( + &from, + std::fs::Permissions::from_mode(mode | 0o200), + ) + .ok(); + } + } + // A leftover from an interrupted install would make rename + // fail, the same way the other promotion paths treat it. + if to.exists() { + if to.is_dir() { + fs::remove_dir_all(&to).ok(); + } else { + fs::remove_file(&to).ok(); + } + } fs::rename(&from, &to).with_context(|| { format!("renaming {} to {}", from.display(), to.display()) })?; @@ -756,8 +1086,38 @@ impl PackageInstaller { fs::remove_dir_all(&extract_path).ok(); } + // Archives conventionally wrap everything in one versioned + // directory (foo-1.2.3-x86_64/). Nothing downstream can guess that + // name, so when extraction leaves exactly one directory behind and + // no extract_root was given, treat it as the root. + let auto_root = if self.extract_root.is_none() && extracted { + let mut dirs = Vec::new(); + let mut files = 0usize; + if let Ok(rd) = fs::read_dir(&self.install_dir) { + for entry in rd.flatten() { + let name = entry.file_name(); + if name.to_string_lossy().starts_with('.') { + continue; + } + if entry.path().is_dir() { + dirs.push(name.to_string_lossy().to_string()); + } else { + files += 1; + } + } + } + if files == 0 && dirs.len() == 1 { + debug!(root = %dirs[0], "auto-detected single extract root"); + Some(dirs.remove(0)) + } else { + None + } + } else { + None + }; + // Handle extract_root: move contents from subdirectory to install root - if let Some(ref root_dir) = self.extract_root { + if let Some(ref root_dir) = self.extract_root.clone().or(auto_root) { let root_dir = substitute_placeholders( root_dir, Some(&self.package.version), @@ -772,6 +1132,19 @@ impl PackageInstaller { root_path.display(), self.install_dir.display() ); + + // A file inside the root can share the root's own name + // (age/age). Promoting it would target the directory + // currently being drained, and the clobber below would + // delete the rest of the package. Move the root aside + // first so source and destination can never collide. + let staged = self.install_dir.join(".soar_extract_root"); + fs::remove_dir_all(&staged).ok(); + fs::rename(&root_path, &staged).with_context(|| { + format!("staging {} for promotion", root_path.display()) + })?; + let root_path = staged; + // Move all contents from root_path to install_dir for entry in fs::read_dir(&root_path).with_context(|| { format!("reading extract_root directory {}", root_path.display()) @@ -798,6 +1171,16 @@ impl PackageInstaller { } } + if let Some(files) = self.package.files.as_deref().filter(|f| !f.is_empty()) { + apply_file_layout(files, &self.install_dir, output_path)?; + } + + if extracted { + mark_elfs_executable(&self.install_dir); + } + + install_extras(&self.package, &self.install_dir).await?; + // Handle nested_extract: extract an archive within the package if let Some(ref nested_archive) = self.nested_extract { let nested_archive = substitute_placeholders( @@ -888,7 +1271,7 @@ impl PackageInstaller { let package = &self.package; let repo_name = &package.repo_name; let pkg_name = &package.pkg_name; - let pkg_id = &package.pkg_id; + let pkg_id = package.pkg_id.as_deref(); let version = &package.version; let size = package.ghcr_size.unwrap_or(package.size.unwrap_or(0)) as i64; let checksum = package.bsum.as_deref(); @@ -914,8 +1297,8 @@ impl PackageInstaller { let record_id = record_id.ok_or_else(|| { SoarError::Custom(format!( - "Failed to record installation for {}#{}: package not found in database", - pkg_name, pkg_id + "Failed to record installation for {}: package not found in database", + pkg_name )) })?; @@ -964,12 +1347,26 @@ impl PackageInstaller { } if !unlinked { - self.db - .with_conn(|conn| CoreRepository::unlink_others(conn, pkg_name, pkg_id, version))?; + self.db.with_conn(|conn| { + CoreRepository::unlink_others( + conn, + pkg_name, + &self.package.repo_name, + pkg_id, + self.package.pkg_family.as_deref(), + Some(version), + ) + })?; let alternate_packages: Vec = self.db.with_conn(|conn| { - CoreRepository::find_alternates(conn, pkg_name, pkg_id, version) + CoreRepository::find_alternates( + conn, + pkg_name, + pkg_id, + self.package.pkg_family.as_deref(), + version, + ) })?; for alt_pkg in alternate_packages { @@ -1007,3 +1404,31 @@ impl PackageInstaller { Ok(()) } } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use super::relative_to; + + #[test] + fn alias_beside_its_target_is_just_the_name() { + assert_eq!( + relative_to("bin/fdfind", "bin/fd").unwrap(), + Path::new("fd") + ); + } + + #[test] + fn alias_in_another_directory_climbs_out() { + assert_eq!( + relative_to("share/man/man1/fdfind.1", "share/man/man1/fd.1").unwrap(), + Path::new("fd.1") + ); + assert_eq!( + relative_to("bin/fd", "libexec/fd").unwrap(), + Path::new("../libexec/fd") + ); + assert_eq!(relative_to("fd", "bin/fd").unwrap(), Path::new("bin/fd")); + } +} diff --git a/crates/soar-core/src/package/local.rs b/crates/soar-core/src/package/local.rs index 2e901f1d1..cd1822ba0 100644 --- a/crates/soar-core/src/package/local.rs +++ b/crates/soar-core/src/package/local.rs @@ -26,8 +26,11 @@ pub struct LocalPackage { pub path: PathBuf, /// Extracted or overridden package name pub pkg_name: String, - /// Generated package ID (lowercase, normalized) - pub pkg_id: String, + /// Package id, set only by an explicit override + pub pkg_id: Option, + /// Where the file came from, used to tell apart two local files that + /// produce the same package name. + pub pkg_family: Option, /// Extracted or overridden version pub version: String, /// Detected package type from extension (e.g., "appimage") @@ -116,18 +119,19 @@ impl LocalPackage { .map(|v| v.strip_prefix('v').unwrap_or(v).to_string()) .unwrap_or(extracted_version); - let pkg_id = pkg_id_override.map(String::from).unwrap_or_else(|| { - if let Some(ref ptype) = pkg_type { - format!("local.{pkg_name}-{ptype}") - } else { - format!("local.{pkg_name}") - } + let pkg_id = pkg_id_override.map(String::from); + // Nothing derives an id. The family carries where the file came from, + // which is what the derived id was standing in for. + let pkg_family = Some(match pkg_type { + Some(ref ptype) => format!("local.{pkg_name}-{ptype}"), + None => format!("local.{pkg_name}"), }); Ok(Self { path, - pkg_name, pkg_id, + pkg_family, + pkg_name, version, pkg_type, size, @@ -143,6 +147,7 @@ impl LocalPackage { id: 0, repo_name: "local".to_string(), pkg_id: self.pkg_id.clone(), + pkg_family: self.pkg_family.clone(), pkg_name: self.pkg_name.clone(), pkg_type: self.pkg_type.clone(), version: self.version.clone(), @@ -194,7 +199,7 @@ mod tests { assert_eq!(pkg.pkg_name, "myapp"); assert_eq!(pkg.version, "2.0.1"); assert_eq!(pkg.pkg_type, Some("appimage".to_string())); - assert_eq!(pkg.pkg_id, "local.myapp-appimage"); + assert_eq!(pkg.pkg_family.as_deref(), Some("local.myapp-appimage")); let package = pkg.to_package(); assert_eq!(package.repo_name, "local"); diff --git a/crates/soar-core/src/package/query.rs b/crates/soar-core/src/package/query.rs index 9ae8df932..da4dcfac3 100644 --- a/crates/soar-core/src/package/query.rs +++ b/crates/soar-core/src/package/query.rs @@ -1,15 +1,20 @@ use std::sync::OnceLock; use regex::Regex; +use tracing::warn; use crate::error::SoarError; /// Parsed package query string. -/// Supports format: `name#pkg_id@version:repo` +/// +/// Supports `family/name@version:repo`, where the family narrows a name that +/// more than one project publishes. #[derive(Debug)] pub struct PackageQuery { pub name: Option, + pub family: Option, pub repo_name: Option, + /// Deprecated. Repositories no longer publish a package id. pub pkg_id: Option, pub version: Option, } @@ -22,8 +27,10 @@ impl TryFrom<&str> for PackageQuery { let re = PACKAGE_RE.get_or_init(|| { Regex::new( r"(?x) + ^ # anchored: a/b/c is not a query + (?:(?P[^\/\#\@:]+)\/)? # optional family before / (?P[^\/\#\@:]+)? # optional package name - (?:\#(?P[^@:]+))? # optional pkg_id after # + (?:\#(?P[^@:]+))? # deprecated pkg_id after # (?:@(?P[^:]+))? # optional version after @ (?::(?P[^:]+))?$ # optional repo after : ", @@ -43,7 +50,11 @@ impl TryFrom<&str> for PackageQuery { ))?; let name = caps.name("name").map(|m| m.as_str().to_string()); + let family = caps.name("family").map(|m| m.as_str().to_string()); let pkg_id = caps.name("pkg_id").map(|m| m.as_str().to_string()); + if pkg_id.is_some() { + warn!("#pkg_id is deprecated and will be removed; use family/name instead"); + } if pkg_id.is_none() && name.is_none() { return Err(SoarError::InvalidPackageQuery( "Either package name or pkg_id is required".into(), @@ -60,9 +71,34 @@ impl TryFrom<&str> for PackageQuery { Ok(PackageQuery { repo_name: caps.name("repo").map(|m| m.as_str().to_string()), + family, pkg_id, name, version: caps.name("version").map(|m| m.as_str().to_string()), }) } } + +#[cfg(test)] +mod tests { + use super::PackageQuery; + + #[test] + fn parses_every_supported_shape() { + let q = PackageQuery::try_from("ripgrep").unwrap(); + assert_eq!(q.name.as_deref(), Some("ripgrep")); + assert_eq!(q.family, None); + + let q = PackageQuery::try_from("bat/bat@0.24.0:bincache").unwrap(); + assert_eq!(q.family.as_deref(), Some("bat")); + assert_eq!(q.name.as_deref(), Some("bat")); + assert_eq!(q.version.as_deref(), Some("0.24.0")); + assert_eq!(q.repo_name.as_deref(), Some("bincache")); + } + + #[test] + fn a_third_segment_is_not_a_query() { + // Unanchored, this matched from the middle and silently dropped `a`. + assert!(PackageQuery::try_from("a/b/c").is_err()); + } +} diff --git a/crates/soar-core/src/package/remove.rs b/crates/soar-core/src/package/remove.rs index 5dbb21ac6..2bc60aa83 100644 --- a/crates/soar-core/src/package/remove.rs +++ b/crates/soar-core/src/package/remove.rs @@ -1,6 +1,7 @@ use std::{ ffi::OsString, fs, + os::unix::fs::PermissionsExt, path::{Path, PathBuf}, }; @@ -14,6 +15,45 @@ use tracing::{debug, trace, warn}; use super::hooks::{run_hook, HookEnv}; +/// Remove every symlink under `dir` that points into `installed_path`. +/// +/// Ownership is decided by where the link goes, not by its name: a completion +/// has to be named after its command, so soar cannot mark its own with a +/// suffix the way it does for desktop files. Anything pointing elsewhere +/// belongs to someone else and is left alone. +fn remove_links_into(dir: &Path, installed_path: &Path, removed: &mut Vec) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_symlink() { + if let Ok(target) = fs::read_link(&path) { + if target.starts_with(installed_path) { + trace!("removing link: {}", path.display()); + if fs::remove_file(&path).is_ok() { + removed.push(path); + } + } + } + } else if path.is_dir() { + remove_links_into(&path, installed_path, removed); + } + } +} + +/// The directories a package's files are linked into, beyond `bin`. +/// +/// Every destination, not only the ones the configured shells ask for: a +/// completion linked while a shell was enabled still has to be unlinked once +/// that shell is turned off. +fn shared_link_dirs(bin_path: &Path) -> Vec { + crate::utils::shared_link_targets(bin_path, &[]) + .into_iter() + .map(|(_, destination, _)| destination) + .collect() +} + /// Removes the bin-directory symlinks a package's `provides` created, keeping /// only those that live directly in `bin_path` and resolve into /// `installed_path`. @@ -78,6 +118,28 @@ pub struct PackageRemover { sandbox: Option, } +/// Give every directory under `path` the owner write bit. +/// +/// Without it `remove_dir_all` cannot unlink the entries inside, so a package +/// that installed cleanly could not be removed. +pub fn make_tree_writable(path: &Path) { + let Ok(entries) = fs::read_dir(path) else { + return; + }; + for entry in entries.flatten() { + let child = entry.path(); + if child.is_dir() && !child.is_symlink() { + make_tree_writable(&child); + } + } + if let Ok(meta) = fs::metadata(path) { + let mode = meta.permissions().mode(); + if mode & 0o200 == 0 { + fs::set_permissions(path, fs::Permissions::from_mode(mode | 0o200)).ok(); + } + } +} + impl PackageRemover { pub async fn new(package: InstalledPackage, db: DieselDatabase, config: Config) -> Self { trace!( @@ -110,9 +172,9 @@ impl PackageRemover { fn run_hook(&self, hook_name: &str, command: &str) -> SoarResult<()> { let install_dir = PathBuf::from(&self.package.installed_path); let env = HookEnv { + pkg_id: self.package.pkg_id.as_deref().unwrap_or_default(), install_dir: &install_dir, pkg_name: &self.package.pkg_name, - pkg_id: &self.package.pkg_id, pkg_version: &self.package.version, }; @@ -137,9 +199,8 @@ impl PackageRemover { version = self.package.version, repo = self.package.repo_name, installed_path = self.package.installed_path, - "removing {}#{}:{} ({})", + "removing {}:{} ({})", self.package.pkg_name, - self.package.pkg_id, self.package.repo_name, self.package.version ); @@ -156,17 +217,20 @@ impl PackageRemover { let bin_path = self.config.get_bin_path()?; let installed_path = PathBuf::from(&self.package.installed_path); - if let Some(provides) = &self.package.provides { + // An empty list is stored rather than null once nothing declares + // provides, and it must not stand in for "this package has links". + if let Some(provides) = self.package.provides.as_deref().filter(|p| !p.is_empty()) { let removed = remove_provide_symlinks(&bin_path, provides, &installed_path)?; removed_symlinks.extend(removed); } else { - let def_bin = bin_path.join(&self.package.pkg_name); - if def_bin.is_symlink() && def_bin.is_file() { - trace!("removing binary symlink: {}", def_bin.display()); - fs::remove_file(&def_bin) - .with_context(|| format!("removing binary {}", def_bin.display()))?; - removed_symlinks.push(def_bin); - } + // Nothing declares the links anymore, so they are found by + // where they point. This also catches aliases, which a name + // built from pkg_name alone would miss. + remove_links_into(&bin_path, &installed_path, &mut removed_symlinks); + } + + for dir in shared_link_dirs(&bin_path) { + remove_links_into(&dir, &installed_path, &mut removed_symlinks); } let mut remove_action = |path: &Path| -> FileSystemResult<()> { @@ -180,7 +244,12 @@ impl PackageRemover { } Ok(()) }; - walk_dir(&self.config.get_desktop_path()?, &mut remove_action)?; + // A missing desktop or icon directory means there is nothing of + // ours in it, not a reason to abandon the removal half-done. + let desktop_path = self.config.get_desktop_path()?; + if desktop_path.is_dir() { + walk_dir(&desktop_path, &mut remove_action)?; + } let mut remove_action = |path: &Path| -> FileSystemResult<()> { if let Ok(real_path) = fs::read_link(path) { @@ -191,7 +260,10 @@ impl PackageRemover { } Ok(()) }; - walk_dir(self.config.get_icons_path(), &mut remove_action)?; + let icons_path = self.config.get_icons_path(); + if icons_path.is_dir() { + walk_dir(icons_path, &mut remove_action)?; + } } // Calculate directory size before removal for logging @@ -214,6 +286,10 @@ impl PackageRemover { self.package.installed_path, size_str ); + // Archives commonly ship directories read-only, and removing a + // directory's entries needs the write bit on that directory. soar owns + // this tree, so it may restore what it needs to delete it. + make_tree_writable(Path::new(&self.package.installed_path)); if let Err(err) = fs::remove_dir_all(&self.package.installed_path) { // if not found, the package is already removed. if err.kind() != std::io::ErrorKind::NotFound { @@ -241,12 +317,8 @@ impl PackageRemover { } debug!( - "removed {}#{}:{} ({}) - reclaimed {}", - self.package.pkg_name, - self.package.pkg_id, - self.package.repo_name, - self.package.version, - size_str + "removed {}:{} ({}) - reclaimed {}", + self.package.pkg_name, self.package.repo_name, self.package.version, size_str ); Ok(()) } diff --git a/crates/soar-core/src/package/update.rs b/crates/soar-core/src/package/update.rs index 71672384a..8cd2f8e25 100644 --- a/crates/soar-core/src/package/update.rs +++ b/crates/soar-core/src/package/update.rs @@ -16,25 +16,43 @@ use crate::{ pub fn remove_old_versions(package: &Package, db: &DieselDatabase, force: bool) -> SoarResult<()> { let Package { pkg_id, + pkg_family, pkg_name, repo_name, .. } = package; let old_packages = db.with_conn(|conn| { - CoreRepository::get_old_package_paths(conn, pkg_id, pkg_name, repo_name, force) + CoreRepository::get_old_package_paths( + conn, + pkg_id.as_deref(), + pkg_family.as_deref(), + pkg_name, + repo_name, + force, + ) })?; for (_id, installed_path) in &old_packages { let path = Path::new(installed_path); if path.exists() { + // An archive may ship its directories read-only, and removing an + // entry needs write permission on the directory holding it. + crate::package::remove::make_tree_writable(path); fs::remove_dir_all(path) .with_context(|| format!("removing old package directory {}", path.display()))?; } } db.with_conn(|conn| { - CoreRepository::delete_old_packages(conn, pkg_id, pkg_name, repo_name, force) + CoreRepository::delete_old_packages( + conn, + pkg_id.as_deref(), + pkg_family.as_deref(), + pkg_name, + repo_name, + force, + ) })?; Ok(()) diff --git a/crates/soar-core/src/package/url.rs b/crates/soar-core/src/package/url.rs index 3aa6db960..d7dd57687 100644 --- a/crates/soar-core/src/package/url.rs +++ b/crates/soar-core/src/package/url.rs @@ -14,8 +14,13 @@ pub struct UrlPackage { pub url: String, /// Extracted or overridden package name pub pkg_name: String, - /// Generated package ID (lowercase, normalized) - pub pkg_id: String, + /// Package id, only when the caller supplied one. Nothing derives an id: + /// repositories on the declarative format publish none, so inventing one + /// here would be the only place they came from. + pub pkg_id: Option, + /// Where the package came from, used to tell apart two sources that + /// produce the same name. This is what an id used to stand in for. + pub pkg_family: Option, /// Extracted or overridden version pub version: String, /// Detected package type from extension (e.g., "appimage") @@ -90,18 +95,25 @@ impl UrlPackage { /// Both are joined into the install dir and interpolated into resource /// paths, so a caller-supplied override containing `/` or `..` would escape /// it. The derived defaults are always dot-separated and unaffected. - fn validate_names(pkg_name: &str, pkg_id: &str) -> SoarResult<()> { + fn validate_names( + pkg_name: &str, + pkg_id: Option<&str>, + pkg_family: Option<&str>, + ) -> SoarResult<()> { if !is_safe_component(pkg_name) { return Err(SoarError::Custom(format!( "Invalid package name '{}': must be a single path component", pkg_name ))); } - if !is_safe_component(pkg_id) { - return Err(SoarError::Custom(format!( - "Invalid package id '{}': must be a single path component", - pkg_id - ))); + for (label, value) in [("id", pkg_id), ("family", pkg_family)] { + if let Some(value) = value { + if !is_safe_component(value) { + return Err(SoarError::Custom(format!( + "Invalid package {label} '{value}': must be a single path component" + ))); + } + } } Ok(()) } @@ -145,18 +157,18 @@ impl UrlPackage { .map(|v| v.strip_prefix('v').unwrap_or(v).to_string()) .unwrap_or_else(|| tag.strip_prefix('v').unwrap_or(&tag).to_string()); - let pkg_id = pkg_id_override - .map(String::from) - .unwrap_or_else(|| package.replace('/', ".")); + let pkg_id = pkg_id_override.map(String::from); + let pkg_family = Some(package.replace('/', ".")); let pkg_type = pkg_type_override.map(|s| s.to_lowercase()); - Self::validate_names(&pkg_name, &pkg_id)?; + Self::validate_names(&pkg_name, pkg_id.as_deref(), pkg_family.as_deref())?; Ok(Self { url: reference.to_string(), - pkg_name, pkg_id, + pkg_family, + pkg_name, version, pkg_type, is_ghcr: true, @@ -219,24 +231,24 @@ impl UrlPackage { .map(|v| v.strip_prefix('v').unwrap_or(v).to_string()) .unwrap_or(extracted_version); - // Generate pkg_id: use override, or extract from URL, or generate from name and type - let pkg_id = pkg_id_override - .map(String::from) - .or_else(|| extract_pkg_id_from_url(url)) - .unwrap_or_else(|| { - if let Some(ref ptype) = pkg_type { - format!("{}-{}", pkg_name, ptype) - } else { - pkg_name.clone() - } - }); + let pkg_id = pkg_id_override.map(String::from); + // The host and project the URL points at, falling back to the name so + // two unrelated downloads sharing a filename stay distinct. + let pkg_family = extract_family_from_url(url).or_else(|| { + if let Some(ref ptype) = pkg_type { + Some(format!("{}-{}", pkg_name, ptype)) + } else { + Some(pkg_name.clone()) + } + }); - Self::validate_names(&pkg_name, &pkg_id)?; + Self::validate_names(&pkg_name, pkg_id.as_deref(), pkg_family.as_deref())?; Ok(Self { url: url.to_string(), - pkg_name, pkg_id, + pkg_family, + pkg_name, version, pkg_type, is_ghcr: false, @@ -251,6 +263,7 @@ impl UrlPackage { id: 0, repo_name: "local".to_string(), pkg_id: self.pkg_id.clone(), + pkg_family: self.pkg_family.clone(), pkg_name: self.pkg_name.clone(), pkg_type: self.pkg_type.clone(), version: self.version.clone(), @@ -265,6 +278,7 @@ impl UrlPackage { id: 0, repo_name: "local".to_string(), pkg_id: self.pkg_id.clone(), + pkg_family: self.pkg_family.clone(), pkg_name: self.pkg_name.clone(), pkg_type: self.pkg_type.clone(), version: self.version.clone(), @@ -277,12 +291,12 @@ impl UrlPackage { } } -/// Extract pkg_id from URL based on host and first two path segments. +/// Extract the family from a URL: host plus the first two path segments. /// /// Examples: /// - `https://github.com/user/repo/...` → `github.com.user.repo` /// - `https://example.com/foo/bar/...` → `example.com.foo.bar` -fn extract_pkg_id_from_url(url: &str) -> Option { +fn extract_family_from_url(url: &str) -> Option { let url = url.trim().to_lowercase(); // Remove protocol @@ -424,7 +438,7 @@ mod tests { assert_eq!(pkg.pkg_name, "myapp"); assert_eq!(pkg.version, "2.0.0"); - assert_eq!(pkg.pkg_id, "github.com.user.repo"); + assert_eq!(pkg.pkg_family.as_deref(), Some("github.com.user.repo")); } #[test] @@ -436,25 +450,25 @@ mod tests { assert_eq!(pkg.pkg_name, "myapp"); assert_eq!(pkg.version, "1.0.0"); assert_eq!(pkg.pkg_type, Some("appimage".to_string())); - assert_eq!(pkg.pkg_id, "example.com.downloads.app"); + assert_eq!(pkg.pkg_family.as_deref(), Some("example.com.downloads.app")); } #[test] - fn test_extract_pkg_id_from_url() { + fn test_extract_family_from_url() { assert_eq!( - extract_pkg_id_from_url("https://github.com/pkgforge/soar/releases/file"), + extract_family_from_url("https://github.com/pkgforge/soar/releases/file"), Some("github.com.pkgforge.soar".to_string()) ); assert_eq!( - extract_pkg_id_from_url("https://gitlab.com/user/project/-/releases"), + extract_family_from_url("https://gitlab.com/user/project/-/releases"), Some("gitlab.com.user.project".to_string()) ); assert_eq!( - extract_pkg_id_from_url("https://example.com/foo/bar/baz"), + extract_family_from_url("https://example.com/foo/bar/baz"), Some("example.com.foo.bar".to_string()) ); assert_eq!( - extract_pkg_id_from_url("https://example.com/app"), + extract_family_from_url("https://example.com/app"), Some("example.com.app".to_string()) ); } @@ -468,7 +482,7 @@ mod tests { assert_eq!(pkg.repo_name, "local"); assert_eq!(pkg.pkg_name, "test"); assert_eq!(pkg.version, "1.0"); - assert_eq!(pkg.pkg_id, "github.com.user.testrepo"); + assert_eq!(pkg.pkg_family.as_deref(), Some("github.com.user.testrepo")); assert_eq!(pkg.download_url, url); } @@ -540,7 +554,7 @@ mod tests { assert_eq!(pkg.pkg_name, "soar"); assert_eq!(pkg.version, "0.8.1"); // 'v' prefix stripped - assert_eq!(pkg.pkg_id, "pkgforge.soar"); + assert_eq!(pkg.pkg_family.as_deref(), Some("pkgforge.soar")); assert!(pkg.is_ghcr); } @@ -551,7 +565,7 @@ mod tests { assert_eq!(pkg.pkg_name, "repo"); assert_eq!(pkg.version, "sha256:deadbeef1234567890"); - assert_eq!(pkg.pkg_id, "org.repo"); + assert_eq!(pkg.pkg_family.as_deref(), Some("org.repo")); assert!(pkg.is_ghcr); } @@ -562,7 +576,7 @@ mod tests { assert_eq!(pkg.pkg_name, "package"); assert_eq!(pkg.version, "latest"); - assert_eq!(pkg.pkg_id, "org.package"); + assert_eq!(pkg.pkg_family.as_deref(), Some("org.package")); assert!(pkg.is_ghcr); } @@ -573,7 +587,7 @@ mod tests { assert_eq!(pkg.pkg_name, "repo"); assert_eq!(pkg.version, "1.0"); - assert_eq!(pkg.pkg_id, "org.team.repo"); + assert_eq!(pkg.pkg_family.as_deref(), Some("org.team.repo")); assert!(pkg.is_ghcr); } @@ -586,7 +600,7 @@ mod tests { assert_eq!(pkg.pkg_name, "myapp"); assert_eq!(pkg.version, "2.0.0"); - assert_eq!(pkg.pkg_id, "custom-id"); + assert_eq!(pkg.pkg_id.as_deref(), Some("custom-id")); assert!(pkg.is_ghcr); } @@ -618,7 +632,7 @@ mod tests { assert_eq!(pkg.repo_name, "local"); assert_eq!(pkg.pkg_name, "soar"); assert_eq!(pkg.version, "0.8.1"); // 'v' prefix stripped - assert_eq!(pkg.pkg_id, "pkgforge.soar"); + assert_eq!(pkg.pkg_family.as_deref(), Some("pkgforge.soar")); assert_eq!(pkg.download_url, ""); assert_eq!( pkg.ghcr_pkg, diff --git a/crates/soar-core/src/utils.rs b/crates/soar-core/src/utils.rs index 9ff5d04ba..237543d8a 100644 --- a/crates/soar-core/src/utils.rs +++ b/crates/soar-core/src/utils.rs @@ -119,3 +119,38 @@ pub fn substitute_placeholders( None => result, } } + +/// Where a package's shared files are exposed on the system. +/// +/// Each entry is the directory inside the package, the destination on the +/// system, and whether the user asked for it. Man pages go beside the bin +/// directory, because man-db derives its search path from PATH: for every +/// `.../bin` it also looks at `.../share/man`. That makes them findable with +/// no MANPATH set. +pub fn shared_link_targets( + bin_dir: &Path, + shells: &[String], +) -> Vec<(&'static str, PathBuf, bool)> { + let prefix = bin_dir.parent().unwrap_or(bin_dir); + let data = soar_utils::path::xdg_data_home(); + let config = soar_utils::path::xdg_config_home(); + let wants = |name: &str| shells.iter().any(|s| s == name); + vec![ + ("share/man", prefix.join("share/man"), true), + ( + "share/bash-completion/completions", + data.join("bash-completion/completions"), + wants("bash"), + ), + ( + "share/zsh/site-functions", + data.join("zsh/site-functions"), + wants("zsh"), + ), + ( + "share/fish/vendor_completions.d", + config.join("fish/completions"), + wants("fish"), + ), + ] +} diff --git a/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql new file mode 100644 index 000000000..37c313772 --- /dev/null +++ b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql @@ -0,0 +1,38 @@ +-- A package without an id cannot be represented once the column is required +-- again. Refuse the downgrade rather than deleting the rows: the CHECK fails +-- when any such row exists, and the table name is what the error reports. +CREATE TEMP TABLE cannot_downgrade_packages_without_pkg_id ( + ok INTEGER NOT NULL CHECK (ok = 1) +); +INSERT INTO cannot_downgrade_packages_without_pkg_id (ok) + SELECT CASE WHEN EXISTS (SELECT 1 FROM packages WHERE pkg_id IS NULL) THEN 0 ELSE 1 END; +DROP TABLE cannot_downgrade_packages_without_pkg_id; + +CREATE TABLE packages_old ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + repo_name TEXT NOT NULL, + pkg_id TEXT NOT NULL COLLATE NOCASE, + pkg_name TEXT NOT NULL COLLATE NOCASE, + pkg_type TEXT COLLATE NOCASE, + version TEXT NOT NULL, + size BIGINT NOT NULL, + checksum TEXT, + installed_path TEXT NOT NULL, + installed_date TEXT NOT NULL, + profile TEXT NOT NULL, + pinned BOOLEAN NOT NULL DEFAULT false, + is_installed BOOLEAN NOT NULL DEFAULT false, + detached BOOLEAN NOT NULL DEFAULT false, + unlinked BOOLEAN NOT NULL DEFAULT false, + provides JSONB, + install_patterns JSONB +); + +INSERT INTO packages_old +SELECT id, repo_name, pkg_id, pkg_name, pkg_type, version, size, checksum, + installed_path, installed_date, profile, pinned, is_installed, detached, + unlinked, provides, install_patterns +FROM packages; + +DROP TABLE packages; +ALTER TABLE packages_old RENAME TO packages; diff --git a/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/metadata.toml b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/metadata.toml new file mode 100644 index 000000000..b6070ebe5 --- /dev/null +++ b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/metadata.toml @@ -0,0 +1,3 @@ +# Dropping the old table cascades rows out of tables referencing it, and +# foreign keys can only be turned off outside a transaction. +run_in_transaction = false diff --git a/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/up.sql b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/up.sql new file mode 100644 index 000000000..92ea98c00 --- /dev/null +++ b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/up.sql @@ -0,0 +1,40 @@ +PRAGMA foreign_keys = OFF; + +-- Rebuilt rather than altered: SQLite cannot relax `pkg_id NOT NULL` in place. +CREATE TABLE packages_new ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + repo_name TEXT NOT NULL, + pkg_id TEXT COLLATE NOCASE, + pkg_name TEXT NOT NULL COLLATE NOCASE, + pkg_family TEXT COLLATE NOCASE, + pkg_type TEXT COLLATE NOCASE, + version TEXT NOT NULL, + size BIGINT NOT NULL, + checksum TEXT, + installed_path TEXT NOT NULL, + installed_date TEXT NOT NULL, + profile TEXT NOT NULL, + pinned BOOLEAN NOT NULL DEFAULT false, + is_installed BOOLEAN NOT NULL DEFAULT false, + detached BOOLEAN NOT NULL DEFAULT false, + unlinked BOOLEAN NOT NULL DEFAULT false, + provides JSONB, + install_patterns JSONB +); + +INSERT INTO packages_new +SELECT id, repo_name, pkg_id, pkg_name, NULL, pkg_type, version, size, checksum, + installed_path, installed_date, profile, pinned, is_installed, detached, + unlinked, provides, install_patterns +FROM packages; + +DROP TABLE packages; +ALTER TABLE packages_new RENAME TO packages; + +-- A synthesised id was how a URL or local install recorded its source; that is +-- the family's job now. +UPDATE packages +SET pkg_family = pkg_id +WHERE repo_name = 'local' AND pkg_family IS NULL AND pkg_id IS NOT NULL; + +PRAGMA foreign_keys = ON; diff --git a/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sql b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sql new file mode 100644 index 000000000..a19ead64d --- /dev/null +++ b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sql @@ -0,0 +1,63 @@ +PRAGMA foreign_keys = OFF; + +-- A package without an id cannot be represented once the column is required +-- again. This table is a cache of the published index, so the rows are simply +-- dropped and the next sync puts them back. +DELETE FROM packages WHERE pkg_id IS NULL; + +DROP INDEX IF EXISTS packages_identity; + +CREATE TABLE packages_old ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + pkg_id TEXT NOT NULL COLLATE NOCASE, + pkg_family TEXT COLLATE NOCASE, + pkg_name TEXT NOT NULL COLLATE NOCASE, + pkg_type TEXT COLLATE NOCASE, + pkg_webpage TEXT, + app_id TEXT COLLATE NOCASE, + description TEXT, + version TEXT NOT NULL, + version_upstream TEXT, + licenses JSONB, + download_url TEXT NOT NULL, + size BIGINT, + ghcr_pkg TEXT, + ghcr_size BIGINT, + ghcr_blob TEXT, + ghcr_url TEXT, + bsum TEXT, + icon TEXT, + desktop TEXT, + appstream TEXT, + homepages JSONB, + notes JSONB, + source_urls JSONB, + tags JSONB, + categories JSONB, + build_id TEXT, + build_date TEXT, + build_action TEXT, + build_script TEXT, + build_log TEXT, + provides JSONB, + snapshots JSONB, + replaces JSONB, + soar_syms BOOLEAN NOT NULL DEFAULT false, + desktop_integration BOOLEAN, + portable BOOLEAN, + UNIQUE (pkg_id, pkg_name, version) +); + +INSERT INTO packages_old SELECT + id, pkg_id, pkg_family, pkg_name, pkg_type, NULL, app_id, description, + version, NULL, licenses, download_url, size, ghcr_pkg, ghcr_size, ghcr_blob, + ghcr_url, bsum, icon, desktop, appstream, homepages, notes, source_urls, + NULL, categories, build_id, build_date, build_action, build_script, + build_log, provides, snapshots, replaces, soar_syms, desktop_integration, + portable +FROM packages; + +DROP TABLE packages; +ALTER TABLE packages_old RENAME TO packages; + +PRAGMA foreign_keys = ON; diff --git a/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/metadata.toml b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/metadata.toml new file mode 100644 index 000000000..b6070ebe5 --- /dev/null +++ b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/metadata.toml @@ -0,0 +1,3 @@ +# Dropping the old table cascades rows out of tables referencing it, and +# foreign keys can only be turned off outside a transaction. +run_in_transaction = false diff --git a/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql new file mode 100644 index 000000000..82b030fe4 --- /dev/null +++ b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql @@ -0,0 +1,59 @@ +PRAGMA foreign_keys = OFF; + +-- Rebuilt rather than altered: SQLite cannot relax `pkg_id NOT NULL` in place. +CREATE TABLE packages_new ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + pkg_id TEXT COLLATE NOCASE, + pkg_family TEXT COLLATE NOCASE, + pkg_name TEXT NOT NULL COLLATE NOCASE, + pkg_type TEXT COLLATE NOCASE, + app_id TEXT COLLATE NOCASE, + description TEXT, + version TEXT NOT NULL, + licenses JSONB, + download_url TEXT NOT NULL, + size BIGINT, + ghcr_pkg TEXT, + ghcr_size BIGINT, + ghcr_blob TEXT, + ghcr_url TEXT, + bsum TEXT, + icon TEXT, + desktop TEXT, + appstream TEXT, + homepages JSONB, + notes JSONB, + source_urls JSONB, + categories JSONB, + build_id TEXT, + build_date TEXT, + build_action TEXT, + build_script TEXT, + build_log TEXT, + provides JSONB, + snapshots JSONB, + replaces JSONB, + soar_syms BOOLEAN NOT NULL DEFAULT false, + desktop_integration BOOLEAN, + portable BOOLEAN, + extra JSONB, + files JSONB +); + +INSERT INTO packages_new SELECT + id, pkg_id, pkg_family, pkg_name, pkg_type, app_id, description, version, + licenses, download_url, size, ghcr_pkg, ghcr_size, ghcr_blob, ghcr_url, + bsum, icon, desktop, appstream, homepages, notes, source_urls, categories, + build_id, build_date, build_action, build_script, build_log, provides, + snapshots, replaces, soar_syms, desktop_integration, portable, NULL, NULL +FROM packages; + +DROP TABLE packages; +ALTER TABLE packages_new RENAME TO packages; + +-- NULLs are collapsed first: SQLite treats every NULL as distinct, so an +-- id-less package would insert a duplicate on every sync instead of conflicting. +CREATE UNIQUE INDEX packages_identity + ON packages (COALESCE(pkg_id, ''), COALESCE(pkg_family, ''), pkg_name, version); + +PRAGMA foreign_keys = ON; diff --git a/crates/soar-db/src/migration.rs b/crates/soar-db/src/migration.rs index 5c2fc8376..343ab005b 100644 --- a/crates/soar-db/src/migration.rs +++ b/crates/soar-db/src/migration.rs @@ -110,10 +110,6 @@ pub fn migrate_json_to_jsonb( "UPDATE packages SET source_urls = jsonb(source_urls) WHERE {}", json_condition("source_urls") ), - format!( - "UPDATE packages SET tags = jsonb(tags) WHERE {}", - json_condition("tags") - ), format!( "UPDATE packages SET categories = jsonb(categories) WHERE {}", json_condition("categories") diff --git a/crates/soar-db/src/models/core.rs b/crates/soar-db/src/models/core.rs index 430d06884..cd987d9d9 100644 --- a/crates/soar-db/src/models/core.rs +++ b/crates/soar-db/src/models/core.rs @@ -7,8 +7,9 @@ use crate::{json_vec, models::types::PackageProvide, schema::core::*}; pub struct Package { pub id: i32, pub repo_name: String, - pub pkg_id: String, + pub pkg_id: Option, pub pkg_name: String, + pub pkg_family: Option, pub pkg_type: Option, pub version: String, pub size: i64, @@ -28,8 +29,9 @@ impl Queryable for Package { type Row = ( i32, String, + Option, String, - String, + Option, Option, String, i64, @@ -51,19 +53,20 @@ impl Queryable for Package { repo_name: row.1, pkg_id: row.2, pkg_name: row.3, - pkg_type: row.4, - version: row.5, - size: row.6, - checksum: row.7, - installed_path: row.8, - installed_date: row.9, - profile: row.10, - pinned: row.11, - is_installed: row.12, - detached: row.13, - unlinked: row.14, - provides: json_vec!(row.15), - install_patterns: json_vec!(row.16), + pkg_family: row.4, + pkg_type: row.5, + version: row.6, + size: row.7, + checksum: row.8, + installed_path: row.9, + installed_date: row.10, + profile: row.11, + pinned: row.12, + is_installed: row.13, + detached: row.14, + unlinked: row.15, + provides: json_vec!(row.16), + install_patterns: json_vec!(row.17), }) } } @@ -84,8 +87,9 @@ pub struct PortablePackage { #[diesel(table_name = packages)] pub struct NewPackage<'a> { pub repo_name: &'a str, - pub pkg_id: &'a str, + pub pkg_id: Option<&'a str>, pub pkg_name: &'a str, + pub pkg_family: Option<&'a str>, pub pkg_type: Option<&'a str>, pub version: &'a str, pub size: i64, diff --git a/crates/soar-db/src/models/metadata.rs b/crates/soar-db/src/models/metadata.rs index dee3beb31..dbf3b63b7 100644 --- a/crates/soar-db/src/models/metadata.rs +++ b/crates/soar-db/src/models/metadata.rs @@ -1,16 +1,19 @@ use diesel::{prelude::*, sqlite::Sqlite}; use serde_json::Value; -use crate::{json_vec, models::types::PackageProvide, schema::metadata::*}; +use crate::{ + json_vec, + models::types::{PackageExtra, PackageFile, PackageProvide}, + schema::metadata::*, +}; #[derive(Debug, Clone, Selectable)] pub struct Package { pub id: i32, - pub pkg_id: String, + pub pkg_id: Option, pub pkg_name: String, pub pkg_family: Option, pub pkg_type: Option, - pub pkg_webpage: Option, pub app_id: Option, pub description: Option, pub version: String, @@ -28,7 +31,6 @@ pub struct Package { pub homepages: Option>, pub notes: Option>, pub source_urls: Option>, - pub tags: Option>, pub categories: Option>, pub build_id: Option, pub build_date: Option, @@ -41,14 +43,18 @@ pub struct Package { pub soar_syms: bool, pub desktop_integration: Option, pub portable: Option, + /// Executables inside the artifact, as source path -> installed name. + /// Pinned side files installed alongside the artifact. + pub extra: Option>, + /// What the package takes out of its artifact. + pub files: Option>, } impl Queryable for Package { type Row = ( i32, - String, - String, Option, + String, Option, Option, Option, @@ -69,7 +75,6 @@ impl Queryable for Package { Option, Option, Option, - Option, Option, Option, Option, @@ -81,6 +86,8 @@ impl Queryable for Package { bool, Option, Option, + Option, + Option, ); fn build(row: Self::Row) -> diesel::deserialize::Result { @@ -90,37 +97,37 @@ impl Queryable for Package { pkg_name: row.2, pkg_family: row.3, pkg_type: row.4, - pkg_webpage: row.5, - app_id: row.6, - description: row.7, - version: row.8, - licenses: json_vec!(row.9), - download_url: row.10, - size: row.11, - ghcr_pkg: row.12, - ghcr_size: row.13, - ghcr_blob: row.14, - ghcr_url: row.15, - bsum: row.16, - icon: row.17, - desktop: row.18, - appstream: row.19, - homepages: json_vec!(row.20), - notes: json_vec!(row.21), - source_urls: json_vec!(row.22), - tags: json_vec!(row.23), - categories: json_vec!(row.24), - build_id: row.25, - build_date: row.26, - build_action: row.27, - build_script: row.28, - build_log: row.29, - provides: json_vec!(row.30), - snapshots: json_vec!(row.31), - replaces: json_vec!(row.32), - soar_syms: row.33, - desktop_integration: row.34, - portable: row.35, + app_id: row.5, + description: row.6, + version: row.7, + licenses: json_vec!(row.8), + download_url: row.9, + size: row.10, + ghcr_pkg: row.11, + ghcr_size: row.12, + ghcr_blob: row.13, + ghcr_url: row.14, + bsum: row.15, + icon: row.16, + desktop: row.17, + appstream: row.18, + homepages: json_vec!(row.19), + notes: json_vec!(row.20), + source_urls: json_vec!(row.21), + categories: json_vec!(row.22), + build_id: row.23, + build_date: row.24, + build_action: row.25, + build_script: row.26, + build_log: row.27, + provides: json_vec!(row.28), + snapshots: json_vec!(row.29), + replaces: json_vec!(row.30), + soar_syms: row.31, + desktop_integration: row.32, + portable: row.33, + extra: json_vec!(row.34), + files: json_vec!(row.35), }) } } @@ -131,7 +138,8 @@ impl Queryable for Package { #[diesel(table_name = packages)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] pub struct PackageListing { - pub pkg_id: String, + pub pkg_id: Option, + pub pkg_family: Option, pub pkg_name: String, pub pkg_type: Option, pub version: String, @@ -145,7 +153,7 @@ pub struct PackageListing { pub struct FuzzyCandidate { pub id: i32, pub pkg_name: String, - pub pkg_id: String, + pub pkg_id: Option, pub description: Option, } @@ -186,11 +194,10 @@ pub struct PackageMaintainer { #[derive(Default, Insertable)] #[diesel(table_name = packages)] pub struct NewPackage<'a> { - pub pkg_id: &'a str, + pub pkg_id: Option<&'a str>, pub pkg_name: &'a str, pub pkg_family: Option<&'a str>, pub pkg_type: Option<&'a str>, - pub pkg_webpage: Option<&'a str>, pub app_id: Option<&'a str>, pub description: Option<&'a str>, pub version: &'a str, @@ -208,7 +215,6 @@ pub struct NewPackage<'a> { pub homepages: Option, pub notes: Option, pub source_urls: Option, - pub tags: Option, pub categories: Option, pub build_id: Option<&'a str>, pub build_date: Option<&'a str>, @@ -221,6 +227,8 @@ pub struct NewPackage<'a> { pub soar_syms: bool, pub desktop_integration: Option, pub portable: Option, + pub extra: Option, + pub files: Option, } #[derive(Default, Insertable)] diff --git a/crates/soar-db/src/models/types.rs b/crates/soar-db/src/models/types.rs index e75a17dd8..f556ad740 100644 --- a/crates/soar-db/src/models/types.rs +++ b/crates/soar-db/src/models/types.rs @@ -137,3 +137,30 @@ mod tests { assert!(!PackageProvide::from_string("@../evil").is_safe()); } } + +/// One file the package installs out of its artifact. +/// +/// `to` is a path inside the package directory, so where it lands says what it +/// is. An empty `source` means the artifact is itself the file. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct PackageFile { + #[serde(default)] + pub source: String, + pub to: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub alias: Vec, +} + +/// A side file installed alongside the artifact, pinned by hash. +/// +/// Exists because some artifacts are a bare binary with no room for a licence, +/// and a package manager that redistributes them still owes one. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct PackageExtra { + pub url: String, + pub to: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub blake3: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sha256: Option, +} diff --git a/crates/soar-db/src/repository/core.rs b/crates/soar-db/src/repository/core.rs index baf6ee821..2f90bb1d3 100644 --- a/crates/soar-db/src/repository/core.rs +++ b/crates/soar-db/src/repository/core.rs @@ -1,6 +1,10 @@ //! Core database repository for installed packages. -use diesel::{prelude::*, sql_types::Bool, sqlite::Sqlite}; +use diesel::{ + prelude::*, + sql_types::{Bool, Nullable}, + sqlite::Sqlite, +}; use crate::{ models::{ @@ -10,6 +14,35 @@ use crate::{ schema::core::{packages, portable_package}, }; +/// An installed row reduced to what identifies it: id, repository, package id, +/// family and version. +type IdentityRow = (i32, String, Option, Option, String); + +/// Matches a row's package id, including rows that have none. +/// +/// A repository publishing the declarative format produces no package id, so +/// `pkg_id = ?` would never match those rows and `pkg_id != ?` would never +/// exclude them. Asking for no id has to mean the row has none either. +fn match_pkg_id( + pkg_id: Option<&str>, +) -> Box>> { + match pkg_id { + Some(id) => Box::new(packages::pkg_id.eq(id.to_string())), + None => Box::new(packages::pkg_id.is_null().nullable()), + } +} + +/// Same as [`match_pkg_id`] for the family, so cleaning up one package's old +/// versions cannot reach another package that merely shares its name. +fn match_pkg_family( + pkg_family: Option<&str>, +) -> Box>> { + match pkg_family { + Some(family) => Box::new(packages::pkg_family.eq(family.to_string())), + None => Box::new(packages::pkg_family.is_null().nullable()), + } +} + /// Sort direction for queries. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SortDirection { @@ -27,8 +60,9 @@ pub type NewInstalledPackage<'a> = NewPackage<'a>; pub struct InstalledPackageWithPortable { pub id: i32, pub repo_name: String, - pub pkg_id: String, + pub pkg_id: Option, pub pkg_name: String, + pub pkg_family: Option, pub pkg_type: Option, pub version: String, pub size: i64, @@ -56,6 +90,7 @@ impl From<(Package, Option)> for InstalledPackageWithPortable { repo_name: pkg.repo_name, pkg_id: pkg.pkg_id, pkg_name: pkg.pkg_name, + pkg_family: pkg.pkg_family, pkg_type: pkg.pkg_type, version: pkg.version, size: pkg.size, @@ -173,15 +208,27 @@ impl CoreRepository { conn: &mut SqliteConnection, repo_name: &str, pkg_name: &str, - pkg_id: &str, + pkg_id: Option<&str>, + pkg_family: Option<&str>, version: &str, ) -> QueryResult> { - let result: Option<(Package, Option)> = packages::table + // The boxed predicate is typed for the bare table, so the join builds + // its own two-branch filter. + let mut query = packages::table .left_join(portable_package::table) .filter(packages::repo_name.eq(repo_name)) .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.eq(pkg_id)) .filter(packages::version.eq(version)) + .into_boxed(); + query = match pkg_id { + Some(id) => query.filter(packages::pkg_id.eq(id.to_string())), + None => query.filter(packages::pkg_id.is_null()), + }; + query = match pkg_family { + Some(family) => query.filter(packages::pkg_family.eq(family.to_string())), + None => query.filter(packages::pkg_family.is_null()), + }; + let result: Option<(Package, Option)> = query .select((Package::as_select(), Option::::as_select())) .first(conn) .optional()?; @@ -244,8 +291,12 @@ impl CoreRepository { } query + // NULLs are collapsed first: concatenating one yields NULL, and + // COUNT(DISTINCT) skips it, so every package without an id went + // uncounted. .select(sql::( - "COUNT(DISTINCT pkg_id || '\x00' || pkg_name)", + "COUNT(DISTINCT COALESCE(pkg_id, '') || '\x00' \ + || COALESCE(pkg_family, '') || '\x00' || pkg_name)", )) .first(conn) } @@ -296,18 +347,51 @@ impl CoreRepository { Ok(results.into_iter().map(Into::into).collect()) } - /// Finds installed packages by name, excluding specific pkg_id and version. + /// Finds installed packages of the same name that are not this one. + /// + /// The same version under a different id and the same id at a different + /// version are both other packages, so neither condition alone may be + /// required: only the row matching on both is this package itself. pub fn find_alternates( conn: &mut SqliteConnection, pkg_name: &str, - exclude_pkg_id: &str, + exclude_pkg_id: Option<&str>, + pkg_family: Option<&str>, exclude_version: &str, ) -> QueryResult> { - let results: Vec<(Package, Option)> = packages::table + let mut query = packages::table .left_join(portable_package::table) .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.ne(exclude_pkg_id)) - .filter(packages::version.ne(exclude_version)) + .into_boxed(); + // Same family only, matching how old versions are found: another + // family sharing the name is a different package, not an alternate. + query = match pkg_family { + Some(family) => query.filter(packages::pkg_family.eq(family.to_string())), + None => query.filter(packages::pkg_family.is_null()), + }; + + // An id-less row differs from one that carries an id, and SQL answers + // `pkg_id != ?` with NULL rather than true in both directions, so + // neither case can be left to the comparison. + let others = match exclude_pkg_id { + Some(id) => { + query.filter( + packages::pkg_id + .is_null() + .or(packages::pkg_id.ne(id.to_string())) + .or(packages::version.ne(exclude_version)), + ) + } + None => { + query.filter( + packages::pkg_id + .is_not_null() + .or(packages::version.ne(exclude_version)), + ) + } + }; + + let results: Vec<(Package, Option)> = others .select((Package::as_select(), Option::::as_select())) .load(conn)?; @@ -317,11 +401,11 @@ impl CoreRepository { /// Finds an installed package by pkg_id and repo_name. pub fn find_by_pkg_id_and_repo( conn: &mut SqliteConnection, - pkg_id: &str, + pkg_id: Option<&str>, repo_name: &str, ) -> QueryResult> { packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) .filter(packages::repo_name.eq(repo_name)) .select(Package::as_select()) .first(conn) @@ -331,12 +415,12 @@ impl CoreRepository { /// Finds an installed package by pkg_id, pkg_name, and repo_name. pub fn find_by_pkg_id_name_and_repo( conn: &mut SqliteConnection, - pkg_id: &str, + pkg_id: Option<&str>, pkg_name: &str, repo_name: &str, ) -> QueryResult> { packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .select(Package::as_select()) @@ -370,7 +454,7 @@ impl CoreRepository { conn: &mut SqliteConnection, repo_name: &str, pkg_name: &str, - pkg_id: &str, + pkg_id: Option<&str>, version: &str, size: i64, provides: Option>, @@ -383,7 +467,7 @@ impl CoreRepository { packages::table .filter(packages::repo_name.eq(repo_name)) .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) .filter(packages::version.eq(version)) .filter(packages::is_installed.eq(false)), ) @@ -422,33 +506,79 @@ impl CoreRepository { pub fn unlink_others( conn: &mut SqliteConnection, pkg_name: &str, - keep_pkg_id: &str, - keep_version: &str, + keep_repo_name: &str, + keep_pkg_id: Option<&str>, + keep_pkg_family: Option<&str>, + keep_version: Option<&str>, ) -> QueryResult { - diesel::update( - packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter( - packages::pkg_id - .ne(keep_pkg_id) - .or(packages::version.ne(keep_version)), - ), - ) - .set(packages::unlinked.eq(true)) - .execute(conn) + // The row to keep is identified in full. A SQL predicate cannot do it: + // `pkg_id IS NULL` matches every id-less row, which now includes the + // package being installed, so it would unlink itself. + // + // Switching between versions of one package passes no version, since + // there every other version is exactly what has to be unlinked. + let stale = Self::rows_other_than( + conn, + pkg_name, + keep_repo_name, + keep_pkg_id, + keep_pkg_family, + keep_version, + )?; + if stale.is_empty() { + return Ok(0); + } + diesel::update(packages::table.filter(packages::id.eq_any(stale))) + .set(packages::unlinked.eq(true)) + .execute(conn) + } + + /// Ids of installed rows sharing `pkg_name` but not the given identity. + fn rows_other_than( + conn: &mut SqliteConnection, + pkg_name: &str, + keep_repo_name: &str, + keep_pkg_id: Option<&str>, + keep_pkg_family: Option<&str>, + keep_version: Option<&str>, + ) -> QueryResult> { + let rows: Vec = packages::table + .filter(packages::pkg_name.eq(pkg_name)) + .select(( + packages::id, + packages::repo_name, + packages::pkg_id, + packages::pkg_family, + packages::version, + )) + .load(conn)?; + Ok(rows + .into_iter() + .filter(|(_, repo_name, pkg_id, pkg_family, version)| { + // The repository is part of the identity: the same package + // from two of them is two installs, and only one can own the + // command. + let same = repo_name == keep_repo_name + && pkg_id.as_deref() == keep_pkg_id + && pkg_family.as_deref() == keep_pkg_family + && keep_version.is_none_or(|v| version == v); + !same + }) + .map(|(id, ..)| id) + .collect()) } /// Updates the pkg_id for packages matching repo_name and old pkg_id. pub fn update_pkg_id( conn: &mut SqliteConnection, repo_name: &str, - old_pkg_id: &str, + old_pkg_id: Option<&str>, new_pkg_id: &str, ) -> QueryResult { diesel::update( packages::table .filter(packages::repo_name.eq(repo_name)) - .filter(packages::pkg_id.eq(old_pkg_id)), + .filter(match_pkg_id(old_pkg_id)), ) .set(packages::pkg_id.eq(new_pkg_id)) .execute(conn) @@ -463,13 +593,13 @@ impl CoreRepository { /// Used to check if we can resume a partial install. pub fn has_pending_install( conn: &mut SqliteConnection, - pkg_id: &str, + pkg_id: Option<&str>, pkg_name: &str, repo_name: &str, version: &str, ) -> QueryResult { let count: i64 = packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .filter(packages::version.eq(version)) @@ -483,12 +613,12 @@ impl CoreRepository { /// Used to clean up orphaned partial installs before starting a new install. pub fn delete_pending_installs( conn: &mut SqliteConnection, - pkg_id: &str, + pkg_id: Option<&str>, pkg_name: &str, repo_name: &str, ) -> QueryResult> { let paths: Vec = packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .filter(packages::is_installed.eq(false)) @@ -497,7 +627,7 @@ impl CoreRepository { diesel::delete( packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .filter(packages::is_installed.eq(false)), @@ -578,13 +708,15 @@ impl CoreRepository { /// If `force` is true, includes pinned packages. Otherwise only unpinned packages. pub fn get_old_package_paths( conn: &mut SqliteConnection, - pkg_id: &str, + pkg_id: Option<&str>, + pkg_family: Option<&str>, pkg_name: &str, repo_name: &str, force: bool, ) -> QueryResult> { let latest: Option<(i32, String)> = packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) + .filter(match_pkg_family(pkg_family)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .order(packages::id.desc()) @@ -597,7 +729,8 @@ impl CoreRepository { }; let query = packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) + .filter(match_pkg_family(pkg_family)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .filter(packages::id.ne(latest_id)) @@ -619,13 +752,15 @@ impl CoreRepository { /// If `force` is true, deletes pinned packages too. Otherwise only unpinned packages. pub fn delete_old_packages( conn: &mut SqliteConnection, - pkg_id: &str, + pkg_id: Option<&str>, + pkg_family: Option<&str>, pkg_name: &str, repo_name: &str, force: bool, ) -> QueryResult { let latest_id: Option = packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) + .filter(match_pkg_family(pkg_family)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .order(packages::id.desc()) @@ -645,7 +780,8 @@ impl CoreRepository { }; let query = packages::table - .filter(packages::pkg_id.eq(pkg_id)) + .filter(match_pkg_id(pkg_id)) + .filter(match_pkg_family(pkg_family)) .filter(packages::pkg_name.eq(pkg_name)) .filter(packages::repo_name.eq(repo_name)) .filter(packages::id.ne(latest_id)) @@ -654,59 +790,13 @@ impl CoreRepository { diesel::delete(query).execute(conn) } - /// Unlinks all packages with a given name except those matching pkg_id and checksum. - /// Used when switching between alternate package versions. - pub fn unlink_others_by_checksum( - conn: &mut SqliteConnection, - pkg_name: &str, - keep_pkg_id: &str, - keep_checksum: Option<&str>, - ) -> QueryResult { - if let Some(checksum) = keep_checksum { - diesel::update( - packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.ne(keep_pkg_id)) - .filter(packages::checksum.ne(checksum)), - ) - .set(packages::unlinked.eq(true)) - .execute(conn) - } else { - diesel::update( - packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.ne(keep_pkg_id)), - ) - .set(packages::unlinked.eq(true)) - .execute(conn) - } - } - - /// Links a package by pkg_name, pkg_id, and checksum. - /// Used when switching to an alternate package version. - pub fn link_by_checksum( - conn: &mut SqliteConnection, - pkg_name: &str, - pkg_id: &str, - checksum: Option<&str>, - ) -> QueryResult { - if let Some(checksum) = checksum { - diesel::update( - packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.eq(pkg_id)) - .filter(packages::checksum.eq(checksum)), - ) - .set(packages::unlinked.eq(false)) - .execute(conn) - } else { - diesel::update( - packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.eq(pkg_id)), - ) + /// Mark one installed row as the linked one, by its own id. + /// + /// Matching on a checksum cannot do this: two repositories shipping the + /// same build share it, so linking one would link both. + pub fn link_by_row_id(conn: &mut SqliteConnection, id: i32) -> QueryResult { + diesel::update(packages::table.filter(packages::id.eq(id))) .set(packages::unlinked.eq(false)) .execute(conn) - } } } diff --git a/crates/soar-db/src/repository/metadata.rs b/crates/soar-db/src/repository/metadata.rs index cc1854a81..9dd19ef24 100644 --- a/crates/soar-db/src/repository/metadata.rs +++ b/crates/soar-db/src/repository/metadata.rs @@ -6,7 +6,10 @@ use diesel::{dsl::sql, prelude::*, sql_types::Text}; use regex::Regex; use serde_json::json; use soar_registry::RemotePackage; -use soar_utils::path::is_safe_component; +use soar_utils::{ + path::is_safe_component, + version::{compare_versions, is_newer}, +}; use tracing::{debug, trace, warn}; /// Regex for extracting name and contact from maintainer string format "Name (contact)". @@ -31,6 +34,24 @@ struct PkgIdOnly { pkg_id: String, } +/// Narrow candidates to those carrying `pkg_id`, unless none of them do. +/// +/// An id recorded at install time may have disappeared from the metadata, +/// since a repository that moved to the declarative format publishes none. +/// Demanding a match there would report the package as up to date forever, +/// while ignoring the id altogether lets a different package of the same name +/// pass as a newer build of this one. +pub fn narrow_by_pkg_id(candidates: Vec, pkg_id: Option<&str>) -> Vec { + let Some(id) = pkg_id else { + return candidates; + }; + let matches_id = |p: &Package| p.pkg_id.as_deref() == Some(id); + if !candidates.iter().any(matches_id) { + return candidates; + } + candidates.into_iter().filter(matches_id).collect() +} + /// Repository for package metadata operations. pub struct MetadataRepository; @@ -285,7 +306,9 @@ impl MetadataRepository { conn: &mut SqliteConnection, pkg_id: &str, ) -> QueryResult> { - let query = "SELECT pkg_id FROM packages WHERE EXISTS \ + // Only rows that have an id: the projection cannot hold a NULL, and a + // package without an id has nothing to answer with anyway. + let query = "SELECT pkg_id FROM packages WHERE pkg_id IS NOT NULL AND EXISTS \ (SELECT 1 FROM json_each(replaces) WHERE json_each.value = ?) LIMIT 1"; diesel::sql_query(query) @@ -390,10 +413,12 @@ impl MetadataRepository { } /// Finds packages with flexible filtering using Diesel DSL. + #[allow(clippy::too_many_arguments)] pub fn find_filtered( conn: &mut SqliteConnection, pkg_name: Option<&str>, pkg_id: Option<&str>, + pkg_family: Option<&str>, version: Option<&str>, limit: Option, sort_by_name: Option, @@ -403,6 +428,11 @@ impl MetadataRepository { if let Some(name) = pkg_name { query = query.filter(packages::pkg_name.eq(name)); } + if let Some(family) = pkg_family { + if family != "all" { + query = query.filter(packages::pkg_family.eq(family)); + } + } if let Some(id) = pkg_id { if id != "all" { query = query.filter(packages::pkg_id.eq(id)); @@ -432,41 +462,35 @@ impl MetadataRepository { pub fn find_newer_version( conn: &mut SqliteConnection, pkg_name: &str, - pkg_id: &str, + pkg_id: Option<&str>, + pkg_family: Option<&str>, current_version: &str, ) -> QueryResult> { trace!( pkg_name = pkg_name, - pkg_id = pkg_id, current_version = current_version, "checking for newer version" ); - // Handle both regular versions and HEAD- versions - let head_version = if current_version.starts_with("HEAD-") && current_version.len() > 14 { - current_version[14..].to_string() - } else { - String::new() - }; + // Ordering cannot be left to SQL: a string comparison puts 10 below 9 + // and the rebuild suffix in 1.14.0-1 below 1.14.0. Candidates are + // loaded and compared segment-wise instead. + let mut query = packages::table + .into_boxed() + .filter(packages::pkg_name.eq(pkg_name)); + // Narrowed by family only when the install records one. An older + // install has none, and demanding a match would report it as up to + // date forever. + if let Some(family) = pkg_family { + query = query.filter(packages::pkg_family.eq(family.to_string())); + } + let candidates = narrow_by_pkg_id(query.select(Package::as_select()).load(conn)?, pkg_id); - let result = packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.eq(pkg_id)) - .filter( - sql::("(version > ") - .bind::(current_version) - .sql(" OR (version LIKE 'HEAD-%' AND substr(version, 15) > ") - .bind::(&head_version) - .sql("))"), - ) - .order(packages::version.desc()) - .select(Package::as_select()) - .first(conn) - .optional(); + let result: QueryResult> = Ok(candidates + .into_iter() + .filter(|p| is_newer(&p.version, current_version)) + .max_by(|a, b| compare_versions(&a.version, &b.version))); if let Ok(Some(ref p)) = result { - debug!( - "newer version available: {}#{} -> {}", - pkg_name, pkg_id, p.version - ); + debug!("newer version available: {} -> {}", pkg_name, p.version); } result } @@ -484,7 +508,7 @@ impl MetadataRepository { conn: &mut SqliteConnection, metadata: &[RemotePackage], repo_name: &str, - ) -> QueryResult<()> { + ) -> QueryResult { debug!( repo_name = repo_name, count = metadata.len(), @@ -502,23 +526,29 @@ impl MetadataRepository { .execute(conn)?; trace!(repo_name = repo_name, "repository record upserted"); + let mut imported = 0usize; for package in metadata { - Self::insert_remote_package(conn, package)?; + if Self::insert_remote_package(conn, package)? { + imported += 1; + } } debug!( repo_name = repo_name, - count = metadata.len(), + offered = metadata.len(), + imported, "package import completed" ); - Ok(()) + Ok(imported) }) } /// Inserts a single remote package. + /// Returns whether the package was accepted; rejected entries are skipped + /// rather than failing the whole import. fn insert_remote_package( conn: &mut SqliteConnection, package: &RemotePackage, - ) -> QueryResult<()> { + ) -> QueryResult { trace!( pkg_id = package.pkg_id, pkg_name = package.pkg_name, @@ -526,15 +556,20 @@ impl MetadataRepository { "inserting remote package" ); + // Stored as absent rather than invented: a repository whose names are + // already unique has no id to give, and inventing one from the name + // makes a fabricated value indistinguishable from a published one. + let pkg_id = package.pkg_id.as_deref().filter(|s| !s.is_empty()); + // pkg_name and pkg_id are joined into the install dir and interpolated // into resource paths, so a name with separators or '..' would escape it. - if !is_safe_component(&package.pkg_name) || !is_safe_component(&package.pkg_id) { + if !is_safe_component(&package.pkg_name) || pkg_id.is_some_and(|id| !is_safe_component(id)) + { warn!( - pkg_id = package.pkg_id, pkg_name = package.pkg_name, "skipping package with unsafe path component in pkg_name/pkg_id" ); - return Ok(()); + return Ok(false); } let provides = package.provides.as_ref().map(|vec| { @@ -556,17 +591,16 @@ impl MetadataRepository { }); let new_package = NewPackage { - pkg_id: &package.pkg_id, + pkg_id, pkg_name: &package.pkg_name, pkg_family: package.pkg_family.as_deref(), pkg_type: package.pkg_type.as_deref(), - pkg_webpage: package.pkg_webpage.as_deref(), app_id: package.app_id.as_deref(), description: Some(&package.description), version: &package.version, licenses: Some(json!(package.licenses)), download_url: &package.download_url, - size: package.size_raw.map(|s| s as i64), + size: package.size_raw.or(package.size).map(|s| s as i64), ghcr_pkg: package.ghcr_pkg.as_deref(), ghcr_size: package.ghcr_size_raw.map(|s| s as i64), ghcr_blob: package.ghcr_blob.as_deref(), @@ -578,7 +612,6 @@ impl MetadataRepository { homepages: Some(json!(package.homepages)), notes: Some(json!(package.notes)), source_urls: Some(json!(package.src_urls)), - tags: Some(json!(&package.tags)), categories: Some(json!(package.categories)), build_id: package.build_id.as_deref(), build_date: package.build_date.as_deref(), @@ -591,17 +624,18 @@ impl MetadataRepository { soar_syms: package.soar_syms.unwrap_or(false), desktop_integration: package.desktop_integration, portable: package.portable, + extra: package.extra.as_ref().map(|e| json!(e)), + files: package.files.as_ref().map(|f| json!(f)), }; let inserted = diesel::insert_into(packages::table) .values(&new_package) - .on_conflict((packages::pkg_id, packages::pkg_name, packages::version)) - .do_nothing() + .on_conflict_do_nothing() .execute(conn)?; if inserted == 0 { - trace!(pkg_id = package.pkg_id, "package already exists, skipping"); - return Ok(()); + trace!(pkg_id, "package already exists, skipping"); + return Ok(false); } let package_id = Self::last_insert_id(conn)?; @@ -615,7 +649,7 @@ impl MetadataRepository { } } - Ok(()) + Ok(true) } /// Extracts name and contact from maintainer string format "Name (contact)". diff --git a/crates/soar-db/src/schema/core.rs b/crates/soar-db/src/schema/core.rs index ae55e180f..512c755d1 100644 --- a/crates/soar-db/src/schema/core.rs +++ b/crates/soar-db/src/schema/core.rs @@ -2,8 +2,9 @@ diesel::table! { packages (id) { id -> Integer, repo_name -> Text, - pkg_id -> Text, + pkg_id -> Nullable, pkg_name -> Text, + pkg_family -> Nullable, pkg_type -> Nullable, version -> Text, size -> BigInt, diff --git a/crates/soar-db/src/schema/metadata.rs b/crates/soar-db/src/schema/metadata.rs index 72e513b85..462cd3c11 100644 --- a/crates/soar-db/src/schema/metadata.rs +++ b/crates/soar-db/src/schema/metadata.rs @@ -17,11 +17,10 @@ diesel::table! { diesel::table! { packages (id) { id -> Integer, - pkg_id -> Text, + pkg_id -> Nullable, pkg_name -> Text, pkg_family -> Nullable, pkg_type -> Nullable, - pkg_webpage -> Nullable, app_id -> Nullable, description -> Nullable, version -> Text, @@ -39,7 +38,6 @@ diesel::table! { homepages -> Nullable, notes -> Nullable, source_urls -> Nullable, - tags -> Nullable, categories -> Nullable, build_id -> Nullable, build_date -> Nullable, @@ -52,6 +50,8 @@ diesel::table! { soar_syms -> Bool, desktop_integration -> Nullable, portable -> Nullable, + extra -> Nullable, + files -> Nullable, } } diff --git a/crates/soar-dl/src/download.rs b/crates/soar-dl/src/download.rs index 4f3e37893..594f44521 100644 --- a/crates/soar-dl/src/download.rs +++ b/crates/soar-dl/src/download.rs @@ -322,16 +322,33 @@ impl Download { remove_resume(&output_path)?; + // Extraction is driven by what the file actually is, not by what the + // caller guessed it would be. compak detects by magic number, so an + // ELF (an AppImage, say) is never mistaken for an archive even when + // extraction was requested. if self.extract { - let extract_dir = self.extract_to.unwrap_or_else(|| { - output_path - .parent() - .map(PathBuf::from) - .unwrap_or_else(|| PathBuf::from(".")) - }); - debug!(archive = %output_path.display(), dest = %extract_dir.display(), "extracting archive"); - - compak::extract_archive(&output_path, &extract_dir)?; + match compak::detect_from_file(&output_path) { + Ok(format) => { + let extract_dir = self.extract_to.unwrap_or_else(|| { + output_path + .parent() + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from(".")) + }); + debug!(archive = %output_path.display(), dest = %extract_dir.display(), + ?format, "extracting archive"); + // A failure here fails the install. A bare .gz of a single + // file shares its magic number with the tar-wrapped form, + // but detection decompresses far enough to tell the two + // apart, so what is left is a download that really is + // broken, and swallowing that installs an empty package. + compak::extract_archive(&output_path, &extract_dir)?; + } + Err(_) => { + trace!(path = %output_path.display(), + "not an archive, installing as-is"); + } + } } debug!(path = %output_path.display(), "download completed successfully"); diff --git a/crates/soar-events/src/event.rs b/crates/soar-events/src/event.rs index 4f0682822..6216daed5 100644 --- a/crates/soar-events/src/event.rs +++ b/crates/soar-events/src/event.rs @@ -7,14 +7,12 @@ pub enum SoarEvent { DownloadStarting { op_id: OperationId, pkg_name: String, - pkg_id: String, total: u64, }, /// Download is resuming from a previous checkpoint. DownloadResuming { op_id: OperationId, pkg_name: String, - pkg_id: String, current: u64, total: u64, }, @@ -22,7 +20,6 @@ pub enum SoarEvent { DownloadProgress { op_id: OperationId, pkg_name: String, - pkg_id: String, current: u64, total: u64, }, @@ -30,59 +27,50 @@ pub enum SoarEvent { DownloadComplete { op_id: OperationId, pkg_name: String, - pkg_id: String, total: u64, }, /// Download error, retrying. DownloadRetry { op_id: OperationId, pkg_name: String, - pkg_id: String, }, /// Download permanently failed after retries. DownloadAborted { op_id: OperationId, pkg_name: String, - pkg_id: String, }, /// Download recovered from an error. DownloadRecovered { op_id: OperationId, pkg_name: String, - pkg_id: String, }, /// Verification stage. Verifying { op_id: OperationId, pkg_name: String, - pkg_id: String, stage: VerifyStage, }, /// Install/extraction stage. Installing { op_id: OperationId, pkg_name: String, - pkg_id: String, stage: InstallStage, }, /// Package removal stage. Removing { op_id: OperationId, pkg_name: String, - pkg_id: String, stage: RemoveStage, }, /// Update check for a package. UpdateCheck { pkg_name: String, - pkg_id: String, status: UpdateCheckStatus, }, /// Old version cleanup after update. UpdateCleanup { op_id: OperationId, pkg_name: String, - pkg_id: String, old_version: String, stage: UpdateCleanupStage, }, @@ -90,7 +78,6 @@ pub enum SoarEvent { Hook { op_id: OperationId, pkg_name: String, - pkg_id: String, hook_name: String, stage: HookStage, }, @@ -98,27 +85,23 @@ pub enum SoarEvent { Running { op_id: OperationId, pkg_name: String, - pkg_id: String, stage: RunStage, }, /// Build stage (for source packages). Building { op_id: OperationId, pkg_name: String, - pkg_id: String, stage: BuildStage, }, /// Operation completed successfully. OperationComplete { op_id: OperationId, pkg_name: String, - pkg_id: String, }, /// Operation failed. OperationFailed { op_id: OperationId, pkg_name: String, - pkg_id: String, error: String, }, /// Repository sync progress. diff --git a/crates/soar-events/src/lib.rs b/crates/soar-events/src/lib.rs index 142c5be3e..7c57ee50f 100644 --- a/crates/soar-events/src/lib.rs +++ b/crates/soar-events/src/lib.rs @@ -31,20 +31,17 @@ mod tests { sink.emit(SoarEvent::DownloadStarting { op_id: 1, pkg_name: "test-pkg".to_string(), - pkg_id: "test-pkg-id".to_string(), total: 1024, }); sink.emit(SoarEvent::DownloadProgress { op_id: 1, pkg_name: "test-pkg".to_string(), - pkg_id: "test-pkg-id".to_string(), current: 512, total: 1024, }); sink.emit(SoarEvent::DownloadComplete { op_id: 1, pkg_name: "test-pkg".to_string(), - pkg_id: "test-pkg-id".to_string(), total: 1024, }); @@ -126,7 +123,6 @@ mod tests { sink.emit(SoarEvent::OperationComplete { op_id: 42, pkg_name: "pkg".to_string(), - pkg_id: "pkg-id".to_string(), }); assert_eq!(collector.len(), 1); } @@ -147,68 +143,57 @@ mod tests { collector.emit(SoarEvent::DownloadStarting { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), total: 100, }); collector.emit(SoarEvent::DownloadResuming { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), current: 50, total: 100, }); collector.emit(SoarEvent::DownloadProgress { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), current: 75, total: 100, }); collector.emit(SoarEvent::DownloadComplete { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), total: 100, }); collector.emit(SoarEvent::DownloadRetry { op_id: 2, pkg_name: "b".into(), - pkg_id: "b-id".into(), }); collector.emit(SoarEvent::DownloadAborted { op_id: 2, pkg_name: "b".into(), - pkg_id: "b-id".into(), }); collector.emit(SoarEvent::DownloadRecovered { op_id: 3, pkg_name: "c".into(), - pkg_id: "c-id".into(), }); // Verification collector.emit(SoarEvent::Verifying { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: VerifyStage::Checksum, }); collector.emit(SoarEvent::Verifying { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: VerifyStage::Signature, }); collector.emit(SoarEvent::Verifying { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: VerifyStage::Passed, }); collector.emit(SoarEvent::Verifying { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: VerifyStage::Failed("bad checksum".into()), }); @@ -216,49 +201,41 @@ mod tests { collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::Extracting, }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::ExtractingNested, }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::LinkingBinaries, }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::DesktopIntegration, }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::SetupPortable, }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::RecordingDatabase, }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::RunningHook("post_install".into()), }); collector.emit(SoarEvent::Installing { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), stage: InstallStage::Complete, }); @@ -266,43 +243,36 @@ mod tests { collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::RunningHook("pre_remove".into()), }); collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::UnlinkingBinaries, }); collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::UnlinkingDesktop, }); collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::UnlinkingIcons, }); collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::RemovingDirectory, }); collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::CleaningDatabase, }); collector.emit(SoarEvent::Removing { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), stage: RemoveStage::Complete { size_freed: Some(1024 * 1024), }, @@ -311,7 +281,6 @@ mod tests { // Update checks collector.emit(SoarEvent::UpdateCheck { pkg_name: "f".into(), - pkg_id: "f-id".into(), status: UpdateCheckStatus::Available { current_version: "1.0.0".into(), new_version: "2.0.0".into(), @@ -319,14 +288,12 @@ mod tests { }); collector.emit(SoarEvent::UpdateCheck { pkg_name: "g".into(), - pkg_id: "g-id".into(), status: UpdateCheckStatus::UpToDate { version: "1.0.0".into(), }, }); collector.emit(SoarEvent::UpdateCheck { pkg_name: "h".into(), - pkg_id: "h-id".into(), status: UpdateCheckStatus::Skipped { reason: "pinned".into(), }, @@ -336,14 +303,12 @@ mod tests { collector.emit(SoarEvent::UpdateCleanup { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), old_version: "1.0.0".into(), stage: UpdateCleanupStage::Removing, }); collector.emit(SoarEvent::UpdateCleanup { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), old_version: "1.0.0".into(), stage: UpdateCleanupStage::Complete { size_freed: Some(512), @@ -352,7 +317,6 @@ mod tests { collector.emit(SoarEvent::UpdateCleanup { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), old_version: "1.0.0".into(), stage: UpdateCleanupStage::Kept, }); @@ -361,21 +325,18 @@ mod tests { collector.emit(SoarEvent::Hook { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), hook_name: "pre_remove".into(), stage: HookStage::Starting, }); collector.emit(SoarEvent::Hook { op_id: 5, pkg_name: "e".into(), - pkg_id: "e-id".into(), hook_name: "pre_remove".into(), stage: HookStage::Complete, }); collector.emit(SoarEvent::Hook { op_id: 6, pkg_name: "i".into(), - pkg_id: "i-id".into(), hook_name: "post_install".into(), stage: HookStage::Failed { exit_code: Some(1), @@ -386,25 +347,21 @@ mod tests { collector.emit(SoarEvent::Running { op_id: 7, pkg_name: "j".into(), - pkg_id: "j-id".into(), stage: RunStage::CacheHit, }); collector.emit(SoarEvent::Running { op_id: 8, pkg_name: "k".into(), - pkg_id: "k-id".into(), stage: RunStage::Downloading, }); collector.emit(SoarEvent::Running { op_id: 7, pkg_name: "j".into(), - pkg_id: "j-id".into(), stage: RunStage::Executing, }); collector.emit(SoarEvent::Running { op_id: 7, pkg_name: "j".into(), - pkg_id: "j-id".into(), stage: RunStage::Complete { exit_code: 0, }, @@ -414,13 +371,11 @@ mod tests { collector.emit(SoarEvent::Building { op_id: 4, pkg_name: "d".into(), - pkg_id: "d-id".into(), stage: BuildStage::Sandboxing, }); collector.emit(SoarEvent::Building { op_id: 4, pkg_name: "d".into(), - pkg_id: "d-id".into(), stage: BuildStage::Running { command_index: 0, total_commands: 3, @@ -429,7 +384,6 @@ mod tests { collector.emit(SoarEvent::Building { op_id: 4, pkg_name: "d".into(), - pkg_id: "d-id".into(), stage: BuildStage::CommandComplete { command_index: 0, }, @@ -439,12 +393,10 @@ mod tests { collector.emit(SoarEvent::OperationComplete { op_id: 1, pkg_name: "a".into(), - pkg_id: "a-id".into(), }); collector.emit(SoarEvent::OperationFailed { op_id: 2, pkg_name: "b".into(), - pkg_id: "b-id".into(), error: "not found".into(), }); diff --git a/crates/soar-operations/src/apply.rs b/crates/soar-operations/src/apply.rs index 82b344c3a..ec96de015 100644 --- a/crates/soar-operations/src/apply.rs +++ b/crates/soar-operations/src/apply.rs @@ -52,10 +52,10 @@ pub async fn compute_diff( let diesel_db = ctx.diesel_core_db()?.clone(); let mut diff = ApplyDiff::default(); - let mut declared_keys: HashSet<(String, Option, Option)> = HashSet::new(); + let mut declared_keys: DeclaredKeys = HashSet::new(); for pkg in resolved { - declared_keys.insert((pkg.name.clone(), pkg.pkg_id.clone(), pkg.repo.clone())); + declared_keys.insert(declared_key(pkg)); let is_github_or_gitlab = pkg.github.is_some() || pkg.gitlab.is_some(); if is_github_or_gitlab || pkg.url.is_some() { @@ -70,7 +70,8 @@ pub async fn compute_diff( MetadataRepository::find_filtered( conn, Some(&pkg.name), - pkg.pkg_id.as_deref(), + declared_pkg_id(pkg), + pkg.family.as_deref(), pkg.version.as_deref(), None, Some(SortDirection::Asc), @@ -89,7 +90,8 @@ pub async fn compute_diff( let pkgs = MetadataRepository::find_filtered( conn, Some(&pkg.name), - pkg.pkg_id.as_deref(), + declared_pkg_id(pkg), + pkg.family.as_deref(), pkg.version.as_deref(), None, Some(SortDirection::Asc), @@ -118,7 +120,7 @@ pub async fn compute_diff( conn, Some(&metadata_pkg.repo_name), Some(&metadata_pkg.pkg_name), - Some(&metadata_pkg.pkg_id), + metadata_pkg.pkg_id.as_deref(), None, None, None, @@ -130,24 +132,25 @@ pub async fn compute_diff( .map(Into::into) .collect(); - let existing_install = installed_packages.into_iter().find(|ip| ip.is_installed); + // The query cannot narrow by family, so it is compared here. + let existing_install = installed_packages.into_iter().find(|ip| { + ip.is_installed && ip.pkg_family.as_deref() == metadata_pkg.pkg_family.as_deref() + }); if let Some(ref existing) = existing_install { let version_matches = pkg.version.as_ref().is_none_or(|v| existing.version == *v); if version_matches && existing.version == metadata_pkg.version { - diff.in_sync.push(format!( - "{}#{}@{}", - existing.pkg_name, existing.pkg_id, existing.version - )); + diff.in_sync + .push(format!("{}@{}", existing.pkg_name, existing.version)); } else if !existing.pinned || pkg.version.is_some() { let resolved_pkg = metadata_pkg.resolve(pkg.version.as_deref()); let target = create_install_target(pkg, resolved_pkg, Some(existing.clone())); diff.to_update.push((pkg.clone(), target)); } else { diff.in_sync.push(format!( - "{}#{}@{} (pinned)", - existing.pkg_name, existing.pkg_id, existing.version + "{}@{} (pinned)", + existing.pkg_name, existing.version )); } } else { @@ -178,11 +181,16 @@ pub async fn compute_diff( .collect(); for installed in all_installed { - let is_declared = declared_keys.iter().any(|(name, pkg_id, repo)| { + let is_declared = declared_keys.iter().any(|(name, pkg_id, family, repo)| { let name_matches = *name == installed.pkg_name; - let pkg_id_matches = pkg_id.as_ref().is_none_or(|id| *id == installed.pkg_id); + let pkg_id_matches = pkg_id + .as_deref() + .is_none_or(|id| Some(id) == installed.pkg_id.as_deref()); + let family_matches = family + .as_deref() + .is_none_or(|f| Some(f) == installed.pkg_family.as_deref()); let repo_matches = repo.as_ref().is_none_or(|r| *r == installed.repo_name); - name_matches && pkg_id_matches && repo_matches + name_matches && pkg_id_matches && family_matches && repo_matches }); if !is_declared { @@ -320,7 +328,6 @@ pub async fn execute_apply( ctx.events().emit(SoarEvent::Removing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: RemoveStage::RunningHook("pre_remove".into()), }); @@ -336,7 +343,6 @@ pub async fn execute_apply( ctx.events().emit(SoarEvent::Removing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: RemoveStage::Complete { size_freed: None, }, @@ -347,7 +353,6 @@ pub async fn execute_apply( ctx.events().emit(SoarEvent::OperationFailed { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), error: e.to_string(), }); failed_count += 1; @@ -365,22 +370,36 @@ pub async fn execute_apply( } /// Handle local (URL/github/gitlab) packages in apply diff. +/// What a declaration identifies: name, package id, family and repository. +type DeclaredKeys = HashSet<(String, Option, Option, Option)>; + +/// The deprecated package id a declaration still carries, if any. +/// +/// Reading it is the whole point of keeping the field, so the deprecation is +/// answered once here rather than at every use. +#[allow(deprecated)] +fn declared_pkg_id(pkg: &ResolvedPackage) -> Option<&str> { + pkg.pkg_id.as_deref() +} + +fn declared_key(pkg: &ResolvedPackage) -> (String, Option, Option, Option) { + ( + pkg.name.clone(), + declared_pkg_id(pkg).map(str::to_string), + pkg.family.clone(), + pkg.repo.clone(), + ) +} + fn handle_local_package( pkg: &ResolvedPackage, is_github_or_gitlab: bool, diesel_db: &DieselDatabase, diff: &mut ApplyDiff, ) -> SoarResult<()> { - let local_pkg_id = if is_github_or_gitlab { - pkg.pkg_id.clone().or_else(|| { - pkg.github - .as_ref() - .or(pkg.gitlab.as_ref()) - .map(|repo| repo.replace('/', ".")) - }) - } else { - pkg.pkg_id.clone() - }; + // Only what the user set. The family is derived from the download URL, + // which identifies the source just as well without minting an id. + let local_pkg_id = declared_pkg_id(pkg).map(str::to_string); let installed: Option = diesel_db .with_conn(|conn| { @@ -548,7 +567,7 @@ fn handle_local_package( Some(&pkg.name), pkg.version.as_deref(), pkg.pkg_type.as_deref(), - pkg.pkg_id.as_deref(), + declared_pkg_id(pkg), )?; match check_url_package_status(&url_pkg, pkg, "local", diesel_db)? { @@ -573,7 +592,7 @@ fn check_url_package_status( conn, Some("local"), Some(&url_pkg.pkg_name), - Some(&url_pkg.pkg_id), + url_pkg.pkg_id.as_deref(), None, None, None, @@ -585,9 +604,11 @@ fn check_url_package_status( .map(Into::into) .collect(); + // A name alone is not an identity: the family says which source this + // install came from, and only a matching one is the same package. let installed = installed_packages .iter() - .find(|ip| ip.is_installed) + .find(|ip| ip.is_installed && ip.pkg_family == url_pkg.pkg_family) .cloned(); if let Some(ref existing) = installed { diff --git a/crates/soar-operations/src/context.rs b/crates/soar-operations/src/context.rs index 5af1bc3fe..a0a6cc7fc 100644 --- a/crates/soar-operations/src/context.rs +++ b/crates/soar-operations/src/context.rs @@ -46,6 +46,13 @@ fn handle_json_metadata>( Ok(()) } +/// Bring a published metadata database up to the schema soar reads. +fn migrate_metadata(path: &Path) -> SoarResult<()> { + DbConnection::open(path, DbType::Metadata) + .map_err(|e| SoarError::Custom(format!("migrating repository metadata: {}", e)))?; + Ok(()) +} + #[derive(Clone)] pub struct SoarContext { inner: Arc, @@ -162,6 +169,17 @@ impl SoarContext { MetadataContent::SqliteDb(db_bytes) => { write_metadata_db(&db_bytes, &metadata_db_path) .map_err(|e| SoarError::Custom(e.to_string()))?; + // A published database was built by whatever soar + // the repository runs, so it can predate the + // columns these queries name. Opening it through + // the migration runner brings it up to them. The + // rebuild runs outside a transaction, so a failure + // leaves a half-migrated file: it is discarded + // rather than queried. + if let Err(e) = migrate_metadata(&metadata_db_path) { + fs::remove_file(&metadata_db_path).ok(); + return Err(e); + } } MetadataContent::Json(packages) => { handle_json_metadata(&packages, &metadata_db_path, &repo.name)?; @@ -230,25 +248,30 @@ impl SoarContext { })?; for pkg in installed_packages { - let exists = metadata_db - .with_conn(|conn| MetadataRepository::exists_by_pkg_id(conn, &pkg.pkg_id))?; + // Replacement tracking is keyed by package id, which the + // declarative format does not produce. Those rows have nothing to + // look up here. + let Some(pkg_id) = pkg.pkg_id.as_deref() else { + continue; + }; + let exists = + metadata_db.with_conn(|conn| MetadataRepository::exists_by_pkg_id(conn, pkg_id))?; if !exists { - let replacement = metadata_db.with_conn(|conn| { - MetadataRepository::find_replacement_pkg_id(conn, &pkg.pkg_id) - })?; + let replacement = metadata_db + .with_conn(|conn| MetadataRepository::find_replacement_pkg_id(conn, pkg_id))?; if let Some(new_pkg_id) = replacement { self.inner.events.emit(SoarEvent::Log { level: LogLevel::Info, message: format!( "{} is replaced by {} in {}", - pkg.pkg_id, new_pkg_id, repo_name + pkg_id, new_pkg_id, repo_name ), }); diesel_core_db.with_conn(|conn| { - CoreRepository::update_pkg_id(conn, &repo_name, &pkg.pkg_id, &new_pkg_id) + CoreRepository::update_pkg_id(conn, &repo_name, Some(pkg_id), &new_pkg_id) })?; } } diff --git a/crates/soar-operations/src/health.rs b/crates/soar-operations/src/health.rs index ea8860e37..cc58e6138 100644 --- a/crates/soar-operations/src/health.rs +++ b/crates/soar-operations/src/health.rs @@ -22,12 +22,38 @@ pub fn check_health(ctx: &SoarContext) -> SoarResult { .split(':') .any(|p| resolve_path(p).unwrap_or_default() == bin_path); + // Whether `man` can find what soar installed. An explicit MANPATH replaces + // everything man-db would work out for itself, so a package's manual pages + // can be installed correctly and still be invisible. + let man_dir = bin_path.parent().unwrap_or(&bin_path).join("share/man"); + let man_path = man_dir.is_dir().then(|| man_dir.clone()); + let man_path_configured = match &man_path { + None => true, + Some(dir) => { + let listed = |value: String| { + value + .split(':') + .any(|p| !p.is_empty() && resolve_path(p).unwrap_or_default() == *dir) + }; + let from_env = std::env::var("MANPATH").map(&listed).unwrap_or(false); + from_env + || std::process::Command::new("manpath") + .output() + .ok() + .and_then(|o| String::from_utf8(o.stdout).ok()) + .map(|out| listed(out.trim().to_string())) + .unwrap_or(false) + } + }; + let broken_packages = get_broken_packages(ctx)?; let broken_symlinks = get_broken_symlinks(ctx)?; Ok(HealthReport { path_configured, bin_path, + man_path, + man_path_configured, broken_packages, broken_symlinks, }) @@ -46,14 +72,12 @@ pub async fn remove_broken_packages(ctx: &SoarContext) -> SoarResult SoarResult SoarResult SoarResult> { .map(|p| { BrokenPackage { pkg_name: p.pkg_name, - pkg_id: p.pkg_id, installed_path: p.installed_path, } }) @@ -137,16 +156,21 @@ fn get_broken_symlinks(ctx: &SoarContext) -> SoarResult> { let config = ctx.config(); let mut broken = Vec::new(); + // A directory that does not exist holds nothing broken. Walking it is an + // error, and reporting that as a failed health check tells the user + // something is wrong when nothing is. let bin_path = config.get_bin_path()?; - walk_dir( - &bin_path, - &mut |path: &std::path::Path| -> FileSystemResult<()> { - if !path.exists() { - broken.push(path.to_path_buf()); - } - Ok(()) - }, - )?; + if bin_path.is_dir() { + walk_dir( + &bin_path, + &mut |path: &std::path::Path| -> FileSystemResult<()> { + if !path.exists() { + broken.push(path.to_path_buf()); + } + Ok(()) + }, + )?; + } let desktop_path = config.get_desktop_path()?; let mut soar_check = |path: &std::path::Path| -> FileSystemResult<()> { @@ -158,8 +182,13 @@ fn get_broken_symlinks(ctx: &SoarContext) -> SoarResult> { Ok(()) }; - walk_dir(&desktop_path, &mut soar_check)?; - walk_dir(config.get_icons_path(), &mut soar_check)?; + if desktop_path.is_dir() { + walk_dir(&desktop_path, &mut soar_check)?; + } + let icons_path = config.get_icons_path(); + if icons_path.is_dir() { + walk_dir(&icons_path, &mut soar_check)?; + } Ok(broken) } diff --git a/crates/soar-operations/src/install.rs b/crates/soar-operations/src/install.rs index dd3420428..6f34a4d18 100644 --- a/crates/soar-operations/src/install.rs +++ b/crates/soar-operations/src/install.rs @@ -21,6 +21,7 @@ use soar_core::{ install::{InstallMarker, InstallTarget, PackageInstaller}, local::LocalPackage, query::PackageQuery, + remove::make_tree_writable, update::remove_old_versions, url::UrlPackage, }, @@ -28,7 +29,7 @@ use soar_core::{ }; use soar_db::repository::{ core::{CoreRepository, SortDirection}, - metadata::MetadataRepository, + metadata::{narrow_by_pkg_id, MetadataRepository}, }; use soar_events::{InstallStage, SoarEvent, VerifyStage}; use soar_package::integrate_package; @@ -37,20 +38,63 @@ use soar_utils::{ lock::FileLock, path::is_safe_component, pattern::apply_sig_variants, + version::compare_versions, }; use tokio::sync::Semaphore; use tracing::{debug, trace, warn}; use crate::{ progress::{create_progress_bridge, next_op_id}, - utils::{has_desktop_integration, mangle_package_symlinks}, + utils::{has_desktop_integration, link_shared_files, mangle_package_symlinks}, FailedInfo, InstallOptions, InstallReport, InstalledInfo, ResolveResult, SoarContext, }; +/// Build an install target for a package the caller has already chosen. +/// +/// Resolving it again by name would pose the same ambiguous question that the +/// choice just answered. +pub fn target_for( + ctx: &SoarContext, + package: Package, + options: &InstallOptions, +) -> SoarResult { + let diesel_db = ctx.diesel_core_db()?.clone(); + let existing_install: Option = diesel_db + .with_conn(|conn| { + CoreRepository::list_filtered( + conn, + Some(&package.repo_name), + Some(&package.pkg_name), + package.pkg_id.as_deref(), + None, + None, + None, + None, + None, + ) + })? + .into_iter() + // The query cannot narrow by family, and an uninstalled row of the + // same name would otherwise stand in for the installed one. + .filter(|ip| ip.pkg_family.as_deref() == package.pkg_family.as_deref()) + .find(|ip| ip.is_installed) + .map(Into::into); + let pinned = options.version_override.is_some(); + let package = package.resolve(options.version_override.as_deref()); + Ok(InstallTarget { + package, + existing_install, + pinned, + profile: None, + ..Default::default() + }) +} + /// Resolve package queries into install targets or ambiguity results. /// -/// For each query string, returns a [`ResolveResult`] indicating whether the package -/// was resolved, is ambiguous (multiple candidates), not found, or already installed. +/// For each query string, returns a [`ResolveResult`] indicating whether the +/// package was resolved, is ambiguous (multiple candidates), not found, or +/// already installed. pub async fn resolve_packages( ctx: &SoarContext, packages: &[String], @@ -153,7 +197,7 @@ fn resolve_synthetic_target( conn, Some("local"), Some(&package.pkg_name), - Some(&package.pkg_id), + package.pkg_id.as_deref(), None, None, None, @@ -171,7 +215,6 @@ fn resolve_synthetic_target( if !options.force { return Ok(ResolveResult::AlreadyInstalled { pkg_name: installed.pkg_name.clone(), - pkg_id: installed.pkg_id.clone(), repo_name: installed.repo_name.clone(), version: installed.version.clone(), }); @@ -206,6 +249,7 @@ fn resolve_all_variants( None, None, None, + None, Some(SortDirection::Asc), ) })? @@ -225,6 +269,7 @@ fn resolve_all_variants( None, None, None, + None, Some(SortDirection::Asc), )?; Ok(pkgs @@ -257,15 +302,32 @@ fn resolve_all_variants( } let target_pkg_id = variants[0].pkg_id.clone(); + let target_pkg_name = variants[0].pkg_name.clone(); + let target_pkg_family = variants[0].pkg_family.clone(); + + // Without an id every filter below would be None, which reads as "no + // filter" and returns the whole metadata table. Fall back to the selected + // package's own name and family instead. + let (name_filter, id_filter, family_filter) = match target_pkg_id.as_deref() { + Some(id) => (None, Some(id), None), + None => { + ( + Some(target_pkg_name.as_str()), + None, + target_pkg_family.as_deref(), + ) + } + }; - // Find all packages with this pkg_id + // Find all packages with this identity let all_pkgs: Vec = if let Some(ref repo_name) = query.repo_name { metadata_mgr .query_repo(repo_name, |conn| { MetadataRepository::find_filtered( conn, - None, - Some(&target_pkg_id), + name_filter, + id_filter, + family_filter, None, None, Some(SortDirection::Asc), @@ -283,8 +345,9 @@ fn resolve_all_variants( metadata_mgr.query_all_flat(|repo_name, conn| { let pkgs = MetadataRepository::find_filtered( conn, - None, - Some(&target_pkg_id), + name_filter, + id_filter, + family_filter, None, None, Some(SortDirection::Asc), @@ -305,8 +368,8 @@ fn resolve_all_variants( CoreRepository::list_filtered( conn, query.repo_name.as_deref(), - None, - Some(&target_pkg_id), + name_filter, + id_filter, None, None, None, @@ -376,6 +439,7 @@ fn resolve_by_pkg_id( conn, None, query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -395,6 +459,7 @@ fn resolve_by_pkg_id( conn, None, query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -488,13 +553,16 @@ fn resolve_normal( 0 => Ok(ResolveResult::NotFound(package_name.to_string())), 1 => { let pkg = packages.into_iter().next().unwrap(); - let installed_pkg = installed_packages.iter().find(|ip| ip.is_installed); + // Same name from another repository is a different package, so it + // must not be mistaken for this one already being installed. + let installed_pkg = installed_packages + .iter() + .find(|ip| ip.is_installed && ip.repo_name == pkg.repo_name); if let Some(installed) = installed_pkg { if !options.force { return Ok(ResolveResult::AlreadyInstalled { pkg_name: installed.pkg_name.clone(), - pkg_id: installed.pkg_id.clone(), repo_name: installed.repo_name.clone(), version: installed.version.clone(), }); @@ -517,6 +585,54 @@ fn resolve_normal( }])) } _ => { + // Several versions of one package are not competing variants, and + // asking which to install would be asking the same question the + // caller already answered by naming it. The newest wins; anything + // else is chosen with an explicit @version. + let identity = |p: &Package| { + ( + p.pkg_name.clone(), + p.pkg_id.clone(), + p.pkg_family.clone(), + p.repo_name.clone(), + ) + }; + let first = identity(&packages[0]); + if packages.iter().all(|p| identity(p) == first) { + let newest = packages + .into_iter() + .max_by(|a, b| compare_versions(&a.version, &b.version)) + .unwrap(); + let installed_pkg = installed_packages + .iter() + .find(|ip| ip.is_installed && ip.repo_name == newest.repo_name); + if let Some(installed) = installed_pkg { + if !options.force { + return Ok(ResolveResult::AlreadyInstalled { + pkg_name: installed.pkg_name.clone(), + repo_name: installed.repo_name.clone(), + version: installed.version.clone(), + }); + } + } + let existing_install = installed_packages + .iter() + .find(|ip| { + ip.version == newest.version + && ip.repo_name == newest.repo_name + && ip.pkg_family.as_deref() == newest.pkg_family.as_deref() + }) + .cloned(); + let newest = newest.resolve(query.version.as_deref()); + return Ok(ResolveResult::Resolved(vec![InstallTarget { + package: newest, + existing_install, + pinned: query.version.is_some(), + profile: None, + ..Default::default() + }])); + } + Ok(ResolveResult::Ambiguous(crate::AmbiguousPackage { query: package_name.to_string(), candidates: packages, @@ -530,27 +646,36 @@ fn find_packages( query: &PackageQuery, existing_install: &Option, ) -> SoarResult> { + // Naming a repository is a choice, so it outranks where an existing install + // happened to come from. // If we have an existing install, try to find it in its original repo first - if let Some(existing) = existing_install { - let existing_pkgs: Vec = metadata_mgr - .query_repo(&existing.repo_name, |conn| { - MetadataRepository::find_filtered( - conn, - Some(&existing.pkg_name), - Some(&existing.pkg_id), - None, - None, - None, - ) - })? - .unwrap_or_default() - .into_iter() - .map(|p| { - let mut pkg: Package = p.into(); - pkg.repo_name = existing.repo_name.clone(); - pkg - }) - .collect(); + if let Some(existing) = existing_install + .as_ref() + .filter(|_| query.repo_name.is_none()) + { + let existing_pkgs: Vec = narrow_by_pkg_id( + metadata_mgr + .query_repo(&existing.repo_name, |conn| { + MetadataRepository::find_filtered( + conn, + Some(&existing.pkg_name), + None, + existing.pkg_family.as_deref(), + None, + None, + None, + ) + })? + .unwrap_or_default(), + existing.pkg_id.as_deref(), + ) + .into_iter() + .map(|p| { + let mut pkg: Package = p.into(); + pkg.repo_name = existing.repo_name.clone(); + pkg + }) + .collect(); if !existing_pkgs.is_empty() { return Ok(existing_pkgs); @@ -564,6 +689,7 @@ fn find_packages( conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -583,6 +709,7 @@ fn find_packages( conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -656,37 +783,40 @@ pub async fn perform_installation( .await; match result { - Ok((install_dir, symlinks)) => { + Ok((install_dir, symlinks, shared)) => { if !install_dir.as_os_str().is_empty() { installed.lock().unwrap().push(InstalledInfo { pkg_name: target.package.pkg_name.clone(), - pkg_id: target.package.pkg_id.clone(), + pkg_family: target.package.pkg_family.clone(), repo_name: target.package.repo_name.clone(), version: target.package.version.clone(), install_dir, symlinks, + shared, notes: target.package.notes.clone(), }); } - let _ = remove_old_versions(&target.package, &db, false); + if let Err(err) = remove_old_versions(&target.package, &db, false) { + warn!(error = %err, "could not remove the superseded version"); + } } Err(err) => { match err { SoarError::Warning(msg) => { warnings.lock().unwrap().push(msg); - let _ = remove_old_versions(&target.package, &db, false); + if let Err(err) = remove_old_versions(&target.package, &db, false) { + warn!(error = %err, "could not remove the superseded version"); + } } _ => { let op_id = next_op_id(); ctx.events().emit(SoarEvent::OperationFailed { op_id, pkg_name: target.package.pkg_name.clone(), - pkg_id: target.package.pkg_id.clone(), error: err.to_string(), }); failed.lock().unwrap().push(FailedInfo { pkg_name: target.package.pkg_name.clone(), - pkg_id: target.package.pkg_id.clone(), error: err.to_string(), }); failed_count.fetch_add(1, Ordering::Relaxed); @@ -733,6 +863,8 @@ fn source_skips_integrity_gate(pkg: &Package) -> bool { } #[allow(clippy::too_many_arguments)] +// Reads install_patterns while the OCI path exists; see the field's deprecation. +#[allow(deprecated)] async fn install_single_package( ctx: &SoarContext, target: &InstallTarget, @@ -744,7 +876,7 @@ async fn install_single_package( portable_config: Option<&str>, portable_share: Option<&str>, portable_cache: Option<&str>, -) -> SoarResult<(PathBuf, Vec<(PathBuf, PathBuf)>)> { +) -> SoarResult<(PathBuf, Vec<(PathBuf, PathBuf)>, Vec<(PathBuf, PathBuf)>)> { let op_id = next_op_id(); let events = ctx.events().clone(); let pkg = &target.package; @@ -788,7 +920,7 @@ async fn install_single_package( conn, Some(&pkg.repo_name), Some(&pkg.pkg_name), - Some(&pkg.pkg_id), + pkg.pkg_id.as_deref(), Some(&pkg.version), Some(true), None, @@ -797,10 +929,12 @@ async fn install_single_package( ) })? .into_iter() - .find(|ip| ip.is_installed); + // The query cannot narrow by family, so it is compared here: two + // packages sharing a name in one batch are still two packages. + .find(|ip| ip.is_installed && ip.pkg_family.as_deref() == pkg.pkg_family.as_deref()); if freshly_installed.is_some() { - return Ok((PathBuf::new(), Vec::new())); + return Ok((PathBuf::new(), Vec::new(), Vec::new())); } let config = ctx.config(); @@ -815,34 +949,51 @@ async fn install_single_package( .unwrap_or(false); if !has_signing { return Err(SoarError::Custom(format!( - "Refusing to install {}#{}: no checksum or signature available to verify integrity (use --no-verify to override)", - pkg.pkg_name, pkg.pkg_id + "Refusing to install {}: no checksum or signature available to verify integrity (use --no-verify to override)", + pkg.pkg_name ))); } } - let dir_suffix: String = pkg + // Keyed on the whole identity rather than the artifact's hash. Two + // repositories shipping byte-identical builds are still two installs, and + // sharing a directory means removing one deletes the other's files. + let content = pkg .bsum - .as_ref() - .filter(|s| s.len() >= 12) - .map(|s| s[..12].to_string()) - .unwrap_or_else(|| { - let input = format!("{}:{}:{}", pkg.pkg_id, pkg.pkg_name, pkg.version); - hash_string(&input)[..12].to_string() - }); - - // pkg_name/pkg_id are joined into install_dir and interpolated into resource - // paths downstream, so they must not be able to escape the packages dir. - if !is_safe_component(&pkg.pkg_name) || !is_safe_component(&pkg.pkg_id) { + .as_deref() + .or(pkg.pkg_id.as_deref()) + .or(pkg.ghcr_pkg.as_deref()) + .unwrap_or(pkg.download_url.as_str()); + let dir_suffix: String = hash_string(&format!( + "{}:{}:{}:{}:{}", + pkg.repo_name, + pkg.pkg_family.as_deref().unwrap_or_default(), + pkg.pkg_name, + pkg.version, + content + ))[..12] + .to_string(); + + // pkg_name is joined into install_dir and interpolated into resource paths + // downstream, so it must not be able to escape the packages dir. + if !is_safe_component(&pkg.pkg_name) { return Err(SoarError::Custom(format!( - "Refusing to install {}#{}: package name or id is not a valid path component", - pkg.pkg_name, pkg.pkg_id + "Refusing to install {}: package name is not a valid path component", + pkg.pkg_name ))); } + // The version is in the name so a directory can be read at a glance. It is + // not the identity: the hash still is, since two builds of one version must + // not collide. + let dir_name = if is_safe_component(&pkg.version) { + format!("{}-{}-{}", pkg.pkg_name, pkg.version, dir_suffix) + } else { + format!("{}-{}", pkg.pkg_name, dir_suffix) + }; let install_dir = config .get_packages_path(target.profile.clone())? - .join(format!("{}-{}-{}", pkg.pkg_name, pkg.pkg_id, dir_suffix)); + .join(dir_name); let main_binary_name = pkg .provides .as_ref() @@ -896,6 +1047,9 @@ async fn install_single_package( if should_cleanup && install_dir.exists() { debug!(path = %install_dir.display(), "cleaning up existing installation directory"); + // An archive may ship its directories read-only, and removing an entry + // needs write permission on the directory holding it. + make_tree_writable(&install_dir); fs::remove_dir_all(&install_dir).map_err(|err| { SoarError::Custom(format!( "Failed to clean up install directory {}: {}", @@ -921,12 +1075,7 @@ async fn install_single_package( let install_patterns = apply_sig_variants(install_patterns); // Create progress bridge for download events - let progress_callback = create_progress_bridge( - events.clone(), - op_id, - pkg.pkg_name.clone(), - pkg.pkg_id.clone(), - ); + let progress_callback = create_progress_bridge(events.clone(), op_id, pkg.pkg_name.clone()); trace!(install_dir = %install_dir.display(), "creating package installer"); let installer = PackageInstaller::new( @@ -951,7 +1100,6 @@ async fn install_single_package( events.emit(SoarEvent::Verifying { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: VerifyStage::Signature, }); @@ -959,8 +1107,8 @@ async fn install_single_package( verified_sig_count = verify_signatures(pubkey, &install_dir)?; } else { warn!( - "{}#{} - Signature verification skipped as no pubkey was found.", - pkg.pkg_name, pkg.pkg_id + "{} - Signature verification skipped as no pubkey was found.", + pkg.pkg_name ); } } @@ -971,8 +1119,8 @@ async fn install_single_package( if !no_verify && !skip_integrity_gate && pkg.bsum.is_none() && verified_sig_count == 0 { return Err(SoarError::Custom(format!( - "Refusing to install {}#{}: no checksum and no valid signature found to verify integrity (use --no-verify to override)", - pkg.pkg_name, pkg.pkg_id + "Refusing to install {}: no checksum and no valid signature found to verify integrity (use --no-verify to override)", + pkg.pkg_name ))); } @@ -981,7 +1129,6 @@ async fn install_single_package( events.emit(SoarEvent::Verifying { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: VerifyStage::Checksum, }); @@ -1003,7 +1150,6 @@ async fn install_single_package( events.emit(SoarEvent::Verifying { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: VerifyStage::Failed("checksum mismatch".into()), }); return Err(SoarError::Custom( @@ -1014,7 +1160,6 @@ async fn install_single_package( events.emit(SoarEvent::Verifying { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: VerifyStage::Passed, }); } @@ -1022,12 +1167,11 @@ async fn install_single_package( events.emit(SoarEvent::Verifying { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: VerifyStage::Failed("checksum unavailable".into()), }); return Err(SoarError::Custom(format!( - "Could not verify {}#{}: expected a checksum but none could be computed", - pkg.pkg_name, pkg.pkg_id + "Could not verify {}: expected a checksum but none could be computed", + pkg.pkg_name ))); } _ => {} @@ -1038,10 +1182,13 @@ async fn install_single_package( events.emit(SoarEvent::Installing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: InstallStage::LinkingBinaries, }); + // Only what packages.toml declares: a repository says where its files go + // through `files`, not through a binary mapping. + let binaries = target.binaries.clone().filter(|bins| !bins.is_empty()); + let symlinks = mangle_package_symlinks( &install_dir, &bin_dir, @@ -1049,17 +1196,21 @@ async fn install_single_package( &pkg.pkg_name, &pkg.version, target.entrypoint.as_deref(), - target.binaries.as_deref(), + binaries.as_deref(), target.arch_map.as_ref(), + pkg.files.as_deref(), ) .await?; + // Man pages and completions only mean anything where the system looks for + // them, so they are linked out of the package the same way binaries are. + let shared = link_shared_files(&install_dir, &bin_dir, &ctx.config().completion_shells())?; + // Desktop integration if !unlinked || has_desktop_integration(pkg, ctx.config()) { events.emit(SoarEvent::Installing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: InstallStage::DesktopIntegration, }); @@ -1082,7 +1233,6 @@ async fn install_single_package( events.emit(SoarEvent::Installing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: InstallStage::RecordingDatabase, }); @@ -1102,7 +1252,6 @@ async fn install_single_package( events.emit(SoarEvent::OperationComplete { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), }); debug!( @@ -1111,7 +1260,7 @@ async fn install_single_package( version = pkg.version, "installation complete" ); - Ok((install_dir, symlinks)) + Ok((install_dir, symlinks, shared)) } fn verify_signatures(pubkey_str: &str, install_dir: &Path) -> SoarResult { diff --git a/crates/soar-operations/src/list.rs b/crates/soar-operations/src/list.rs index cd4d04085..fe46cbeb9 100644 --- a/crates/soar-operations/src/list.rs +++ b/crates/soar-operations/src/list.rs @@ -9,10 +9,11 @@ use soar_db::{ models::metadata::PackageListing, repository::{core::CoreRepository, metadata::MetadataRepository}, }; -use soar_utils::fs::dir_size; +use soar_utils::{fs::dir_size, version::compare_versions}; use tracing::{debug, trace}; use crate::{ + utils::{is_installed, InstalledIndex, PackageKey}, InstalledEntry, InstalledListResult, PackageListEntry, PackageListResult, SoarContext, }; @@ -57,31 +58,102 @@ pub async fn list_packages( })? }; - let installed_pkgs: HashMap<(String, String, String), bool> = diesel_db + // One row per package, not per version. A repository publishes every + // version it knows, and listing them all buries the packages themselves. + let mut newest: HashMap = HashMap::new(); + let mut counts: HashMap> = HashMap::new(); + for entry in packages { + // family included: two packages sharing a name are different + // packages, not two versions of one + let key = ( + entry.repo_name.clone(), + entry.pkg.pkg_name.clone(), + entry.pkg.pkg_id.clone(), + entry.pkg.pkg_family.clone(), + ); + counts + .entry(key.clone()) + .or_default() + .push(entry.pkg.version.clone()); + match newest.get(&key) { + Some(kept) if compare_versions(&kept.pkg.version, &entry.pkg.version).is_ge() => {} + _ => { + newest.insert(key, entry); + } + } + } + let mut packages: Vec = newest.into_values().collect(); + packages.sort_by(|a, b| { + a.pkg + .pkg_name + .cmp(&b.pkg.pkg_name) + .then(a.repo_name.cmp(&b.repo_name)) + }); + + let installed_pkgs: InstalledIndex = diesel_db .with_conn(|conn| { CoreRepository::list_filtered(conn, None, None, None, None, None, None, None, None) })? .into_par_iter() - .map(|pkg| ((pkg.repo_name, pkg.pkg_id, pkg.pkg_name), pkg.is_installed)) - .collect(); + // Keyed by name, not id: a package installed before ids became + // optional still carries one, while its metadata no longer does, and + // keying on both would stop matching the two. Rows sharing a key are + // merged rather than overwritten, so one uninstalled version cannot + // mask an installed one. + // Keyed by family too, or a package merely sharing a name would + // inherit the marker. The family is recorded at install time, so a + // package without one matches only entries without one. + .map(|pkg| { + ( + (pkg.repo_name, pkg.pkg_name), + (pkg.pkg_family, pkg.is_installed), + ) + }) + .fold(HashMap::new, |mut acc: HashMap<_, Vec<_>>, (key, value)| { + acc.entry(key).or_default().push(value); + acc + }) + .reduce(HashMap::new, |mut acc: HashMap<_, Vec<_>>, part| { + for (key, values) in part { + acc.entry(key).or_default().extend(values); + } + acc + }); let total = packages.len(); let entries: Vec = packages .into_iter() .map(|entry| { - let key = ( - entry.repo_name.clone(), - entry.pkg.pkg_id.clone(), - entry.pkg.pkg_name.clone(), + let installed = is_installed( + &installed_pkgs, + &entry.repo_name, + &entry.pkg.pkg_name, + entry.pkg.pkg_family.as_deref(), ); - let installed = installed_pkgs.get(&key).copied().unwrap_or(false); + let other_versions = counts + .get(&( + entry.repo_name.clone(), + entry.pkg.pkg_name.clone(), + entry.pkg.pkg_id.clone(), + entry.pkg.pkg_family.clone(), + )) + .map(|all| { + let mut rest: Vec = all + .iter() + .filter(|v| **v != entry.pkg.version) + .cloned() + .collect(); + rest.sort_by(|a, b| compare_versions(b, a)); + rest + }) + .unwrap_or_default(); // Build a minimal Package for the entry let package = Package { repo_name: entry.repo_name, - pkg_id: entry.pkg.pkg_id, pkg_name: entry.pkg.pkg_name, + pkg_family: entry.pkg.pkg_family, pkg_type: entry.pkg.pkg_type, version: entry.pkg.version, ..Default::default() @@ -90,6 +162,7 @@ pub async fn list_packages( PackageListEntry { package, installed, + other_versions, } }) .collect(); diff --git a/crates/soar-operations/src/progress.rs b/crates/soar-operations/src/progress.rs index 6e741b85b..f762f8503 100644 --- a/crates/soar-operations/src/progress.rs +++ b/crates/soar-operations/src/progress.rs @@ -14,7 +14,6 @@ pub fn create_progress_bridge( events: EventSinkHandle, op_id: OperationId, pkg_name: String, - pkg_id: String, ) -> Arc { Arc::new(move |progress| { let event = match progress { @@ -24,7 +23,6 @@ pub fn create_progress_bridge( SoarEvent::DownloadStarting { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), total, } } @@ -35,7 +33,6 @@ pub fn create_progress_bridge( SoarEvent::DownloadResuming { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), current, total, } @@ -47,7 +44,6 @@ pub fn create_progress_bridge( SoarEvent::DownloadProgress { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), current, total, } @@ -58,7 +54,6 @@ pub fn create_progress_bridge( SoarEvent::DownloadComplete { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), total, } } @@ -66,21 +61,18 @@ pub fn create_progress_bridge( SoarEvent::DownloadRetry { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), } } Progress::Aborted => { SoarEvent::DownloadAborted { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), } } Progress::Recovered => { SoarEvent::DownloadRecovered { op_id, pkg_name: pkg_name.clone(), - pkg_id: pkg_id.clone(), } } }; @@ -114,7 +106,7 @@ mod tests { let collector = Arc::new(CollectorSink::default()); let events: EventSinkHandle = collector.clone(); - let bridge = create_progress_bridge(events, 1, "pkg".into(), "pkg-id".into()); + let bridge = create_progress_bridge(events, 1, "pkg".into()); bridge(Progress::Starting { total: 1000, diff --git a/crates/soar-operations/src/remove.rs b/crates/soar-operations/src/remove.rs index 43198752f..ff4cf1eb5 100644 --- a/crates/soar-operations/src/remove.rs +++ b/crates/soar-operations/src/remove.rs @@ -34,7 +34,7 @@ pub fn resolve_removals( let query = PackageQuery::try_from(package.as_str())?; // --all flag: remove all installed variants matching the name - if let (true, None, Some(ref name)) = (all, &query.pkg_id, &query.name) { + if let (true, None, Some(ref name)) = (all, query.pkg_id.as_deref(), &query.name) { let installed: Vec = diesel_db .with_conn(|conn| { CoreRepository::list_filtered( @@ -94,14 +94,22 @@ pub fn resolve_removals( }); } else { let target_pkg_id = installed[0].pkg_id.clone(); - // Find all packages with this pkg_id + let target_pkg_name = installed[0].pkg_name.clone(); + // Without an id, filtering on it alone selects every + // installed package in the repository, which would remove + // far more than was asked for. + let (name_filter, id_filter) = match target_pkg_id.as_deref() { + Some(id) => (None, Some(id)), + None => (Some(target_pkg_name.as_str()), None), + }; + // Find all packages with this identity let all_installed: Vec = diesel_db .with_conn(|conn| { CoreRepository::list_filtered( conn, query.repo_name.as_deref(), - None, - Some(&target_pkg_id), + name_filter, + id_filter, None, None, None, @@ -170,7 +178,6 @@ pub async fn perform_removal( ctx.events().emit(SoarEvent::Removing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: RemoveStage::RunningHook("pre_remove".into()), }); @@ -191,7 +198,6 @@ pub async fn perform_removal( ctx.events().emit(SoarEvent::Removing { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), stage: RemoveStage::Complete { size_freed: None, }, @@ -199,12 +205,10 @@ pub async fn perform_removal( ctx.events().emit(SoarEvent::OperationComplete { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), }); removed.push(RemovedInfo { pkg_name: pkg.pkg_name, - pkg_id: pkg.pkg_id, repo_name: pkg.repo_name, version: pkg.version, }); @@ -213,13 +217,11 @@ pub async fn perform_removal( ctx.events().emit(SoarEvent::OperationFailed { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), error: err.to_string(), }); failed.push(FailedInfo { pkg_name: pkg.pkg_name, - pkg_id: pkg.pkg_id, error: err.to_string(), }); } diff --git a/crates/soar-operations/src/run.rs b/crates/soar-operations/src/run.rs index 8f6eb78cd..726b3c788 100644 --- a/crates/soar-operations/src/run.rs +++ b/crates/soar-operations/src/run.rs @@ -3,13 +3,16 @@ use std::{fs, path::Path, process::Command, sync::Arc}; use soar_core::{ database::models::Package, error::{ErrorContext, SoarError}, - package::query::PackageQuery, + package::{install::apply_file_layout, query::PackageQuery}, utils::get_extract_dir, SoarResult, }; use soar_db::repository::metadata::MetadataRepository; use soar_dl::{download::Download, oci::OciDownload, types::OverwriteMode}; -use soar_utils::hash::calculate_checksum; +use soar_utils::{ + hash::{calculate_checksum, hash_string}, + version::compare_versions, +}; use tracing::debug; use crate::{ @@ -36,10 +39,9 @@ pub async fn prepare_run( let package_name = query.name.as_deref().unwrap_or(package_name); let repo_name = query.repo_name.as_deref().or(repo_name); let pkg_id = query.pkg_id.as_deref().or(pkg_id); + let family = query.family.as_deref(); let version = query.version.as_deref(); - let output_path = cache_bin.join(package_name); - let metadata_mgr = ctx.metadata_manager().await?; let packages: Vec = if let Some(repo_name) = repo_name { @@ -49,7 +51,8 @@ pub async fn prepare_run( conn, Some(package_name), pkg_id, - None, + family, + version, None, None, ) @@ -68,7 +71,8 @@ pub async fn prepare_run( conn, Some(package_name), pkg_id, - None, + family, + version, None, None, )?; @@ -83,7 +87,7 @@ pub async fn prepare_run( })? }; - let packages: Vec = if let Some(version) = version { + let mut packages: Vec = if let Some(version) = version { packages .into_iter() .filter(|p| p.has_version(version)) @@ -96,24 +100,110 @@ pub async fn prepare_run( 0 => return Err(SoarError::PackageNotFound(package_name.to_string())), 1 => {} _ => { - return Ok(PrepareRunResult::Ambiguous(AmbiguousPackage { - query: package_name.to_string(), - candidates: packages, - })); + // Several versions of one package are not a choice to put to the + // caller: running a command means running the current one, unless + // an explicit @version says otherwise. + let identity = |p: &Package| { + ( + p.pkg_name.clone(), + p.pkg_id.clone(), + p.pkg_family.clone(), + p.repo_name.clone(), + ) + }; + let first = identity(&packages[0]); + if packages.iter().all(|p| identity(p) == first) { + let newest = packages + .into_iter() + .max_by(|a, b| compare_versions(&a.version, &b.version)) + .unwrap(); + packages = vec![newest]; + } else { + return Ok(PrepareRunResult::Ambiguous(AmbiguousPackage { + query: package_name.to_string(), + candidates: packages, + })); + } } } let package = packages.into_iter().next().unwrap().resolve(version); + // Named like an install. A package that published a checksum is keyed by + // it, so identical content is shared and different content never is; + // without one the key is the identity the package was resolved from, + // repository included. + let suffix = package + .bsum + .as_deref() + .filter(|s| s.len() >= 12) + .map(|s| s[..12].to_string()) + .unwrap_or_else(|| { + let source = package + .pkg_id + .as_deref() + .or(package.ghcr_pkg.as_deref()) + .unwrap_or(package.download_url.as_str()); + hash_string(&format!( + "{}:{}:{}:{}:{}", + package.repo_name, + package.pkg_family.as_deref().unwrap_or_default(), + package.pkg_name, + package.version, + source + ))[..12] + .to_string() + }); + let cache_dir = cache_bin.join(format!( + "{}-{}-{}", + package.pkg_name, package.version, suffix + )); + let output_path = cache_dir.join(&package.pkg_name); + // Refuse to execute a package whose integrity cannot be checked. OCI // artifacts are digest-verified during download, so they are exempt. if !no_verify && package.bsum.is_none() && package.ghcr_blob.is_none() { return Err(SoarError::Custom(format!( - "Refusing to run {}#{}: no checksum to verify integrity (use --no-verify to override)", - package.pkg_name, package.pkg_id + "Refusing to run {}: no checksum to verify integrity (use --no-verify to override)", + package.pkg_name ))); } + // A laid-out package keeps its binary, not the artifact it came from, so a + // cache hit is that binary. Where the binary is the artifact itself the + // published checksum still describes it, and a cache entry is only reused + // once it matches; a binary taken out of an archive has no checksum of its + // own, and the content-addressed directory is what stands in for one. + let laid_out = package.files.as_deref().and_then(|files| { + files.iter().find_map(|f| { + f.to.strip_prefix("bin/") + .filter(|rest| !rest.contains('/')) + .map(|_| (cache_dir.join(&f.to), f.source.is_empty())) + }) + }); + if let Some((binary, is_artifact)) = laid_out.as_ref().filter(|(p, _)| p.exists()) { + let verified = match package.bsum { + Some(ref bsum) if *is_artifact && !no_verify => { + let matches = calculate_checksum(binary)? == *bsum; + if !matches { + debug!( + package = %package.pkg_name, + "cached binary checksum mismatch; re-downloading" + ); + fs::remove_dir_all(&cache_dir).ok(); + } + matches + } + _ => true, + }; + if verified { + return Ok(PrepareRunResult::Ready { + path: binary.clone(), + downloaded: false, + }); + } + } + // Reuse a cached binary only after re-verifying it against the expected // checksum, so a stale or tampered cache entry is never executed blindly. if output_path.exists() { @@ -121,7 +211,10 @@ pub async fn prepare_run( Some(ref bsum) if !no_verify => { let checksum = calculate_checksum(&output_path)?; if checksum == *bsum { - return Ok(PrepareRunResult::Ready(output_path)); + return Ok(PrepareRunResult::Ready { + path: output_path, + downloaded: false, + }); } debug!( package = %package.pkg_name, @@ -129,30 +222,52 @@ pub async fn prepare_run( ); fs::remove_file(&output_path).ok(); } - _ => return Ok(PrepareRunResult::Ready(output_path)), + _ => { + return Ok(PrepareRunResult::Ready { + path: output_path, + downloaded: false, + }) + } } } - fs::create_dir_all(&cache_bin) - .with_context(|| format!("creating directory {}", cache_bin.display()))?; + fs::create_dir_all(&cache_dir) + .with_context(|| format!("creating directory {}", cache_dir.display()))?; let op_id = next_op_id(); - let progress_callback = create_progress_bridge( - ctx.events().clone(), - op_id, - package.pkg_name.clone(), - package.pkg_id.clone(), - ); + let progress_callback = + create_progress_bridge(ctx.events().clone(), op_id, package.pkg_name.clone()); download_to_cache( &package, &output_path, - &cache_bin, + &cache_dir, no_verify, progress_callback, )?; - Ok(PrepareRunResult::Ready(output_path)) + // The artifact may be an archive, in which case the thing to execute is + // wherever the package says it is rather than the download itself. + if let Some(files) = package.files.as_deref().filter(|f| !f.is_empty()) { + apply_file_layout(files, &cache_dir, &output_path)?; + if let Some(binary) = files.iter().find_map(|f| { + f.to.strip_prefix("bin/") + .filter(|rest| !rest.contains('/')) + .map(|_| cache_dir.join(&f.to)) + }) { + if binary.exists() { + return Ok(PrepareRunResult::Ready { + path: binary, + downloaded: true, + }); + } + } + } + + Ok(PrepareRunResult::Ready { + path: output_path, + downloaded: true, + }) } /// Execute a binary with the given arguments. diff --git a/crates/soar-operations/src/search.rs b/crates/soar-operations/src/search.rs index 1d99c777f..643d1e2b2 100644 --- a/crates/soar-operations/src/search.rs +++ b/crates/soar-operations/src/search.rs @@ -14,13 +14,18 @@ use soar_db::{ metadata::MetadataRepository, }, }; +use soar_utils::version::compare_versions; use tracing::{debug, trace}; -use crate::{SearchEntry, SearchResult, SoarContext}; +use crate::{ + utils::{is_installed, InstalledIndex, PackageKey}, + SearchEntry, SearchResult, SoarContext, +}; /// Search for packages across all repositories. /// -/// Uses fuzzy matching by default. Falls back to SQL LIKE for case-sensitive searches. +/// Uses fuzzy matching by default. Falls back to SQL LIKE for case-sensitive +/// searches. pub async fn search_packages( ctx: &SoarContext, query: &str, @@ -55,13 +60,67 @@ pub async fn search_packages( fuzzy_search(ctx, query, search_limit).await? }; - let installed_pkgs: HashMap<(String, String, String), bool> = diesel_db + // One row per package: a result repeated once per published version says + // nothing extra and pushes real matches off the list. + let mut newest: HashMap = HashMap::new(); + let mut counts: HashMap> = HashMap::new(); + let mut order: Vec = Vec::new(); + for pkg in packages { + let key = ( + pkg.repo_name.clone(), + pkg.pkg_name.clone(), + pkg.pkg_id.clone(), + pkg.pkg_family.clone(), + ); + counts + .entry(key.clone()) + .or_default() + .push(pkg.version.clone()); + match newest.get(&key) { + Some(kept) if compare_versions(&kept.version, &pkg.version).is_ge() => {} + _ => { + if !newest.contains_key(&key) { + order.push(key.clone()); + } + newest.insert(key, pkg); + } + } + } + // ranking order is the point of a search, so it is preserved + let packages: Vec = order + .into_iter() + .filter_map(|k| newest.remove(&k)) + .collect(); + + let installed_pkgs: InstalledIndex = diesel_db .with_conn(|conn| { CoreRepository::list_filtered(conn, None, None, None, None, None, None, None, None) })? .into_par_iter() - .map(|pkg| ((pkg.repo_name, pkg.pkg_id, pkg.pkg_name), pkg.is_installed)) - .collect(); + // Keyed by name, not id: a package installed before ids became + // optional still carries one, while its metadata no longer does, and + // keying on both would stop matching the two. Rows sharing a key are + // merged rather than overwritten, so one uninstalled version cannot + // mask an installed one. + // Keyed by family too, or a package merely sharing a name would + // inherit the marker. The family is recorded at install time, so a + // package without one matches only entries without one. + .map(|pkg| { + ( + (pkg.repo_name, pkg.pkg_name), + (pkg.pkg_family, pkg.is_installed), + ) + }) + .fold(HashMap::new, |mut acc: HashMap<_, Vec<_>>, (key, value)| { + acc.entry(key).or_default().push(value); + acc + }) + .reduce(HashMap::new, |mut acc: HashMap<_, Vec<_>>, part| { + for (key, values) in part { + acc.entry(key).or_default().extend(values); + } + acc + }); let total_count = packages.len(); @@ -69,15 +128,33 @@ pub async fn search_packages( .into_iter() .take(search_limit) .map(|package| { - let key = ( - package.repo_name.clone(), - package.pkg_id.clone(), - package.pkg_name.clone(), + let installed = is_installed( + &installed_pkgs, + &package.repo_name, + &package.pkg_name, + package.pkg_family.as_deref(), ); - let installed = installed_pkgs.get(&key).copied().unwrap_or(false); + let other_versions = counts + .get(&( + package.repo_name.clone(), + package.pkg_name.clone(), + package.pkg_id.clone(), + package.pkg_family.clone(), + )) + .map(|all| { + let mut rest: Vec = all + .iter() + .filter(|v| **v != package.version) + .cloned() + .collect(); + rest.sort_by(|a, b| compare_versions(b, a)); + rest + }) + .unwrap_or_default(); SearchEntry { package, installed, + other_versions, } }) .collect(); @@ -184,8 +261,11 @@ fn score_candidates(query: &str, candidates: &[(String, FuzzyCandidate)]) -> Vec let name_buf = Utf32String::from(candidate.pkg_name.as_str()); let name_score = pattern.score(name_buf.slice(..), &mut matcher); - let id_buf = Utf32String::from(candidate.pkg_id.as_str()); - let id_score = pattern.score(id_buf.slice(..), &mut matcher); + // A package without an id simply has nothing extra to match on. + let id_score = candidate.pkg_id.as_deref().and_then(|id| { + let id_buf = Utf32String::from(id); + pattern.score(id_buf.slice(..), &mut matcher) + }); let best_score = [name_score, id_score].into_iter().flatten().max(); @@ -222,6 +302,7 @@ pub async fn query_package(ctx: &SoarContext, query_str: &str) -> SoarResult SoarResult SoarResult = if let Some(ref version) = query.version { + let mut packages: Vec = if let Some(ref version) = query.version { packages .into_iter() .filter(|p| p.has_version(version)) @@ -266,5 +348,15 @@ pub async fn query_package(ctx: &SoarContext, query_str: &str) -> SoarResult, pub repo_name: String, pub version: String, pub install_dir: PathBuf, pub symlinks: Vec<(PathBuf, PathBuf)>, + /// Man pages and completions linked out of the package. Counted rather + /// than listed: a package like gh ships over a hundred manual pages. + pub shared: Vec<(PathBuf, PathBuf)>, pub notes: Option>, } @@ -70,7 +72,6 @@ pub struct InstalledInfo { #[derive(Debug)] pub struct FailedInfo { pub pkg_name: String, - pub pkg_id: String, pub error: String, } @@ -93,7 +94,6 @@ pub struct RemoveReport { pub struct RemovedInfo { pub pkg_name: String, - pub pkg_id: String, pub repo_name: String, pub version: String, } @@ -102,7 +102,6 @@ pub struct RemovedInfo { pub struct UpdateInfo { pub pkg_name: String, - pub pkg_id: String, pub repo_name: String, pub current_version: String, pub new_version: String, @@ -133,6 +132,8 @@ pub struct SearchResult { pub struct SearchEntry { pub package: Package, pub installed: bool, + /// The other versions the repository publishes, newest first. + pub other_versions: Vec, } pub struct PackageListResult { @@ -143,6 +144,9 @@ pub struct PackageListResult { pub struct PackageListEntry { pub package: Package, pub installed: bool, + /// The other versions the repository publishes, newest first. Only the + /// newest is listed, so these say what is not being shown. + pub other_versions: Vec, } pub struct InstalledListResult { @@ -162,13 +166,17 @@ pub struct InstalledEntry { pub struct HealthReport { pub path_configured: bool, pub bin_path: PathBuf, + /// Where soar puts manual pages, and whether `man` will actually look + /// there. Set only once a package has installed one, since there is + /// nothing to warn about otherwise. + pub man_path: Option, + pub man_path_configured: bool, pub broken_packages: Vec, pub broken_symlinks: Vec, } pub struct BrokenPackage { pub pkg_name: String, - pub pkg_id: String, pub installed_path: String, } @@ -211,7 +219,7 @@ pub struct ApplyReport { // ---- Run ---- pub enum PrepareRunResult { - Ready(PathBuf), + Ready { path: PathBuf, downloaded: bool }, Ambiguous(AmbiguousPackage), } diff --git a/crates/soar-operations/src/update.rs b/crates/soar-operations/src/update.rs index 57b130841..a29847814 100644 --- a/crates/soar-operations/src/update.rs +++ b/crates/soar-operations/src/update.rs @@ -137,7 +137,13 @@ fn check_repo_update( ) -> SoarResult> { let new_pkg: Option = metadata_mgr .query_repo(&pkg.repo_name, |conn| { - MetadataRepository::find_newer_version(conn, &pkg.pkg_name, &pkg.pkg_id, &pkg.version) + MetadataRepository::find_newer_version( + conn, + &pkg.pkg_name, + pkg.pkg_id.as_deref(), + pkg.pkg_family.as_deref(), + &pkg.version, + ) })? .flatten() .map(|p| { @@ -150,7 +156,6 @@ fn check_repo_update( let Some(package) = new_pkg else { ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::UpToDate { version: pkg.version.clone(), }, @@ -168,7 +173,6 @@ fn check_repo_update( ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::Available { current_version: pkg.version.clone(), new_version: package.version.clone(), @@ -177,7 +181,6 @@ fn check_repo_update( Ok(Some(UpdateInfo { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), repo_name: pkg.repo_name.clone(), current_version: pkg.version.clone(), new_version: package.version.clone(), @@ -202,14 +205,19 @@ fn check_local_update( resolved_packages: &[ResolvedPackage], ctx: &SoarContext, ) -> SoarResult> { - let resolved = resolved_packages - .iter() - .find(|r| r.name == pkg.pkg_name && has_update_source(r)); + // A declaration that names a family only speaks for that family, so a + // package of the same name from another one is not updated by it. + let resolved = resolved_packages.iter().find(|r| { + r.name == pkg.pkg_name + && r.family + .as_deref() + .is_none_or(|f| Some(f) == pkg.pkg_family.as_deref()) + && has_update_source(r) + }); let Some(resolved) = resolved else { ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::Skipped { reason: "no update source configured".into(), }, @@ -220,7 +228,6 @@ fn check_local_update( if resolved.pinned { ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::Skipped { reason: "pinned".into(), }, @@ -250,7 +257,6 @@ fn check_local_update( if v == installed_version { ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::UpToDate { version: pkg.version.clone(), }, @@ -311,7 +317,6 @@ fn check_local_update( if v == installed_version { ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::UpToDate { version: pkg.version.clone(), }, @@ -332,13 +337,12 @@ fn check_local_update( Some(&pkg.pkg_name), Some(&version), pkg.pkg_type.as_deref(), - Some(&pkg.pkg_id), + pkg.pkg_id.as_deref(), )?; updated_url_pkg.size = size; ctx.events().emit(SoarEvent::UpdateCheck { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), status: UpdateCheckStatus::Available { current_version: pkg.version.clone(), new_version: version.clone(), @@ -367,7 +371,6 @@ fn check_local_update( Ok(Some(UpdateInfo { pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), repo_name: pkg.repo_name.clone(), current_version: pkg.version.clone(), new_version: version, @@ -389,7 +392,8 @@ fn get_existing( conn, &package.repo_name, &package.pkg_name, - &package.pkg_id, + package.pkg_id.as_deref(), + package.pkg_family.as_deref(), &package.version, ) })?; @@ -449,15 +453,29 @@ pub async fn perform_update( // Clean up old versions only for successfully updated packages if !keep_old { let diesel_db = ctx.diesel_core_db()?.clone(); - let succeeded: HashSet<(&str, &str)> = install_report + // Keyed by the whole identity: a name alone would let a package from + // another repository or family inherit this one's success. + let succeeded: HashSet<(&str, Option<&str>, &str, &str)> = install_report .installed .iter() - .map(|i| (i.pkg_name.as_str(), i.pkg_id.as_str())) + .map(|i| { + ( + i.pkg_name.as_str(), + i.pkg_family.as_deref(), + i.repo_name.as_str(), + i.version.as_str(), + ) + }) .collect(); for target in &targets { let pkg = &target.package; - if !succeeded.contains(&(pkg.pkg_name.as_str(), pkg.pkg_id.as_str())) { + if !succeeded.contains(&( + pkg.pkg_name.as_str(), + pkg.pkg_family.as_deref(), + pkg.repo_name.as_str(), + pkg.version.as_str(), + )) { continue; } @@ -465,7 +483,6 @@ pub async fn perform_update( ctx.events().emit(SoarEvent::UpdateCleanup { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), old_version: target .existing_install .as_ref() @@ -474,12 +491,13 @@ pub async fn perform_update( stage: UpdateCleanupStage::Removing, }); - let _ = remove_old_versions(pkg, &diesel_db, false); + if let Err(err) = remove_old_versions(pkg, &diesel_db, false) { + warn!(error = %err, "could not remove the superseded version"); + } ctx.events().emit(SoarEvent::UpdateCleanup { op_id, pkg_name: pkg.pkg_name.clone(), - pkg_id: pkg.pkg_id.clone(), old_version: target .existing_install .as_ref() diff --git a/crates/soar-operations/src/utils.rs b/crates/soar-operations/src/utils.rs index bf7d65fa2..9d131536d 100644 --- a/crates/soar-operations/src/utils.rs +++ b/crates/soar-operations/src/utils.rs @@ -12,12 +12,12 @@ use soar_config::{ use soar_core::{ database::models::Package, error::{ErrorContext, SoarError}, - utils::substitute_placeholders, + utils::{shared_link_targets, substitute_placeholders}, SoarResult, }; -use soar_db::models::types::PackageProvide; +use soar_db::models::types::{PackageFile, PackageProvide}; use soar_utils::fs::is_elf; -use tracing::warn; +use tracing::{debug, warn}; /// Check if a package should have desktop integration (desktop files, icons). pub fn has_desktop_integration(package: &Package, config: &Config) -> bool { @@ -87,6 +87,78 @@ fn create_provide_symlinks( Ok(symlinks) } +/// Link a package's man pages and completions where the system looks for them. +/// +/// A destination already holding something soar did not put there is left +/// alone: a distro package or a hand-written completion outranks ours. +pub fn link_shared_files( + install_dir: &Path, + bin_dir: &Path, + shells: &[String], +) -> SoarResult> { + // A link pointing anywhere inside soar's own package tree is soar's, + // including one left by an older version of this package: an install + // directory carries its version, so the path never matches the new one. + let packages_root = install_dir.parent().unwrap_or(install_dir); + let mut linked = Vec::new(); + for (relative, destination, enabled) in shared_link_targets(bin_dir, shells) { + if !enabled { + continue; + } + let source_root = install_dir.join(relative); + if !source_root.is_dir() { + continue; + } + for source in walk_files(&source_root) { + let Ok(rest) = source.strip_prefix(&source_root) else { + continue; + }; + let link = destination.join(rest); + if let Some(parent) = link.parent() { + if fs::create_dir_all(parent).is_err() { + continue; + } + } + match fs::read_link(&link) { + // ours, from this package or an older version of it + Ok(target) if target.starts_with(packages_root) => { + fs::remove_file(&link).ok(); + } + Ok(_) | Err(_) if link.exists() || link.is_symlink() => { + debug!(path = %link.display(), "leaving a file soar does not own"); + continue; + } + _ => {} + } + if unix::fs::symlink(&source, &link).is_ok() { + linked.push((source, link)); + } + } + } + Ok(linked) +} + +/// Every regular file under `dir`, recursively, skipping symlinks and the +/// bookkeeping entries soar writes alongside a package. +fn walk_files(dir: &Path) -> Vec { + let mut out = Vec::new(); + let Ok(entries) = fs::read_dir(dir) else { + return out; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_symlink() { + continue; + } + if path.is_dir() { + out.extend(walk_files(&path)); + } else if !entry.file_name().to_string_lossy().starts_with('.') { + out.push(path); + } + } + out +} + /// Creates symlinks from installed package binaries to the bin directory. #[allow(clippy::too_many_arguments)] pub async fn mangle_package_symlinks( @@ -98,23 +170,121 @@ pub async fn mangle_package_symlinks( entrypoint: Option<&str>, binaries: Option<&[BinaryMapping]>, arch_map: Option<&HashMap>, + files: Option<&[PackageFile]>, ) -> SoarResult> { let mut symlinks = Vec::new(); + // A package laid out by its file list has already said what its commands + // are: everything in `bin/`. + let listed: Option> = files.filter(|f| !f.is_empty()).map(|files| { + files + .iter() + .flat_map(|f| std::iter::once(&f.to).chain(f.alias.iter())) + .cloned() + .collect() + }); + // A package installed before the file list existed carries none, so its own + // `bin/` stands in, but only where nothing else describes the package. + // Reading the directory otherwise publishes every executable an archive + // happens to ship, which for a toolchain is dozens of them. + let listed = listed.or_else(|| { + if entrypoint.is_some() + || binaries.is_some_and(|b| !b.is_empty()) + || provides.is_some_and(|p| !p.is_empty()) + { + return None; + } + // Read rather than walk: an alias is a symlink, which walking skips. + let entries: Vec = fs::read_dir(install_dir.join("bin")) + .into_iter() + .flatten() + .flatten() + .map(|e| format!("bin/{}", e.file_name().to_string_lossy())) + .collect(); + (!entries.is_empty()).then_some(entries) + }); + if let Some(listed) = listed { + for path in &listed { + { + let Some(name) = path.strip_prefix("bin/").filter(|n| !n.contains('/')) else { + continue; + }; + let source_path = install_dir.join(path); + if !source_path.exists() { + continue; + } + let link_path = bin_dir.join(name); + set_executable(&source_path)?; + if link_path.is_symlink() || link_path.is_file() { + std::fs::remove_file(&link_path).with_context(|| { + format!("removing existing file/symlink at {}", link_path.display()) + })?; + } + unix::fs::symlink(&source_path, &link_path) + .with_context(|| format!("creating symlink {}", link_path.display()))?; + symlinks.push((source_path, link_path)); + } + } + return Ok(symlinks); + } + if let Some(bins) = binaries { if !bins.is_empty() { + // Walked once: the tree does not change while the mappings are + // resolved against it. + let present = walk_files(install_dir); + let rel_of = |path: &PathBuf| { + path.strip_prefix(install_dir) + .unwrap_or(path) + .to_string_lossy() + .to_string() + }; + let matching = |pat: &str| -> Vec { + present + .iter() + .filter(|p| fast_glob::glob_match(pat, rel_of(p))) + .cloned() + .collect() + }; + // One name cannot stand for two files, so the first mapping to + // claim it keeps it. + let mut claimed: HashSet = HashSet::new(); for mapping in bins { let source_pattern = substitute_placeholders(&mapping.source, Some(version), arch_map); - let source_paths: Vec = fs::read_dir(install_dir) - .with_context(|| format!("reading directory {}", install_dir.display()))? - .filter_map(|entry| entry.ok()) - .filter(|entry| { - let name = entry.file_name(); - fast_glob::glob_match(&source_pattern, name.to_string_lossy().to_string()) - }) - .map(|entry| entry.path()) - .collect(); + // Try the most specific reading of the pattern first. Falling + // straight back to the file name would pick an arbitrary one + // when an archive ships the same binary for several + // architectures, each under its own directory. + + let mut source_paths = matching(&source_pattern); + // An archive with a single top-level directory has it promoted + // away, so the recorded path still carries a component the + // installed tree no longer has. + if source_paths.is_empty() { + if let Some((_, rest)) = source_pattern.split_once('/') { + source_paths = matching(rest); + } + } + // Last resort: the artifact was rearranged and only the name + // survives. Several files can answer to it, so `link_as` is + // dropped below and each keeps its own name. + if source_paths.is_empty() { + source_paths = present + .iter() + .filter(|p| { + p.file_name() + .map(|n| { + fast_glob::glob_match( + &source_pattern, + n.to_string_lossy().to_string(), + ) + }) + .unwrap_or(false) + }) + .cloned() + .collect(); + } if source_paths.is_empty() { return Err(SoarError::Custom(format!( @@ -137,6 +307,14 @@ pub async fn mangle_package_symlinks( .unwrap_or(&mapping.source) }); let link_path = bin_dir.join(link_name); + if !claimed.insert(link_path.clone()) { + warn!( + link = %link_path.display(), + source = %source_path.display(), + "skipping a second source for the same command" + ); + continue; + } set_executable(&source_path)?; @@ -328,6 +506,27 @@ fn find_matching_executable( .cloned() } +/// What identifies a package apart from its version: repository, name, id and +/// family. Two entries sharing this are two versions of one package. +pub type PackageKey = (String, String, Option, Option); + +/// Installed packages by repository and name, each with the family it was +/// installed under, so a package sharing a name is not mistaken for it. +pub type InstalledIndex = HashMap<(String, String), Vec<(Option, bool)>>; + +pub fn is_installed( + map: &InstalledIndex, + repo_name: &str, + pkg_name: &str, + pkg_family: Option<&str>, +) -> bool { + map.get(&(repo_name.to_string(), pkg_name.to_string())) + .is_some_and(|rows| { + rows.iter() + .any(|(family, installed)| *installed && family.as_deref() == pkg_family) + }) +} + #[cfg(test)] mod tests { use std::{ diff --git a/crates/soar-package/src/formats/common.rs b/crates/soar-package/src/formats/common.rs index e2b49fc9c..c91a99f44 100644 --- a/crates/soar-package/src/formats/common.rs +++ b/crates/soar-package/src/formats/common.rs @@ -17,7 +17,7 @@ use regex::Regex; use soar_config::config::get_config; use soar_utils::{ fs::{create_symlink, walk_dir}, - path::icons_dir, + path::{icons_dir, is_safe_component}, }; use tracing::{debug, trace}; @@ -291,11 +291,19 @@ pub fn setup_portable_dir, T: PackageExt>( portable_share: Option<&str>, portable_cache: Option<&str>, ) -> Result<()> { - let portable_dir_base = get_config().get_portable_dirs()?.join(format!( - "{}-{}", - package.pkg_name(), - package.pkg_id() - )); + // Packages that carry an id keep their existing directory name. Without + // one the family has to stand in, or two packages sharing a name would + // share a portable directory. Neither is trusted to be a single path + // component: both come from metadata, and one holding `..` would put the + // directory outside the portable root. + let family = package.pkg_family().filter(|f| is_safe_component(f)); + let portable_dir_base = get_config().get_portable_dirs()?.join( + match (package.pkg_id().filter(|id| is_safe_component(id)), family) { + (Some(pkg_id), _) => format!("{}-{}", package.pkg_name(), pkg_id), + (None, Some(family)) => format!("{}-{}", package.pkg_name(), family), + (None, None) => package.pkg_name().to_string(), + }, + ); let bin_path = bin_path.as_ref(); let pkg_name = package.pkg_name(); diff --git a/crates/soar-package/src/traits.rs b/crates/soar-package/src/traits.rs index d305609df..e6d9b2df7 100644 --- a/crates/soar-package/src/traits.rs +++ b/crates/soar-package/src/traits.rs @@ -8,8 +8,12 @@ pub trait PackageExt { /// Returns the package name (human-readable name). fn pkg_name(&self) -> &str; - /// Returns the unique package identifier. - fn pkg_id(&self) -> &str; + /// Returns the package identifier, when the repository publishes one. + fn pkg_id(&self) -> Option<&str>; + + /// Returns the family the package came from, when the repository + /// publishes one. This is what distinguishes two packages sharing a name. + fn pkg_family(&self) -> Option<&str>; /// Returns the package version string. fn version(&self) -> &str; diff --git a/crates/soar-registry/src/error.rs b/crates/soar-registry/src/error.rs index 840288f4e..40b74d373 100644 --- a/crates/soar-registry/src/error.rs +++ b/crates/soar-registry/src/error.rs @@ -12,6 +12,13 @@ use thiserror::Error; /// processing, or storing package metadata. #[derive(Error, Diagnostic, Debug)] pub enum RegistryError { + #[error( + "repository index is format {found}, but this soar understands up to \ + {supported}; upgrade soar to use this repository" + )] + #[diagnostic(code(soar_registry::unsupported_format))] + UnsupportedFormat { found: u32, supported: u32 }, + #[error("Error while {action}: {source}")] #[diagnostic(code(soar_registry::io))] IoError { diff --git a/crates/soar-registry/src/lib.rs b/crates/soar-registry/src/lib.rs index d2d582455..d96df41d2 100644 --- a/crates/soar-registry/src/lib.rs +++ b/crates/soar-registry/src/lib.rs @@ -39,7 +39,7 @@ pub mod package; pub use error::{ErrorContext, RegistryError, Result}; pub use metadata::{ - fetch_metadata, process_metadata_content, write_metadata_db, MetadataContent, - SQLITE_MAGIC_BYTES, ZST_MAGIC_BYTES, + fetch_metadata, parse_index, process_metadata_content, write_metadata_db, MetadataContent, + SQLITE_MAGIC_BYTES, SUPPORTED_FORMAT, ZST_MAGIC_BYTES, }; pub use package::RemotePackage; diff --git a/crates/soar-registry/src/metadata.rs b/crates/soar-registry/src/metadata.rs index 19b034f2b..a971415d5 100644 --- a/crates/soar-registry/src/metadata.rs +++ b/crates/soar-registry/src/metadata.rs @@ -5,12 +5,13 @@ use std::{ fs::{self, File}, - io::{self, BufReader, BufWriter, Write}, + io::{self, BufRead, BufReader, BufWriter, Write}, path::{Path, PathBuf}, time::UNIX_EPOCH, }; use minisign_verify::{PublicKey, Signature}; +use serde::Deserialize; use soar_config::repository::Repository; use soar_dl::http_client::SHARED_AGENT; use soar_utils::path::resolve_path; @@ -435,7 +436,7 @@ pub fn process_metadata_content( let tmp_file = File::open(&tmp_path) .with_context(|| format!("opening temporary file {tmp_path}"))?; let reader = BufReader::new(tmp_file); - let metadata: Vec = serde_json::from_reader(reader)?; + let metadata = parse_index_reader(reader)?; fs::remove_file(&tmp_path) .with_context(|| format!("removing temporary file {tmp_path}"))?; Ok(MetadataContent::Json(metadata)) @@ -443,11 +444,73 @@ pub fn process_metadata_content( } else if content[..4] == SQLITE_MAGIC_BYTES { Ok(MetadataContent::SqliteDb(content)) } else { - let metadata: Vec = serde_json::from_slice(&content)?; + let metadata = parse_index(&content)?; Ok(MetadataContent::Json(metadata)) } } +/// The highest index format this build understands. +pub const SUPPORTED_FORMAT: u32 = 1; + +/// The versioned shape of a metadata index. +/// +/// The original shape is a bare array of packages; this one wraps it so a +/// client can tell an index it cannot read from one that merely lacks a field. +#[derive(Deserialize)] +struct VersionedIndex { + format: u32, + packages: Vec, +} + +impl VersionedIndex { + /// Unwrap to the packages, refusing an index newer than this build. + fn into_packages(self) -> Result> { + if self.format > SUPPORTED_FORMAT { + return Err(RegistryError::UnsupportedFormat { + found: self.format, + supported: SUPPORTED_FORMAT, + }); + } + Ok(self.packages) + } +} + +/// Whether an index is the versioned shape, judged by its opening character. +/// +/// The two are told apart here rather than by an untagged enum, which reports +/// only that no variant matched and so turns one malformed field anywhere in +/// the index into a message that names nothing. +fn is_versioned(bytes: &[u8]) -> bool { + bytes + .iter() + .find(|b| !b.is_ascii_whitespace()) + .is_some_and(|b| *b == b'{') +} + +/// Parse an index in either shape, refusing one newer than this build. +pub fn parse_index(bytes: &[u8]) -> Result> { + if is_versioned(bytes) { + serde_json::from_slice::(bytes)?.into_packages() + } else { + Ok(serde_json::from_slice(bytes)?) + } +} + +/// Parse an index that is still on disk, without holding it twice in memory. +fn parse_index_reader(mut reader: impl BufRead) -> Result> { + let versioned = is_versioned(reader.fill_buf().map_err(|e| { + RegistryError::IoError { + action: "reading metadata".to_string(), + source: e, + } + })?); + if versioned { + serde_json::from_reader::<_, VersionedIndex>(reader)?.into_packages() + } else { + Ok(serde_json::from_reader(reader)?) + } +} + /// Writes SQLite database content to a file. /// /// This is a convenience function for writing [`MetadataContent::SqliteDb`] diff --git a/crates/soar-registry/src/package.rs b/crates/soar-registry/src/package.rs index a94176a8d..76ea13648 100644 --- a/crates/soar-registry/src/package.rs +++ b/crates/soar-registry/src/package.rs @@ -130,7 +130,11 @@ pub struct RemotePackage { #[serde(alias = "_disabled_reason")] pub disabled_reason: Option, - pub pkg_id: String, + /// Optional. It exists to disambiguate identically-named packages within + /// one repository; where names are already unique a repository can omit + /// it, and the name is used instead. + #[serde(default, deserialize_with = "empty_is_none")] + pub pkg_id: Option, pub pkg_name: String, #[serde(default, deserialize_with = "empty_is_none")] @@ -139,17 +143,21 @@ pub struct RemotePackage { #[serde(default, deserialize_with = "empty_is_none")] pub pkg_type: Option, - #[serde(default, deserialize_with = "empty_is_none")] - pub pkg_webpage: Option, - pub description: String, pub version: String, pub download_url: String, + /// Bytes, as the older format publishes them. #[serde(default, deserialize_with = "optional_number")] pub size_raw: Option, + /// Bytes, as the port format publishes them. The older format uses this + /// name for a human-readable string, which parses as no value, so the two + /// can coexist in one index without either being mistaken for the other. + #[serde(default, deserialize_with = "optional_number")] + pub size: Option, + #[serde(default, deserialize_with = "empty_is_none")] pub ghcr_pkg: Option, @@ -179,9 +187,6 @@ pub struct RemotePackage { #[serde(alias = "note")] pub notes: Option>, - #[serde(alias = "tag")] - pub tags: Option>, - #[serde(default, deserialize_with = "empty_is_none")] pub bsum: Option, @@ -232,6 +237,14 @@ pub struct RemotePackage { pub repology: Option>, pub snapshots: Option>, pub replaces: Option>, + /// Executables inside the artifact, as source path -> installed name. + /// Pinned side files to install alongside the artifact. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub extra: Option>, + /// What the package takes out of its artifact. Absent means the whole + /// artifact is the package, which is how the older format always behaved. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub files: Option>, } #[cfg(test)] @@ -249,11 +262,25 @@ mod tests { }"#; let pkg: RemotePackage = serde_json::from_str(json).unwrap(); - assert_eq!(pkg.pkg_id, "test-pkg"); + assert_eq!(pkg.pkg_id.as_deref(), Some("test-pkg")); assert_eq!(pkg.pkg_name, "test"); assert_eq!(pkg.version, "1.0.0"); } + #[test] + fn test_pkg_id_is_optional() { + let json = r#"{ + "pkg_name": "test", + "description": "A test package", + "version": "1.0.0", + "download_url": "https://example.com/test.tar.gz" + }"#; + + let pkg: RemotePackage = serde_json::from_str(json).unwrap(); + assert_eq!(pkg.pkg_id, None); + assert_eq!(pkg.pkg_name, "test"); + } + #[test] fn test_flexible_bool() { let json = r#"{ @@ -269,3 +296,29 @@ mod tests { assert_eq!(pkg.disabled, Some(true)); } } + +/// One file the package installs, as published in the index. +/// +/// `to` is a path inside the package directory, so the directory it lands in +/// says what it is: `bin/` is a command, `share/man/` a manual page. An empty +/// `source` means the artifact is itself the file. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct RemoteFile { + #[serde(default)] + pub source: String, + pub to: String, + /// Extra paths, relative to the package directory, resolving to this file. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub alias: Vec, +} + +/// A pinned side file as published in the index. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct RemoteExtra { + pub url: String, + pub to: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub blake3: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sha256: Option, +} diff --git a/crates/soar-utils/src/lib.rs b/crates/soar-utils/src/lib.rs index 640b1e88d..d0718b8af 100644 --- a/crates/soar-utils/src/lib.rs +++ b/crates/soar-utils/src/lib.rs @@ -7,3 +7,4 @@ pub mod path; pub mod pattern; pub mod system; pub mod time; +pub mod version; diff --git a/crates/soar-utils/src/version.rs b/crates/soar-utils/src/version.rs new file mode 100644 index 000000000..a24e9e0a1 --- /dev/null +++ b/crates/soar-utils/src/version.rs @@ -0,0 +1,238 @@ +//! Ordering for the version strings repositories actually publish. +//! +//! Comparison is segment-wise: a version is split into runs of digits and runs +//! of non-digits, and matching runs are compared numerically when both are +//! numeric and lexically otherwise. This avoids the two ways a plain string +//! comparison gets it wrong, `10` sorting below `9` and `1.10` below `1.9`. +//! +//! Strict semver is deliberately not used. Most published versions carry a +//! rebuild revision as `-N`, which semver reads as a prerelease and therefore +//! ranks *below* the plain version, the opposite of what it means here. Others +//! (`1.05`, `7.1-2`, `r1287.fef2b38-1`) are not valid semver at all. +//! +//! A version built from a commit hash has no order to recover: hashes carry no +//! time. Those compare equal to each other and below any ordinary version, so a +//! repository that wants upgrades between snapshots has to publish something +//! ordered, such as a date. + +use std::cmp::Ordering; + +/// Compare two version strings. +/// +/// ``` +/// use std::cmp::Ordering; +/// use soar_utils::version::compare_versions; +/// +/// assert_eq!(compare_versions("10.4.2", "9.0.0"), Ordering::Greater); +/// assert_eq!(compare_versions("1.10.0", "1.9.0"), Ordering::Greater); +/// assert_eq!(compare_versions("1.14.0-2", "1.14.0-1"), Ordering::Greater); +/// assert_eq!(compare_versions("2026.05.24", "2026.05.23"), Ordering::Greater); +/// ``` +pub fn compare_versions(a: &str, b: &str) -> Ordering { + // Two commit hashes carry no order at all, and segment rules would invent + // one, letting an arbitrary hash read as an upgrade or a downgrade. A hash + // ranks below any ordinary version rather than being compared segment-wise + // against one, which would leave the ordering intransitive: two hashes + // equal to each other yet landing on opposite sides of the same tag, which + // is enough to panic `sort_by`. + match (is_commit_hash(a), is_commit_hash(b)) { + (true, true) => return Ordering::Equal, + (true, false) => return Ordering::Less, + (false, true) => return Ordering::Greater, + (false, false) => {} + } + + let mut left = segments(a); + let mut right = segments(b); + + loop { + match (left.next(), right.next()) { + (None, None) => return Ordering::Equal, + // A trailing segment means opposite things depending on its + // shape: a number is a further release (1.2.1 over 1.2, or the + // rebuild in 1.14.0-1 over 1.14.0), while text is a prerelease + // (1.2rc and 0.5.7-beta both precede their release). + (Some(x), None) => return extra_segment_order(x), + (None, Some(y)) => return extra_segment_order(y).reverse(), + (Some(x), Some(y)) => { + match compare_segment(x, y) { + Ordering::Equal => continue, + other => return other, + } + } + } + } +} + +/// Whether `candidate` supersedes `current`. +pub fn is_newer(candidate: &str, current: &str) -> bool { + compare_versions(candidate, current) == Ordering::Greater +} + +/// Whether a version is nothing but a commit hash. +/// +/// Requires a letter, so a long run of digits stays a version: 20260412 is a +/// date, not a hash. +fn is_commit_hash(version: &str) -> bool { + version.len() >= 7 + && version.len() <= 40 + && version.chars().all(|c| c.is_ascii_hexdigit()) + && version.chars().any(|c| c.is_ascii_alphabetic()) +} + +/// How a version compares against one that stopped earlier, judged by the +/// first segment the longer one carries alone. +fn extra_segment_order(segment: &str) -> Ordering { + if segment.starts_with(|c: char| c.is_ascii_digit()) { + Ordering::Greater + } else { + Ordering::Less + } +} + +fn compare_segment(a: &str, b: &str) -> Ordering { + match (a.parse::(), b.parse::()) { + (Ok(x), Ok(y)) => x.cmp(&y), + // A numeric segment outranks a textual one, so 1.2 beats 1.2rc. + (Ok(_), Err(_)) => Ordering::Greater, + (Err(_), Ok(_)) => Ordering::Less, + (Err(_), Err(_)) => a.cmp(b), + } +} + +/// Split a version into runs of digits and runs of everything else, +/// discarding the separators between them. +fn segments(version: &str) -> impl Iterator { + let mut rest = version; + std::iter::from_fn(move || { + while let Some(c) = rest.chars().next() { + if c.is_ascii_alphanumeric() { + break; + } + rest = &rest[c.len_utf8()..]; + } + if rest.is_empty() { + return None; + } + let numeric = rest.starts_with(|c: char| c.is_ascii_digit()); + let end = rest + .find(|c: char| c.is_ascii_digit() != numeric || !c.is_ascii_alphanumeric()) + .unwrap_or(rest.len()); + let (seg, tail) = rest.split_at(end); + rest = tail; + Some(seg) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn numeric_segments_beat_string_order() { + assert_eq!(compare_versions("10.4.2", "9.0.0"), Ordering::Greater); + assert_eq!(compare_versions("1.10.0", "1.9.0"), Ordering::Greater); + assert_eq!(compare_versions("1.06", "1.05"), Ordering::Greater); + } + + #[test] + fn rebuild_revision_is_newer_not_older() { + // semver would call these prereleases and rank them below 1.14.0 + assert_eq!(compare_versions("1.14.0-1", "1.14.0"), Ordering::Greater); + assert_eq!(compare_versions("2.0.19-4", "2.0.19-3"), Ordering::Greater); + assert_eq!(compare_versions("0.2.4-4", "0.2.4-3"), Ordering::Greater); + } + + #[test] + fn dates_order_naturally() { + assert_eq!( + compare_versions("2026.05.24", "2026.05.23"), + Ordering::Greater + ); + assert_eq!( + compare_versions("2026.05.24.1.dda726e-1", "2026.05.23.1.aaa111b-1"), + Ordering::Greater + ); + } + + #[test] + fn dates_compare_whatever_separates_them() { + // hyphen-separated, the ISO form + assert_eq!( + compare_versions("2026-04-12", "2026-04-11"), + Ordering::Greater + ); + assert_eq!( + compare_versions("2026-05-01", "2026-04-30"), + Ordering::Greater + ); + assert_eq!( + compare_versions("2027-01-01", "2026-12-31"), + Ordering::Greater + ); + // leading zeros are numeric, not text + assert_eq!(compare_versions("2026-04-09", "2026-04-10"), Ordering::Less); + // the separator itself carries no meaning + assert_eq!( + compare_versions("2026-04-12", "2026.04.12"), + Ordering::Equal + ); + assert_eq!( + compare_versions("2026-04-12-2", "2026-04-12"), + Ordering::Greater + ); + } + + #[test] + fn equal_versions_compare_equal() { + assert_eq!(compare_versions("1.2.3", "1.2.3"), Ordering::Equal); + assert_eq!(compare_versions("89c99d2a9", "89c99d2a9"), Ordering::Equal); + } + + #[test] + fn commit_hashes_have_no_meaningful_order() { + // Neither supersedes the other, so neither can drive an upgrade. + assert_eq!(compare_versions("89c99d2a9", "0f3a21b"), Ordering::Equal); + assert_eq!(compare_versions("0f3a21b", "89c99d2a9"), Ordering::Equal); + assert!(!is_newer("89c99d2a9", "0f3a21b")); + assert!(!is_newer("0f3a21b", "89c99d2a9")); + } + + #[test] + fn hashes_rank_below_ordinary_versions() { + // Sorting a mixed list needs a total order. Comparing each hash + // segment-wise against the tag would put one above it and one below, + // while the two stay equal to each other. + assert_eq!(compare_versions("0f3a21b", "5.0"), Ordering::Less); + assert_eq!(compare_versions("89c99d2a9", "5.0"), Ordering::Less); + assert_eq!(compare_versions("5.0", "89c99d2a9"), Ordering::Greater); + + let mut versions = ["89c99d2a9", "5.0", "0f3a21b", "4.9"]; + versions.sort_by(|a, b| compare_versions(b, a)); + assert_eq!(&versions[..2], &["5.0", "4.9"]); + } + + #[test] + fn digit_runs_are_versions_not_hashes() { + // a date-like version must keep comparing normally + assert_eq!(compare_versions("20260413", "20260412"), Ordering::Greater); + } + + #[test] + fn longer_version_wins_when_prefix_matches() { + assert_eq!(compare_versions("1.2.1", "1.2"), Ordering::Greater); + assert_eq!(compare_versions("3.13.1", "3.13"), Ordering::Greater); + } + + #[test] + fn text_segments_rank_below_numeric() { + // a text suffix is a prerelease and precedes the plain version + assert_eq!(compare_versions("1.2", "1.2rc"), Ordering::Greater); + assert_eq!(compare_versions("0.5.7", "0.5.7-beta"), Ordering::Greater); + // between two prereleases, order is lexical + assert_eq!( + compare_versions("0.5.7-beta", "0.5.7-alpha"), + Ordering::Greater + ); + } +}