nerdctl-stub: Better support foreign flags

This fixes `nerdctl run ... -flag` to not warn on flags that are not to be
parsed by nerdctl.

Signed-off-by: Mark Yen <mark.yen@suse.com>

Author: mook-as

src/go/nerdctl-stub/generate/main_linux.go

@@ -32,8 +32,11 @@ type helpData struct {
 	// (`--version`) or the short option (`-v`), and the value is whether the
 	// option takes an argument.
 	Options map[string]bool
-	// Whether this command has positional arguments.
-	hasPositionalArguments bool
+	// If set, this command can have subcommands; this alters argument parsing.
+	canHaveSubcommands bool
+	// If set, this command can pass flags to foreign commands, as in `nerdctl run`.
+	// This alters argument parsing by letting us ignore unknown flags.
+	HasForeignFlags bool
 	// mergedOptions includes local options plus inherited options.
 	mergedOptions map[string]struct{}
 }
@@ -103,14 +106,19 @@ func buildSubcommand(args []string, parentData helpData, writer io.Writer) error
 	subcommands := parseHelp(help, parentData)
 
 	fields := logrus.Fields{"args": args}
-	if subcommands.hasPositionalArguments {
-		fields["positional"] = true
+	if subcommands.HasForeignFlags {
+		fields["type"] = "arguments"
+	} else if !subcommands.canHaveSubcommands {
+		fields["type"] = "positional"
 	}
 	logrus.WithFields(fields).Trace("building subcommand")
 
-	if subcommands.hasPositionalArguments && len(subcommands.Commands) > 0 {
+	if !subcommands.canHaveSubcommands && len(subcommands.Commands) > 0 {
 		return fmt.Errorf("Invalid command %v: has positional arguments, but also subcommands %+v", args, subcommands.Commands)
 	}
+	if subcommands.canHaveSubcommands && subcommands.HasForeignFlags {
+		return fmt.Errorf("Invalid command %v: has subcommands and foreign flags", args)
+	}
 
 	err = emitCommand(args, subcommands, writer)
 	if err != nil {
@@ -178,8 +186,10 @@ func parseHelp(help string, parentData helpData) helpData {
 				newArgs = strings.TrimSpace(newArgs)
 				// Unlike everything else, `nerdctl compose` has a usage string of
 				// `nerdctl compose [flags] COMMAND` so we need to ignore that.
-				if newArgs != "" && newArgs != "COMMAND" {
-					result.hasPositionalArguments = true
+				if newArgs == "" || newArgs == "COMMAND" {
+					result.canHaveSubcommands = true
+				} else if strings.Contains(newArgs, "COMMAND") && strings.Contains(newArgs, "...") {
+					result.HasForeignFlags = true
 				}
 			} else {
 				state = STATE_OTHER
@@ -245,6 +255,9 @@ const commandTemplate = `
 				{{- printf "%q" $k -}}: {{ if $v -}} ignoredArgHandler {{- else -}} nil {{- end -}},
 			{{ end }}
 		},
+		{{- if .Data.HasForeignFlags }}
+			hasForeignFlags: true,
+		{{- end }}
 	},
 `
 

src/go/nerdctl-stub/nerdctl_commands_generated.go

@@ -47,6 +47,10 @@ type commandDefinition struct {
 	// options for this (sub) command.  If the handler is null, the option does
 	// not take arguments.
 	options map[string]argHandler
+	// if set, this command can include foreign flags that should not be parsed.
+	// This should be set for things like `nerdctl run` where flags can be passed
+	// to the command to be run.
+	hasForeignFlags bool
 	// handler for any positional arguments and subcommands.  This should not
 	// include the name of the subcommand itself.  If this is not given, all
 	// subcommands are searched for, and positional arguments are ignored.
@@ -138,8 +142,14 @@ func (c commandDefinition) parse(args []string) (*parsedArgs, error) {
 	// - At each command level, short options (-x) at that level can be parsed.
 	// - At each command level, long options from the current or any previous level can be parsed.
 	// - If a command contains positional arguments, it may not contain any subcommands.
-	//   (We check this in ./generate to make sure this stays true.)
+	//   (We check this in `./generate` to make sure this stays true.)
 	// - Positional arguments can be intermixed with (both long and short) options.
+	// - If a command can have foreign flags (e.g. `nerdctl run`), we stop parsing
+	//   options on first positional argument.  This means we parse the flag in
+	//   `nerdctl run --env foo=bar image sh -c ...` but not the `--env` flag in
+	//   `nerdctl run image --env foo=bar sh -c ...`.
+	// - Having foreign flags is mutually exclusive with having subcommands; this
+	//   is also checked in `./generate`.
 	// - `--` stops parsing of options.
 	var result parsedArgs
 	var positionalArgs []string
@@ -196,9 +206,17 @@ func (c commandDefinition) parse(args []string) (*parsedArgs, error) {
 			}
 			break
 		} else {
-			// This command doesn't have subcommands; everything is positional arguments.
-			// We still have to parse other arguments for flags, though.
-			positionalArgs = append(positionalArgs, arg)
+			if c.hasForeignFlags {
+				// If we have foreign flags, assume the rest of the arguments starting
+				// from the first positional argument is foreign.
+				positionalArgs = append(positionalArgs, args[argIndex:]...)
+				break
+			} else {
+				// This command doesn't have subcommands, nor foreign arguments.
+				// Everything is positional arguments; we still have to parse other
+				// arguments for flags, though.
+				positionalArgs = append(positionalArgs, arg)
+			}
 		}
 	}
 	// At this point, `result` is filled with options, and `positionalArgs`

src/go/nerdctl-stub/parse_args.go

@@ -258,4 +258,36 @@ func TestParse(t *testing.T) {
 			assert.Equal(t, []string{"subcommand", "--foo", "FOO", "--bar", "BAR"}, result.args)
 		}
 	})
+	t.Run("subcommand with foreign flags", func(t *testing.T) {
+		t.Parallel()
+		var seenArgs []string
+		localCommands := make(map[string]commandDefinition)
+		localCommands[""] = commandDefinition{
+			commands: &localCommands,
+			subcommands: map[string]struct{}{
+				"subcommand": {},
+			},
+			options: map[string]argHandler{
+				"--foo": ignoredArgHandler,
+			},
+		}
+		localCommands["subcommand"] = commandDefinition{
+			commandPath: "subcommand",
+			commands:    &localCommands,
+			options: map[string]argHandler{
+				"--bar": ignoredArgHandler,
+			},
+			hasForeignFlags: true,
+			handler: func(cd *commandDefinition, s []string, argHandlers argHandlersType) (*parsedArgs, error) {
+				seenArgs = s
+				return &parsedArgs{}, nil
+			},
+		}
+		result, err := localCommands[""].parse([]string{"subcommand", "--foo", "FOO", "qq", "--bar", "BAR", "zz"})
+		if assert.NoError(t, err) {
+			assert.Equal(t, []string{"qq", "--bar", "BAR", "zz"}, seenArgs)
+			// Because we have a custom handler, they don't show up in result.args
+			assert.Equal(t, []string{"subcommand", "--foo", "FOO"}, result.args)
+		}
+	})
 }