IntenseWebs/distrobox
3
0
Fork 0
mirror of https://github.com/89luca89/distrobox.git synced 2026-08-19 01:14:49 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix: various bug fixes and code improvements (#2047)

Browse Source 
* fix(containermanager): fix typos in ContanerID and ContanerManagerType

* fix(rm): pass ContainerHome to RmOptions when removing container

* fix(containermanager): handle empty names slice in podman container list

* fix(create): initialize prompter in NewCreateCommand to prevent nil dereference

* fix(userenv): trim trailing newline from uid and gid command output

* refactor(containermanager): move shared types and constants to providers common file

* fix(containermanager): skip /dev/shm volume mount if EvalSymlinks fails

* fix(containermanager): rename ContainerManagerType to ManagerType to avoid stutter

* restore: ManagerType -> ContainerManagerType

* test(containermanager): add empty names fallback test for parsePodmanContainerList

* fix(containermanager): suppress revive stutter lint for ContainerManagerType

* fix(containermanager): use empty string fallback when podman container has no name

* fix(commands): pass real prompter to create command in assemble and ephemeral

* Revert "refactor(containermanager): move shared types and constants to providers common file"

This reverts commit 59faeb71f8.
This commit is contained in:
Mirko Brombin
2026-06-14 13:04:28 +02:00
committed by Alessio Biancalana
parent 9d4955a23e
commit c31f0e9c71
11 changed files with 83 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/cli/create.go
+3 -1
View File
@@ -1,6 +1,7 @@
package cli
import (
"bufio"
"context"
"errors"
"fmt"
@@ -213,8 +214,9 @@ func createAction(ctx context.Context, cmd *cli.Command) error {
}
progress := ui.NewProgress(os.Stderr)
prompter := ui.NewPrompter(*bufio.NewReader(os.Stdin), os.Stdout)
createCmd := commands.NewCreateCommand(containerManager, progress)
createCmd := commands.NewCreateCommand(containerManager, progress, prompter)
err := createCmd.Execute(ctx, opts)
var containerAlreadyExistsErr *commands.ContainerAlreadyExistsError
internal/cli/ephemeral.go
+3 -1
View File
@@ -1,6 +1,7 @@
package cli
import (
"bufio"
"context"
"errors"
"fmt"
@@ -78,8 +79,9 @@ func ephemeralAction(ctx context.Context, cmd *cli.Command) error {
progress := ui.NewProgress(os.Stderr)
printer := ui.NewPrinter(os.Stderr, true)
prompter := ui.NewPrompter(*bufio.NewReader(os.Stdin), os.Stdout)
ephemeralCmd := commands.NewEphemeralCommand(containerManager, progress, printer)
ephemeralCmd := commands.NewEphemeralCommand(containerManager, progress, printer, prompter)
err := ephemeralCmd.Execute(ctx, opts)
if err != nil {
internal/userenv/user_environment.go
+2 -2
View File
@@ -71,14 +71,14 @@ func LoadUserEnvironment(ctx context.Context) *UserEnvironment {
if uid := os.Getuid(); uid >= 0 {
env.UserID = strconv.Itoa(uid)
} else if uid, err := exec.CommandContext(ctx, "id", "-ru").Output(); err == nil {
env.UserID = string(uid)
env.UserID = strings.TrimSpace(string(uid))
}
// GROUP ID
if gid := os.Getgid(); gid >= 0 {
env.GroupID = strconv.Itoa(gid)
} else if gid, err := exec.CommandContext(ctx, "id", "-rg").Output(); err == nil {
env.GroupID = string(gid)
env.GroupID = strings.TrimSpace(string(gid))
}
return env
pkg/commands/assemble.go
+1 -1
View File
@@ -42,7 +42,7 @@ func NewAssembleCommand(
) *AssembleCommand {
return &AssembleCommand{
containermanager: cm,
createCmd: NewCreateCommand(cm, ui.NewDevNullProgress()),
createCmd: NewCreateCommand(cm, ui.NewDevNullProgress(), prompter),
rmCmd: NewRmCommand(cm, prompter),
enterCmd: NewEnterCommand(cm, progress, printer),
progress: progress,
pkg/commands/create.go
+4 -3
View File
@@ -84,11 +84,12 @@ type CreateOptions struct {
NonInteractive bool
}
func NewCreateCommand(cm containermanager.ContainerManager, progress *ui.Progress) *CreateCommand {
func NewCreateCommand(cm containermanager.ContainerManager, progress *ui.Progress, prompter *ui.Prompter) *CreateCommand {
return &CreateCommand{
containerManager: cm,
generateEntryCmd: NewGenerateEntryCommand(NewListCommand(cm)),
progress: progress,
prompter: prompter,
}
}
@@ -267,9 +268,9 @@ func (c *CreateCommand) clone(ctx context.Context, containerName string) (string
commitTag := fmt.Sprintf("%s:%s", strings.ToLower(containerName), time.Now().Format("2006-01-02"))
err = c.containerManager.Commit(ctx, i.ContanerID, commitTag)
err = c.containerManager.Commit(ctx, i.ContainerID, commitTag)
if err != nil {
return "", fmt.Errorf("failed to commit container '%s:%s': %w", i.ContanerID, commitTag, err)
return "", fmt.Errorf("failed to commit container '%s:%s': %w", i.ContainerID, commitTag, err)
}
return commitTag, nil
pkg/commands/ephemeral.go
+3 -2
View File
@@ -26,12 +26,13 @@ func NewEphemeralCommand(
cm containermanager.ContainerManager,
progress *ui.Progress,
printer *ui.Printer,
prompter *ui.Prompter,
) *EphemeralCommand {
return &EphemeralCommand{
containerManager: cm,
createCmd: NewCreateCommand(cm, progress),
createCmd: NewCreateCommand(cm, progress, prompter),
enterCmd: NewEnterCommand(cm, progress, printer),
rmCmd: NewRmCommand(cm, nil),
rmCmd: NewRmCommand(cm, prompter),
}
}
pkg/commands/rm.go
+3 -2
View File
@@ -108,8 +108,9 @@ func (c *RmCommand) removeContainer(
}
cmOptions := containermanager.RmOptions{
Force: forceRemove,
RemoveHome: removeHome,
Force: forceRemove,
RemoveHome: removeHome,
ContainerHome: inspectOutput.ContainerHome,
}
err = c.containerManager.Remove(ctx, container.Name, cmOptions)
if err != nil {
pkg/containermanager/containermanager.go
+3 -2
View File
@@ -16,7 +16,7 @@ type Container struct {
}
type InspectResult struct {
ContanerID string
ContainerID string
ContainerStatus string
ContainerHome string
ContainerPath string
@@ -73,7 +73,8 @@ func (c Container) IsRunning() bool {
return strings.Contains(s, "up") || strings.Contains(s, "running")
}
type ContanerManagerType string
//nolint:revive // ContainerManagerType is intentionally named for clarity despite the stutter
type ContainerManagerType string
type ContainerManager interface {
Name() string
pkg/containermanager/providers/docker.go
+5 -3
View File
@@ -305,8 +305,10 @@ func (d *Docker) makeCreateCommand(
// Resolve this detecting if /dev/shm is a symlink and mount original
// source also in the container.
if isSymlink("/dev/shm") && !unshareIPC {
realPath, _ := filepath.EvalSymlinks("/dev/shm")
options = append(options, "--volume", fmt.Sprintf("%s:%s", realPath, realPath))
realPath, err := filepath.EvalSymlinks("/dev/shm")
if err == nil {
options = append(options, "--volume", fmt.Sprintf("%s:%s", realPath, realPath))
}
}
// Ensure support forwarding of RedHat subscription-manager
@@ -675,7 +677,7 @@ func (d *Docker) InspectContainer(ctx context.Context, containerName string) (*c
}
inspect := inspects[0]
config.ContanerID = inspect.ID
config.ContainerID = inspect.ID
config.ContainerStatus = inspect.State.Status
// Check for unshare_groups label
pkg/containermanager/providers/podman.go
+10 -3
View File
@@ -266,8 +266,10 @@ func (p *Podman) makeCreateCommand(
// Resolve this detecting if /dev/shm is a symlink and mount original
// source also in the container.
if isSymlink("/dev/shm") && !unshareIPC {
realPath, _ := filepath.EvalSymlinks("/dev/shm")
options = append(options, "--volume", fmt.Sprintf("%s:%s", realPath, realPath))
realPath, err := filepath.EvalSymlinks("/dev/shm")
if err == nil {
options = append(options, "--volume", fmt.Sprintf("%s:%s", realPath, realPath))
}
}
// Ensure support forwarding of RedHat subscription-manager
@@ -615,10 +617,15 @@ func parsePodmanContainerList(output string) ([]containermanager.Container, erro
id = id[:containerIDMaxLength]
}
name := ""
if len(c.Names) > 0 {
name = c.Names[0]
}
containers = append(containers, containermanager.Container{
ID: id,
Image: c.Image,
Name: c.Names[0],
Name: name,
Status: c.Status,
Labels: c.Labels,
})
pkg/containermanager/providers/podman_internal_test.go
+46
View File
@@ -663,6 +663,52 @@ func TestParsePodmanContainerListEmpty(t *testing.T) {
}
}
func TestParsePodmanContainerListEmptyNames(t *testing.T) {
tests := []struct {
name string
json string
wantName string
wantID string
}{
{
name: "empty names array falls back to empty string",
json: `[{"ID":"abc123def456789012345678","Image":"fedora:39","Names":[],"Status":"running","Labels":{}}]`,
wantName: "",
wantID: "abc123def456",
},
{
name: "null names falls back to empty string",
json: `[{"ID":"xyz789abc123456789012345","Image":"ubuntu:22.04","Names":null,"Status":"exited","Labels":{}}]`,
wantName: "",
wantID: "xyz789abc123",
},
{
name: "short ID (under 12 chars) is used as-is",
json: `[{"ID":"shortid","Image":"alpine:latest","Names":[],"Status":"running","Labels":{}}]`,
wantName: "",
wantID: "shortid",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
containers, err := parsePodmanContainerList(tt.json)
if err != nil {
t.Fatalf("parsePodmanContainerList returned error: %v", err)
}
if len(containers) != 1 {
t.Fatalf("Expected 1 container, got %d", len(containers))
}
if containers[0].Name != tt.wantName {
t.Errorf("Expected Name %q, got %q", tt.wantName, containers[0].Name)
}
if containers[0].ID != tt.wantID {
t.Errorf("Expected ID %q, got %q", tt.wantID, containers[0].ID)
}
})
}
}
func TestCommandExists(t *testing.T) {
// Test with a command that should exist on all systems
if !commandExists("sh") {