fix(enter,ephemeral): support -e/--exec marker for custom command

This commit is contained in:
Alessio Biancalana authored and Luca Di Maio committed 2026-06-25 09:07:20 +02:00
1 parent ab1b3ac660
commit 3d59d11ab5
6 files changed
+457 -7

No files matched your search

+35 -5
View File
@@ -24,6 +24,13 @@ func newEnterCommand(cfg *config.Values) *cli.Command {
Aliases: []string{"n"},
Usage: "name for the distrobox",
},
&cli.BoolFlag{
Name: "exec",
Aliases: []string{"e"},
Usage: "end arguments: execute the rest as command to execute at login\n" +
"(equivalent to the bare `--` separator; the custom command is\n" +
"always taken from the positional args after the container name)",
},
&cli.BoolFlag{
Name: "dry-run",
Aliases: []string{"d"},
@@ -31,7 +38,6 @@ func newEnterCommand(cfg *config.Values) *cli.Command {
},
&cli.BoolFlag{
Name: "clean-path",
Aliases: []string{"c"},
Usage: "only print the container manager command generated",
},
&cli.StringFlag{
@@ -55,7 +61,8 @@ func newEnterCommand(cfg *config.Values) *cli.Command {
Usage: "always start the container from container's home directory",
},
},
UseShortOptionHandling: true,
UseShortOptionHandling: false,
StopOnNthArg: ptr(1),
SkipFlagParsing: false,
Action: func(ctx context.Context, cmd *cli.Command) error {
return enterAction(ctx, cmd, cfg)
@@ -71,12 +78,35 @@ func enterAction(ctx context.Context, cmd *cli.Command, cfg *config.Values) erro
// Container name: --name flag takes priority, otherwise first positional arg.
// Everything after the container name (or after --) is the custom command.
//
// The CLI is configured with StopOnNthArg: 1, so urfave/cli stops flag
// parsing as soon as the first positional arg is seen. The trailing
// positional args (which include the custom command and any -e/--exec
// marker that came after the container name) are returned verbatim.
containerName := cmd.String("name")
args := cmd.Args().Slice()
if containerName == "" && len(args) > 0 {
// If the user placed -e/--exec AFTER the container name, it lands in
// the positional tail. In that case the first positional arg is still
// the container name and the custom command starts right after the
// marker. When the marker is consumed as a flag (i.e. it appeared
// before the container name) the tail is just the custom command.
markerIndex := findExecMarkerIndex(args)
var customCommand []string
switch {
case markerIndex >= 0:
// -e/--exec was placed after the container name.
if containerName == "" {
containerName = args[0]
}
customCommand = args[markerIndex+1:]
case containerName == "" && len(args) > 0:
containerName = args[0]
args = args[1:]
customCommand = args[1:]
default:
customCommand = args
}
if containerName == "" {
@@ -86,7 +116,7 @@ func enterAction(ctx context.Context, cmd *cli.Command, cfg *config.Values) erro
options := commands.EnterOptions{
ContainerName: containerName,
AdditionalFlags: cmd.String("additional-flags"),
CustomCommand: args,
CustomCommand: customCommand,
DryRun: cmd.Bool("dry-run"),
NoTTY: cmd.Bool("no-tty"),
CleanPath: cmd.Bool("clean-path"),
+223
View File
@@ -0,0 +1,223 @@
package cli
import (
"context"
"strings"
"sync"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v3"
"github.com/89luca89/distrobox/pkg/config"
"github.com/89luca89/distrobox/pkg/containermanager"
"github.com/89luca89/distrobox/pkg/ui"
)
// spyContainerManager records every Enter call so the test can assert on
// the EnterOptions the enter subcommand built from its argv. All other
// methods are no-ops — enterAction only ever calls Enter.
type spyContainerManager struct {
mu sync.Mutex
calls []containermanager.EnterOptions
failOn bool
}
func (s *spyContainerManager) Enter(_ context.Context, opts containermanager.EnterOptions, _ *ui.Progress, _ *ui.Printer) error {
s.mu.Lock()
defer s.mu.Unlock()
s.calls = append(s.calls, opts)
return nil
}
// The remaining methods are stubs to satisfy containermanager.ContainerManager.
// enterAction never reaches them.
func (s *spyContainerManager) Name() string { return "spy" }
func (s *spyContainerManager) CloneAsRoot() containermanager.ContainerManager { return s }
func (s *spyContainerManager) Create(_ context.Context, _ containermanager.CreateOptions) error { return nil }
func (s *spyContainerManager) ListContainers(_ context.Context) ([]containermanager.Container, error) {
return nil, nil
}
func (s *spyContainerManager) Remove(_ context.Context, _ string, _ containermanager.RmOptions) error {
return nil
}
func (s *spyContainerManager) Exists(_ context.Context, _ string) bool { return false }
func (s *spyContainerManager) Stop(_ context.Context, _ []string) error { return nil }
func (s *spyContainerManager) InspectContainer(_ context.Context, _ string) (*containermanager.InspectResult, error) {
return &containermanager.InspectResult{}, nil
}
func (s *spyContainerManager) Commit(_ context.Context, _, _ string) error { return nil }
func (s *spyContainerManager) ImageExists(_ context.Context, _ string) bool {
return false
}
func (s *spyContainerManager) PullImage(_ context.Context, _, _ string, _ bool) error { return nil }
// runEnter runs the enter subcommand with the given argv (starting from
// "enter") against a spy container manager and returns the EnterOptions
// the spy was invoked with.
func runEnter(t *testing.T, argv ...string) containermanager.EnterOptions {
t.Helper()
spy := &spyContainerManager{}
cmd := newEnterCommand(config.DefaultValues())
// The production Before hook (installed by withContainerManager in
// root.go) tries to detect a real container manager and validate
// sudo. We don't need any of that for parsing tests — we just want
// to plant the spy where enterAction looks for it.
cmd.Before = func(ctx context.Context, _ *cli.Command) (context.Context, error) {
return context.WithValue(ctx, containerManagerKey, spy), nil
}
root := &cli.Command{Commands: []*cli.Command{cmd}}
full := append([]string{"distrobox"}, argv...)
require.NoError(t, root.Run(context.Background(), full))
require.NotEmpty(t, spy.calls, "expected Enter to be called for argv %v", argv)
return spy.calls[0]
}
// TestEnterCommand_CustomCommandVariants locks down the three equivalent
// ways of supplying a custom command to `distrobox enter` — `-e`,
// `--exec`, and the bare `--` separator — as well as the implicit form
// where positional args after the container name are taken as the
// custom command. All four must produce the same (container,
// customCommand) pair, matching the original bash distrobox-enter.
func TestEnterCommand_CustomCommandVariants(t *testing.T) {
cases := []struct {
name string
argv []string
}{
{
name: "short -e flag",
argv: []string{"enter", "suse", "-e", "echo", "ciao"},
},
{
name: "long --exec flag",
argv: []string{"enter", "suse", "--exec", "echo", "ciao"},
},
{
name: "bare -- separator",
argv: []string{"enter", "suse", "--", "echo", "ciao"},
},
{
name: "implicit (no -e/--exec/--)",
argv: []string{"enter", "suse", "echo", "ciao"},
},
{
name: "explicit --name with -e",
argv: []string{"enter", "--name", "suse", "-e", "echo", "ciao"},
},
{
name: "explicit --name with --",
argv: []string{"enter", "--name", "suse", "--", "echo", "ciao"},
},
{
// Regression: the custom command itself contains a short
// flag (e.g. `bash -c echo`). The CLI must not eat it as
// a distrobox flag — matching the original bash script.
name: "custom command with short flag after -e",
argv: []string{"enter", "suse", "-e", "bash", "-c", "echo"},
},
{
name: "custom command with short flag after --",
argv: []string{"enter", "suse", "--", "bash", "-c", "echo"},
},
{
name: "-e before the container name",
argv: []string{"enter", "-e", "suse", "bash", "-c", "echo"},
},
{
name: "short flag combo with --no-tty before container name",
argv: []string{"enter", "--no-tty", "suse", "-e", "bash", "-c", "echo"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
opts := runEnter(t, tc.argv...)
assert.Equal(t, "suse", opts.ContainerName)
want := []string{"echo", "ciao"}
if strings.HasPrefix(tc.name, "custom command with short flag") ||
strings.HasPrefix(tc.name, "-e before the container name") ||
strings.HasPrefix(tc.name, "short flag combo with --no-tty") {
want = []string{"bash", "-c", "echo"}
}
assert.Equal(t, want, opts.CustomCommand)
})
}
}
func TestFindExecMarkerIndex(t *testing.T) {
cases := []struct {
name string
args []string
want int
}{
{
name: "empty args",
args: nil,
want: -1,
},
{
name: "no marker",
args: []string{"suse", "echo", "ciao"},
want: -1,
},
{
name: "short -e is the only arg",
args: []string{"-e"},
want: 0,
},
{
name: "long --exec is the only arg",
args: []string{"--exec"},
want: 0,
},
{
name: "-e at the start",
args: []string{"-e", "bash", "-c", "echo"},
want: 0,
},
{
name: "--exec in the middle",
args: []string{"suse", "--exec", "bash", "-c", "echo"},
want: 1,
},
{
name: "-e at the end with no command after it",
args: []string{"suse", "-e"},
want: 1,
},
{
name: "first match wins when both forms are present",
args: []string{"-e", "suse", "--exec", "echo"},
want: 0,
},
{
name: "marker-looking arg inside the custom command is not a match",
// `bash` happens to start with `b`, not `-`, so it must
// never be picked up. This guards against a future
// regression that would substring-match.
args: []string{"suse", "bash", "-e", "echo"},
want: 2,
},
{
name: "looks similar but is not the marker",
// `--exec-foo` shares a prefix with `--exec` but is a
// distinct token; it must not be treated as the marker.
args: []string{"suse", "--exec-foo", "echo"},
want: -1,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.want, findExecMarkerIndex(tc.args))
})
}
}
+44 -2
View File
@@ -23,13 +23,27 @@ func newEphemeralCommand(cfg *config.Values) *cli.Command {
"compatibility",
"no-entry",
}
flags := make([]cli.Flag, 0, len(createCmd.Flags))
flags := make([]cli.Flag, 0, len(createCmd.Flags)+1)
for _, f := range createCmd.Flags {
if slices.Contains(ignoredFlags, f.Names()[0]) {
continue
}
flags = append(flags, f)
}
// -e/--exec is the marker that tells distrobox to treat everything
// after it as the custom command to run inside the ephemeral
// container. It is a no-op marker from urfave/cli's point of view
// because the custom command is always taken from the positional
// args — we just need the alias to exist so the parser doesn't
// reject the flag. The original distrobox-ephemeral bash script
// accepts -e, --exec, and `--` interchangeably.
flags = append(flags, &cli.BoolFlag{
Name: "exec",
Aliases: []string{"e"},
Usage: "end arguments: execute the rest as command to execute at login\n" +
"(equivalent to the bare `--` separator; the custom command is\n" +
"always taken from the positional args)",
})
return &cli.Command{
Name: "ephemeral",
@@ -42,6 +56,17 @@ Examples:
distrobox ephemeral --root --image fedora:39
distrobox ephemeral -- bash -c "echo hello"`,
Flags: flags,
// StopOnNthArg: 1 mirrors the semantics of the original bash
// distrobox-ephemeral: every flag must appear before the first
// positional arg, and everything from the first positional arg
// onward is treated as the custom command. Without this, short
// flags inherited from `distrobox-create` (e.g. -c for --clone)
// would be eaten out of the custom command. The bare `--`
// separator still works because urfave/cli checks for it before
// honouring StopOnNthArg.
UseShortOptionHandling: false,
StopOnNthArg: ptr(1),
SkipFlagParsing: false,
Action: func(ctx context.Context, cmd *cli.Command) error {
return ephemeralAction(ctx, cmd, cfg)
},
@@ -53,6 +78,23 @@ func ephemeralAction(ctx context.Context, cmd *cli.Command, cfg *config.Values)
if !ok {
return errors.New("container manager not found in context")
}
// The CLI is configured with StopOnNthArg: 1, so urfave/cli stops
// flag parsing as soon as it sees the first positional arg. From
// that point on, anything — including the -e/--exec marker, if the
// user placed it after a positional arg — is captured verbatim into
// the positional tail. We use findExecMarkerIndex to split the
// custom command out of the tail.
args := cmd.Args().Slice()
markerIndex := findExecMarkerIndex(args)
var customCommand []string
if markerIndex >= 0 {
customCommand = args[markerIndex+1:]
} else {
customCommand = args
}
opts := commands.EphemeralOptions{
CreateOptions: commands.CreateOptions{
ContainerImage: cmd.String("image"),
@@ -77,7 +119,7 @@ func ephemeralAction(ctx context.Context, cmd *cli.Command, cfg *config.Values)
GenerateEntry: false,
Rootful: cmd.Bool("root"),
},
CustomCommand: cmd.Args().Slice(),
CustomCommand: customCommand,
DryRun: cmd.Bool("dry-run"),
}
+130
View File
@@ -0,0 +1,130 @@
package cli
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v3"
"github.com/89luca89/distrobox/pkg/config"
"github.com/89luca89/distrobox/pkg/containermanager"
)
// runEphemeral runs the ephemeral subcommand with the given argv against a
// spy container manager and returns the EnterOptions that the enter step
// was finally invoked with. Ephemeral always calls Enter exactly once (the
// create + enter are paired), so this is enough to assert the custom
// command shape.
func runEphemeral(t *testing.T, argv ...string) containermanager.EnterOptions {
t.Helper()
spy := &spyContainerManager{}
cmd := newEphemeralCommand(config.DefaultValues())
// Replace the production Before hook (which would auto-detect a
// real container manager) with a stub that just installs the spy
// onto the context.
cmd.Before = func(ctx context.Context, _ *cli.Command) (context.Context, error) {
return context.WithValue(ctx, containerManagerKey, spy), nil
}
root := &cli.Command{Commands: []*cli.Command{cmd}}
full := append([]string{"distrobox"}, argv...)
require.NoError(t, root.Run(context.Background(), full))
require.NotEmpty(t, spy.calls, "expected Enter to be called for argv %v", argv)
return spy.calls[0]
}
// TestEphemeralCommand_CustomCommandVariants locks down the three
// equivalent ways of supplying a custom command to `distrobox ephemeral`:
// `-e`, `--exec`, and the bare `--` separator. All three must produce
// the same CustomCommand slice on the EnterOptions that ephemeral passes
// to the enter step, matching the original bash distrobox-ephemeral.
func TestEphemeralCommand_CustomCommandVariants(t *testing.T) {
cases := []struct {
name string
argv []string
want []string
}{
{
name: "short -e flag",
argv: []string{"ephemeral", "--image", "alpine", "-e", "echo", "ciao"},
want: []string{"echo", "ciao"},
},
{
name: "long --exec flag",
argv: []string{"ephemeral", "--image", "alpine", "--exec", "echo", "ciao"},
want: []string{"echo", "ciao"},
},
{
name: "bare -- separator",
argv: []string{"ephemeral", "--image", "alpine", "--", "echo", "ciao"},
want: []string{"echo", "ciao"},
},
{
name: "implicit (no -e/--exec/--)",
argv: []string{"ephemeral", "--image", "alpine", "echo", "ciao"},
want: []string{"echo", "ciao"},
},
{
name: "explicit --name with -e",
argv: []string{"ephemeral", "--name", "foo", "--image", "alpine", "-e", "echo", "ciao"},
want: []string{"echo", "ciao"},
},
{
// Regression: the custom command itself contains a short
// flag (e.g. `bash -c echo`). The CLI must not eat `-c` as
// the inherited `--clone` flag from `distrobox-create`.
name: "custom command with short flag after -e",
argv: []string{"ephemeral", "--image", "alpine", "-e", "bash", "-c", "echo"},
want: []string{"bash", "-c", "echo"},
},
{
name: "custom command with short flag after --",
argv: []string{"ephemeral", "--image", "alpine", "--", "bash", "-c", "echo"},
want: []string{"bash", "-c", "echo"},
},
{
name: "-e before the image flag",
argv: []string{"ephemeral", "-e", "echo", "ciao"},
want: []string{"echo", "ciao"},
},
{
name: "short alias for image with -- separator",
argv: []string{"ephemeral", "-i", "alpine", "--", "bash", "-c", "echo"},
want: []string{"bash", "-c", "echo"},
},
{
name: "no custom command (only flags)",
argv: []string{"ephemeral", "--image", "alpine"},
want: nil,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
opts := runEphemeral(t, tc.argv...)
if tc.want == nil {
assert.Empty(t, opts.CustomCommand)
} else {
assert.Equal(t, tc.want, opts.CustomCommand)
}
})
}
}
// TestEphemeralCommand_ContainerNameFromFlag ensures that an explicit
// --name is propagated through the create+enter pair. This is the
// ephemeral-specific path (the normal flow auto-generates a name), so
// it's worth pinning down separately.
func TestEphemeralCommand_ContainerNameFromFlag(t *testing.T) {
opts := runEphemeral(t, "ephemeral", "--image", "alpine", "--name", "my-ephemeral", "-e", "echo", "ciao")
assert.Equal(t, "my-ephemeral", opts.ContainerName)
assert.Equal(t, []string{"echo", "ciao"}, opts.CustomCommand)
}
+17
View File
@@ -0,0 +1,17 @@
package cli
// findExecMarkerIndex returns the index of the first -e or --exec in
// args, or -1 if neither is present. It is used by the enter and
// ephemeral commands to recover the marker when the user placed it
// after the container name (or, for ephemeral, after the first
// positional arg) — in those cases urfave/cli leaves the marker in
// the positional tail instead of consuming it as a flag, and we need
// to know where the custom command starts.
func findExecMarkerIndex(args []string) int {
for i, arg := range args {
if arg == "-e" || arg == "--exec" {
return i
}
}
return -1
}
+8
View File
@@ -0,0 +1,8 @@
package cli
// ptr returns a pointer to v. Go has no built-in syntax to take the
// address of a literal, so we need this tiny helper to populate pointer
// fields like urfave/cli's StopOnNthArg (a *int) — the zero value of a
// pointer is nil, and nil keeps the default behaviour, so the call site
// has to be explicit about wanting a non-nil pointer.
func ptr[T any](v T) *T { return &v }