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..3fc03bb58 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::{ @@ -61,10 +61,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 +93,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 +108,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..03b75476e 100644 --- a/crates/soar-cli/src/inspect.rs +++ b/crates/soar-cli/src/inspect.rs @@ -44,7 +44,7 @@ fn get_installed_path( conn, &package.repo_name, &package.pkg_name, - &package.pkg_id, + package.pkg_id.as_deref(), &package.version, ) })?; @@ -71,6 +71,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 +91,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..f501a6729 100644 --- a/crates/soar-cli/src/install.rs +++ b/crates/soar-cli/src/install.rs @@ -75,9 +75,15 @@ 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); + // Re-resolve with the specific selected package. The + // family has to come along, or a name ambiguous within one + // repository resolves right back to the same choice. + let specific_query = match &pkg.pkg_family { + Some(family) => { + format!("{}/{}:{}", family, pkg.pkg_name, pkg.repo_name) + } + None => format!("{}:{}", pkg.pkg_name, pkg.repo_name), + }; let re_results = install::resolve_packages(ctx, &[specific_query], &options).await?; for r in re_results { @@ -97,13 +103,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 +167,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,8 +206,12 @@ async fn install_with_show( }; if let Some(pkg) = pkg { - let specific_query = - format!("{}#{}:{}", pkg.pkg_name, pkg.pkg_id, pkg.repo_name); + let specific_query = match &pkg.pkg_family { + Some(family) => { + format!("{}/{}:{}", family, pkg.pkg_name, pkg.repo_name) + } + None => format!("{}:{}", pkg.pkg_name, pkg.repo_name), + }; let re_results = install::resolve_packages(ctx, &[specific_query], options).await?; for r in re_results { @@ -221,13 +231,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 +257,7 @@ async fn install_with_show( conn, query.name.as_deref(), None, + query.family.as_deref(), None, None, Some(SortDirection::Asc), @@ -266,6 +277,7 @@ async fn install_with_show( conn, query.name.as_deref(), None, + query.family.as_deref(), None, None, Some(SortDirection::Asc), @@ -302,7 +314,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 +329,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 +349,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, @@ -352,9 +364,8 @@ async fn install_with_show( 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,10 +413,9 @@ 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()) ); @@ -424,7 +434,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 +448,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..3fb4f06c1 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,61 @@ 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. + let mut tmp_name = output_path.file_name().unwrap_or_default().to_os_string(); + tmp_name.push(".tmp"); + 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..07b75b68d 100644 --- a/crates/soar-cli/src/list.rs +++ b/crates/soar-cli/src/list.rs @@ -53,10 +53,9 @@ pub async fn search_packages( version = package.version, description = package.description, size = package.ghcr_size.or(package.size), - "[{}] {}#{}:{} | {} | {} - {} ({})", + "[{}] {}:{} | {} | {} - {} ({})", state_icon, Colored(Blue, &package.pkg_name), - Colored(Cyan, &package.pkg_id), Colored(Green, &package.repo_name), Colored(LightRed, &package.version), package @@ -124,9 +123,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 +235,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 +264,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,10 +296,9 @@ 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, Colored(Blue, &package.pkg_name), - Colored(Cyan, &package.pkg_id), Colored(Cyan, &package.repo_name), Colored(LightRed, &package.version), package @@ -408,9 +397,11 @@ 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.pkg_id.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..daaea3a2e 100644 --- a/crates/soar-cli/src/run.rs +++ b/crates/soar-cli/src/run.rs @@ -38,7 +38,7 @@ pub async fn run_package( ctx, package_name, Some(&pkg.repo_name), - Some(&pkg.pkg_id), + pkg.pkg_id.as_deref(), no_verify, ) .await?; 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..13f361ef2 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,12 +129,16 @@ 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]")) @@ -142,10 +146,9 @@ pub fn select_package_interactively_with_installed( String::new() }; info!( - "[{}] {}#{}:{} | {}{}", + "[{}] {}:{} | {}{}", idx + 1, Colored(Blue, &pkg.pkg_name()), - Colored(Cyan, &pkg.pkg_id()), Colored(Green, pkg.repo_name()), Colored(LightRed, pkg.version()), installed_marker @@ -180,9 +183,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..48686b983 100644 --- a/crates/soar-config/src/config.rs +++ b/crates/soar-config/src/config.rs @@ -79,8 +79,13 @@ 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. + /// 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 +194,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() { @@ -280,6 +287,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={}", @@ -405,6 +414,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) { @@ -777,6 +788,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..79cfdfd60 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. @@ -269,7 +273,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. @@ -326,6 +334,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 +345,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) => { @@ -452,6 +466,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 { diff --git a/crates/soar-core/src/database/models.rs b/crates/soar-core/src/database/models.rs index e31edda4d..7a64b7e93 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::{PackageBinary, PackageExtra, 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,10 @@ pub struct Package { pub deprecated: bool, pub desktop_integration: Option, pub portable: Option, + /// Executables inside the artifact, as source path -> installed name. + pub binaries: Option>, + /// Pinned side files installed alongside the artifact. + pub extra: Option>, } impl PackageExt for Package { @@ -72,8 +77,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 +141,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 +169,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 +194,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 +225,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 +260,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 +275,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 +293,8 @@ impl From for Package { deprecated: false, desktop_integration: pkg.desktop_integration, portable: pkg.portable, + binaries: pkg.binaries, + extra: pkg.extra, } } } diff --git a/crates/soar-core/src/package/install.rs b/crates/soar-core/src/package/install.rs index 45cf3e38a..ce37eae26 100644 --- a/crates/soar-core/src/package/install.rs +++ b/crates/soar-core/src/package/install.rs @@ -29,6 +29,7 @@ use soar_utils::{ error::FileSystemResult, fs::{safe_remove, walk_dir}, hash::calculate_checksum, + path::is_safe_component, }; use tracing::{debug, trace, warn}; @@ -45,6 +46,75 @@ use crate::{ /// /// AppImages and plain binaries are ELF and need the executable bit; archives /// are not and are extracted instead. +/// 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(()) +} + +/// 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(); + } + } +} + fn is_elf(path: &Path) -> bool { let mut magic = [0u8; 4]; fs::File::open(path) @@ -109,7 +179,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 +271,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 +303,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 +326,7 @@ impl PackageInstaller { repo_name, pkg_id, pkg_name, + pkg_family: package.pkg_family.as_deref(), pkg_type, version, size, @@ -296,16 +368,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 +487,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 +502,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 +516,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 +549,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 +568,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, }, @@ -697,11 +765,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 +805,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 +817,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 +847,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 +893,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 +932,12 @@ impl PackageInstaller { } } + 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 +1028,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 +1054,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,8 +1104,15 @@ 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, + pkg_id, + self.package.pkg_family.as_deref(), + version, + ) + })?; let alternate_packages: Vec = self.db.with_conn(|conn| { diff --git a/crates/soar-core/src/package/local.rs b/crates/soar-core/src/package/local.rs index 2e901f1d1..4819a0f87 100644 --- a/crates/soar-core/src/package/local.rs +++ b/crates/soar-core/src/package/local.rs @@ -126,8 +126,8 @@ impl LocalPackage { Ok(Self { path, - pkg_name, pkg_id, + pkg_name, version, pkg_type, size, @@ -142,7 +142,7 @@ impl LocalPackage { Package { id: 0, repo_name: "local".to_string(), - pkg_id: self.pkg_id.clone(), + pkg_id: Some(self.pkg_id.clone()), pkg_name: self.pkg_name.clone(), pkg_type: self.pkg_type.clone(), version: self.version.clone(), diff --git a/crates/soar-core/src/package/query.rs b/crates/soar-core/src/package/query.rs index 9ae8df932..17d715c26 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,9 @@ impl TryFrom<&str> for PackageQuery { let re = PACKAGE_RE.get_or_init(|| { Regex::new( r"(?x) + (?:(?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 +49,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,6 +70,7 @@ 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()), diff --git a/crates/soar-core/src/package/remove.rs b/crates/soar-core/src/package/remove.rs index 5dbb21ac6..e2e01e7ff 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}, }; @@ -78,6 +79,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. +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 +133,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 +160,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 ); @@ -214,6 +236,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 +267,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..7b0185049 100644 --- a/crates/soar-core/src/package/update.rs +++ b/crates/soar-core/src/package/update.rs @@ -16,13 +16,21 @@ 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 { @@ -34,7 +42,14 @@ pub fn remove_old_versions(package: &Package, db: &DieselDatabase, force: bool) } 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..99beef6ae 100644 --- a/crates/soar-core/src/package/url.rs +++ b/crates/soar-core/src/package/url.rs @@ -155,8 +155,8 @@ impl UrlPackage { Ok(Self { url: reference.to_string(), - pkg_name, pkg_id, + pkg_name, version, pkg_type, is_ghcr: true, @@ -235,8 +235,8 @@ impl UrlPackage { Ok(Self { url: url.to_string(), - pkg_name, pkg_id, + pkg_name, version, pkg_type, is_ghcr: false, @@ -250,7 +250,7 @@ impl UrlPackage { Package { id: 0, repo_name: "local".to_string(), - pkg_id: self.pkg_id.clone(), + pkg_id: Some(self.pkg_id.clone()), pkg_name: self.pkg_name.clone(), pkg_type: self.pkg_type.clone(), version: self.version.clone(), @@ -264,7 +264,7 @@ impl UrlPackage { Package { id: 0, repo_name: "local".to_string(), - pkg_id: self.pkg_id.clone(), + pkg_id: Some(self.pkg_id.clone()), pkg_name: self.pkg_name.clone(), pkg_type: self.pkg_type.clone(), version: self.version.clone(), @@ -468,7 +468,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_id.as_deref(), Some("github.com.user.testrepo")); assert_eq!(pkg.download_url, url); } @@ -618,7 +618,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_id.as_deref(), Some("pkgforge.soar")); assert_eq!(pkg.download_url, ""); assert_eq!( pkg.ghcr_pkg, 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/up.sql b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/up.sql new file mode 100644 index 000000000..8be8cdbde --- /dev/null +++ b/crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/up.sql @@ -0,0 +1,33 @@ +-- Installed rows record the family a package came from, so variants can be +-- told apart without the package id repositories no longer publish, and the +-- id itself stops being required. SQLite cannot relax NOT NULL in place, so +-- the table is rebuilt. +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; 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..56e64639c --- /dev/null +++ b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sql @@ -0,0 +1,59 @@ +-- 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; 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..d0d727597 --- /dev/null +++ b/crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql @@ -0,0 +1,65 @@ +-- The declarative format carries what a package contains rather than leaving +-- it to be guessed: `binaries` says where each executable lives inside the +-- artifact and what to call it, `extra` lists side files installed alongside +-- it. A repository publishing that format has no package id to give, so the +-- id stops being required, and SQLite cannot relax NOT NULL in place. +-- +-- Three columns go: `pkg_webpage` was derivable from the package's own fields, +-- `tags` was stored and displayed but never searched, and `version_upstream` +-- was never read at all. +-- +-- The uniqueness key keeps the id, but NULLs are collapsed first: SQLite +-- treats every NULL as distinct, so an id-less package would otherwise insert +-- a fresh duplicate on every sync instead of conflicting with itself. +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, + binaries JSONB, + extra 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; + +CREATE UNIQUE INDEX packages_identity + ON packages (COALESCE(pkg_id, ''), pkg_name, version); 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..2ec252351 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::{PackageBinary, PackageExtra, 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,17 @@ pub struct Package { pub soar_syms: bool, pub desktop_integration: Option, pub portable: Option, + /// Executables inside the artifact, as source path -> installed name. + pub binaries: Option>, + /// Pinned side files installed alongside the artifact. + pub extra: Option>, } impl Queryable for Package { type Row = ( i32, - String, - String, Option, + String, Option, Option, Option, @@ -69,7 +74,6 @@ impl Queryable for Package { Option, Option, Option, - Option, Option, Option, Option, @@ -81,6 +85,8 @@ impl Queryable for Package { bool, Option, Option, + Option, + Option, ); fn build(row: Self::Row) -> diesel::deserialize::Result { @@ -90,37 +96,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, + binaries: json_vec!(row.34), + extra: json_vec!(row.35), }) } } @@ -131,7 +137,7 @@ 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_name: String, pub pkg_type: Option, pub version: String, @@ -145,7 +151,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 +192,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 +213,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 +225,8 @@ pub struct NewPackage<'a> { pub soar_syms: bool, pub desktop_integration: Option, pub portable: Option, + pub binaries: Option, + pub extra: Option, } #[derive(Default, Insertable)] diff --git a/crates/soar-db/src/models/types.rs b/crates/soar-db/src/models/types.rs index e75a17dd8..2fac0e262 100644 --- a/crates/soar-db/src/models/types.rs +++ b/crates/soar-db/src/models/types.rs @@ -137,3 +137,29 @@ mod tests { assert!(!PackageProvide::from_string("@../evil").is_safe()); } } + +/// One executable inside a package artifact. +/// +/// `source` is a path relative to the extracted artifact and may be a glob, +/// since archives often wrap their contents in a versioned directory. +/// `link_as` is the name to install it under, defaulting to the file's own. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct PackageBinary { + pub source: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub link_as: Option, +} + +/// 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..28b44a8e7 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,31 @@ use crate::{ schema::core::{packages, portable_package}, }; +/// 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 +56,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 +86,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 +204,22 @@ impl CoreRepository { conn: &mut SqliteConnection, repo_name: &str, pkg_name: &str, - pkg_id: &str, + pkg_id: 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()), + }; + let result: Option<(Package, Option)> = query .select((Package::as_select(), Option::::as_select())) .first(conn) .optional()?; @@ -300,13 +338,17 @@ impl CoreRepository { pub fn find_alternates( conn: &mut SqliteConnection, pkg_name: &str, - exclude_pkg_id: &str, + exclude_pkg_id: Option<&str>, exclude_version: &str, ) -> QueryResult> { let results: Vec<(Package, Option)> = packages::table .left_join(portable_package::table) .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.ne(exclude_pkg_id)) + .filter( + packages::pkg_id + .is_null() + .or(packages::pkg_id.ne(exclude_pkg_id)), + ) .filter(packages::version.ne(exclude_version)) .select((Package::as_select(), Option::::as_select())) .load(conn)?; @@ -317,11 +359,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 +373,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 +412,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 +425,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 +464,68 @@ impl CoreRepository { pub fn unlink_others( conn: &mut SqliteConnection, pkg_name: &str, - keep_pkg_id: &str, + keep_pkg_id: Option<&str>, + keep_pkg_family: Option<&str>, keep_version: &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. + let stale = Self::rows_other_than( + conn, + pkg_name, + keep_pkg_id, + keep_pkg_family, + Some(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_pkg_id: Option<&str>, + keep_pkg_family: Option<&str>, + keep_version: Option<&str>, + ) -> QueryResult> { + let rows: Vec<(i32, Option, Option, String)> = packages::table + .filter(packages::pkg_name.eq(pkg_name)) + .select(( + packages::id, + packages::pkg_id, + packages::pkg_family, + packages::version, + )) + .load(conn)?; + Ok(rows + .into_iter() + .filter(|(_, pkg_id, pkg_family, version)| { + let same = 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 +540,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 +560,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 +574,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 +655,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 +676,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 +699,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 +727,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)) @@ -659,26 +742,30 @@ impl CoreRepository { pub fn unlink_others_by_checksum( conn: &mut SqliteConnection, pkg_name: &str, - keep_pkg_id: &str, + keep_pkg_id: Option<&str>, + keep_pkg_family: Option<&str>, keep_checksum: Option<&str>, ) -> QueryResult { if let Some(checksum) = keep_checksum { + let others = Self::rows_other_than(conn, pkg_name, keep_pkg_id, keep_pkg_family, None)?; + if others.is_empty() { + return Ok(0); + } diesel::update( packages::table - .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.ne(keep_pkg_id)) + .filter(packages::id.eq_any(others)) .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) + let others = Self::rows_other_than(conn, pkg_name, keep_pkg_id, keep_pkg_family, None)?; + if others.is_empty() { + return Ok(0); + } + diesel::update(packages::table.filter(packages::id.eq_any(others))) + .set(packages::unlinked.eq(true)) + .execute(conn) } } @@ -687,14 +774,14 @@ impl CoreRepository { pub fn link_by_checksum( conn: &mut SqliteConnection, pkg_name: &str, - pkg_id: &str, + pkg_id: Option<&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(match_pkg_id(pkg_id)) .filter(packages::checksum.eq(checksum)), ) .set(packages::unlinked.eq(false)) @@ -703,7 +790,7 @@ impl CoreRepository { diesel::update( packages::table .filter(packages::pkg_name.eq(pkg_name)) - .filter(packages::pkg_id.eq(pkg_id)), + .filter(match_pkg_id(pkg_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..bbb4dc34b 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)". @@ -390,10 +393,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 +408,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 +442,38 @@ impl MetadataRepository { pub fn find_newer_version( conn: &mut SqliteConnection, pkg_name: &str, - pkg_id: &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() - }; + // Matched by name alone. An id recorded at install time may no longer + // appear in the metadata at all, and requiring it to match would + // report every such package as already up to date. + // + // 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: Vec = query.select(Package::as_select()).load(conn)?; - 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 +491,7 @@ impl MetadataRepository { conn: &mut SqliteConnection, metadata: &[RemotePackage], repo_name: &str, - ) -> QueryResult<()> { + ) -> QueryResult { debug!( repo_name = repo_name, count = metadata.len(), @@ -502,23 +509,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 +539,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 +574,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 +595,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 +607,18 @@ impl MetadataRepository { soar_syms: package.soar_syms.unwrap_or(false), desktop_integration: package.desktop_integration, portable: package.portable, + binaries: package.binaries.as_ref().map(|b| json!(b)), + extra: package.extra.as_ref().map(|e| json!(e)), }; 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 +632,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..d4834faa3 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, + binaries -> Nullable, + extra -> Nullable, } } diff --git a/crates/soar-dl/src/download.rs b/crates/soar-dl/src/download.rs index 4f3e37893..c074916c2 100644 --- a/crates/soar-dl/src/download.rs +++ b/crates/soar-dl/src/download.rs @@ -322,16 +322,44 @@ 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(".")) + }); + // Cleanup below must never reach a directory that was + // already there: the default destination is the download's + // own parent, so removing it would take the user's other + // files with it. + let dir_existed = extract_dir.exists(); + debug!(archive = %output_path.display(), dest = %extract_dir.display(), + ?format, "extracting archive"); + // A bare .gz or .bz2 of a single file shares its magic + // number with the tar-wrapped form, so detection can be + // right about the compression and wrong about the + // container. Leave the download in place rather than + // failing the install outright. + if let Err(e) = compak::extract_archive(&output_path, &extract_dir) { + warn!(archive = %output_path.display(), error = %e, + "extraction failed, installing the download as-is"); + if !dir_existed { + std::fs::remove_dir_all(&extract_dir).ok(); + } + } + } + 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..673feb57e 100644 --- a/crates/soar-operations/src/apply.rs +++ b/crates/soar-operations/src/apply.rs @@ -71,6 +71,7 @@ pub async fn compute_diff( conn, Some(&pkg.name), pkg.pkg_id.as_deref(), + None, pkg.version.as_deref(), None, Some(SortDirection::Asc), @@ -90,6 +91,7 @@ pub async fn compute_diff( conn, Some(&pkg.name), pkg.pkg_id.as_deref(), + None, 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, @@ -136,18 +138,16 @@ pub async fn compute_diff( 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 { @@ -180,7 +180,9 @@ pub async fn compute_diff( for installed in all_installed { let is_declared = declared_keys.iter().any(|(name, pkg_id, 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 repo_matches = repo.as_ref().is_none_or(|r| *r == installed.repo_name); name_matches && pkg_id_matches && repo_matches }); @@ -320,7 +322,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 +337,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 +347,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; @@ -573,7 +572,7 @@ fn check_url_package_status( conn, Some("local"), Some(&url_pkg.pkg_name), - Some(&url_pkg.pkg_id), + Some(url_pkg.pkg_id.as_str()), None, None, None, diff --git a/crates/soar-operations/src/context.rs b/crates/soar-operations/src/context.rs index 5af1bc3fe..d04c2b3da 100644 --- a/crates/soar-operations/src/context.rs +++ b/crates/soar-operations/src/context.rs @@ -230,25 +230,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..b05d5a204 100644 --- a/crates/soar-operations/src/health.rs +++ b/crates/soar-operations/src/health.rs @@ -46,14 +46,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, } }) diff --git a/crates/soar-operations/src/install.rs b/crates/soar-operations/src/install.rs index dd3420428..c316b3e31 100644 --- a/crates/soar-operations/src/install.rs +++ b/crates/soar-operations/src/install.rs @@ -10,7 +10,7 @@ use std::{ }; use minisign_verify::{PublicKey, Signature}; -use soar_config::utils::default_install_patterns; +use soar_config::{packages::BinaryMapping, utils::default_install_patterns}; use soar_core::{ database::{ connection::{DieselDatabase, MetadataManager}, @@ -153,7 +153,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 +171,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 +205,7 @@ fn resolve_all_variants( None, None, None, + None, Some(SortDirection::Asc), ) })? @@ -225,6 +225,7 @@ fn resolve_all_variants( None, None, None, + None, Some(SortDirection::Asc), )?; Ok(pkgs @@ -257,15 +258,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 +301,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 +324,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 +395,7 @@ fn resolve_by_pkg_id( conn, None, query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -395,6 +415,7 @@ fn resolve_by_pkg_id( conn, None, query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -494,7 +515,6 @@ fn resolve_normal( 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(), }); @@ -537,7 +557,8 @@ fn find_packages( MetadataRepository::find_filtered( conn, Some(&existing.pkg_name), - Some(&existing.pkg_id), + None, + existing.pkg_id.as_deref(), None, None, None, @@ -564,6 +585,7 @@ fn find_packages( conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -583,6 +605,7 @@ fn find_packages( conn, query.name.as_deref(), query.pkg_id.as_deref(), + query.family.as_deref(), None, None, None, @@ -660,7 +683,7 @@ pub async fn perform_installation( 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, @@ -681,12 +704,10 @@ pub async fn perform_installation( 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 +754,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, @@ -788,7 +811,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, @@ -815,8 +838,8 @@ 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 ))); } } @@ -827,22 +850,29 @@ async fn install_single_package( .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); + // Name and version alone collide across sources: the same + // release packaged by two repositories would share a directory. + let source = pkg + .pkg_id + .as_deref() + .or(pkg.ghcr_pkg.as_deref()) + .unwrap_or(pkg.download_url.as_str()); + let input = format!("{}:{}:{}", pkg.pkg_name, pkg.version, source); 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) { + // 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 ))); } let install_dir = config .get_packages_path(target.profile.clone())? - .join(format!("{}-{}-{}", pkg.pkg_name, pkg.pkg_id, dir_suffix)); + .join(format!("{}-{}", pkg.pkg_name, dir_suffix)); let main_binary_name = pkg .provides .as_ref() @@ -921,12 +951,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 +976,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 +983,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 +995,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 +1005,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 +1026,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 +1036,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 +1043,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 +1058,24 @@ 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, }); + // A repository can describe where its executables live; local + // configuration still wins, so an override in packages.toml is never + // silently replaced by whatever the index says. + let index_binaries: Option> = pkg.binaries.as_ref().map(|bins| { + bins.iter() + .map(|b| { + BinaryMapping { + source: b.source.clone(), + link_as: b.link_as.clone(), + } + }) + .collect() + }); + let binaries = target.binaries.clone().or(index_binaries); + let symlinks = mangle_package_symlinks( &install_dir, &bin_dir, @@ -1049,7 +1083,7 @@ 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(), ) .await?; @@ -1059,7 +1093,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::DesktopIntegration, }); @@ -1082,7 +1115,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 +1134,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!( diff --git a/crates/soar-operations/src/list.rs b/crates/soar-operations/src/list.rs index cd4d04085..665cc79ff 100644 --- a/crates/soar-operations/src/list.rs +++ b/crates/soar-operations/src/list.rs @@ -57,30 +57,39 @@ pub async fn list_packages( })? }; - let installed_pkgs: HashMap<(String, String, String), bool> = diesel_db + let installed_pkgs: HashMap<(String, String), bool> = 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. + .map(|pkg| ((pkg.repo_name, pkg.pkg_name), pkg.is_installed)) + .fold(HashMap::new, |mut acc, (key, installed)| { + *acc.entry(key).or_insert(false) |= installed; + acc + }) + .reduce(HashMap::new, |mut acc, part| { + for (key, installed) in part { + *acc.entry(key).or_insert(false) |= installed; + } + 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 key = (entry.repo_name.clone(), entry.pkg.pkg_name.clone()); let installed = installed_pkgs.get(&key).copied().unwrap_or(false); // 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_type: entry.pkg.pkg_type, version: entry.pkg.version, 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..37e22fcbb 100644 --- a/crates/soar-operations/src/run.rs +++ b/crates/soar-operations/src/run.rs @@ -36,6 +36,7 @@ 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); @@ -49,7 +50,8 @@ pub async fn prepare_run( conn, Some(package_name), pkg_id, - None, + family, + version, None, None, ) @@ -68,7 +70,8 @@ pub async fn prepare_run( conn, Some(package_name), pkg_id, - None, + family, + version, None, None, )?; @@ -109,8 +112,8 @@ pub async fn prepare_run( // 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 ))); } @@ -137,12 +140,8 @@ pub async fn prepare_run( .with_context(|| format!("creating directory {}", cache_bin.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, diff --git a/crates/soar-operations/src/search.rs b/crates/soar-operations/src/search.rs index 1d99c777f..cb97b9436 100644 --- a/crates/soar-operations/src/search.rs +++ b/crates/soar-operations/src/search.rs @@ -55,13 +55,27 @@ pub async fn search_packages( fuzzy_search(ctx, query, search_limit).await? }; - let installed_pkgs: HashMap<(String, String, String), bool> = diesel_db + let installed_pkgs: HashMap<(String, String), bool> = 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. + .map(|pkg| ((pkg.repo_name, pkg.pkg_name), pkg.is_installed)) + .fold(HashMap::new, |mut acc, (key, installed)| { + *acc.entry(key).or_insert(false) |= installed; + acc + }) + .reduce(HashMap::new, |mut acc, part| { + for (key, installed) in part { + *acc.entry(key).or_insert(false) |= installed; + } + acc + }); let total_count = packages.len(); @@ -69,11 +83,7 @@ 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 key = (package.repo_name.clone(), package.pkg_name.clone()); let installed = installed_pkgs.get(&key).copied().unwrap_or(false); SearchEntry { package, @@ -184,8 +194,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 +235,7 @@ pub async fn query_package(ctx: &SoarContext, query_str: &str) -> SoarResult SoarResult, pub repo_name: String, pub version: String, pub install_dir: PathBuf, @@ -70,7 +69,6 @@ pub struct InstalledInfo { #[derive(Debug)] pub struct FailedInfo { pub pkg_name: String, - pub pkg_id: String, pub error: String, } @@ -93,7 +91,6 @@ pub struct RemoveReport { pub struct RemovedInfo { pub pkg_name: String, - pub pkg_id: String, pub repo_name: String, pub version: String, } @@ -102,7 +99,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, @@ -168,7 +164,6 @@ pub struct HealthReport { pub struct BrokenPackage { pub pkg_name: String, - pub pkg_id: String, pub installed_path: String, } diff --git a/crates/soar-operations/src/update.rs b/crates/soar-operations/src/update.rs index 57b130841..f7379a3e9 100644 --- a/crates/soar-operations/src/update.rs +++ b/crates/soar-operations/src/update.rs @@ -137,7 +137,12 @@ 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_family.as_deref(), + &pkg.version, + ) })? .flatten() .map(|p| { @@ -150,7 +155,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 +172,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 +180,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(), @@ -209,7 +211,6 @@ fn check_local_update( 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 +221,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 +250,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 +310,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 +330,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 +364,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 +385,7 @@ fn get_existing( conn, &package.repo_name, &package.pkg_name, - &package.pkg_id, + package.pkg_id.as_deref(), &package.version, ) })?; @@ -449,15 +445,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 +475,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() @@ -479,7 +488,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() diff --git a/crates/soar-operations/src/utils.rs b/crates/soar-operations/src/utils.rs index bf7d65fa2..aaee7f7ac 100644 --- a/crates/soar-operations/src/utils.rs +++ b/crates/soar-operations/src/utils.rs @@ -88,6 +88,28 @@ fn create_provide_symlinks( } /// Creates symlinks from installed package binaries to the bin directory. +#[allow(clippy::too_many_arguments)] +/// 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 +} + #[allow(clippy::too_many_arguments)] pub async fn mangle_package_symlinks( install_dir: &Path, @@ -106,15 +128,52 @@ pub async fn mangle_package_symlinks( 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 files = 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 { + files + .iter() + .filter(|p| fast_glob::glob_match(pat, rel_of(p))) + .cloned() + .collect() + }; + + 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. Ambiguity here is handled below. + if source_paths.is_empty() { + source_paths = files + .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!( diff --git a/crates/soar-package/src/formats/common.rs b/crates/soar-package/src/formats/common.rs index e2b49fc9c..a4cfd8702 100644 --- a/crates/soar-package/src/formats/common.rs +++ b/crates/soar-package/src/formats/common.rs @@ -291,11 +291,17 @@ 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. + let portable_dir_base = + get_config() + .get_portable_dirs()? + .join(match (package.pkg_id(), package.pkg_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..b7b176975 100644 --- a/crates/soar-registry/src/error.rs +++ b/crates/soar-registry/src/error.rs @@ -12,6 +12,12 @@ 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" + )] + 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..97700702c 100644 --- a/crates/soar-registry/src/metadata.rs +++ b/crates/soar-registry/src/metadata.rs @@ -11,6 +11,7 @@ use std::{ }; 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,8 @@ 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 = + serde_json::from_reader::<_, MetadataDocument>(reader)?.into_packages()?; fs::remove_file(&tmp_path) .with_context(|| format!("removing temporary file {tmp_path}"))?; Ok(MetadataContent::Json(metadata)) @@ -443,11 +445,55 @@ 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 = serde_json::from_slice::(&content)?.into_packages()?; Ok(MetadataContent::Json(metadata)) } } +/// The highest index format this build understands. +pub const SUPPORTED_FORMAT: u32 = 1; + +/// A metadata index, in either shape it may arrive in. +/// +/// The original form is a bare array of packages. The versioned form wraps it +/// so a client can tell an index it cannot read from one that merely lacks a +/// field, and say so instead of failing on whichever field it noticed first. +#[derive(Deserialize)] +#[serde(untagged)] +enum MetadataDocument { + Versioned { + format: u32, + packages: Vec, + }, + Legacy(Vec), +} + +/// Parse an index in either shape, refusing one newer than this build. +pub fn parse_index(bytes: &[u8]) -> Result> { + serde_json::from_slice::(bytes)?.into_packages() +} + +impl MetadataDocument { + /// Unwrap to the packages, refusing an index newer than this build. + fn into_packages(self) -> Result> { + match self { + MetadataDocument::Legacy(packages) => Ok(packages), + MetadataDocument::Versioned { + format, + packages, + } => { + if format > SUPPORTED_FORMAT { + return Err(RegistryError::UnsupportedFormat { + found: format, + supported: SUPPORTED_FORMAT, + }); + } + Ok(packages) + } + } + } +} + /// 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..773dc2813 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)] + 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. + /// Lets a package whose binary is named differently from the package + /// itself be installed from the index alone. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub binaries: Option>, + /// Pinned side files to install alongside the artifact. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub extra: 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,25 @@ mod tests { assert_eq!(pkg.disabled, Some(true)); } } + +/// One executable inside a package artifact, as published in the index. +/// +/// `source` is relative to the extracted artifact and may be a glob, since +/// archives commonly wrap their contents in a versioned directory. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct RemoteBinary { + pub source: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub link_as: Option, +} + +/// 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..a021e6639 --- /dev/null +++ b/crates/soar-utils/src/version.rs @@ -0,0 +1,216 @@ +//! 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 unless identical, 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. + if a != b && is_commit_hash(a) && is_commit_hash(b) { + return Ordering::Equal; + } + + 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 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 + ); + } +}