feat: Add container image tree command#2005
Conversation
|
For anything that's non-trivial, please first create an issue describing the proposed feature, and link the PR to it. |
I am sorry for any misunderstanding but I think I did it here ig. #2004 |
|
Looks like |
noted . thanks @gnavadev |
| for node in nodes { | ||
| var foundParent = false | ||
| // Find a parent: a node with maximum diffIDs that is a strict prefix of current node's diffIDs | ||
| for potentialParent in roots.flatMap({ getAllNodes(in: $0) }).sorted(by: { ($0.allDiffIDs.first?.count ?? 0) > ($1.allDiffIDs.first?.count ?? 0) }) { |
There was a problem hiding this comment.
Some questions that I have in mind for this:
1 - You are sorting the nodes based only on the first variant's DiffIDs ($0.allDiffIDs.first), but assigning parents using isPrefixOfAny, which loops through all variants. Doesn't this mismatch break the tree's ordering guarantee?
2 - You are calling flatMap and .sorted() inside the for node in nodes loop. This means you are unpacking the entire tree into a flat list and re-sorting it from scratch on every single iteration, which will severely bottleneck performance.
|
|
||
| for resource in resources { | ||
| let allDiffIDs = resource.variants.map { $0.config.rootfs.diffIDs } | ||
| let sizeStr = resource.variants.first.map { formatter.string(fromByteCount: $0.size) } ?? "0 MB" |
There was a problem hiding this comment.
This would be a good use case for reduce
| // limitations under the License. | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import ArgumentParser |
There was a problem hiding this comment.
ContainerizationError - ContainerPersistence - ContainerPlugin are all unused imports
There was a problem hiding this comment.
Indeed, these are redundancies that I inadvertently overlooked. Furthermore, I encountered an additional bottleneck during performance optimization and testing.
Should you find merit in this feature proposal, please consider actively contributing to its development.
As an active container user I felt this is a needed requirement for the project
@gnavadev
There was a problem hiding this comment.
I think this is a nice QoL improvement. However, I wonder if something like this would be better as a plugin or in a separate repository, maybe something like apple-container-tools? What do you think @jglogan ?
There was a problem hiding this comment.
First of all, thank you both for the valuable discussion.@gnavadev and @jglogan
@gnavadev, thank you for the insightful suggestions and direction. I agree that this should be developed as a separate utility. When working with more complex workflows, it becomes difficult to keep track of active containers, and having a dedicated utility would make that much more manageable. It also aligns well with Apple's approach of providing focused, user-friendly utilities.
|
@dev-kvt Thanks for the contribution! Right now we've been focused on other work so we haven't been able to review the backlog nearly as much as we'd like. Starting with the next release we're going to communicate that areas that we're going to be focusing on. PRs that address issues in the focus area will tend to get reviewed more quickly. @gnavadev Thanks for the review! We certainly could use a repo for plugins and samples but that might be a while in coming still. I could see something like this being an image subcommand as it is now, but to avoid cluttering the world with verbs we could have |
| public static let configuration = CommandConfiguration( | ||
| commandName: "tree", | ||
| abstract: "Show images in a tree view based on parent-child relationships", | ||
| aliases: ["tr"] |
There was a problem hiding this comment.
This is likely a lesser used command, and it only saves two characters.
3 participants
Important
All commits must be signed and verified. Pull requests containing unsigned or unverified commits cannot be built or merged. See the GitHub documentation for instructions.
For all but trivial fixes, make sure to first create a GitHub issue that concisely describes the bug or desired enhancement as justification for the change. Large PRs with no justifying issue will be closed.
Type of Change
Motivation and Context
This PR introduces the
container image treesubcommand. Previously, when dealing with many locally built images,container image lsproduced a flat list that became difficult to navigate and clean up.This new command dynamically calculates image dependencies by inspecting layer architecture (
rootfs.diffIDs) and displays the images in a visual parent-child hierarchy. It seamlessly matches variants across different architectures, and includes a--sizeflag to display human-readable byte counts alongside the tree.Testing