diff --git a/src/clone.rs b/src/clone.rs index bc5b27e..5fbe730 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -114,7 +114,7 @@ pub(crate) fn execute_git_clone( // Output what we know - just the meta repo clone command // (Child repos are in .meta file which hasn't been cloned yet) println!("{} Would clone meta repository:", style("[DRY RUN]").cyan()); - println!(" {clone_cmd_str}"); + println!(" {}", meta_core::display::escape(&clone_cmd_str)); return Ok(CommandResult::Message(String::new())); } @@ -124,7 +124,10 @@ pub(crate) fn execute_git_clone( ssh_setup::SshMasters::UserManaged | ssh_setup::SshMasters::Failed => None, }; - println!("Cloning meta repository: {url}"); + println!( + "Cloning meta repository: {}", + meta_core::display::escape(&url) + ); let mut clone_cmd = Command::new("git"); clone_cmd.arg("clone").args(&git_clone_args).arg(&url); if let Some(ref dir) = dir_arg { diff --git a/src/commands/worktree/add.rs b/src/commands/worktree/add.rs index 7c11198..7545951 100644 --- a/src/commands/worktree/add.rs +++ b/src/commands/worktree/add.rs @@ -30,7 +30,11 @@ pub(crate) fn handle_add(args: AddArgs, verbose: bool, json: bool) -> Result<()> let wt_dir = worktree_set_dir(&worktree_root, name)?; if !wt_dir.exists() { - anyhow::bail!("Worktree '{}' not found at {}", name, escape_path(&wt_dir)); + anyhow::bail!( + "Worktree '{}' not found at {}", + escape(name), + escape_path(&wt_dir) + ); } // Check existing repos in the worktree @@ -42,58 +46,81 @@ pub(crate) fn handle_add(args: AddArgs, verbose: bool, json: bool) -> Result<()> // Check every repo before adding any: one refused after others were // added would leave a set the caller didn't ask for - let mut planned = Vec::new(); - for (i, spec) in repo_specs.iter().enumerate() { - // As in create: a repeat is one worktree, a repeat on another branch - // is refused - if let Some(first) = repo_specs.iter().take(i).find(|o| o.alias == spec.alias) { - if first.branch != spec.branch { + let mut planned: Vec<(&RepoSpec, std::path::PathBuf, String)> = Vec::new(); + for spec in repo_specs { + // Use recursive lookup for nested paths (containing '/') + let (source, _project) = lookup_nested_project(&meta_dir, &spec.alias)?; + // The repo goes where it is in the workspace + let place = place_in_set(&meta_dir, &spec.alias, &source); + // As in create: a repo given twice, by name or by path, is one + // worktree, and refused on two branches + if let Some((first, ..)) = planned.iter().find(|(_, _, p)| same_place(p, &place)) { + let first_branch = resolve_branch(name, None, first.branch.as_deref()); + let branch = resolve_branch(name, None, spec.branch.as_deref()); + if first_branch != branch { anyhow::bail!( "Repo '{}' is given twice, on branches '{}' and '{}'", - escape(&spec.alias), - escape(first.branch.as_deref().unwrap_or("(default)")), - escape(spec.branch.as_deref().unwrap_or("(default)")) + escape(&place), + escape(&first_branch), + escape(&branch) ); } continue; } - if existing.iter().any(|r| r.alias == spec.alias) { - anyhow::bail!( - "Repo '{}' already exists in worktree '{name}'", - escape(&spec.alias) - ); - } - - // Use recursive lookup for nested paths (containing '/') - let (source, _project) = lookup_nested_project(&meta_dir, &spec.alias)?; - - // Preserve the full alias as the destination path so that relative - // references (e.g., Cargo.toml workspace members) remain valid. - let dest = wt_dir.join(&spec.alias); // A checkout already in the worktree (such as the root repo's) may // hold a symlink where this one goes; git must not follow it out - meta_core::config::resolve_inside(&wt_dir, &spec.alias).map_err(|why| { + let dest = dest_in_set(&wt_dir, &place).map_err(|why| { anyhow::anyhow!( "Refusing to add the worktree for '{}': {why}", escape(&spec.alias) ) })?; - planned.push((spec, source, dest)); + // There already, under its place or where a link there leads, or + // nested in a member (`parent/leaf`); or its place holds other files + let real = |path: &std::path::Path| path.canonicalize().ok(); + let member = dest.join(".git").symlink_metadata().is_ok() + || existing.iter().any(|r| { + same_place(&r.alias, &place) + || (real(&r.path).is_some() && real(&r.path) == real(&dest)) + }); + if member { + anyhow::bail!( + "Repo '{}' already exists in worktree '{}'", + escape(&spec.alias), + escape(name) + ); + } + let taken = std::fs::read_dir(&dest) + .map_or(dest.symlink_metadata().is_ok(), |mut entries| { + entries.next().is_some() + }); + if taken { + anyhow::bail!( + "{} in worktree '{}' already holds files that aren't a checkout of '{}'; move them away first", + escape_path(&dest), + escape(name), + escape(&spec.alias) + ); + } + planned.push((spec, source, place)); } let mut added = Vec::new(); let mut failed = None; - for (spec, source, dest) in planned { + for (spec, source, place) in planned { let branch = resolve_branch(name, None, spec.branch.as_deref()); // Again now: a checkout added above (a parent repo's) may have put a // symlink where this one goes - if let Err(why) = meta_core::config::resolve_inside(&wt_dir, &spec.alias) { - failed = Some(anyhow::anyhow!( - "Refusing to add the worktree for '{}': {why}", - escape(&spec.alias) - )); - break; - } + let dest = match dest_in_set(&wt_dir, &place) { + Ok(dest) => dest, + Err(why) => { + failed = Some(anyhow::anyhow!( + "Refusing to add the worktree for '{}': {why}", + escape(&spec.alias) + )); + break; + } + }; // Ensure parent directories exist for nested paths (e.g., "vendor/") if let Some(parent) = dest.parent() { @@ -125,12 +152,7 @@ pub(crate) fn handle_add(args: AddArgs, verbose: bool, json: bool) -> Result<()> break; } }; - added.push(CreateRepoEntry { - alias: spec.alias.clone(), - path: dest.display().to_string(), - branch, - created_branch, - }); + // A worktree that can't be recorded is reported as failed, not added if let Some(set_record) = &set_record { if let Err(e) = linked_worktree_admin_dir(&dest).and_then(|admin| record::write(&admin, set_record)) @@ -142,6 +164,12 @@ pub(crate) fn handle_add(args: AddArgs, verbose: bool, json: bool) -> Result<()> break; } } + added.push(CreateRepoEntry { + alias: spec.alias.clone(), + path: dest.display().to_string(), + branch, + created_branch, + }); } if json { @@ -166,3 +194,23 @@ pub(crate) fn handle_add(args: AddArgs, verbose: bool, json: bool) -> Result<()> failed.map_or(Ok(()), Err) } + +/// Whether two places in a set name the same directory. Discovery spells a +/// place with the platform's separator (`vendor\\app` on Windows), plans with +/// `/`; as paths they compare equal. +fn same_place(a: &str, b: &str) -> bool { + std::path::Path::new(a) == std::path::Path::new(b) +} + +#[cfg(test)] +mod tests { + use super::same_place; + + #[test] + fn places_compare_as_paths() { + assert!(same_place("vendor/app", "vendor/app")); + assert!(!same_place("vendor/app", "vendor/other")); + #[cfg(windows)] + assert!(same_place("vendor\\app", "vendor/app")); + } +} diff --git a/src/commands/worktree/cli_types.rs b/src/commands/worktree/cli_types.rs index adff9ab..c421543 100644 --- a/src/commands/worktree/cli_types.rs +++ b/src/commands/worktree/cli_types.rs @@ -89,7 +89,8 @@ pub struct CreateArgs { /// /// By default, worktree create includes the root repo and resolves /// dependencies via provides/depends_on from .meta.yaml. - /// Use --no-deps to include only explicitly specified repos. + /// Use --no-deps to include only the named repos (and, for a repo nested + /// inside another project's path, that parent project). #[arg(long)] pub no_deps: bool, @@ -169,8 +170,8 @@ pub struct ExecArgs { /// Start from a specific commit-ish (ephemeral only) pub commit_ish: Option, - /// Only run in these repos (comma-separated): each value is a repo's alias, - /// its whole path in the set (. for the root), never part of one + /// Only run in these repos (comma-separated): each value is a repo's name, + /// or its whole path in the set (. for the root), never part of one #[arg(long, value_delimiter = ',')] pub include: Vec, diff --git a/src/commands/worktree/create.rs b/src/commands/worktree/create.rs index 42a0ba7..8adcb90 100644 --- a/src/commands/worktree/create.rs +++ b/src/commands/worktree/create.rs @@ -43,7 +43,10 @@ pub(crate) fn handle_create( } else { super::warn_or_bail( strict, - format!("--meta value '{s}' missing '=' separator (expected key=value), skipping"), + format!( + "--meta value '{}' missing '=' separator (expected key=value), skipping", + escape(s) + ), )?; } } @@ -91,7 +94,7 @@ pub(crate) fn handle_create( let nearest_meta_dir = require_meta_dir()?; let meta_dir = if recursive { - meta_core::config::find_root_meta_dir(&nearest_meta_dir) + meta_core::config::find_root_meta_dir(&nearest_meta_dir)? } else { nearest_meta_dir }; @@ -114,18 +117,28 @@ pub(crate) fn handle_create( // Determine which repos to include: Vec<(alias, source_path, branch)> let repos_to_create: Vec<(String, std::path::PathBuf, String)> = if use_all { - projects + let mut all: Vec<(String, std::path::PathBuf)> = projects .iter() - .map(|p| { + .filter(|p| !recursive || p.name == ".") + .map(|p| (p.name.clone(), meta_dir.join(&p.path))) + .collect(); + // --recursive: every repo of the nested tree, by its full path + if recursive { + let tree = meta_core::config::walk_meta_tree(&meta_dir, None)?; + all.extend( + meta_core::config::build_project_map(&tree, &meta_dir, "") + .into_iter() + .map(|(alias, (source, _))| (alias, source)), + ); + } + all.into_iter() + .map(|(alias, source)| { let per_branch = repo_specs .iter() - .find(|r| r.alias == p.name) + .find(|r| r.alias == alias) .and_then(|r| r.branch.as_deref()); - ( - p.name.clone(), - meta_dir.join(&p.path), - resolve_branch(name, branch_flag, per_branch), - ) + let branch = resolve_branch(name, branch_flag, per_branch); + (alias, source, branch) }) .collect() } else if no_deps { @@ -165,13 +178,37 @@ pub(crate) fn handle_create( // parents ("gitkb") get worktrees too — the thin spine from root to target. let mut repos_to_create = ensure_intermediate_parents(&meta_dir, repos_to_create, name, branch_flag)?; - // One worktree per alias: a repeated --repo would try the same add twice + // One worktree per place in the set: a repeated --repo, or a repo named + // both by its name and by its path, would try the same add twice + let mut branches: HashMap = HashMap::new(); + for (alias, source, branch) in &repos_to_create { + let place = place_in_set(&meta_dir, alias, source); + match branches.get(place.as_str()) { + Some(first) if *first != branch.as_str() => anyhow::bail!( + "Repo '{}' is given twice, on branches '{}' and '{}'", + escape(&place), + escape(first), + escape(branch) + ), + Some(_) => {} + None => { + branches.insert(place, branch); + } + } + } let mut seen = HashSet::new(); - repos_to_create.retain(|(alias, _, _)| seen.insert(alias.clone())); + repos_to_create + .retain(|(alias, source, _)| seen.insert(place_in_set(&meta_dir, alias, source))); + // A repo goes in before any repo inside it, whatever order they were named in + repos_to_create.sort_by_key(|(alias, source, _)| { + let place = place_in_set(&meta_dir, alias, source); + (alias != ".", place.split('/').count()) + }); if dry_run { let plan = CreateDryRunPlan { name, + meta_dir: &meta_dir, wt_dir: &wt_dir, repos_to_create: &repos_to_create, from_ref, @@ -192,11 +229,17 @@ pub(crate) fn handle_create( .find(|(alias, source, _)| alias != "." && repo_matches_spec(source, pr_repo_spec)) .ok_or_else(|| { anyhow::anyhow!( - "No selected repo has origin '{pr_repo_spec}', so PR branch '{pr_branch}' can't be applied" + "No selected repo has origin '{}', so PR branch '{}' can't be applied", + escape(pr_repo_spec), + escape(pr_branch) ) })?; - git_fetch_branch(source, pr_branch) - .map_err(|e| e.context(format!("Cannot fetch the PR branch into '{alias}'")))?; + git_fetch_branch(source, pr_branch).map_err(|e| { + e.context(format!( + "Cannot fetch the PR branch into '{}'", + escape(alias) + )) + })?; *branch = pr_branch.clone(); } @@ -219,6 +262,8 @@ pub(crate) fn handle_create( // Anything that fails from here rolls back what this call created. let mut created_repos = Vec::new(); let mut new_branches = Vec::new(); + // The checkout whose sets these are: inside a set, the one it stands for + let owner = owner_dir(&meta_dir); let built = (|| -> Result<()> { // Each worktree gets this record as soon as it exists, so `remove` // and `prune` know meta made it; a worktree that can't be recorded @@ -227,7 +272,9 @@ pub(crate) fn handle_create( set: wt_dir .canonicalize() .with_context(|| format!("Cannot resolve {}", escape_path(&wt_dir)))?, - project: meta_dir.clone(), + // Inside a set, the checkout it stands for: a record never names + // a set's checkout, which can be removed before this set + project: owner.clone(), created_at: Utc::now().to_rfc3339(), ephemeral, ttl_seconds, @@ -243,7 +290,10 @@ pub(crate) fn handle_create( if let Some((_, source, branch)) = repos_to_create.iter().find(|(a, _, _)| a == ".") { if let Some(r) = missing_ref(source, from_ref)? { // --from-ref: skip root repo if ref doesn't exist (same as child repos) - super::warn_or_bail(strict, format!("Skipping '.': ref '{r}' not found"))?; + super::warn_or_bail( + strict, + format!("Skipping '.': ref '{}' not found", escape(r)), + )?; } else { if verbose { eprintln!( @@ -272,18 +322,22 @@ pub(crate) fn handle_create( } if let Some(r) = missing_ref(source, from_ref)? { // --from-ref: skip repos where ref doesn't exist - super::warn_or_bail(strict, format!("Skipping '{alias}': ref '{r}' not found"))?; + super::warn_or_bail( + strict, + format!( + "Skipping '{}': ref '{}' not found", + escape(alias), + escape(r) + ), + )?; continue; } - // Preserve the full alias as the destination path so that relative - // references (e.g., Cargo.toml workspace members) remain valid. - // "vendor/tree-sitter-markdown" -> "vendor/tree-sitter-markdown" - // Simple aliases are unchanged: "core" -> "core" - let dest = wt_dir.join(alias); + // The repo goes where it is in the workspace + let place = place_in_set(&meta_dir, alias, source); // A checkout already in the worktree (such as the root repo's) may // hold a symlink where this one goes; git must not follow it out - meta_core::config::resolve_inside(&wt_dir, alias).map_err(|why| { + let dest = dest_in_set(&wt_dir, &place).map_err(|why| { anyhow::anyhow!( "Refusing to add the worktree for '{}': {why}", escape(alias) @@ -322,7 +376,10 @@ pub(crate) fn handle_create( if created_repos.is_empty() { // Every repo was skipped because none has the ref. match from_ref { - Some(r) => anyhow::bail!("No selected repo has ref '{r}'; no worktree was created"), + Some(r) => anyhow::bail!( + "No selected repo has ref '{}'; no worktree was created", + escape(r) + ), None => anyhow::bail!("No repos selected; no worktree was created"), } } @@ -332,7 +389,8 @@ pub(crate) fn handle_create( .file_name() .and_then(|n| n.to_str()) .unwrap_or(".worktrees"); - ensure_worktrees_in_gitignore(&meta_dir, dirname, json) + // in the checkout whose sets these are + ensure_worktrees_in_gitignore(&owner, dirname, json) })(); built.map_err(|e| roll_back(&wt_dir, &new_branches, e))?; @@ -344,7 +402,7 @@ pub(crate) fn handle_create( ephemeral, ttl_seconds, &custom_meta, - Some(&meta_dir), + Some(&owner), ); // Output @@ -428,9 +486,9 @@ fn journal_new_branch( /// Undo a failed create: delete the set this call claimed, if the removal /// check finds nothing to lose, and the branches it created, if they still -/// point where it left them. A record that can't be used (one whose write -/// failed, say) doesn't stop it: this call made the set, so it needs no -/// proof of that. Returns the error to report. +/// point where it left them. A record that can't be used or is missing (one +/// whose write failed, say) doesn't stop it: this call made the set, so it +/// needs no proof of that. Returns the error to report. fn roll_back( wt_dir: &std::path::Path, new_branches: &[(std::path::PathBuf, String, String)], @@ -440,7 +498,7 @@ fn roll_back( let blockers = check .blockers .iter() - .filter(|b| !check.unusable_records.contains(b)); + .filter(|b| !check.unusable_records.contains(b) && !check.missing_records.contains(b)); let problems: Vec = blockers.chain(&check.unsaved).cloned().collect(); anyhow::ensure!(problems.is_empty(), "{}", problems.join("; ")); delete_checked_set(wt_dir, &check, false, false) @@ -476,6 +534,7 @@ fn roll_back( struct CreateDryRunPlan<'a> { name: &'a str, + meta_dir: &'a std::path::Path, wt_dir: &'a std::path::Path, repos_to_create: &'a [(String, std::path::PathBuf, String)], from_ref: Option<&'a str>, @@ -494,10 +553,13 @@ fn print_create_dry_run(plan: &CreateDryRunPlan<'_>) { println!(); if let Some(from_ref) = plan.from_ref { - println!("Start ref: {from_ref}"); + println!("Start ref: {}", escape(from_ref)); } if let Some(from_pr_spec) = plan.from_pr_spec { - println!("PR source: would resolve {from_pr_spec} and fetch the matching head branch"); + println!( + "PR source: would resolve {} and fetch the matching head branch", + escape(from_pr_spec) + ); } if plan.ephemeral { println!("Ephemeral: true"); @@ -510,7 +572,7 @@ fn print_create_dry_run(plan: &CreateDryRunPlan<'_>) { "Metadata: {}", plan.custom_meta .iter() - .map(|(k, v)| format!("{k}={v}")) + .map(|(k, v)| format!("{}={}", escape(k), escape(v))) .collect::>() .join(", ") ); @@ -524,7 +586,7 @@ fn print_create_dry_run(plan: &CreateDryRunPlan<'_>) { let dest = if alias == "." { plan.wt_dir.to_path_buf() } else { - plan.wt_dir.join(alias) + plan.wt_dir.join(place_in_set(plan.meta_dir, alias, source)) }; let mut cmd = format!( "git -C {} worktree add -b {} {}", @@ -538,6 +600,13 @@ fn print_create_dry_run(plan: &CreateDryRunPlan<'_>) { } println!(" {}:", escape(alias)); println!(" source: {}", escape_path(source)); + if alias != "." + && source + .symlink_metadata() + .is_ok_and(|m| m.file_type().is_symlink()) + { + println!(" (a link: if the root checkout holds it, the repo goes where it leads)"); + } println!(" dest: {}", escape_path(&dest)); println!(" branch: {}", escape(branch)); println!(" bash: {}", escape(&cmd)); @@ -610,7 +679,11 @@ fn resolve_repos_with_dependencies( for dep in deps { repos_to_include.insert(dep.to_string()); if verbose { - eprintln!(" Including '{}' (dependency of '{}')", dep, resolved_alias); + eprintln!( + " Including '{}' (dependency of '{}')", + escape(dep), + escape(&resolved_alias) + ); } } } @@ -696,13 +769,13 @@ fn resolve_alias_in_graph(graph: &DependencyGraph, alias: &str) -> Result anyhow::bail!( "Repo '{}' not found in the recursive project graph. \ With --recursive, use the full path from root (e.g., 'open-source/gitkb/core').", - alias + escape(alias) ), _ => anyhow::bail!( "Repo alias '{}' is ambiguous — matches: {}. \ Use the full path to disambiguate.", - alias, - matches.join(", ") + escape(alias), + escape(&matches.join(", ")) ), } } @@ -755,6 +828,26 @@ fn build_nested_dep_graph(meta_dir: &std::path::Path) -> Result DependencyGraph::build(project_deps) } +/// The source of the project at `place` from `meta_dir`, reading only the +/// configs of the meta repos on the way to it. +fn project_at(meta_dir: &std::path::Path, place: &str) -> Result> { + let Some((config, _)) = meta_core::config::find_meta_config_in(meta_dir)? else { + return Ok(None); + }; + let (projects, _) = meta_core::config::parse_meta_config(&config)?; + for project in &projects { + if project.path == place { + return Ok(Some(meta_dir.join(&project.path))); + } + if let Some(rest) = place.strip_prefix(&format!("{}/", project.path)) { + if let Some(found) = project_at(&meta_dir.join(&project.path), rest)? { + return Ok(Some(found)); + } + } + } + Ok(None) +} + /// Ensure intermediate `meta: true` parent repos get worktrees for nested aliases. /// /// When `repos_to_create` contains a nested alias like `"gitkb/core"`, the @@ -770,20 +863,27 @@ fn ensure_intermediate_parents( worktree_name: &str, branch_flag: Option<&str>, ) -> Result> { - let existing: HashSet = repos.iter().map(|(a, _, _)| a.clone()).collect(); + // Where the selected repos go: a parent is there when a repo is at its + // path, whatever name selected it + let existing: HashSet = repos + .iter() + .map(|(a, s, _)| place_in_set(meta_dir, a, s)) + .collect(); let mut to_add: Vec<(String, std::path::PathBuf, String)> = Vec::new(); let mut added: HashSet = HashSet::new(); - for (alias, _source, _branch) in &repos { - // Only nested aliases need intermediate parents - if !alias.contains('/') { + for (alias, source, _branch) in &repos { + // Only a repo nested in the set needs its parents: they follow from + // where it goes, not from its name + let place = place_in_set(meta_dir, alias, source); + if !place.contains('/') { continue; } // Walk up through path segments: "gitkb/core" → check "gitkb" // "a/b/c" → check "a", "a/b" - let parts: Vec<&str> = alias.split('/').collect(); + let parts: Vec<&str> = place.split('/').collect(); for i in 1..parts.len() { let parent_alias = parts[..i].join("/"); @@ -792,44 +892,29 @@ fn ensure_intermediate_parents( continue; } - // Resolve the parent's source path via config lookup (respects custom - // `path` settings) with a fallback to direct path joining. - // A fallback directory must lie inside the workspace and not be a - // symlink: it becomes a source repo for git - let parent_source = match lookup_nested_project(meta_dir, &parent_alias) { - Ok((source, _project)) => source, - Err(_) => { - let candidate = meta_dir.join(&parent_alias); - let is_link = candidate - .symlink_metadata() - .is_ok_and(|meta| meta.file_type().is_symlink()); - if is_link - || meta_core::config::resolve_inside(meta_dir, &parent_alias).is_err() - { - log::warn!( - "Intermediate parent '{}' is not a plain directory inside the workspace, skipping", - escape(&parent_alias) - ); - continue; - } - candidate + // A parent is the project at that path, not one by that name; a + // repo the workspace doesn't list is not added (remove would + // refuse it as foreign) + let Some(parent_source) = project_at(meta_dir, &parent_alias)? else { + // A plain directory on the way is no parent; a repo that + // isn't a project is worth saying so + if !meta_dir.join(&parent_alias).join(".git").exists() { + continue; } - }; - if !parent_source.exists() { - log::warn!( - "Intermediate parent '{}' for nested repo '{}' not found at {:?}, skipping", - parent_alias, - alias, - parent_source + eprintln!( + "{} Intermediate parent '{}' for nested repo '{}' is not a project of the workspace, skipping", + "warning:".yellow().bold(), + escape(&parent_alias), + escape(alias) ); continue; - } - - // Only add if it's actually a git repo (has .git dir or .git file) + }; if !parent_source.join(".git").exists() { - log::warn!( - "Intermediate parent '{}' for nested repo '{}' exists but is not a git repo (no .git), skipping", - parent_alias, alias + eprintln!( + "{} Intermediate parent '{}' for nested repo '{}' is not cloned, skipping", + "warning:".yellow().bold(), + escape(&parent_alias), + escape(alias) ); continue; } @@ -837,8 +922,8 @@ fn ensure_intermediate_parents( let branch = resolve_branch(worktree_name, branch_flag, None); log::info!( "Adding intermediate parent '{}' for nested repo '{}'", - parent_alias, - alias + escape(&parent_alias), + escape(alias) ); to_add.push((parent_alias, parent_source, branch)); } @@ -895,6 +980,40 @@ mod tests { ); } + #[test] + fn roll_back_deletes_a_set_whose_records_were_not_all_written() { + let tmp = tempfile::tempdir().unwrap(); + let set = tmp.path().join("set"); + for (repo, wt) in [("root", set.clone()), ("leaf", set.join("leaf"))] { + let source = tmp.path().join(repo); + std::fs::create_dir(&source).unwrap(); + git(&source, &["init", "-q"]); + git(&source, &["commit", "-q", "--allow-empty", "-m", "init"]); + git( + &source, + &["worktree", "add", "-q", "-b", repo, wt.to_str().unwrap()], + ); + } + // The root was recorded; the leaf's record write failed + let set_record = SetRecord { + set: set.canonicalize().unwrap(), + project: tmp.path().to_path_buf(), + created_at: Utc::now().to_rfc3339(), + ephemeral: false, + ttl_seconds: None, + custom: HashMap::new(), + }; + record::write(&linked_worktree_admin_dir(&set).unwrap(), &set_record).unwrap(); + assert_eq!( + check_worktree_removal(&set).unwrap().missing_records.len(), + 1 + ); + + let err = roll_back(&set, &[], anyhow::anyhow!("record write failed")); + assert_eq!(format!("{err:#}"), "record write failed"); + assert!(!set.exists()); + } + #[test] fn journal_keeps_only_branches_this_call_created() { let tmp = tempfile::tempdir().unwrap(); diff --git a/src/commands/worktree/diff.rs b/src/commands/worktree/diff.rs index 1b94fef..a0b60d5 100644 --- a/src/commands/worktree/diff.rs +++ b/src/commands/worktree/diff.rs @@ -59,7 +59,7 @@ pub(crate) fn handle_diff(args: DiffArgs, _verbose: bool, json: bool) -> Result< println!("{}", serde_json::to_string_pretty(&output)?); } else { // Human mode: always show stat summary - println!("{} vs {}:", name.bold(), base_ref); + println!("{} vs {}:", name.bold(), escape(base_ref)); for d in &diff_entries { if d.files_changed > 0 { let insertions = d.insertions; @@ -84,7 +84,7 @@ pub(crate) fn handle_diff(args: DiffArgs, _verbose: bool, json: bool) -> Result< total_repos_changed, ); } else { - println!(" No changes vs {base_ref}"); + println!(" No changes vs {}", escape(base_ref)); } } diff --git a/src/commands/worktree/exec.rs b/src/commands/worktree/exec.rs index 58a3976..819a20b 100644 --- a/src/commands/worktree/exec.rs +++ b/src/commands/worktree/exec.rs @@ -1,7 +1,7 @@ use anyhow::Result; use colored::*; use meta_core::display::escape_lines; -use std::path::Path; +use std::path::{Path, PathBuf}; use meta_cli::repo_filter; use meta_cli::worktree::{discover_worktree_repos, WorktreeRepoInfo}; @@ -22,13 +22,27 @@ fn build_loop_config( json: bool, ) -> Result { let directories: Vec = repos.iter().map(|r| r.path.display().to_string()).collect(); - // A member sits at its alias, so its path in the set is its name + // A set lays its repos out as the workspace, so the set's config names + // them, by project name or path; only a filter reads it + let names = if include_filters.is_empty() && exclude_filters.is_empty() { + Vec::new() + } else { + let paths: Vec = repos.iter().map(|r| r.path.clone()).collect(); + match meta_cli::worktree::set_config(wt_dir, &paths, None)? { + Some((config, _)) => { + let (projects, _) = meta_core::config::parse_meta_config(&config)?; + let tree = meta_cli::worktree::set_tree(wt_dir, &paths, &projects)?; + repo_filter::repo_names(wt_dir, &projects, &tree) + } + None => Vec::new(), + } + }; let directories = repo_filter::select( &include_filters, &exclude_filters, &directories, wt_dir, - &[], + &names, )?; Ok(loop_lib::LoopConfig { directories, diff --git a/src/commands/worktree/list.rs b/src/commands/worktree/list.rs index 513c65e..f9be449 100644 --- a/src/commands/worktree/list.rs +++ b/src/commands/worktree/list.rs @@ -113,7 +113,7 @@ pub(crate) fn handle_list( .expect("worktree location has a parent") .to_path_buf(), _ => { - let meta_dir = meta_cli::config::find_meta_config(&cwd, None) + let meta_dir = meta_cli::config::find_meta_config(&cwd, None)? .and_then(|(path, _)| path.parent().map(std::path::Path::to_path_buf)); let root = resolve_worktree_root(Some(meta_dir.as_deref().unwrap_or(&cwd)))?; if root.is_absolute() { diff --git a/src/commands/worktree/mod.rs b/src/commands/worktree/mod.rs index 9933980..0e2a95c 100644 --- a/src/commands/worktree/mod.rs +++ b/src/commands/worktree/mod.rs @@ -13,7 +13,7 @@ mod status; use anyhow::Result; use clap::Parser; use colored::*; -use meta_core::display::escape_lines; +use meta_core::display::{escape, escape_lines}; use meta_plugin_protocol::{CommandResult, PluginRequestOptions}; use std::path::Path; @@ -105,8 +105,12 @@ pub fn execute_worktree_command( CommandResult::Message(String::new()) } _ => { - // Actual parse errors - CommandResult::ShowHelp(Some(e.to_string())) + // Actual parse errors: clap's message names the problem + // and where to get help; the whole git help would bury it + let message = + meta_cli::args::escaped_error::(&clap_args).to_string(); + let message = message.strip_prefix("error: ").unwrap_or(&message); + CommandResult::Error(message.trim_end().to_string()) } } } @@ -133,7 +137,7 @@ fn handle_worktree_command( WorktreeCommands::Exec(args) => exec::handle_exec(args, verbose, json), WorktreeCommands::Prune(args) => prune::handle_prune(args, verbose, json, global_strict), WorktreeCommands::Unknown(args) => { - let cmd = args.first().map(|s| s.as_str()).unwrap_or(""); + let cmd = escape(args.first().map(|s| s.as_str()).unwrap_or("")); eprintln!( "{}: unrecognized worktree subcommand '{cmd}'", "error".red().bold(), diff --git a/src/commands/worktree/prune.rs b/src/commands/worktree/prune.rs index 51e7f13..e6b049a 100644 --- a/src/commands/worktree/prune.rs +++ b/src/commands/worktree/prune.rs @@ -1,18 +1,17 @@ -use anyhow::{Context, Result}; +use anyhow::Result; use chrono::Utc; use colored::*; -use meta_core::display::{escape, escape_path}; +use meta_core::display::escape; use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; use meta_cli::worktree::discover_worktree_members; -use meta_git_lib::worktree::helpers::{require_meta_dir, resolve_worktree_root}; +use meta_git_lib::worktree::helpers::{owner_dir, require_meta_dir, resolve_worktree_root}; use meta_git_lib::worktree::hooks::fire_post_prune; use meta_git_lib::worktree::record::{self, SetRecord}; -use meta_git_lib::worktree::safety::{ - check_worktree_removal, delete_checked_set, linked_worktree_admin_dir, member_record, -}; +use meta_git_lib::worktree::safety::{check_worktree_removal, delete_checked_set}; use meta_git_lib::worktree::types::*; +use meta_git_lib::worktree::workspace::Workspace; use super::cli_types::PruneArgs; @@ -31,66 +30,6 @@ fn create_prune_entry( } } -/// Check if a set is orphaned because `project` no longer has any of the -/// repos its worktrees (`aliases`, their paths in the set) belong to. -/// Uses a config cache to avoid redundant file I/O when multiple worktrees -/// share the same source project. -fn check_repo_orphaned( - project: &Path, - aliases: &[String], - config_cache: &mut std::collections::HashMap< - PathBuf, - Option>, - >, -) -> Option { - let config = config_cache - .entry(project.to_path_buf()) - .or_insert_with(|| { - meta_core::config::find_meta_config_in(project) - .ok() - .flatten() - .and_then(|(meta_path, _)| { - meta_core::config::parse_meta_config(&meta_path) - .ok() - .map(|(projects, _)| projects) - }) - }); - - let Some(projects) = config else { - return None; // Can't check without config - }; - - // "." is the project's own repo and always exists, so a set holding it is - // never orphaned. Nested aliases ("sub/leaf") live in child .meta files, - // not the top-level project list. - let missing_count = aliases - .iter() - .map(String::as_str) - .filter(|alias| { - *alias != "." - && !projects.iter().any(|p| p.name == *alias) - && !(alias.contains('/') && nested_alias_present(project, alias)) - }) - .count(); - - if missing_count > 0 && missing_count == aliases.len() { - Some("orphaned (all source repos removed from project)".to_string()) - } else { - None - } -} - -/// Whether nested alias `alias` still names a repo of `project`. A nested -/// config that can't be read or is refused says nothing about the repo, so -/// it counts as present: only a tree that reads cleanly without the alias -/// makes the repo removed. -fn nested_alias_present(project: &Path, alias: &str) -> bool { - match meta_core::config::walk_meta_tree(project, None) { - Ok(tree) => meta_core::config::build_project_map(&tree, project, "").contains_key(alias), - Err(_) => true, - } -} - /// Only a path that is really gone counts as missing. A dangling link or a /// path prune can't inspect (permission denied) exists, so it gets checked /// and kept, never forgotten. @@ -101,89 +40,39 @@ fn is_missing(path: &Path) -> bool { /// Every record naming each set path, with the admin directory it is in. type Records = BTreeMap>; -/// The sets meta created that prune can find from the current workspace: -/// the sets under the worktree root, and any other set with a record in a -/// repository of the workspace (one under `META_WORKTREES`, or one whose -/// directory is gone). There is no global list, so sets recorded only in -/// other workspaces' repositories are not seen. Records that can't be used -/// are returned as kept entries. -fn find_sets(meta_dir: &Path) -> Result<(Records, Vec)> { +/// The sets meta created that have a record in a repository of +/// `workspace`. There is no global list, and the worktree directory may be +/// shared with other workspaces, so sets are found only through these +/// records. Records that can't be used are returned as kept entries, named +/// after their set when it is under `worktree_root`. +fn find_sets(workspace: &Workspace, worktree_root: &Path) -> (Records, Vec) { let mut found: Vec<(PathBuf, SetRecord)> = Vec::new(); let mut unusable: Vec = Vec::new(); - let mut not_used = |name: String, path: &Path, e: anyhow::Error| { + // Records hold resolved paths + let worktree_root = worktree_root + .canonicalize() + .unwrap_or_else(|_| worktree_root.to_path_buf()); + let mut not_used = |path: &Path, e: &anyhow::Error| { + let path = set_of_admin(path, &worktree_root).unwrap_or_else(|| path.to_path_buf()); + let name = path.file_name().map(|n| n.to_string_lossy().into_owned()); unusable.push(create_prune_entry( - name, + name.unwrap_or_default(), path.display().to_string(), format!("meta's records there can't be read: {e:#}"), None, )) }; - let worktree_root = resolve_worktree_root(Some(meta_dir))?; - // A root that can't be read is reported; its sets are still found - // through their repositories, and checked one by one - let entries = match std::fs::read_dir(&worktree_root) { - Ok(entries) => entries.collect(), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Vec::new(), - Err(e) => vec![Err(e)], - }; - for entry in entries { - let entry = match entry { - Ok(entry) => entry, - Err(e) => { - let name = worktree_root - .file_name() - .map(|n| n.to_string_lossy().into_owned()); - not_used(name.unwrap_or_default(), &worktree_root, e.into()); - continue; - } - }; - if !entry.file_type().is_ok_and(|t| t.is_dir()) { - continue; - } - let name = entry.file_name().to_string_lossy().into_owned(); - for member in discover_worktree_members(&entry.path()).unwrap_or_default() { - match member_record(&member.path) { - Ok(Some(found_here)) => found.push(found_here), - Ok(None) => {} - // Named by its admin directory, as the repository scan names it - Err(e) => { - let admin = linked_worktree_admin_dir(&member.path).unwrap_or(member.path); - not_used(name.clone(), &admin, e) - } - } - } - } - let root = meta_core::config::find_root_meta_dir(meta_dir); - let mut repos = vec![root.clone()]; - match meta_core::config::walk_meta_tree(&root, None) { - Ok(tree) => repos.extend( - meta_core::config::build_project_map(&tree, &root, "") - .into_values() - .map(|(path, _)| path), - ), - // Only some repositories can be searched: that is reported - Err(e) => { - let name = root.file_name().map(|n| n.to_string_lossy().into_owned()); - not_used(name.unwrap_or_default(), &root, e) - } + // Only some repositories can be searched: that is reported + for (path, e) in &workspace.unreadable { + not_used(path, e); } - for repo in repos { - let git_dir = match repo_git_dir(&repo) { - Ok(Some(git_dir)) => git_dir, - Ok(None) => continue, - Err(e) => { - let name = repo.file_name().map(|n| n.to_string_lossy().into_owned()); - not_used(name.unwrap_or_default(), &repo, e); - continue; - } - }; - for (admin, record) in record::in_repo(&git_dir) { + for git_dir in &workspace.repos { + for (admin, record) in record::in_repo(git_dir) { match record { + // A workspace sharing this repository made it: its set, not ours + Ok(record) if !workspace.made_here(&record.project) => {} Ok(record) => found.push((admin, record)), - Err(e) => { - let name = admin.file_name().map(|n| n.to_string_lossy().into_owned()); - not_used(name.unwrap_or_default(), &admin, e) - } + Err(e) => not_used(&admin, &e), } } } @@ -196,31 +85,29 @@ fn find_sets(meta_dir: &Path) -> Result<(Records, Vec)> { .push((admin, record)); } } - let mut paths = BTreeSet::new(); - unusable.retain(|e| paths.insert(e.path.clone())); - Ok((sets, unusable)) + (sets, unusable) } -/// Why an existing set with `record`, whose worktrees are at `aliases` in -/// it, is due for pruning, if it is: the reason and, for an expired TTL, -/// its age. -fn eligible( - record: &SetRecord, - aliases: &[String], - config_cache: &mut std::collections::HashMap< - PathBuf, - Option>, - >, - now: i64, -) -> Option<(String, Option)> { - // Check if source project directory still exists - if !record.project.exists() { - return Some(("orphaned (source project missing)".to_string(), None)); - } - // Check if source repos still exist in project (with config caching) - if let Some(reason) = check_repo_orphaned(&record.project, aliases, config_cache) { - return Some((reason, None)); - } +/// The set under `worktree_root` holding the worktree whose admin directory +/// is `admin`, if git's link back to the worktree leads there. +fn set_of_admin(admin: &Path, worktree_root: &Path) -> Option { + let gitdir = std::fs::read_to_string(admin.join("gitdir")).ok()?; + // git may write it relative to `admin` (worktree.useRelativePaths) + let checkout = admin.join(gitdir.trim()); + let checkout = checkout.canonicalize().unwrap_or(checkout); + let name = checkout + .strip_prefix(worktree_root) + .ok()? + .components() + .next()?; + Some(worktree_root.join(name)) +} + +/// Why an existing set with `record` is due for pruning, if it is: the +/// reason and, for an expired TTL, its age. (A set whose repositories were +/// removed from the project, or whose project is gone, is no longer this +/// workspace's: prune leaves it.) +fn eligible(record: &SetRecord, now: i64) -> Option<(String, Option)> { // Check TTL expiration let remaining = record.ttl_remaining(now).filter(|r| *r <= 0)?; // Total age = configured TTL + seconds past expiry @@ -231,66 +118,6 @@ fn eligible( Some(("ttl_expired".to_string(), Some(age))) } -/// The git directory holding the worktrees of the repository at `repo`: -/// `None` if there is no repository there (not cloned). -fn repo_git_dir(repo: &Path) -> Result> { - let dot_git = repo.join(".git"); - // Links are followed: a `.git` linked to the real git directory is one - let meta = match std::fs::metadata(&dot_git) { - Ok(meta) => meta, - // A bare repository is its own git directory - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - let bare = repo.join("HEAD").is_file() - && repo.join("objects").is_dir() - && repo.join("refs").is_dir(); - return Ok(bare.then(|| repo.to_path_buf())); - } - Err(e) => { - return Err( - anyhow::Error::from(e).context(format!("Cannot inspect {}", escape_path(&dot_git))) - ) - } - }; - if meta.is_dir() { - return Ok(Some(dot_git)); - } - // A `.git` file: a linked worktree's (its admin directory names the - // repository's git directory in `commondir`) or a `--separate-git-dir` - // checkout's (it names the git directory itself, with no `commondir`) - let content = std::fs::read_to_string(&dot_git) - .with_context(|| format!("Cannot read {}", escape_path(&dot_git)))?; - let target = content - .trim() - .strip_prefix("gitdir:") - .map(|target| repo.join(target.trim())) - .with_context(|| format!("{} has no gitdir line", escape_path(&dot_git)))?; - let commondir = target.join("commondir"); - match std::fs::read_to_string(&commondir) { - Ok(common) => Ok(Some(target.join(common.trim()))), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(Some(target)), - Err(e) => { - Err(anyhow::Error::from(e).context(format!("Cannot read {}", escape_path(&commondir)))) - } - } -} - -/// The paths of `members` from `set`, as repo aliases (`/`-separated, as -/// in `.meta`, on every platform). -fn aliases<'a>(set: &Path, members: impl IntoIterator) -> Vec { - members - .into_iter() - .map(|path| match path.strip_prefix(set) { - Ok(rel) if rel.as_os_str().is_empty() => ".".to_string(), - Ok(rel) => rel - .components() - .map(|c| c.as_os_str().to_string_lossy()) - .collect::>() - .join("/"), - Err(_) => path.to_string_lossy().into_owned(), - }) - .collect() -} - pub(crate) fn handle_prune( args: PruneArgs, _verbose: bool, @@ -300,7 +127,12 @@ pub(crate) fn handle_prune( let dry_run = args.dry_run; let meta_dir = require_meta_dir()?; - let (sets, mut kept) = find_sets(&meta_dir)?; + // Hooks come from here: inside a set being pruned, its own config goes + // with it + let owner = owner_dir(&meta_dir); + let workspace = Workspace::of(&meta_dir); + let worktree_root = resolve_worktree_root(Some(&meta_dir))?; + let (sets, mut kept) = find_sets(&workspace, &worktree_root); if sets.is_empty() && kept.is_empty() { if json { println!( @@ -312,14 +144,13 @@ pub(crate) fn handle_prune( })? ); } else { - println!("No worktree sets that meta created were found. Nothing to prune."); + println!("No worktree sets of this workspace were found. Nothing to prune."); } return Ok(()); } let now = Utc::now().timestamp(); let mut to_remove: Vec<(PruneEntry, SetRecord)> = Vec::new(); - let mut config_cache = std::collections::HashMap::new(); for (set, records) in &sets { let path_key = set.display().to_string(); @@ -363,8 +194,7 @@ pub(crate) fn handle_prune( None => found.clone(), }; - let found_aliases = aliases(set, members.iter().map(|m| m.path.as_path())); - if let Some((reason, age)) = eligible(&record, &found_aliases, &mut config_cache, now) { + if let Some((reason, age)) = eligible(&record, now) { to_remove.push((create_prune_entry(name, path_key, reason, age), record)); } } @@ -387,23 +217,21 @@ pub(crate) fn handle_prune( )); continue; } - // A set holding another set is blocked by the check. - let problems = match check_worktree_removal(wt_path) { - Ok(check) if check.is_clear() && check.record() == Some(&record) => { - // Judged again with every repo the full scan found: only it - // finds repos nested in other repos - let all = aliases(wt_path, check.members.iter().map(|m| m.path.as_path())); - if let Some((reason, age)) = eligible(&record, &all, &mut config_cache, now) { - let entry = create_prune_entry(entry.name, entry.path, reason, age); - candidates.push((entry, Some(check))); - } + // A set holding another set is blocked by the check, and one holding + // another workspace's worktrees is that workspace's to prune. + let checked = check_worktree_removal(wt_path) + .map(|check| (workspace.foreign_members(wt_path, &check), check)); + let problems = match checked { + Ok((foreign, _)) if !foreign.is_empty() => foreign.join("; "), + Ok((_, check)) if check.is_clear() && check.record() == Some(&record) => { + candidates.push((entry, Some(check))); continue; } - Ok(check) if check.is_clear() && check.record().is_some() => { + Ok((_, check)) if check.is_clear() && check.record().is_some() => { "its record changed after prune read it".to_string() } - Ok(check) if check.is_clear() => "meta's record of it is gone".to_string(), - Ok(check) => [check.blockers, check.unsaved].concat().join("; "), + Ok((_, check)) if check.is_clear() => "meta's record of it is gone".to_string(), + Ok((_, check)) => [check.blockers, check.unsaved].concat().join("; "), Err(e) => format!("cannot inspect it: {e:#}"), }; kept.push(create_prune_entry( @@ -414,6 +242,7 @@ pub(crate) fn handle_prune( )); } + once_each(&mut kept); if dry_run || candidates.is_empty() { let would_remove: Vec = candidates.into_iter().map(|(e, _)| e).collect(); if json { @@ -494,7 +323,8 @@ pub(crate) fn handle_prune( } // Fire post-prune hook - fire_post_prune(&removed, Some(&meta_dir)); + fire_post_prune(&removed, Some(&owner)); + once_each(&mut kept); if json { println!( @@ -557,6 +387,13 @@ fn path_moved(path: &Path) -> Option { } } +/// Keep the first entry for each path: a set kept for several reasons (an +/// unusable record in each of its worktrees, say) is listed once. +fn once_each(kept: &mut Vec) { + let mut paths = BTreeSet::new(); + kept.retain(|e| paths.insert(e.path.clone())); +} + fn print_kept(kept: &[PruneEntry]) { if kept.is_empty() { return; @@ -577,32 +414,39 @@ fn warn_kept(kept: &[PruneEntry], strict: bool) -> Result<()> { if kept.is_empty() { return Ok(()); } - let names: Vec<&str> = kept.iter().map(|e| e.name.as_str()).collect(); + let names: Vec<_> = kept.iter().map(|e| escape(&e.name)).collect(); super::warn_or_bail(strict, format!("Kept worktrees: {}", names.join(", "))) } #[cfg(test)] mod tests { use super::*; - use std::collections::HashMap; - use tempfile::TempDir; - - /// A set's project and aliases, as check_repo_orphaned takes them. - fn make_set(project: &str, repos: Vec<(&str, &str)>) -> (PathBuf, Vec) { - let aliases = repos - .into_iter() - .map(|(alias, _)| alias.to_string()) - .collect(); - (PathBuf::from(project), aliases) + + #[test] + fn a_relative_gitdir_finds_its_set() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().canonicalize().unwrap().join("wt"); + let checkout = root.join("broken/a"); + std::fs::create_dir_all(&checkout).unwrap(); + std::fs::write(checkout.join(".git"), "").unwrap(); + let admin = tmp.path().join("repo/.git/worktrees/a"); + std::fs::create_dir_all(&admin).unwrap(); + std::fs::write(admin.join("gitdir"), "../../../../wt/broken/a/.git\n").unwrap(); + assert_eq!(set_of_admin(&admin, &root), Some(root.join("broken"))); } #[test] - fn aliases_use_slashes_on_every_platform() { - let set = Path::new("set"); - let members = [set.to_path_buf(), set.join("group").join("core")]; - assert_eq!( - aliases(set, members.iter().map(PathBuf::as_path)), - [".", "group/core"] + fn kept_names_are_escaped_on_one_line() { + let kept = [create_prune_entry( + "bad\nFORGED\u{1b}[2J".to_string(), + String::new(), + "", + None, + )]; + let message = warn_kept(&kept, true).unwrap_err().to_string(); + assert!( + !message.contains('\n') && !message.contains('\u{1b}'), + "{message}" ); } @@ -645,164 +489,4 @@ mod tests { ); assert_eq!(entry.reason, "test reason"); } - - // ── check_repo_orphaned ───────────────────────────── - - #[test] - fn check_repo_orphaned_returns_none_when_all_repos_exist() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - - // Create .meta config - let meta_config = r#"{"projects":{"repo1":"git@github.com:org/repo1.git","repo2":"git@github.com:org/repo2.git"}}"#; - std::fs::write(project_path.join(".meta"), meta_config).unwrap(); - - let entry = make_set( - project_path.to_str().unwrap(), - vec![("repo1", "main"), ("repo2", "feat-x")], - ); - - let mut cache = HashMap::new(); - let result = check_repo_orphaned(&entry.0, &entry.1, &mut cache); - - assert!(result.is_none()); - } - - #[test] - fn check_repo_orphaned_detects_when_all_repos_missing() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - - // Create .meta config with only repo3 (repos 1 and 2 are missing) - let meta_config = r#"{"projects":{"repo3":"git@github.com:org/repo3.git"}}"#; - std::fs::write(project_path.join(".meta"), meta_config).unwrap(); - - let entry = make_set( - project_path.to_str().unwrap(), - vec![("repo1", "main"), ("repo2", "feat-x")], - ); - - let mut cache = HashMap::new(); - let result = check_repo_orphaned(&entry.0, &entry.1, &mut cache); - - assert_eq!( - result, - Some("orphaned (all source repos removed from project)".to_string()) - ); - } - - #[test] - fn check_repo_orphaned_returns_none_when_some_repos_missing() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - - // Create .meta config with repo1 but not repo2 - let meta_config = r#"{"projects":{"repo1":"git@github.com:org/repo1.git"}}"#; - std::fs::write(project_path.join(".meta"), meta_config).unwrap(); - - let entry = make_set( - project_path.to_str().unwrap(), - vec![("repo1", "main"), ("repo2", "feat-x")], - ); - - let mut cache = HashMap::new(); - let result = check_repo_orphaned(&entry.0, &entry.1, &mut cache); - - // Only partial orphan - should return None (worktree is still partially valid) - assert!(result.is_none()); - } - - #[test] - fn check_repo_orphaned_never_counts_the_root_repo_as_missing() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - let meta_config = r#"{"projects":{"repo3":"git@github.com:org/repo3.git"}}"#; - std::fs::write(project_path.join(".meta"), meta_config).unwrap(); - let project = project_path.to_str().unwrap(); - let mut cache = HashMap::new(); - - let root_only = make_set(project, vec![(".", "main")]); - assert!(check_repo_orphaned(&root_only.0, &root_only.1, &mut cache).is_none()); - - let root_and_removed = make_set(project, vec![(".", "main"), ("repo1", "main")]); - assert!( - check_repo_orphaned(&root_and_removed.0, &root_and_removed.1, &mut cache).is_none() - ); - } - - #[test] - fn check_repo_orphaned_finds_nested_aliases() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - let meta_config = - r#"{"projects":{"sub":{"repo":"git@github.com:org/sub.git","meta":true}}}"#; - std::fs::write(project_path.join(".meta"), meta_config).unwrap(); - std::fs::create_dir(project_path.join("sub")).unwrap(); - let sub_config = r#"{"projects":{"leaf":"git@github.com:org/leaf.git"}}"#; - std::fs::write(project_path.join("sub/.meta"), sub_config).unwrap(); - - let entry = make_set(project_path.to_str().unwrap(), vec![("sub/leaf", "main")]); - assert!(check_repo_orphaned(&entry.0, &entry.1, &mut HashMap::new()).is_none()); - } - - #[test] - fn check_repo_orphaned_returns_none_when_no_config() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - - // No .meta config file - let entry = make_set(project_path.to_str().unwrap(), vec![("repo1", "main")]); - - let mut cache = HashMap::new(); - let result = check_repo_orphaned(&entry.0, &entry.1, &mut cache); - - // Can't determine without config - assert!(result.is_none()); - } - - #[test] - fn check_repo_orphaned_uses_cache() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - - // Create .meta config - let meta_config = r#"{"projects":{"repo1":"git@github.com:org/repo1.git"}}"#; - std::fs::write(project_path.join(".meta"), meta_config).unwrap(); - - let entry1 = make_set(project_path.to_str().unwrap(), vec![("repo1", "main")]); - - let entry2 = make_set(project_path.to_str().unwrap(), vec![("repo1", "feat-x")]); - - let mut cache = HashMap::new(); - - // First call should populate cache - let result1 = check_repo_orphaned(&entry1.0, &entry1.1, &mut cache); - assert!(result1.is_none()); - assert_eq!(cache.len(), 1); - - // Second call with same project should use cache - let result2 = check_repo_orphaned(&entry2.0, &entry2.1, &mut cache); - assert!(result2.is_none()); - assert_eq!(cache.len(), 1); // Still 1 entry (cached) - - // Verify cache contains the correct project - assert!(cache.contains_key(project_path)); - } - - #[test] - fn check_repo_orphaned_handles_invalid_json() { - let temp_dir = TempDir::new().unwrap(); - let project_path = temp_dir.path(); - - // Create invalid .meta config - std::fs::write(project_path.join(".meta"), "invalid json").unwrap(); - - let entry = make_set(project_path.to_str().unwrap(), vec![("repo1", "main")]); - - let mut cache = HashMap::new(); - let result = check_repo_orphaned(&entry.0, &entry.1, &mut cache); - - // Should return None (can't parse config) - assert!(result.is_none()); - } } diff --git a/src/commands/worktree/remove.rs b/src/commands/worktree/remove.rs index b85da19..29e3387 100644 --- a/src/commands/worktree/remove.rs +++ b/src/commands/worktree/remove.rs @@ -1,11 +1,12 @@ use anyhow::Result; use colored::*; -use meta_core::display::escape_path; +use meta_core::display::{escape, escape_path}; use meta_git_lib::worktree::helpers::*; use meta_git_lib::worktree::hooks::fire_post_destroy; use meta_git_lib::worktree::safety::{check_worktree_removal, delete_checked_set}; use meta_git_lib::worktree::types::*; +use meta_git_lib::worktree::workspace::Workspace; use super::cli_types::DestroyArgs; @@ -14,8 +15,11 @@ pub(crate) fn handle_remove(args: DestroyArgs, verbose: bool, json: bool) -> Res validate_worktree_name(name)?; let force = args.force; - let meta_dir = find_meta_dir(); - let worktree_root = resolve_worktree_root(meta_dir.as_deref())?; + let meta_dir = require_meta_dir()?; + // Hooks come from here: inside the set being removed, its own config + // goes with it + let owner = owner_dir(&meta_dir); + let worktree_root = resolve_worktree_root(Some(&meta_dir))?; let wt_dir = worktree_set_dir(&worktree_root, name)?; if !wt_dir.exists() { @@ -23,6 +27,29 @@ pub(crate) fn handle_remove(args: DestroyArgs, verbose: bool, json: bool) -> Res } let check = check_worktree_removal(&wt_dir)?; + // Only this workspace's sets: a shared worktree directory may hold + // another workspace's. No flag overrides this. + let workspace = Workspace::of(&meta_dir); + let foreign = workspace.foreign_members(&wt_dir, &check); + if !foreign.is_empty() { + // Repositories meta couldn't read make sets from them look foreign + let unreadable: String = workspace + .unreadable + .iter() + .map(|(path, e)| format!("\n {}: {}", escape_path(path), escape(&format!("{e:#}")))) + .collect(); + anyhow::bail!( + "Refusing to remove worktree '{name}': it is not a set of this workspace ({}):\n {}\n\ + Remove it from the workspace that made it.{}", + escape_path(&workspace.root), + foreign.join("\n "), + if unreadable.is_empty() { + String::new() + } else { + format!("\nParts of this workspace couldn't be read:{unreadable}") + } + ); + } if !check.blockers.is_empty() { anyhow::bail!( "Refusing to remove worktree '{name}':\n {}\n\ @@ -62,7 +89,7 @@ pub(crate) fn handle_remove(args: DestroyArgs, verbose: bool, json: bool) -> Res delete_checked_set(&wt_dir, &check, force, verbose)?; // Fire post-destroy hook - fire_post_destroy(name, &wt_dir, force, meta_dir.as_deref()); + fire_post_destroy(name, &wt_dir, force, Some(&owner)); if json { let output = DestroyOutput { diff --git a/src/helpers.rs b/src/helpers.rs index 7ea2151..4aec413 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -18,7 +18,7 @@ pub(crate) fn get_project_directories_with_fallback( pub(crate) fn get_project_directories(cwd: &Path) -> anyhow::Result> { // Without a .meta there is only the root; a .meta that is refused is an // error, not a reason to act on the root alone - if config::find_meta_config(cwd, None).is_none() { + if config::find_meta_config(cwd, None)?.is_none() { return Ok(vec![".".to_string()]); } let tree = config::walk_meta_tree(cwd, Some(0))?; diff --git a/src/ssh.rs b/src/ssh.rs index 0c06c36..86e80d4 100644 --- a/src/ssh.rs +++ b/src/ssh.rs @@ -8,7 +8,7 @@ use std::path::Path; /// pass them directly to `establish_ssh_masters` without information loss. /// Returns an empty list if no .meta config is found or no SSH URLs exist. pub fn discover_ssh_urls(cwd: &Path) -> Vec { - let Some((config_path, _format)) = meta_core::config::find_meta_config(cwd, None) else { + let Ok(Some((config_path, _format))) = meta_core::config::find_meta_config(cwd, None) else { return vec![]; }; @@ -38,7 +38,7 @@ pub(crate) struct RemoteMismatch { /// Check child repos for remote URL mismatches against .meta config. /// Returns a list of mismatches found (non-interactive, no prompts). pub(crate) fn find_remote_mismatches(cwd: &Path) -> Vec { - let Some((config_path, _format)) = meta_core::config::find_meta_config(cwd, None) else { + let Ok(Some((config_path, _format))) = meta_core::config::find_meta_config(cwd, None) else { return vec![]; }; diff --git a/tests/safety.rs b/tests/safety.rs index e8d253a..d3dd492 100644 --- a/tests/safety.rs +++ b/tests/safety.rs @@ -648,12 +648,16 @@ fn a_set_holding_another_recorded_set_is_not_deleted() { let ws = workspace(root, &["a"]); let out = worktree(root, &ws, &["create", "outer", "--repo", ".", "--no-deps"]); assert!(out.status.success(), "{}", stderr(&out)); - // A set created from inside the outer one lands in its .worktrees. + // A set placed inside the outer one (from inside a set, sets go where + // the workspace keeps them, so the place is given) let outer = ws.join(".worktrees/outer"); - let out = worktree( + let out = meta_git_in( root, &outer, - &["create", "inner", "--repo", ".", "--no-deps"], + "git worktree create", + &["inner", "--repo", ".", "--no-deps"], + json!({}), + Some(&outer.join(".worktrees")), ); assert!(out.status.success(), "{}", stderr(&out)); assert!(outer.join(".worktrees/inner").exists()); @@ -765,8 +769,8 @@ fn prune_keeps_a_worktree_it_cannot_inspect() { .iter() .filter_map(|e| e["name"].as_str()) .collect(); - // The set, found through its repository, and the root it couldn't list - assert_eq!(kept, [".worktrees", "x"], "{report}"); + // The set, found through its repository (prune doesn't list the root) + assert_eq!(kept, ["x"], "{report}"); assert!(ws.join(".worktrees/x/a/README").exists()); }