From b6ed98f86ed0749ea076ff2a412c70b440b228f3 Mon Sep 17 00:00:00 2001 From: Emanuele De Cupis Date: Thu, 21 May 2026 13:56:37 +0200 Subject: [PATCH] fix(assemble): accept manifest file as positional argument (#2105) `distrobox assemble create /path/to/file.ini` and `distrobox assemble rm /path/to/file.ini` now work the same way as the original Bash `distrobox-assemble` script, in addition to the existing `--file` flag. When both are supplied, `--file` takes precedence (explicit over implicit). When neither is given, the default `./distrobox.ini` is still used. The resolution logic is extracted into a `resolveManifestPath` helper with unit tests covering flag precedence, positional fallback and the default path. --- internal/cli/assemble.go | 40 ++++++++++++---- internal/cli/assemble_internal_test.go | 66 ++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 internal/cli/assemble_internal_test.go diff --git a/internal/cli/assemble.go b/internal/cli/assemble.go index 1cde9940..eb41fda9 100644 --- a/internal/cli/assemble.go +++ b/internal/cli/assemble.go @@ -36,12 +36,15 @@ func newAssembleCommand(cfg *config.Values) *cli.Command { distrobox assemble create distrobox assemble rm distrobox assemble create --file /path/to/file.ini + distrobox assemble create /path/to/file.ini distrobox assemble rm --file /path/to/file.ini + distrobox assemble rm /path/to/file.ini distrobox assemble create --replace --file /path/to/file.ini Options: --file: path or URL to the distrobox manifest/ini file + (may also be supplied as a positional argument; --file takes precedence) --name/-n: run against a single entry in the manifest/ini file --replace/-R: replace already existing distroboxes with matching names --dry-run/-d: only print the container manager command generated @@ -50,7 +53,8 @@ Options: `, Commands: []*cli.Command{ { - Name: "create", + Name: "create", + ArgsUsage: "[manifest-file]", Flags: []cli.Flag{ fileFlag, nameFlag, @@ -66,7 +70,8 @@ Options: }, }, { - Name: "rm", + Name: "rm", + ArgsUsage: "[manifest-file]", Flags: []cli.Flag{ fileFlag, nameFlag, @@ -80,19 +85,36 @@ Options: } } +// defaultManifestPath is the path used when neither --file nor a positional +// argument is provided to `distrobox assemble`. +const defaultManifestPath = "./distrobox.ini" + +// resolveManifestPath returns the manifest file path to use for an assemble +// invocation. Precedence is: +// 1. the value of the --file flag, if non-empty; +// 2. the first positional argument, if any; +// 3. the default manifest path ("./distrobox.ini"). +// +// This mirrors the behavior of the original Bash `distrobox-assemble` script +// while keeping the explicit `--file` flag dominant over the implicit +// positional argument. +func resolveManifestPath(flagValue string, positional []string) string { + if flagValue != "" { + return flagValue + } + if len(positional) > 0 && positional[0] != "" { + return positional[0] + } + return defaultManifestPath +} + func assembleAction(ctx context.Context, cmd *cli.Command, cfg *config.Values, deleteFlag bool) error { containerManager, ok := ctx.Value(containerManagerKey).(containermanager.ContainerManager) if !ok { return errors.New("container manager not found in context") } - // TODO: handle file name as a positional argument - // https://github.com/89luca89/distrobox-next/blob/07b3abf2015effafc5596b9dc7f02c35a17eb8a7/distrobox-assemble#L205 - - manifestFilePath := cmd.String("file") - if manifestFilePath == "" { - manifestFilePath = "./distrobox.ini" - } + manifestFilePath := resolveManifestPath(cmd.String("file"), cmd.Args().Slice()) manifest, err := manifest.Parse(ctx, manifestFilePath) if err != nil { diff --git a/internal/cli/assemble_internal_test.go b/internal/cli/assemble_internal_test.go new file mode 100644 index 00000000..12daef93 --- /dev/null +++ b/internal/cli/assemble_internal_test.go @@ -0,0 +1,66 @@ +package cli + +import "testing" + +func TestResolveManifestPath_FlagOnly(t *testing.T) { + got := resolveManifestPath("/etc/distrobox.ini", nil) + if got != "/etc/distrobox.ini" { + t.Fatalf("expected flag value, got %q", got) + } +} + +func TestResolveManifestPath_PositionalOnly(t *testing.T) { + got := resolveManifestPath("", []string{"/srv/manifest.ini"}) + if got != "/srv/manifest.ini" { + t.Fatalf("expected positional value, got %q", got) + } +} + +func TestResolveManifestPath_FlagTakesPrecedenceOverPositional(t *testing.T) { + got := resolveManifestPath("/etc/distrobox.ini", []string{"/srv/manifest.ini"}) + if got != "/etc/distrobox.ini" { + t.Fatalf("expected flag value to win, got %q", got) + } +} + +func TestResolveManifestPath_NoInputsReturnsDefault(t *testing.T) { + got := resolveManifestPath("", nil) + if got != defaultManifestPath { + t.Fatalf("expected default %q, got %q", defaultManifestPath, got) + } +} + +func TestResolveManifestPath_EmptyPositionalSliceReturnsDefault(t *testing.T) { + got := resolveManifestPath("", []string{}) + if got != defaultManifestPath { + t.Fatalf("expected default %q, got %q", defaultManifestPath, got) + } +} + +func TestResolveManifestPath_EmptyFirstPositionalReturnsDefault(t *testing.T) { + got := resolveManifestPath("", []string{""}) + if got != defaultManifestPath { + t.Fatalf("expected default %q, got %q", defaultManifestPath, got) + } +} + +func TestResolveManifestPath_FirstPositionalUsedWhenMultiple(t *testing.T) { + got := resolveManifestPath("", []string{"/first.ini", "/second.ini"}) + if got != "/first.ini" { + t.Fatalf("expected first positional, got %q", got) + } +} + +func TestResolveManifestPath_URLFlagValue(t *testing.T) { + got := resolveManifestPath("https://example.com/manifest.ini", nil) + if got != "https://example.com/manifest.ini" { + t.Fatalf("expected URL flag value, got %q", got) + } +} + +func TestResolveManifestPath_URLPositionalValue(t *testing.T) { + got := resolveManifestPath("", []string{"https://example.com/manifest.ini"}) + if got != "https://example.com/manifest.ini" { + t.Fatalf("expected URL positional value, got %q", got) + } +}