feat: clean up distrobox exports/entries even if container is already deleted#2062
feat: clean up distrobox exports/entries even if container is already deleted#2062iTrooz wants to merge 6 commits into89luca89:nextfrom
Conversation
|
I also added a failsafe on cleanup() because I accidentally triggered it with an empty container name during tests, and ended up deleting a lot of other files on my computer. Tell me if you'd prefer me to remove it :) |
db0ba66 to
a48ad88
Compare
There was a problem hiding this comment.
Pull request overview
Adds behavior to distrobox rm to also clean up exported binaries/desktop entries for explicitly requested containers even when the container no longer exists, aligning with the “Remove exported apps” workflow discussed in #2059.
Changes:
- Track explicitly requested container names that were not found/removed and run cleanup for them.
- Add a small slice helper to remove handled names from the explicit list.
- Add a guard in
cleanup()for empty container names.
Comments suppressed due to low confidence (1)
pkg/commands/rm.go:88
- This fmt.Printf call lacks a trailing newline, which can make output from multiple deletions run together and be harder to read. Consider adding "\n" (and/or routing through the logger once available).
//nolint:forbidigo // waiting for the logger implementation
fmt.Printf("error deleting %s: %s", currentDistrobox.Name, err)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clean up exported files of all remaining explicitely requested distroboxes, | ||
| // even if the container doesn't exist anymore | ||
| for _, containerName := range explicitelyRequested { | ||
| c.cleanup(ctx, userHome, containerName) | ||
| } |
There was a problem hiding this comment.
New behavior to run cleanup for explicitly requested but non-existent containers isn’t covered by tests. Consider adding a unit test to ensure it deletes only the expected exported artifacts and does not act on unsafe container names (e.g., empty, path traversal, glob chars).
See #2059