diff --git a/CHANGELOG.md b/CHANGELOG.md index 15f5e219..6a516673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 path, and a `metadata.json` written by an older `dl` reads exactly as it did. [docs/cleanup.md](docs/cleanup.md) has the detail. +### Changed + +- **`dl --purge` names where each surviving workspace came from.** The list of + workspaces a purge is leaving standing printed ids and nothing else, and an id + is the one thing you cannot decide on: `pythontemplate` reads exactly the same + whether it is a `dl ` of yours, a `dl ./project` whose checkout you + care about, or something another tool made. Each line now carries the source + beside the id, the same string `dl --ls` shows in its `SOURCE` column. + + The block also says what removing the cache costs the workspaces that stay. + They keep working, but a clone an older `dl` placed outside the cache, under the + retired `worktree.repos_dir` key, is named only by a record inside the cache + that is about to go: after the purge, `dl rm` deletes the workspace + and leaves that directory standing with nothing on the machine pointing at it. + Removing such a workspace first is what takes its clone with it. The copy of + their volume names goes the same way, so a survivor deleted with a bare `devpod + delete` after a purge leaves volumes nothing can reclaim, where `dl --prune` + would have. `--purge` also reports a `config.toml` that still sets that key now, + which is the only mention a stranded tree gets when no workspace opens it any + more. + ### Fixed - **Sixty-six citations that pointed at nothing now point at something, and a diff --git a/README.md b/README.md index 6b6faca7..a9639bfe 100644 --- a/README.md +++ b/README.md @@ -369,7 +369,7 @@ different jobs: | Command | Takes | Leaves | |---|---|---| | `dl --prune` | Clone directories no workspace opens, and the volumes of workspaces devpod no longer lists | Every workspace, container and image | -| `dl --purge` | The workspaces devlaunch created, and its caches | Workspaces it did not create, named before it asks | +| `dl --purge` | The workspaces devlaunch created, and its caches | Workspaces it did not create, each named with its source before it asks | | `dl --reconcile` | Nothing | Repairs records that stopped matching the disk | Two promises worth knowing. **Nothing deletes work that exists nowhere else:** a clone with diff --git a/docs/cleanup.md b/docs/cleanup.md index 2f4ebd33..59483f82 100644 --- a/docs/cleanup.md +++ b/docs/cleanup.md @@ -166,7 +166,8 @@ created, meaning the clones it made under its own cache directory (`$XDG_CACHE_H directory the purge is about to remove anyway. Everything else keeps working afterwards, because nothing a purge touches backs it. -Anything it is leaving is named before it asks: +Anything it is leaving is named before it asks, by where it came from and not +only by its id: ``` $ dl --purge @@ -175,12 +176,23 @@ This will remove all devlaunch data: - /home/you/.cache/devlaunch/ (workspace clones, repo caches, the shared pixi cache, completions) Leaving 2 workspace(s) devlaunch did not create: - - pythontemplate - - my-hand-made-workspace + - pythontemplate: https://github.com/blooop/pythontemplate + - my-hand-made-workspace: /home/you/projects/thing + +Removing the cache also drops what dl recorded about them, the copy of their volume names included. They keep working, and `dl rm` still removes one and its volumes while devpod still lists it. +A clone an older dl placed outside the cache is named only by a record in there, though, so remove such a workspace now if the clone should go with it. Are you sure? [y/N] ``` +The source is the half you can decide on. An id is what devpod addresses a +workspace by and says nothing about where it came from, so `pythontemplate` reads +the same whether it is a `dl ` of yours, a `dl ./project`, or something +somebody else's tool made. It is the same string `dl --ls` shows in its `SOURCE` +column, read the same way, so the two never describe one workspace differently. +A source `dl` cannot read at all, which is devpod's own object rather than a path +or a URL, is printed as that object and said to be one. + Three things `dl` does create can land in that second list rather than the first. `dl ./some/path` and `dl ` open a source `dl` did not clone, so it cannot tell them from a workspace you made by hand. The third is older: a @@ -192,6 +204,32 @@ Erring this way is deliberate: a purge that skips one of your own workspaces costs you a command, and the other kind of mistake costs you work you cannot get back. +That third one is what the two sentences under the list are about. The workspace +stays, and the record naming its clone was in the cache that has just gone, so a +`dl rm` afterwards deletes the workspace and leaves the directory +standing with nothing on the machine pointing at it. Removing it before the purge +is what takes the clone too. If `config.toml` still sets `worktree.repos_dir`, +`--purge` now says so as well, before the plan: where no workspace opens such a +clone any more there is no line in any list for it, and that notice is the only +mention that tree will get. + +Under `-y` the second sentence is the same fact in the tense that run has earned. +"Remove such a workspace now" is an action only somebody who still has the +question in front of them can take, and printing it into a run that deletes the +records three lines later would be advice arriving after the door shut, so what +`-y` says instead is what will be true of `dl rm` from then on. + +The volume names are part of that loss, and this is the one place it shows. +Deleting a survivor with `dl rm` still takes its volumes: that read +happens at delete time, out of devpod's own record under `DEVPOD_HOME`, which a +purge does not touch. What goes is [the copy `dl` +keeps](#the-volumes-of-a-workspace-devpod-has-already-forgotten), which is what +`--prune` reclaims from once devpod has forgotten a workspace. So a survivor +deleted with a bare `devpod delete` after a purge leaves both its volumes with +nothing on the machine naming them, where before the purge `dl --prune` would +have reclaimed them. Deleting such a workspace through `dl` is what avoids that, +and it is the same advice the sentence above gives for its clone. + #### When part of the cache will not go A container writes into its clone as its own user, `vscode` at uid 1000 in the diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index 01bcfadc..a8420b12 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -1129,12 +1129,48 @@ fn render_purge(context: &mut CommandContext<'_>, cache: &Path, yes: bool) -> En purge_devlaunch_data(context, cache, yes).with_the_boundary() } +/// `worktree.repos_dir`'s notice on a path that opens no records (devlaunch#461). +/// +/// [`report`] says this for every command that opens dl's records, which is where +/// it belongs and is not here: a purge reads `metadata.json` for nothing, and +/// opening it would run the cache migration, writing records into the tree this +/// command is about to remove and into one an aborted purge was asked to leave +/// alone. So the config is read on its own, which creates nothing and touches +/// nothing. +/// +/// It is worth saying *here* in particular, and #467 left the decision to this +/// ticket. A clone under that retired root is not devlaunch's by the only test +/// `--purge` has, so the purge leaves it and removes the record that was the last +/// thing pointing at it. Where a workspace still opens such a clone the leaving +/// list now names the path; where none does, this line is the only mention that +/// tree will ever get. +/// +/// A `config.toml` that cannot be read says nothing rather than refusing: a purge +/// does not otherwise need the file, and the next command that opens dl's records +/// is where a broken config is somebody's problem. +fn say_retired_keys() { + if let Ok((_, retired)) = session::worktree_config() { + for line in render::retired_keys(&retired) { + eprintln!("{line}"); + } + } +} + fn purge_devlaunch_data(context: &mut CommandContext<'_>, cache: &Path, yes: bool) -> Cleanup { + say_retired_keys(); let plan = match lifecycle::purge_plan(context, cache) { Err(refused) => return Cleanup::Raised(refuse_listing(&refused)), Ok(plan) => plan, }; - print(&render::purge_plan_lines(&plan)); + // The plan is told which of the two it is. Everything in it is preventable + // while the question is still coming, and one sentence of it offers an action + // that only an interactive reader can still take. + let confirmation = if yes { + render::Confirmation::AnsweredOnTheLine + } else { + render::Confirmation::WillBeAsked + }; + print(&render::purge_plan_lines(&plan, confirmation)); if !yes && !confirmed("Are you sure? [y/N] ") { println!("Aborted."); return Cleanup::Ended(Ending::Done); diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index bda90964..9f0ce55d 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -9,7 +9,7 @@ use std::fmt::Write as _; use std::io; use std::path::Path; -use devlaunch_core::clients::devpod::{ListingUnreadable, NotAListing, NotRun}; +use devlaunch_core::clients::devpod::{ListingUnreadable, NotAListing, NotRun, Workspace}; use devlaunch_core::clients::devpod_home::RepointFailure; use devlaunch_core::clients::gh::{GhEvent, GhUnavailable}; use devlaunch_core::clients::git::Failure as GitFailure; @@ -37,7 +37,7 @@ use devlaunch_core::flows::lifecycle::{ Unlocatable, VolumeRefusal, VolumesKeptBecause, }; use devlaunch_core::flows::listing::{ - LastUsed, SizeCell, Sizes, SweptRepoNote, TableRow, WorkspaceTable, + self, LastUsed, SizeCell, Sizes, SourceKind, SweptRepoNote, TableRow, WorkspaceTable, }; use devlaunch_core::flows::migration::{Listing, MigrationReport}; use devlaunch_core::flows::provision::{BundleFailed, FailureLevel, ProvisionEvent}; @@ -1769,12 +1769,94 @@ pub(crate) const DOCKER_BOUNDARY: &str = concat!( // --purge // --------------------------------------------------------------------------- +/// Whether the plan above the leaving list is still a decision. +/// +/// `dl --purge` prints the block and *then* asks, so every line of it is something +/// the reader can still prevent. `dl --purge -y` answered on the command line, and +/// the same lines are a record of what is about to happen instead. One sentence in +/// the block turns on that difference, which is why the renderer is told rather +/// than left to print advice into a run that has stopped taking any. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Confirmation { + /// `Are you sure? [y/N]` is coming. + WillBeAsked, + /// `-y` answered it before the plan was printed. + AnsweredOnTheLine, +} + +/// What removing the cache costs the workspaces the purge is *not* deleting. +/// +/// Said in the block that lists them, because it is a reason to answer `n` and +/// remove one of them properly first (devlaunch#461). Two things are true of a +/// survivor and only the first is obvious: it keeps working, and dl stops knowing +/// anything about it. +/// +/// **The volume names are named because they are now in the cache** +/// (devlaunch#456, merged while this was open). dl keeps its own copy of the two +/// volumes a workspace's devcontainer made, under the cache, so a purge that +/// leaves a foreign workspace standing destroys the copy of *its* names while +/// leaving its volumes -- which is the exact case #452 predicted this sentence +/// would have to cover. What is not lost is the ordinary route: `dl rm` +/// reads devpod's own `workspace_result.json` under `DEVPOD_HOME`, which a purge +/// does not touch, so a survivor deleted through dl still takes its volumes with +/// it. The copy is what `--prune` reclaims from *after* devpod has forgotten a +/// workspace, and that is the reach a purge costs it. +const SURVIVORS_KEEP_WORKING: &str = "Removing the cache also drops what dl recorded about them, the copy of their volume \ + names included. They keep working, and `dl rm` still removes one and its \ + volumes while devpod still lists it."; + +/// The sentence under that one, in the tense the run has earned. +/// +/// Where the loss is a loss rather than untidiness. A clone a pre-#467 dl placed +/// under `worktree.repos_dir` is outside the cache, so the workspace opening it is +/// foreign here and stays; the record naming that directory is *inside* the cache +/// and goes. Afterwards `dl rm` answers `NothingRecorded` and leaves the tree +/// standing, and nothing else on the machine mentions it. +/// +/// **Two spellings of one fact, and what separates them is whether it is still +/// actionable.** Printed above the question, "remove such a workspace now" is the +/// action the whole block exists to offer. Printed under `-y` it would be asking +/// for something the same run makes impossible three lines later, which is advice +/// arriving after the door shut. The subject clause is the same either way; only +/// what follows from it moves. +fn stranded_clones(confirmation: Confirmation) -> &'static str { + match confirmation { + Confirmation::WillBeAsked => { + "A clone an older dl placed outside the cache is named only by a record in there, \ + though, so remove such a workspace now if the clone should go with it." + } + Confirmation::AnsweredOnTheLine => { + "A clone an older dl placed outside the cache is named only by a record in there, \ + though, so from here on `dl rm` takes such a workspace and leaves its \ + clone standing." + } + } +} + +/// One survivor's line in the leaving list. +/// +/// A function rather than a `format!` in the loop so that the sample output in +/// `docs/cleanup.md` can be diffed against the real shape of the line. The page is +/// a second hand-maintained copy of it, and `the_cleanup_page_quotes_what_a_purge_ +/// really_prints` is the test the standing rule asks for beside such a copy. +fn leaving_line(id: &str, source: &str) -> String { + format!(" - {id}: {source}") +} + /// What a purge would take, printed before the question is asked. /// /// The workspaces devlaunch did not create are *named* rather than merely left out /// of the count: a user who asked for a clean slate and gets survivors should /// learn it while saying no is still an option. -pub(crate) fn purge_plan_lines(plan: &PurgePlan) -> Vec { +/// +/// **Named by their source and not only by their id** (devlaunch#461). An id is +/// what devpod addresses a workspace by and carries nothing about where it came +/// from, so `someones-project` reads the same whether it is a `dl ./project` of +/// yours, a `dl `, or a workspace somebody made with `devpod up` -- and +/// this is the one screen where that difference is being decided on. The source is +/// the same string `dl --ls` puts in its `SOURCE` column, from the same reading of +/// it, so the two surfaces cannot describe one workspace differently. +pub(crate) fn purge_plan_lines(plan: &PurgePlan, confirmation: Confirmation) -> Vec { let ownership = plan.ownership(); let mut lines = vec![ "This will remove all devlaunch data:".to_owned(), @@ -1794,13 +1876,31 @@ pub(crate) fn purge_plan_lines(plan: &PurgePlan) -> Vec { ownership .foreign .iter() - .map(|workspace| format!(" - {}", workspace.id)), + .map(|workspace| leaving_line(&workspace.id, &left_standing_source(workspace))), ); + lines.push(String::new()); + lines.push(SURVIVORS_KEEP_WORKING.to_owned()); + lines.push(stranded_clones(confirmation).to_owned()); } lines.push(String::new()); lines } +/// Where one surviving workspace came from, as the leaving list names it. +/// +/// [`describe_source`](listing::describe_source) is what `dl --ls` reads, and the +/// detail alone carries the answer for the two arms that have one: a path is a +/// path and a URL is a URL, and neither needs the `TYPE` column's word repeated +/// beside it. The third arm does, because devpod's own object is not a source in +/// any readable sense and would otherwise sit after a colon looking like one. +fn left_standing_source(workspace: &Workspace) -> String { + let described = listing::describe_source(workspace.source()); + match described.kind { + SourceKind::Local | SourceKind::Git => described.detail, + SourceKind::Unknown => format!("a source dl cannot read, {}", described.detail), + } +} + /// One rendered line, and which stream it belongs on. /// /// Every other renderer here answers with lines for one stream, because its caller @@ -2930,6 +3030,46 @@ mod tests { assert!(retired_keys(&[]).is_empty()); } + // ------------------------------------------------------------- --purge + + #[test] + fn the_cleanup_page_quotes_what_a_purge_really_prints() { + // `docs/cleanup.md` reproduces the block `--purge` prints above its + // question, which makes the page a second hand-maintained copy of it -- + // and a sample output that has drifted from the command is worse than no + // sample. This is the diff test the standing rule asks for beside such a + // copy. If that section moves to another page, this path moves with it, + // in the same change. + // + // The survivor line is in here as well as the two sentences, because the + // line is what this change is about: the sample would go on reading + // `- pythontemplate` on its own if the renderer's format ever went back to + // an id, and nothing else would notice. + let page = std::fs::read_to_string( + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../docs/cleanup.md"), + ) + .expect("docs/cleanup.md"); + let quoted = [ + SURVIVORS_KEEP_WORKING.to_owned(), + stranded_clones(Confirmation::WillBeAsked).to_owned(), + leaving_line("pythontemplate", "https://github.com/blooop/pythontemplate"), + leaving_line("my-hand-made-workspace", "/home/you/projects/thing"), + ]; + for said in quoted { + assert!( + page.contains(&said), + "docs/cleanup.md no longer quotes what the purge says: {said}" + ); + } + // And the `-y` spelling is deliberately *not* quoted there: the page + // describes it in prose instead, so there is no second copy of it to + // drift. + assert!( + !page.contains(stranded_clones(Confirmation::AnsweredOnTheLine)), + "the page grew a copy of the -y sentence; guard it here or take it out" + ); + } + // ---------------------------------------------- the refusal advice line /// The one line a person is meant to paste, with paths a shell would diff --git a/rust/dl/tests/lifecycle.rs b/rust/dl/tests/lifecycle.rs index 14e3237b..135b2904 100644 --- a/rust/dl/tests/lifecycle.rs +++ b/rust/dl/tests/lifecycle.rs @@ -1714,14 +1714,50 @@ fn a_fresh_cache_stops_the_child_before_it_sweeps() { // --purge // =========================================================================== -/// Python's `dl --purge` plan for the base world, verbatim. +/// Python's `dl --purge` plan for the base world, with devlaunch#461's two +/// additions to the block that names the survivors. +/// +/// **The second deliberate divergence from Python in this file**, beside +/// [`DOCKER_BOUNDARY`] below. Python printed a bare workspace id per survivor, and +/// an id is exactly what a user cannot decide on: `someones-project` could be a +/// `dl ./project` of theirs, a `dl `, or a workspace somebody made with +/// `devpod up`. So each one is named by its source, and the sentence under the +/// list says what removing the cache costs the workspaces that are staying. +/// +/// This is the plan a run that is **about to ask** prints. `-y` prints +/// [`PURGE_PLAN_YES`], which differs in the one line that offers an action, and +/// the two are spelled out separately rather than derived from each other so that +/// a change to either is read as the output change it is. const PURGE_PLAN: &str = "\ This will remove all devlaunch data: - 2 DevPod workspace(s) - {ROOT}/cache/devlaunch/ (workspace clones, repo caches, the shared pixi cache, completions) Leaving 1 workspace(s) devlaunch did not create: - - someones-project + - someones-project: {ROOT}/foreign/proj + +Removing the cache also drops what dl recorded about them, the copy of their volume names included. They keep working, and `dl rm` still removes one and its volumes while devpod still lists it. +A clone an older dl placed outside the cache is named only by a record in there, though, so remove such a workspace now if the clone should go with it. + +"; + +/// The same plan under `-y`, where the last sentence is in the tense that run has +/// earned. +/// +/// "Remove such a workspace now" is an action only a reader with the question +/// still in front of them can take; the same run deletes the records that make it +/// possible three lines later. So `-y` gets the same fact as what will be true +/// from then on, and every `-y` golden below is built from this one. +const PURGE_PLAN_YES: &str = "\ +This will remove all devlaunch data: + - 2 DevPod workspace(s) + - {ROOT}/cache/devlaunch/ (workspace clones, repo caches, the shared pixi cache, completions) + +Leaving 1 workspace(s) devlaunch did not create: + - someones-project: {ROOT}/foreign/proj + +Removing the cache also drops what dl recorded about them, the copy of their volume names included. They keep working, and `dl rm` still removes one and its volumes while devpod still lists it. +A clone an older dl placed outside the cache is named only by a record in there, though, so from here on `dl rm` takes such a workspace and leaves its clone standing. "; @@ -1738,6 +1774,114 @@ const DOCKER_BOUNDARY: &str = "devlaunch does not manage Docker images: the imag workspaces built may still hold disk, and `docker system df` \ shows what Docker is holding.\n"; +#[test] +fn the_leaving_list_names_each_survivors_source_beside_its_id() { + // devlaunch#461. The id on its own is not something a user can decide on: a + // `dl ./project` of theirs, a `dl ` and a workspace they made with + // `devpod up` all read the same, and this is the one screen where somebody is + // deciding. The source is what tells them apart. + let world = World::base(); + let run = world.answering("n\n", &["--purge"]); + run.exited(0); + assert!( + run.out + .contains(" - someones-project: {ROOT}/foreign/proj\n"), + "the leaving list named an id and no source:\n{}", + run.out + ); + assert!( + run.out.contains( + "Removing the cache also drops what dl recorded about them, the copy of \ + their volume names included." + ), + "the block did not say what the purge costs the survivors:\n{}", + run.out + ); +} + +#[test] +fn a_survivor_whose_source_dl_cannot_read_is_said_to_be_one() { + // The third arm of the source, and the reason the leaving list does not simply + // print the detail: devpod's own object after a colon reads like a source, and + // this is the one row where dl has nothing truer to say than the object. + let world = World::with(&["--unplaceable"]); + let run = world.answering("n\n", &["--purge"]); + run.exited(0); + assert!( + run.out.contains( + " - a-source-nobody-can-read: a source dl cannot read, {\"localFolder\": 42}\n" + ), + "{}", + run.out + ); +} + +#[test] +fn a_purge_names_the_clone_a_retired_repos_dir_left_outside_the_cache() { + // devlaunch#461, the case #467's review reproduced. A pre-#467 `dl` put this + // clone under `worktree.repos_dir`, so the workspace opening it is foreign + // here: the purge leaves it standing and removes the record that is the last + // thing on the machine pointing at the tree. It used to print `devlaunch-main-3j1t` + // and nothing else, which names neither the clone nor the fact that it is one. + let world = World::with(&["--stranded-clone"]); + let run = world.answering("n\n", &["--purge"]); + run.exited(0); + assert!( + run.out.contains( + "Leaving 2 workspace(s) devlaunch did not create:\n \ + - someones-project: {ROOT}/foreign/proj\n \ + - devlaunch-main-3j1t: {ROOT}/old-repos/blooop/devlaunch/devlaunch-main-3j1t\n" + ), + "the stranded clone's path is not in the plan:\n{}", + run.out + ); + // And the retired key earns its notice on this path too, which is the half the + // list cannot cover: a clone under that root with no workspace left opening it + // has no line in any plan, and this run is what removes its record. + assert!( + run.err.contains("worktree.repos_dir = '{ROOT}/old-repos'"), + "the purge said nothing about the key that put a tree there:\n{}", + run.err + ); + + // What the sentence under the list is warning about, on disk: the tree stays + // and the record naming it does not. + let purged = world.dl(&["--purge", "-y"]); + purged.exited(0); + assert!( + world.exists("old-repos/blooop/devlaunch/devlaunch-main-3j1t"), + "the purge removed a clone outside its own cache" + ); + assert!( + !world.exists("cache/devlaunch/metadata.json"), + "the record survived, so the sentence about losing it is wrong" + ); + assert_eq!( + world.devpod_calls().last().map(String::as_str), + Some("devpod delete devlaunch-dirty-fqta --force"), + "the stranded workspace was deleted, or the ownership scope moved" + ); +} + +#[test] +fn removing_a_stranded_workspace_before_the_purge_takes_its_clone() { + // What the plan's last sentence advises, checked rather than assumed. It is + // true because `resolve_clone_path` prefers the record's absolute `local_path` + // over the path derived from the cache root, and every unit test around that + // function uses a path *under* the clone root -- so a later "only remove trees + // under the cache" hardening would turn a printed sentence into bad advice + // with nothing failing. Found in review of devlaunch#461. + let world = World::with(&["--stranded-clone"]); + let run = world.dl(&["devlaunch-main-3j1t", "rm"]); + assert!( + !world.exists("old-repos/blooop/devlaunch/devlaunch-main-3j1t"), + "the clone stayed: exit {:?}\nout:{}\nerr:{}", + run.code, + run.out, + run.err + ); +} + #[test] fn a_purge_answered_no_removes_nothing_and_still_names_the_disk_it_does_not_free() { let world = World::with(&["--prunable"]); @@ -1801,7 +1945,7 @@ fn a_purge_deletes_the_workspaces_devlaunch_made_and_its_cache() { assert_eq!( run.out, format!( - "{PURGE_PLAN}Deleting DevPod workspace: devlaunch-main-legacy\n\ + "{PURGE_PLAN_YES}Deleting DevPod workspace: devlaunch-main-legacy\n\ Deleting DevPod workspace: devlaunch-dirty-fqta\n\ Removed: {{ROOT}}/cache/devlaunch\n{DOCKER_BOUNDARY}" ) @@ -1847,7 +1991,7 @@ fn a_purge_that_could_not_remove_everything_says_which_paths_refused() { assert_eq!( run.out, format!( - "{PURGE_PLAN}Deleting DevPod workspace: devlaunch-main-legacy\n\ + "{PURGE_PLAN_YES}Deleting DevPod workspace: devlaunch-main-legacy\n\ Deleting DevPod workspace: devlaunch-dirty-fqta\n\ Removed what was permitted under {{ROOT}}/cache/devlaunch. These refused:\n \ - {{ROOT}}/cache/devlaunch/repos/blooop/devlaunch/devlaunch-gone-locked/held: \ @@ -1874,7 +2018,7 @@ fn a_purge_that_removed_not_one_path_says_that_rather_than_the_other_sentence() assert_eq!( run.out, format!( - "{PURGE_PLAN}Deleting DevPod workspace: devlaunch-main-legacy\n\ + "{PURGE_PLAN_YES}Deleting DevPod workspace: devlaunch-main-legacy\n\ Deleting DevPod workspace: devlaunch-dirty-fqta\n\ Removed nothing under {{ROOT}}/cache/devlaunch. These refused:\n \ - {{ROOT}}/cache/devlaunch: is a symbolic link to {{ROOT}}/elsewhere/devlaunch, \ @@ -1918,7 +2062,7 @@ fn a_purge_that_deleted_workspaces_and_found_no_cache_says_nothing_about_the_cac assert_eq!( run.out, format!( - "{PURGE_PLAN}Deleting DevPod workspace: devlaunch-main-legacy\n\ + "{PURGE_PLAN_YES}Deleting DevPod workspace: devlaunch-main-legacy\n\ Deleting DevPod workspace: devlaunch-dirty-fqta\n{DOCKER_BOUNDARY}" ) ); diff --git a/rust/dl/tests/lifecycle_scenario.py b/rust/dl/tests/lifecycle_scenario.py index e13c5e5d..6260fb25 100755 --- a/rust/dl/tests/lifecycle_scenario.py +++ b/rust/dl/tests/lifecycle_scenario.py @@ -112,6 +112,15 @@ NOT_A_CLONE_LEAF = "devlaunch-opaque-nogit" NOT_A_CLONE_WS = "devlaunch-opaque-nogit" +# --stranded-clone: a clone a pre-#467 dl placed under `worktree.repos_dir`, a key +# this build no longer reads, and the `config.toml` that still names it. The clone +# sits outside the cache, so the workspace opening it is foreign to `--purge` and +# the tree is out of `--prune`'s reach; the record naming it is inside the cache +# the purge is about to remove. +STRANDED_LEAF = "devlaunch-main-3j1t" +STRANDED_WS = "devlaunch-main-3j1t" +STRANDED_ROOT = "old-repos" + def git(cwd, *args): """Run git in *cwd*, insisting it worked.""" @@ -346,6 +355,29 @@ def build(root: pathlib.Path, shim: pathlib.Path, wanted: set) -> None: NOT_A_CLONE_WS, {"localFolder": str(opaque)}, OLDER, "Stopped" ) + if "stranded-clone" in wanted: + # devlaunch#461, reproduced from #467's review. Everything a machine that + # once set `worktree.repos_dir` still has: the clone at the path that key + # named, a devpod workspace sourcing it, the record in dl's cache that + # points at it, and the key itself, still in `config.toml`. + stranded = _clone( + root, + origin, + root / STRANDED_ROOT / "blooop" / "devlaunch" / STRANDED_LEAF, + "main", + ) + worktrees["blooop/devlaunch/stranded"] = _record( + "blooop", "devlaunch", "stranded", stranded, STRANDED_WS + ) + workspaces[STRANDED_WS] = _workspace( + STRANDED_WS, {"localFolder": str(stranded)}, OLDER, "Stopped" + ) + config = root / "config" / "devlaunch" + config.mkdir(parents=True, exist_ok=True) + (config / "config.toml").write_text( + f'[worktree]\nrepos_dir = "{root / STRANDED_ROOT}"\n', encoding="utf-8" + ) + if "unplaceable" in wanted: workspaces[UNPLACEABLE_WS] = _workspace( UNPLACEABLE_WS, {"localFolder": 42}, OLDER, "Stopped" @@ -501,6 +533,7 @@ def build(root: pathlib.Path, shim: pathlib.Path, wanted: set) -> None: "usage: lifecycle_scenario.py [--prunable] " "[--stale-record] [--orphan] [--unplaceable] [--unwritable] " "[--no-cache] [--no-workspaces] [--not-a-clone] [--unpushed] " + "[--stranded-clone] " "[--tagged-release] " "[--sealed-cache] [--symlinked-cache] [--v1-cache] " "[--devcontainer-volumes]" @@ -515,6 +548,7 @@ def build(root: pathlib.Path, shim: pathlib.Path, wanted: set) -> None: "no-cache", "no-workspaces", "not-a-clone", + "stranded-clone", "unpushed", "tagged-release", "sealed-cache", diff --git a/test/e2e/test_full_workflow.py b/test/e2e/test_full_workflow.py index 1e21de16..1e66ffd5 100644 --- a/test/e2e/test_full_workflow.py +++ b/test/e2e/test_full_workflow.py @@ -331,9 +331,19 @@ def test_purge_deletes_devlaunchs_workspaces_and_leaves_everyone_elses( assert [line for line in printed if line.startswith("Deleting DevPod workspace:")] == [ f"Deleting DevPod workspace: {mine}" ], report - # Named rather than silently passed over. + # Named rather than silently passed over, and named by its *source*: + # an id on its own cannot be told from a `dl ./project` of yours + # (devlaunch#461). The source is asserted as "there is one" rather than + # against a literal, because what devpod echoes back for a git source is + # devpod's normalisation of the URL and not this test's business. assert "Leaving 1 workspace(s) devlaunch did not create:" in printed, report - assert f" - {theirs}" in printed, report + left = [line for line in printed if line.startswith(f" - {theirs}: ")] + assert len(left) == 1, report + assert left[0].split(": ", 1)[1].strip(), report + # And a source, not the fallback for one dl could not read: that arm is + # what a broken reading degrades to, and "something after the colon" + # accepts it just as happily as a real URL. + assert "a source dl cannot read" not in left[0], report listed_after = workspace_ids() assert mine not in listed_after, report