diff --git a/src/cli.rs b/src/cli.rs index 0f93c6d..e222046 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -606,4 +606,18 @@ pub enum ForkSubcommand { /// Project identifiers to promote projects: Vec, }, + + /// Exclude parent projects from the merged export + Exclude { + /// Project slugs to exclude from the parent + #[clap(required = true)] + projects: Vec, + }, + + /// Re-include previously excluded parent projects + Include { + /// Project slugs to stop excluding from the parent + #[clap(required = true)] + projects: Vec, + }, } diff --git a/src/cli/commands/add.rs b/src/cli/commands/add.rs index bc1d79c..ae0b140 100644 --- a/src/cli/commands/add.rs +++ b/src/cli/commands/add.rs @@ -35,8 +35,10 @@ async fn resolve_input( platforms: &HashMap>, lockfile: &LockFile, ) -> Result { - for platform in platforms.values() { - if let Ok(project) = platform + let mut projects = Vec::new(); + + for (platform_name, client) in platforms { + match client .request_project_with_files( input, &lockfile.mc_versions, @@ -44,11 +46,29 @@ async fn resolve_input( ) .await { - return Ok(project); + Ok(project) => { + log::debug!("Resolved '{input}' on {platform_name}"); + projects.push(project); + }, + Err(e) => { + log::debug!("Could not resolve '{input}' on {platform_name}: {e}"); + }, } } - Err(PakkerError::ProjectNotFound(input.to_string())) + if projects.is_empty() { + return Err(PakkerError::ProjectNotFound(input.to_string())); + } + + if projects.len() == 1 { + return Ok(projects.remove(0)); + } + + let mut merged = projects.remove(0); + for project in projects { + merged.merge(project); + } + Ok(merged) } use std::path::Path; @@ -111,16 +131,24 @@ pub async fn execute( } // Load parent lockfile to get metadata - let parent_lockfile = parent_paths + let parent_lock_path = parent_paths .iter() .find(|path| path.exists()) - .and_then(|path| LockFile::load(path.parent()?).ok()) .ok_or_else(|| { PakkerError::IoError(std::io::Error::new( std::io::ErrorKind::NotFound, - "Failed to load parent lockfile metadata", + "Parent lockfile not found at expected paths", )) })?; + let parent_lockfile = LockFile::load_with_validation( + parent_lock_path.parent().ok_or_else(|| { + PakkerError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "Parent lockfile path has no parent directory", + )) + })?, + false, + )?; let minimal_lockfile = LockFile { target: parent_lockfile.target, diff --git a/src/cli/commands/export.rs b/src/cli/commands/export.rs index 6c27cf5..b629a51 100644 --- a/src/cli/commands/export.rs +++ b/src/cli/commands/export.rs @@ -233,14 +233,17 @@ fn merge_lockfiles( // Collect local project slugs for override detection let mut local_slugs = std::collections::HashSet::new(); for project in &local.projects { - // Add all slugs from all platforms for slug in project.slug.values() { local_slugs.insert(slug.clone()); } } - // Add parent projects that are NOT overridden by local - let parent_projects_count = parent.projects.len(); + // Collect excluded slugs from local config + let excluded: std::collections::HashSet<_> = + local_config.excludes.iter().collect(); + + // Add parent projects that are NOT overridden by local and NOT excluded + let mut parent_kept = 0usize; for parent_project in &parent.projects { let is_overridden = parent_project @@ -248,53 +251,63 @@ fn merge_lockfiles( .values() .any(|slug| local_slugs.contains(slug)); - if !is_overridden { - // Check if project has local config overrides - let mut project = parent_project.clone(); + let is_excluded = parent_project + .slug + .values() + .any(|slug| excluded.contains(slug)) + || parent_project + .name + .values() + .any(|name| excluded.contains(name)); - // Apply local config overrides if they exist - for (key, local_proj_cfg) in &local_config.projects { - // Match by slug, name, or pakku_id - let matches = project.slug.values().any(|s| s == key) - || project.name.values().any(|n| n == key) - || project.pakku_id.as_ref() == Some(key); - - if matches { - if let Some(t) = local_proj_cfg.r#type { - project.r#type = t; - } - if let Some(s) = local_proj_cfg.side { - project.side = s; - } - if let Some(us) = local_proj_cfg.update_strategy { - project.update_strategy = us; - } - if let Some(r) = local_proj_cfg.redistributable { - project.redistributable = r; - } - if let Some(ref sp) = local_proj_cfg.subpath { - project.subpath = Some(sp.clone()); - } - if let Some(ref aliases) = local_proj_cfg.aliases { - project.aliases = aliases.iter().cloned().collect(); - } - if let Some(e) = local_proj_cfg.export { - project.export = e; - } - break; - } - } - - merged.projects.push(project); + if is_overridden || is_excluded { + continue; } + + let mut project = parent_project.clone(); + + // Apply local config attribute overrides (side, type, etc.) + for (key, local_proj_cfg) in &local_config.projects { + let matches = project.slug.values().any(|s| s == key) + || project.name.values().any(|n| n == key) + || project.pakku_id.as_ref() == Some(key); + + if matches { + if let Some(t) = local_proj_cfg.r#type { + project.r#type = t; + } + if let Some(s) = local_proj_cfg.side { + project.side = s; + } + if let Some(us) = local_proj_cfg.update_strategy { + project.update_strategy = us; + } + if let Some(r) = local_proj_cfg.redistributable { + project.redistributable = r; + } + if let Some(ref sp) = local_proj_cfg.subpath { + project.subpath = Some(sp.clone()); + } + if let Some(ref aliases) = local_proj_cfg.aliases { + project.aliases = aliases.iter().cloned().collect(); + } + if let Some(e) = local_proj_cfg.export { + project.export = e; + } + break; + } + } + + merged.projects.push(project); + parent_kept += 1; } // Add all local projects merged.projects.extend(local.projects.clone()); println!( - "Merged fork: {} parent projects + {} local projects = {} total projects", - parent_projects_count - local_config.projects.len(), + "Merged fork: {} parent + {} local = {} total projects", + parent_kept, local.projects.len(), merged.projects.len() ); diff --git a/src/cli/commands/fork.rs b/src/cli/commands/fork.rs index 8b69a42..8db8c49 100644 --- a/src/cli/commands/fork.rs +++ b/src/cli/commands/fork.rs @@ -1,11 +1,16 @@ -use std::{fs, io::Write, path::Path}; +use std::{ + collections::{HashMap, HashSet}, + fs, + io::Write, + path::Path, +}; use crate::{ cli::ForkArgs, error::PakkerError, git::{self, VcsType}, model::{ - config::Config, + LockFile, fork::{ForkIntegrity, LocalConfig, ParentConfig, RefType, hash_content}, }, }; @@ -51,6 +56,12 @@ pub fn execute(args: &ForkArgs) -> Result<(), PakkerError> { crate::cli::ForkSubcommand::Promote { projects } => { execute_promote(projects) }, + crate::cli::ForkSubcommand::Exclude { projects } => { + execute_exclude(projects) + }, + crate::cli::ForkSubcommand::Include { projects } => { + execute_include(projects) + }, } } @@ -491,6 +502,41 @@ fn execute_unset() -> Result<(), PakkerError> { Ok(()) } +/// Snapshot parent lockfile as slug → first file name map +fn snapshot_parent_projects( + parent_path: &Path, +) -> HashMap> { + let lockfile_path = if parent_path.join("pakker-lock.json").exists() { + parent_path.join("pakker-lock.json") + } else { + parent_path.join("pakku-lock.json") + }; + + if !lockfile_path.exists() { + return HashMap::new(); + } + + match LockFile::load_with_validation(parent_path, false) { + Ok(lf) => { + lf.projects + .iter() + .map(|p| { + let slug = p + .slug + .values() + .next() + .cloned() + .or_else(|| p.name.values().next().cloned()) + .unwrap_or_default(); + let file = p.files.first().map(|f| f.file_name.clone()); + (slug, file) + }) + .collect() + }, + Err(_) => HashMap::new(), + } +} + fn execute_sync() -> Result<(), PakkerError> { let config_dir = Path::new("."); let mut local_config = LocalConfig::load(config_dir)?; @@ -504,6 +550,9 @@ fn execute_sync() -> Result<(), PakkerError> { let parent_path_str = parent_dir(); let parent_path = Path::new(&parent_path_str); + // Snapshot before update + let before = snapshot_parent_projects(parent_path); + if parent_path.exists() { println!("Fetching parent updates..."); git::fetch_updates(parent_path, &parent.remote_name, &parent.ref_, None)?; @@ -515,6 +564,9 @@ fn execute_sync() -> Result<(), PakkerError> { let commit_sha = git::get_commit_sha(parent_path, &parent.ref_)?; + // Snapshot after update + let after = snapshot_parent_projects(parent_path); + let mut integrity = None; // Try pakker files first, fall back to pakku files @@ -536,15 +588,6 @@ fn execute_sync() -> Result<(), PakkerError> { })?; let lock_hash = hash_content(&lock_content); - - if let Some(prev_hash) = &local_config.parent_lock_hash - && prev_hash != &lock_hash - { - log::warn!("Parent lock file has changed since last sync"); - log::warn!(" Previous hash: {prev_hash}"); - log::warn!(" Current hash: {lock_hash}"); - } - local_config.parent_lock_hash = Some(lock_hash); let config_content = if parent_config_path.exists() { @@ -556,15 +599,6 @@ fn execute_sync() -> Result<(), PakkerError> { }; let config_hash = hash_content(&config_content); - - if let Some(prev_hash) = &local_config.parent_config_hash - && prev_hash != &config_hash - { - log::warn!("Parent config file has changed since last sync"); - log::warn!(" Previous hash: {prev_hash}"); - log::warn!(" Current hash: {config_hash}"); - } - local_config.parent_config_hash = Some(config_hash); integrity = Some(ForkIntegrity::new( @@ -590,6 +624,46 @@ fn execute_sync() -> Result<(), PakkerError> { println!(); println!("✓ Parent sync complete"); println!(" Commit: {}", &commit_sha[..8]); + + // Print diff of parent changes + let before_keys: HashSet<_> = before.keys().collect(); + let after_keys: HashSet<_> = after.keys().collect(); + let added: Vec<_> = after_keys.difference(&before_keys).collect(); + let removed: Vec<_> = before_keys.difference(&after_keys).collect(); + let mut updated: Vec<(&String, &Option, &Option)> = + Vec::new(); + + for slug in before_keys.intersection(&after_keys) { + if before[*slug] != after[*slug] { + updated.push((slug, &before[*slug], &after[*slug])); + } + } + + if added.is_empty() && removed.is_empty() && updated.is_empty() { + println!(" No changes in parent projects."); + } else { + println!(); + println!(" Parent project changes:"); + let mut added: Vec<_> = added; + added.sort(); + for slug in added { + let file = after[*slug].as_deref().unwrap_or("?"); + println!(" + {slug} ({file})"); + } + let mut removed: Vec<_> = removed; + removed.sort(); + for slug in removed { + let file = before[*slug].as_deref().unwrap_or("?"); + println!(" - {slug} ({file})"); + } + updated.sort_by_key(|(slug, ..)| *slug); + for (slug, old_file, new_file) in updated { + let old = old_file.as_deref().unwrap_or("?"); + let new = new_file.as_deref().unwrap_or("?"); + println!(" ~ {slug}: {old} → {new}"); + } + } + println!(); println!("Run 'pakku export' to merge changes from parent."); @@ -613,33 +687,164 @@ fn execute_promote(projects: &[String]) -> Result<(), PakkerError> { )); } - // Load current config - let config = Config::load(config_dir)?; + // Load parent lockfile + let parent_path_str = parent_dir(); + let parent_path = Path::new(&parent_path_str); + if !parent_path.exists() { + return Err(PakkerError::Fork( + "Parent directory not found. Run 'pakku fork sync' first.".to_string(), + )); + } + + let parent_lockfile = LockFile::load_with_validation(parent_path, false) + .map_err(|e| { + PakkerError::Fork(format!("Failed to load parent lockfile: {e}")) + })?; + + // Load or create local lockfile + let lockfile_path = if config_dir.join("pakker-lock.json").exists() { + config_dir.join("pakker-lock.json") + } else { + config_dir.join("pakku-lock.json") + }; + let mut local_lockfile = if lockfile_path.exists() { + LockFile::load_with_validation(config_dir, false).map_err(|e| { + PakkerError::Fork(format!("Failed to load local lockfile: {e}")) + })? + } else { + // Bootstrap from parent metadata + LockFile { + target: parent_lockfile.target, + mc_versions: parent_lockfile.mc_versions.clone(), + loaders: parent_lockfile.loaders.clone(), + projects: Vec::new(), + lockfile_version: parent_lockfile.lockfile_version, + } + }; + + // Track which requested projects we found + let mut promoted = Vec::new(); + let mut not_found = Vec::new(); - // Verify all projects exist for project_arg in projects { - let found = config - .projects - .as_ref() - .and_then(|projs| projs.get(project_arg)) - .is_some(); + let found = parent_lockfile.projects.iter().find(|p| { + p.slug.values().any(|s| s == project_arg) + || p.name.values().any(|n| n == project_arg) + || p.pakku_id.as_deref() == Some(project_arg) + }); - if !found { - return Err(PakkerError::Fork(format!( - "Project not found: {project_arg}" - ))); + if let Some(project) = found { + // Skip if already in local lockfile + let already_local = local_lockfile.projects.iter().any(|lp| { + lp.slug + .values() + .any(|s| project.slug.values().any(|ps| s == ps)) + }); + + if already_local { + println!(" ~ {project_arg}: already in local lockfile, skipping"); + continue; + } + + local_lockfile.add_project(project.clone()); + promoted.push(project_arg); + } else { + not_found.push(project_arg); } } - println!("Note: In the current architecture, projects in pakku.json are"); - println!("automatically merged with parent projects during export."); - println!(); - println!("The following projects are already in pakku.json:"); - for project in projects { - println!(" - {project}"); + if !not_found.is_empty() { + for name in ¬_found { + eprintln!(" ! {name}: not found in parent lockfile"); + } + return Err(PakkerError::Fork(format!( + "{} project(s) not found in parent lockfile", + not_found.len() + ))); + } + + if promoted.is_empty() { + println!("No projects promoted (all already in local lockfile)."); + return Ok(()); + } + + local_lockfile.save(config_dir)?; + + println!("Promoted {} project(s) to local lockfile:", promoted.len()); + for name in &promoted { + println!(" + {name}"); } println!(); - println!("These will be included in exports automatically."); + println!( + "These projects are now locally managed and will override the parent." + ); + + Ok(()) +} + +fn execute_exclude(projects: &[String]) -> Result<(), PakkerError> { + let config_dir = Path::new("."); + let mut local_config = LocalConfig::load(config_dir)?; + + if local_config.parent.is_none() { + return Err(PakkerError::Fork( + "No parent configured. Run 'pakku fork init' first.".to_string(), + )); + } + + let mut added = Vec::new(); + for slug in projects { + if local_config.excludes.contains(slug) { + println!(" ~ {slug}: already excluded"); + } else { + local_config.excludes.push(slug.clone()); + added.push(slug); + } + } + local_config.excludes.sort(); + local_config.save(config_dir)?; + + if !added.is_empty() { + println!("Excluded {} project(s) from parent:", added.len()); + for slug in &added { + println!(" - {slug}"); + } + println!(); + println!("These parent projects will be omitted from exports."); + } + + Ok(()) +} + +fn execute_include(projects: &[String]) -> Result<(), PakkerError> { + let config_dir = Path::new("."); + let mut local_config = LocalConfig::load(config_dir)?; + + if local_config.parent.is_none() { + return Err(PakkerError::Fork( + "No parent configured. Run 'pakku fork init' first.".to_string(), + )); + } + + let mut removed = Vec::new(); + for slug in projects { + if let Some(pos) = local_config.excludes.iter().position(|s| s == slug) { + local_config.excludes.remove(pos); + removed.push(slug); + } else { + println!(" ~ {slug}: not in excludes list"); + } + } + local_config.save(config_dir)?; + + if !removed.is_empty() { + println!("Re-included {} project(s) from parent:", removed.len()); + for slug in &removed { + println!(" + {slug}"); + } + println!(); + println!("These parent projects will be included in exports again."); + } Ok(()) } diff --git a/src/export/rules.rs b/src/export/rules.rs index c82ab96..b5373ab 100644 --- a/src/export/rules.rs +++ b/src/export/rules.rs @@ -87,7 +87,7 @@ impl Effect for CopyProjectFilesEffect { } log::info!("fetched {} (local)", file.file_name); } else if !file.url.is_empty() { - download_file( + match download_file( &context.base_path, &type_dir, &file.file_name, @@ -95,28 +95,36 @@ impl Effect for CopyProjectFilesEffect { curseforge_key.as_deref(), modrinth_token.as_deref(), ) - .await?; - - // Copy into export dir after ensuring it is present in base dir - let downloaded = - context.base_path.join(&type_dir).join(&file.file_name); - if downloaded.exists() { - fs::copy(&downloaded, &dest)?; - if let Some(ui) = &context.ui { - ui.println(format!("fetched {} (download)", file.file_name)); - } - log::info!("fetched {} (download)", file.file_name); - } else { - return Err(crate::error::PakkerError::InternalError(format!( - "download reported success but file is missing: {}", - file.file_name - ))); + .await + { + Ok(()) => { + let downloaded = + context.base_path.join(&type_dir).join(&file.file_name); + if downloaded.exists() { + fs::copy(&downloaded, &dest)?; + if let Some(ui) = &context.ui { + ui.println(format!("fetched {} (download)", file.file_name)); + } + log::info!("fetched {} (download)", file.file_name); + } else { + log::warn!( + "download reported success but file is missing: {}", + file.file_name + ); + } + }, + Err(e) => { + log::warn!( + "failed to download {} (continuing): {e}", + file.file_name + ); + }, } } else { - return Err(crate::error::PakkerError::InternalError(format!( + log::warn!( "missing project file and no download url: {}", file.file_name - ))); + ); } } } diff --git a/src/git/mod.rs b/src/git/mod.rs index 941ddbf..5710ccb 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -45,7 +45,18 @@ pub fn get_current_commit_sha>( let repo = Repository::open(path)?; let commit = if let Some(ref_name) = ref_name { - let obj = repo.revparse_single(ref_name)?; + // Try the ref in several forms: bare name, local branch, remote tracking + let candidates = [ + ref_name.to_string(), + format!("refs/heads/{ref_name}"), + format!("refs/remotes/origin/{ref_name}"), + ]; + let obj = candidates + .iter() + .find_map(|candidate| repo.revparse_single(candidate).ok()) + .ok_or_else(|| { + PakkerError::GitError(format!("revspec '{ref_name}' not found")) + })?; obj.peel_to_commit()? } else { let head = repo.head()?; @@ -222,9 +233,19 @@ pub fn resolve_ref_type>( ) -> Result { let repo = Repository::open(path)?; - // Check if it's a branch - if repo.find_branch(ref_name, git2::BranchType::Local).is_ok() - || repo.find_branch(ref_name, git2::BranchType::Remote).is_ok() + // Check if it's a local branch + if repo.find_branch(ref_name, git2::BranchType::Local).is_ok() { + return Ok(crate::model::fork::RefType::Branch); + } + + // Check remote tracking branches (e.g. origin/main after a fresh clone) + let remote_branch_name = format!("origin/{ref_name}"); + if repo + .find_branch(&remote_branch_name, git2::BranchType::Remote) + .is_ok() + || repo + .find_reference(&format!("refs/remotes/origin/{ref_name}")) + .is_ok() { return Ok(crate::model::fork::RefType::Branch); } @@ -236,7 +257,12 @@ pub fn resolve_ref_type>( } // Try to resolve as commit SHA - if repo.revparse_single(ref_name).is_ok() { + let candidates = [ + ref_name.to_string(), + format!("refs/heads/{ref_name}"), + format!("refs/remotes/origin/{ref_name}"), + ]; + if candidates.iter().any(|c| repo.revparse_single(c).is_ok()) { return Ok(crate::model::fork::RefType::Commit); } @@ -483,7 +509,7 @@ mod tests { let mut index = repo.index().unwrap(); let file_path = path.join(file_name); let mut f = File::create(&file_path).unwrap(); - writeln!(f, "{}", content).unwrap(); + writeln!(f, "{content}").unwrap(); drop(f); index.add_path(std::path::Path::new(file_name)).unwrap(); let tree_id = index.write_tree().unwrap(); @@ -499,7 +525,7 @@ mod tests { repo .branch(branch, &repo.find_commit(head_oid).unwrap(), true) .unwrap(); - repo.set_head(&format!("refs/heads/{}", branch)).unwrap(); + repo.set_head(&format!("refs/heads/{branch}")).unwrap(); repo } diff --git a/src/ipc.rs b/src/ipc.rs index f2c80d9..9f6787d 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -106,42 +106,51 @@ impl IpcCoordinator { } /// Extract modpack hash from pakku.json's parentLockHash field. - /// This is the authoritative content hash for the modpack (Nix-style). + /// This is the authoritative content hash for the modpack. If you've used Nix + /// the model might seem familiar. fn get_modpack_hash(working_dir: &Path) -> Result { + use sha2::{Digest, Sha256}; + + let pakker_path = working_dir.join("pakker.json"); let pakku_path = working_dir.join("pakku.json"); - if !pakku_path.exists() { + let config_path = if pakker_path.exists() { + pakker_path + } else if pakku_path.exists() { + pakku_path + } else { return Err(IpcError::PakkuJsonReadFailed( - "pakku.json not found in working directory".to_string(), + "pakker.json or pakku.json not found in working directory".to_string(), )); - } + }; - let content = fs::read_to_string(&pakku_path) + let content = fs::read_to_string(&config_path) .map_err(|e| IpcError::PakkuJsonReadFailed(e.to_string()))?; - // Parse pakku.json and extract parentLockHash + // Parse config and try to extract parentLockHash (fork modpacks have this) let pakku: serde_json::Value = serde_json::from_str(&content) .map_err(|e| IpcError::PakkuJsonReadFailed(e.to_string()))?; - let hash = pakku + let candidate = pakku .get("pakku") .and_then(|p| p.get("parentLockHash")) .and_then(|v| v.as_str()) - .ok_or_else(|| { - IpcError::PakkuJsonReadFailed( - "parentLockHash not found in pakku.json".to_string(), - ) - })? - .to_string(); + .map(std::string::ToString::to_string); - // Validate it's a valid hex string (SHA256 = 64 chars) - if hash.len() != 64 || !hash.chars().all(|c| c.is_ascii_hexdigit()) { - return Err(IpcError::PakkuJsonReadFailed( - "parentLockHash is not a valid SHA256 hash".to_string(), - )); + if let Some(hash) = candidate { + // Validate it's a valid hex string (SHA256 = 64 chars) + if hash.len() == 64 && hash.chars().all(|c| c.is_ascii_hexdigit()) { + return Ok(hash); + } } - Ok(hash) + // Hash the working directory path for non-fork modpacks as a fallback. + let dir_str = working_dir.to_string_lossy(); + let mut hasher = Sha256::new(); + hasher.update(dir_str.as_bytes()); + Ok(crate::utils::hash::hash_to_hex( + hasher.finalize().as_slice(), + )) } /// Create a new IPC coordinator for the given modpack directory. @@ -470,6 +479,7 @@ impl Drop for OperationGuard { #[cfg(test)] mod tests { + #![expect(clippy::expect_used, reason = "Fine in tests")] use std::fs; use tempfile::TempDir; @@ -495,7 +505,7 @@ mod tests { // Write pakku.json with unique parentLockHash (nested under "pakku" key) let pakku_content = - format!(r#"{{"pakku": {{"parentLockHash": "{}"}}}}"#, unique_hash); + format!(r#"{{"pakku": {{"parentLockHash": "{unique_hash}"}}}}"#); fs::write(temp_dir.path().join("pakku.json"), pakku_content).unwrap(); temp_dir @@ -520,7 +530,7 @@ mod tests { // Write pakku.json with specified parentLockHash (nested under "pakku" key) let pakku_content = - format!(r#"{{"pakku": {{"parentLockHash": "{}"}}}}"#, hash); + format!(r#"{{"pakku": {{"parentLockHash": "{hash}"}}}}"#); fs::write(temp_dir.path().join("pakku.json"), pakku_content).unwrap(); temp_dir @@ -539,8 +549,7 @@ mod tests { "cfe85e0e7e7aa0922d30d8faad071e3a4126cb78b5f0f792f191e90a295aa2c7", ); - let hash = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()).unwrap(); + let hash = IpcCoordinator::get_modpack_hash(temp_dir.path()).unwrap(); assert_eq!( hash, "cfe85e0e7e7aa0922d30d8faad071e3a4126cb78b5f0f792f191e90a295aa2c7" @@ -554,13 +563,13 @@ mod tests { .tempdir() .unwrap(); - let result = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()); + let result = IpcCoordinator::get_modpack_hash(temp_dir.path()); assert!(matches!(result, Err(IpcError::PakkuJsonReadFailed(_)))); } #[test] fn test_get_modpack_hash_missing_parent_lock_hash() { + // Non-fork modpacks have no parentLockHash; should fall back to dir hash let temp_dir = tempfile::Builder::new() .prefix("pakker-ipc-test-") .tempdir() @@ -569,13 +578,15 @@ mod tests { fs::write(temp_dir.path().join("pakku.json"), r#"{"other": "field"}"#) .unwrap(); - let result = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()); - assert!(matches!(result, Err(IpcError::PakkuJsonReadFailed(_)))); + let result = IpcCoordinator::get_modpack_hash(temp_dir.path()); + let hash = result.unwrap(); + assert_eq!(hash.len(), 64); + assert!(hash.chars().all(|c| c.is_ascii_hexdigit())); } #[test] fn test_get_modpack_hash_invalid_hash() { + // Root-level parentLockHash (wrong location) falls back to dir hash let temp_dir = tempfile::Builder::new() .prefix("pakker-ipc-test-") .tempdir() @@ -587,9 +598,10 @@ mod tests { ) .unwrap(); - let result = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()); - assert!(matches!(result, Err(IpcError::PakkuJsonReadFailed(_)))); + let result = IpcCoordinator::get_modpack_hash(temp_dir.path()); + let hash = result.unwrap(); + assert_eq!(hash.len(), 64); + assert!(hash.chars().all(|c| c.is_ascii_hexdigit())); } #[test] @@ -607,8 +619,8 @@ mod tests { shared_hash, ); - let coord1 = IpcCoordinator::new(&temp_dir1.path().to_path_buf()).unwrap(); - let coord2 = IpcCoordinator::new(&temp_dir2.path().to_path_buf()).unwrap(); + let coord1 = IpcCoordinator::new(temp_dir1.path()).unwrap(); + let coord2 = IpcCoordinator::new(temp_dir2.path()).unwrap(); assert_eq!( coord1.ops_file, coord2.ops_file, @@ -624,8 +636,8 @@ mod tests { let temp_dir1 = create_test_modpack(&[("mod1.jar", "content")]); let temp_dir2 = create_test_modpack(&[("mod1.jar", "content")]); - let coord1 = IpcCoordinator::new(&temp_dir1.path().to_path_buf()).unwrap(); - let coord2 = IpcCoordinator::new(&temp_dir2.path().to_path_buf()).unwrap(); + let coord1 = IpcCoordinator::new(temp_dir1.path()).unwrap(); + let coord2 = IpcCoordinator::new(temp_dir2.path()).unwrap(); assert_ne!( coord1.ops_file, coord2.ops_file, @@ -636,8 +648,7 @@ mod tests { #[test] fn test_ipc_coordinator_new_creates_dir() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Check that the parent directory (ipc_dir) exists assert!(coordinator.ops_file.parent().unwrap().exists()); @@ -646,8 +657,7 @@ mod tests { #[test] fn test_load_operations_empty() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let operations = coordinator.load_operations().unwrap(); assert!(operations.is_empty()); @@ -656,8 +666,7 @@ mod tests { #[test] fn test_register_operation() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let id = coordinator .register_operation(OperationType::Fetch) @@ -670,8 +679,7 @@ mod tests { #[test] fn test_register_multiple_operations_different_types() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let fetch_id = coordinator .register_operation(OperationType::Fetch) @@ -689,8 +697,7 @@ mod tests { #[test] fn test_register_conflicting_operation_same_type() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let _id1 = coordinator .register_operation(OperationType::Fetch) @@ -703,8 +710,7 @@ mod tests { #[test] fn test_complete_operation() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let id = coordinator .register_operation(OperationType::Fetch) @@ -719,8 +725,7 @@ mod tests { #[test] fn test_complete_nonexistent_operation() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let result = coordinator.complete_operation("nonexistent-id"); assert!(matches!(result, Err(IpcError::OperationNotFound(_)))); @@ -729,8 +734,7 @@ mod tests { #[test] fn test_has_running_operation() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); assert!(!coordinator.has_running_operation(OperationType::Fetch)); @@ -747,8 +751,7 @@ mod tests { #[test] fn test_get_running_operations() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let operations = coordinator.get_running_operations(OperationType::Fetch); assert!(operations.is_empty()); @@ -764,8 +767,7 @@ mod tests { #[tokio::test] async fn test_wait_for_conflicts_no_conflicts() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let result = coordinator .wait_for_conflicts(OperationType::Fetch, Duration::from_secs(1)) @@ -776,8 +778,7 @@ mod tests { #[tokio::test] async fn test_wait_for_conflicts_with_completion() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Register an operation let id = coordinator @@ -801,8 +802,7 @@ mod tests { #[tokio::test] async fn test_wait_for_conflicts_timeout() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Register a long-running operation (we won't complete it) let _id = coordinator @@ -819,8 +819,7 @@ mod tests { #[test] fn test_operation_guard_completes_on_drop() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let id = coordinator .register_operation(OperationType::Fetch) @@ -842,8 +841,7 @@ mod tests { #[test] fn test_operation_guard_manual_complete() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let id = coordinator .register_operation(OperationType::Export) @@ -862,8 +860,7 @@ mod tests { #[test] fn test_stale_operation_cleanup() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Manually add a stale operation (started 15 minutes ago) let now = SystemTime::now() @@ -896,8 +893,7 @@ mod tests { #[test] fn test_multiple_operations_same_process() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let id1 = coordinator .register_operation(OperationType::Fetch) @@ -929,8 +925,7 @@ mod tests { #[test] fn test_operation_id_format() { let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let id = coordinator .register_operation(OperationType::Export) @@ -971,8 +966,8 @@ mod tests { fs::write(temp_dir1.path().join("file1.txt"), "content1").unwrap(); fs::write(temp_dir2.path().join("file2.txt"), "content2").unwrap(); - let coord1 = IpcCoordinator::new(&temp_dir1.path().to_path_buf()).unwrap(); - let coord2 = IpcCoordinator::new(&temp_dir2.path().to_path_buf()).unwrap(); + let coord1 = IpcCoordinator::new(temp_dir1.path()).unwrap(); + let coord2 = IpcCoordinator::new(temp_dir2.path()).unwrap(); // Both should point to SAME ops file despite different paths assert_eq!(coord1.ops_file, coord2.ops_file); @@ -982,8 +977,7 @@ mod tests { fn test_corrupted_ops_json_trailing_bracket() { // Test handling of corrupted ops.json with trailing characters let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Register an operation to create ops.json let _id = coordinator @@ -992,7 +986,7 @@ mod tests { // Manually corrupt the ops.json by appending extra bracket let ops_content = fs::read_to_string(&coordinator.ops_file).unwrap(); - fs::write(&coordinator.ops_file, format!("{}]", ops_content)).unwrap(); + fs::write(&coordinator.ops_file, format!("{ops_content}]")).unwrap(); // Loading should fail with InvalidFormat error let result = coordinator.load_operations(); @@ -1003,8 +997,7 @@ mod tests { fn test_corrupted_ops_json_invalid_json() { // Test handling of completely invalid JSON let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Write invalid JSON to ops.json fs::write(&coordinator.ops_file, "not valid json {[}").unwrap(); @@ -1017,8 +1010,7 @@ mod tests { fn test_corrupted_ops_json_missing_fields() { // Test handling of JSON with missing required fields let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Write JSON with missing fields fs::write(&coordinator.ops_file, r#"[{"id": "test"}]"#).unwrap(); @@ -1031,8 +1023,7 @@ mod tests { fn test_empty_ops_file() { // Test handling of empty ops.json file let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Create empty ops.json fs::write(&coordinator.ops_file, "").unwrap(); @@ -1045,8 +1036,7 @@ mod tests { fn test_whitespace_only_ops_file() { // Test handling of whitespace-only ops.json let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); fs::write(&coordinator.ops_file, " \n\t \n ").unwrap(); @@ -1074,8 +1064,7 @@ mod tests { }"#; fs::write(temp_dir.path().join("pakku.json"), pakku_content).unwrap(); - let hash = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()).unwrap(); + let hash = IpcCoordinator::get_modpack_hash(temp_dir.path()).unwrap(); assert_eq!( hash, "abc123def456789012345678901234567890abcd123456789012345678901234" @@ -1096,12 +1085,11 @@ mod tests { }"#; fs::write(temp_dir.path().join("pakku.json"), old_format).unwrap(); - let result = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()); - assert!( - matches!(result, Err(IpcError::PakkuJsonReadFailed(_))), - "Old format should be rejected" - ); + // Old format (root-level parentLockHash, not in pakku section) falls back + // to dir-path hash + let hash = IpcCoordinator::get_modpack_hash(temp_dir.path()).unwrap(); + assert_eq!(hash.len(), 64); + assert!(hash.chars().all(|c| c.is_ascii_hexdigit())); } #[test] @@ -1111,12 +1099,13 @@ mod tests { .tempdir() .unwrap(); + // Too-short hash falls back to dir-path hash let pakku_content = r#"{"pakku": {"parentLockHash": "tooshort"}}"#; fs::write(temp_dir.path().join("pakku.json"), pakku_content).unwrap(); - let result = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()); - assert!(matches!(result, Err(IpcError::PakkuJsonReadFailed(_)))); + let hash = IpcCoordinator::get_modpack_hash(temp_dir.path()).unwrap(); + assert_eq!(hash.len(), 64); + assert!(hash.chars().all(|c| c.is_ascii_hexdigit())); } #[test] @@ -1126,13 +1115,13 @@ mod tests { .tempdir() .unwrap(); - // 64 chars but not all hex + // 64 chars but not all hex falls back to dir-path hash let pakku_content = r#"{"pakku": {"parentLockHash": "gggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg"}}"#; fs::write(temp_dir.path().join("pakku.json"), pakku_content).unwrap(); - let result = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()); - assert!(matches!(result, Err(IpcError::PakkuJsonReadFailed(_)))); + let hash = IpcCoordinator::get_modpack_hash(temp_dir.path()).unwrap(); + assert_eq!(hash.len(), 64); + assert!(hash.chars().all(|c| c.is_ascii_hexdigit())); } #[test] @@ -1146,8 +1135,7 @@ mod tests { let pakku_content = r#"{"pakku": {"parentLockHash": "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789"}}"#; fs::write(temp_dir.path().join("pakku.json"), pakku_content).unwrap(); - let hash = - IpcCoordinator::get_modpack_hash(&temp_dir.path().to_path_buf()).unwrap(); + let hash = IpcCoordinator::get_modpack_hash(temp_dir.path()).unwrap(); assert_eq!( hash, "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789" @@ -1158,8 +1146,7 @@ mod tests { fn test_concurrent_registration_race_condition() { // Test that file locking prevents race conditions let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // First registration should succeed let id1 = coordinator @@ -1182,8 +1169,7 @@ mod tests { fn test_file_permissions_ipc_dir() { // Test that IPC directory has correct permissions (700 - owner only) let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let ipc_dir = coordinator.ops_file.parent().unwrap(); let metadata = fs::metadata(ipc_dir).unwrap(); @@ -1195,8 +1181,7 @@ mod tests { fn test_file_permissions_ops_file() { // Test that ops.json has correct permissions (600 - owner read/write only) let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Register operation to create ops.json let _id = coordinator @@ -1215,21 +1200,21 @@ mod tests { let unique_hash = format!( "{:064x}", rand::random::() - ^ (std::time::SystemTime::now() + ^ std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap() - .as_nanos() as u128) + .as_nanos() ); let temp_dir = create_test_modpack_with_hash( &[("test.txt", "test")], &unique_hash[..64], ); - let coord1 = IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coord1 = IpcCoordinator::new(temp_dir.path()).unwrap(); let id = coord1.register_operation(OperationType::Fetch).unwrap(); // Create new coordinator instance - let coord2 = IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coord2 = IpcCoordinator::new(temp_dir.path()).unwrap(); let operations = coord2.load_operations().unwrap(); assert_eq!(operations.len(), 1); @@ -1245,8 +1230,7 @@ mod tests { // Test that stale cleanup only removes running operations, not completed // ones let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); let now = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) @@ -1292,8 +1276,7 @@ mod tests { async fn test_wait_for_conflicts_multiple_types() { // Test that wait_for_conflicts only waits for matching operation types let temp_dir = create_test_modpack(&[("test.txt", "test")]); - let coordinator = - IpcCoordinator::new(&temp_dir.path().to_path_buf()).unwrap(); + let coordinator = IpcCoordinator::new(temp_dir.path()).unwrap(); // Register Export operation (different type) let _export_id = coordinator diff --git a/src/model/fork.rs b/src/model/fork.rs index 0bf4f44..d81392b 100644 --- a/src/model/fork.rs +++ b/src/model/fork.rs @@ -135,6 +135,9 @@ pub struct LocalConfig { pub parent_config_hash: Option, #[serde(default)] pub patches: Vec, + /// Slugs of parent projects to exclude from the merged export + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub excludes: Vec, } impl LocalConfig { @@ -347,6 +350,7 @@ mod tests { parent_lock_hash: None, parent_config_hash: None, patches: vec![], + excludes: vec![], }; assert!(config.parent.is_none()); assert!(config.projects.is_empty()); @@ -361,6 +365,7 @@ mod tests { parent_lock_hash: None, parent_config_hash: None, patches: vec![], + excludes: vec![], }; assert!(!config.has_parent()); } @@ -373,6 +378,7 @@ mod tests { parent_lock_hash: None, parent_config_hash: None, patches: vec![], + excludes: vec![], }; config.parent = Some(ParentConfig { type_: "git".to_string(), @@ -393,6 +399,7 @@ mod tests { parent_lock_hash: None, parent_config_hash: None, patches: vec![], + excludes: vec![], }; config .projects @@ -421,6 +428,7 @@ mod tests { parent_lock_hash: None, parent_config_hash: None, patches: vec![], + excludes: vec![], }; config.patches.push("custom.patch".to_string()); config.patches.push("bugfix.patch".to_string()); @@ -437,6 +445,7 @@ mod tests { parent_lock_hash: None, parent_config_hash: None, patches: vec![], + excludes: vec![], }; config.parent = Some(ParentConfig { type_: "git".to_string(), diff --git a/src/model/lockfile.rs b/src/model/lockfile.rs index dc10595..1e56ab1 100644 --- a/src/model/lockfile.rs +++ b/src/model/lockfile.rs @@ -308,7 +308,7 @@ mod tests { } let found = lockfile.get_project("test-id").unwrap(); - assert_eq!(found.redistributable, false); + assert!(!found.redistributable); } #[test] @@ -566,8 +566,7 @@ mod tests { /// Current lockfile version - bump this when making breaking changes const LOCKFILE_VERSION: u32 = 2; -/// Minimum supported lockfile version for migration -const MIN_SUPPORTED_VERSION: u32 = 1; + const LOCKFILE_NAME: &str = "pakku-lock.json"; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -577,6 +576,7 @@ pub struct LockFile { pub mc_versions: Vec, pub loaders: HashMap, pub projects: Vec, + #[serde(default)] pub lockfile_version: u32, } @@ -642,7 +642,7 @@ impl LockFile { // Check if migration is needed if lockfile.lockfile_version < LOCKFILE_VERSION { - lockfile = lockfile.migrate()?; + lockfile = lockfile.migrate(); // Save migrated lockfile lockfile.save_without_validation(path_ref)?; log::info!( @@ -661,12 +661,11 @@ impl LockFile { } /// Migrate lockfile from older version to current version - fn migrate(mut self) -> Result { - if self.lockfile_version < MIN_SUPPORTED_VERSION { - return Err(PakkerError::InvalidLockFile(format!( - "Lockfile version {} is too old to migrate. Minimum supported: {}", - self.lockfile_version, MIN_SUPPORTED_VERSION - ))); + fn migrate(mut self) -> Self { + // Migration from v0 (pakku format, no explicit version) to v1 + if self.lockfile_version == 0 { + log::info!("Migrating lockfile from v0 (pakku format) to v1..."); + self.lockfile_version = 1; } // Migration from v1 to v2 @@ -693,7 +692,7 @@ impl LockFile { // self.lockfile_version = 3; // } - Ok(self) + self } pub fn save>(&self, path: P) -> Result<()> { @@ -719,13 +718,6 @@ impl LockFile { self.lockfile_version, LOCKFILE_VERSION ))); } - if self.lockfile_version < MIN_SUPPORTED_VERSION { - return Err(PakkerError::InvalidLockFile(format!( - "Lockfile version {} is too old. Minimum supported: {}", - self.lockfile_version, MIN_SUPPORTED_VERSION - ))); - } - if self.mc_versions.is_empty() { return Err(PakkerError::InvalidLockFile( "At least one Minecraft version is required".to_string(), diff --git a/src/model/project.rs b/src/model/project.rs index a9c7469..4ff7504 100644 --- a/src/model/project.rs +++ b/src/model/project.rs @@ -370,6 +370,7 @@ pub struct ProjectFile { #[serde(rename = "type")] pub file_type: String, pub file_name: String, + #[serde(default)] pub mc_versions: Vec, #[serde(default)] pub loaders: Vec, @@ -378,6 +379,7 @@ pub struct ProjectFile { pub id: String, pub parent_id: String, pub hashes: HashMap, + #[serde(default)] pub required_dependencies: Vec, pub size: u64, pub date_published: String, @@ -551,8 +553,7 @@ mod tests { assert!( file.is_compatible(&lockfile_mc, &lockfile_loaders), - "Failed for valid loader: {}", - loader + "Failed for valid loader: {loader}" ); } } diff --git a/src/platform/github.rs b/src/platform/github.rs index 7df7d75..7b8b54a 100644 --- a/src/platform/github.rs +++ b/src/platform/github.rs @@ -533,11 +533,7 @@ mod tests { for (tag, asset, expected) in cases { let result = extract_mc_versions(tag, asset); - assert_eq!( - result, expected, - "Failed for tag: {}, asset: {}", - tag, asset - ); + assert_eq!(result, expected, "Failed for tag: {tag}, asset: {asset}"); } } @@ -568,11 +564,7 @@ mod tests { for (tag, asset, expected) in cases { let result = extract_loaders(tag, asset); - assert_eq!( - result, expected, - "Failed for tag: {}, asset: {}", - tag, asset - ); + assert_eq!(result, expected, "Failed for tag: {tag}, asset: {asset}"); } } @@ -609,8 +601,7 @@ mod tests { let result = detect_project_type(filename, repo_name); assert_eq!( result, expected, - "Failed for filename: {}, repo: {}", - filename, repo_name + "Failed for filename: {filename}, repo: {repo_name}" ); } } diff --git a/src/platform/multiplatform.rs b/src/platform/multiplatform.rs index 734c633..54dac2d 100644 --- a/src/platform/multiplatform.rs +++ b/src/platform/multiplatform.rs @@ -78,21 +78,22 @@ impl PlatformClient for MultiplatformPlatform { let mut cf_project = cf_project; let mut mr_project = mr_project; - let mr_found_and_cf_missing = mr_project.is_some() && cf_project.is_none(); - if mr_found_and_cf_missing + // Cross-reference using each platform's own slug on the other platform. + // Modrinth projects store their slug under "modrinth"; CurseForge under + // "curseforge". Many mods share the same slug across platforms. + if cf_project.is_none() && let Some(ref mr) = mr_project - && let Some(cf_slug) = mr.slug.get("curseforge") + && let Some(mr_slug) = mr.slug.get("modrinth") && let Ok(Some(cf)) = - self.curseforge.request_project_from_slug(cf_slug).await + self.curseforge.request_project_from_slug(mr_slug).await { cf_project = Some(cf); } - let cf_found_and_mr_missing = cf_project.is_some() && mr_project.is_none(); - if cf_found_and_mr_missing + if mr_project.is_none() && let Some(ref cf) = cf_project - && let Some(mr_slug) = cf.slug.get("modrinth") + && let Some(cf_slug) = cf.slug.get("curseforge") && let Ok(Some(mr)) = - self.modrinth.request_project_from_slug(mr_slug).await + self.modrinth.request_project_from_slug(cf_slug).await { mr_project = Some(mr); } @@ -232,3 +233,183 @@ impl PlatformClient for MultiplatformPlatform { Ok(all_projects) } } + +#[cfg(test)] +mod tests { + use std::{collections::HashMap, sync::Arc}; + + use async_trait::async_trait; + + use super::*; + use crate::{ + error::{PakkerError, Result}, + model::{Project, ProjectFile, ProjectSide, ProjectType}, + }; + + struct MockPlatform { + projects: HashMap, + slug_map: HashMap, + } + + impl MockPlatform { + fn new() -> Self { + Self { + projects: HashMap::new(), + slug_map: HashMap::new(), + } + } + + fn with_project(mut self, id: &str, project: Project) -> Self { + self.projects.insert(id.to_string(), project); + self + } + + fn with_slug(mut self, slug: &str, project: Project) -> Self { + self.slug_map.insert(slug.to_string(), project); + self + } + } + + #[async_trait] + impl PlatformClient for MockPlatform { + async fn request_project( + &self, + project_id: &str, + _mc_versions: &[String], + _loaders: &[String], + ) -> Result { + self + .projects + .get(project_id) + .cloned() + .ok_or_else(|| PakkerError::ProjectNotFound(project_id.to_string())) + } + + async fn request_project_files( + &self, + _project_id: &str, + _mc_versions: &[String], + _loaders: &[String], + ) -> Result> { + Ok(vec![]) + } + + async fn request_project_with_files( + &self, + project_id: &str, + mc_versions: &[String], + loaders: &[String], + ) -> Result { + self.request_project(project_id, mc_versions, loaders).await + } + + async fn lookup_by_hash(&self, _hash: &str) -> Result> { + Ok(None) + } + + async fn request_project_from_slug( + &self, + slug: &str, + ) -> Result> { + Ok(self.slug_map.get(slug).cloned()) + } + + async fn request_projects_from_hashes( + &self, + _hashes: &[String], + _algorithm: &str, + ) -> Result> { + Ok(vec![]) + } + } + + fn make_project(platform: &str, id: &str, slug: &str) -> Project { + let mut project = + Project::new(slug.to_string(), ProjectType::Mod, ProjectSide::Both); + project.id.insert(platform.to_string(), id.to_string()); + project.slug.insert(platform.to_string(), slug.to_string()); + project.name.insert(platform.to_string(), slug.to_string()); + project + } + + #[tokio::test] + async fn test_cross_reference_modrinth_to_curseforge() { + let mr_project = make_project("modrinth", "mr-abc", "sodium"); + let cf_project = make_project("curseforge", "12345", "sodium"); + + let modrinth = + Arc::new(MockPlatform::new().with_project("sodium", mr_project.clone())); + let curseforge = + Arc::new(MockPlatform::new().with_slug("sodium", cf_project.clone())); + + let platform = MultiplatformPlatform::new(curseforge, modrinth); + let result = platform.request_project("sodium", &[], &[]).await.unwrap(); + + assert_eq!(result.id.get("modrinth"), Some(&"mr-abc".to_string())); + assert_eq!(result.id.get("curseforge"), Some(&"12345".to_string())); + } + + #[tokio::test] + async fn test_cross_reference_curseforge_to_modrinth() { + let cf_project = make_project("curseforge", "12345", "sodium"); + let mr_project = make_project("modrinth", "mr-abc", "sodium"); + + let modrinth = + Arc::new(MockPlatform::new().with_slug("sodium", mr_project.clone())); + let curseforge = + Arc::new(MockPlatform::new().with_project("sodium", cf_project.clone())); + + let platform = MultiplatformPlatform::new(curseforge, modrinth); + let result = platform.request_project("sodium", &[], &[]).await.unwrap(); + + assert_eq!(result.id.get("curseforge"), Some(&"12345".to_string())); + assert_eq!(result.id.get("modrinth"), Some(&"mr-abc".to_string())); + } + + #[tokio::test] + async fn test_found_on_both_platforms_merged() { + let cf_project = make_project("curseforge", "12345", "sodium"); + let mr_project = make_project("modrinth", "mr-abc", "sodium"); + + let modrinth = + Arc::new(MockPlatform::new().with_project("sodium", mr_project)); + let curseforge = + Arc::new(MockPlatform::new().with_project("sodium", cf_project)); + + let platform = MultiplatformPlatform::new(curseforge, modrinth); + let result = platform.request_project("sodium", &[], &[]).await.unwrap(); + + assert_eq!(result.id.get("curseforge"), Some(&"12345".to_string())); + assert_eq!(result.id.get("modrinth"), Some(&"mr-abc".to_string())); + } + + #[tokio::test] + async fn test_not_found_on_either_platform() { + let modrinth = Arc::new(MockPlatform::new()); + let curseforge = Arc::new(MockPlatform::new()); + + let platform = MultiplatformPlatform::new(curseforge, modrinth); + let result = platform.request_project("nonexistent", &[], &[]).await; + + assert!(matches!(result, Err(PakkerError::ProjectNotFound(_)))); + } + + #[tokio::test] + async fn test_no_cross_reference_when_slug_absent() { + // CurseForge returns a project, but slug lookup on Modrinth finds nothing + let cf_project = make_project("curseforge", "12345", "rare-mod"); + + let modrinth = Arc::new(MockPlatform::new()); + let curseforge = + Arc::new(MockPlatform::new().with_project("rare-mod", cf_project)); + + let platform = MultiplatformPlatform::new(curseforge, modrinth); + let result = platform + .request_project("rare-mod", &[], &[]) + .await + .unwrap(); + + assert_eq!(result.id.get("curseforge"), Some(&"12345".to_string())); + assert!(result.id.get("modrinth").is_none()); + } +} diff --git a/src/utils/flexver.rs b/src/utils/flexver.rs index 1b15bf6..ec2aa4d 100644 --- a/src/utils/flexver.rs +++ b/src/utils/flexver.rs @@ -316,9 +316,7 @@ mod tests { assert_eq!( compare(b, a), inverse, - "Commutativity violation: {} vs {}", - a, - b + "Commutativity violation: {a} vs {b}" ); } } diff --git a/src/utils/id.rs b/src/utils/id.rs index c664e7a..8ac7e03 100644 --- a/src/utils/id.rs +++ b/src/utils/id.rs @@ -23,7 +23,7 @@ mod tests { fn test_generate_pakku_id() { let id = generate_pakku_id(); assert_eq!(id.len(), ID_LENGTH); - assert!(id.chars().all(|c| c.is_alphanumeric())); + assert!(id.chars().all(char::is_alphanumeric)); } #[test]