Unfork spf13/{cobra, pflag}

- Add `SetStringFlagX` to support aliases such as `--storage-driver` and env vars such as `$CONTAINERD_SNAPSHOTTER` - Set `mainHelpTemplate` to support command categories - Fix help string of `nerdctl run` - Re-exec to avoid shorthand collision, e.g, `-a` for `nerdctl --address=ADDRESS ...` and `nerdctl images --all` - Deprecate global shorthand flags (`-a`, `-H`, `-n`) Fix issue 522 Fix issue 510 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
containerd · Nov 17, 2021 · 21452aa · 21452aa
1 parent 3b86328
commit 21452aa
Show file tree

Hide file tree

Showing 12 changed files with 162 additions and 100 deletions.
diff --git a/README.md b/README.md
@@ -1071,10 +1071,12 @@ Unimplemented `docker-compose ps` (V1) flags: `--quiet`, `--services`, `--filter
 Unimplemented `docker compose ps` (V2) flags: `--format`, `--status`
 
 ## Global flags
-- :nerd_face: :blue_square: `-a`, `--address`:  containerd address, optionally with "unix://" prefix
-- :whale:     `-H`, `--host`: Docker-compatible alias for `-a`, `--address`
-- :nerd_face: :blue_square: `-n`, `--namespace`: containerd namespace
+- :nerd_face: :blue_square: `--address`:  containerd address, optionally with "unix://" prefix
+- :nerd_face: :blue_square: `-a`, `--host`, `-H`: deprecated aliases of `--address`
+- :nerd_face: :blue_square: `--namespace`: containerd namespace
+- :nerd_face: :blue_square: `-n`: deprecated alias of `--namespace`
 - :nerd_face: :blue_square: `--snapshotter`: containerd snapshotter
+- :nerd_face: :blue_square: `--storage-driver`: deprecated alias of `--snapshotter`
 - :nerd_face: :blue_square: `--cni-path`: CNI binary path (default: `/opt/cni/bin`) [`$CNI_PATH`]
 - :nerd_face: :blue_square: `--cni-netconfpath`: CNI netconf path (default: `/etc/cni/net.d`) [`$NETCONFPATH`]
 - :nerd_face: :blue_square: `--data-root`: nerdctl data root, e.g. "/var/lib/nerdctl"

diff --git a/cmd/nerdctl/build.go b/cmd/nerdctl/build.go
@@ -34,7 +34,6 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 )
 
 func newBuildCommand() *cobra.Command {
@@ -45,14 +44,7 @@ func newBuildCommand() *cobra.Command {
 		SilenceUsage:  true,
 		SilenceErrors: true,
 	}
-	buildCommand.Flags().AddFlag(
-		&pflag.Flag{
-			Name:    "buildkit-host",
-			Usage:   `BuildKit address`,
-			EnvVars: []string{"BUILDKIT_HOST"},
-			Value:   pflag.NewStringValue(defaults.BuildKitHost(), new(string)),
-		},
-	)
+	SetStringFlagX(buildCommand.Flags(), "buildkit-host", nil, defaults.BuildKitHost(), "BUILDKIT_HOST", "BuildKit address")
 	buildCommand.Flags().StringArrayP("tag", "t", nil, "Name and optionally a tag in the 'name:tag' format")
 	buildCommand.Flags().StringP("file", "f", "", "Name of the Dockerfile")
 	buildCommand.Flags().String("target", "", "Set the target build stage to build")

diff --git a/cmd/nerdctl/container.go b/cmd/nerdctl/container.go
@@ -22,7 +22,7 @@ import (
 
 func newContainerCommand() *cobra.Command {
 	containerCommand := &cobra.Command{
-		Category:      CategoryManagement,
+		Annotations:   map[string]string{Category: Management},
 		Use:           "container",
 		Short:         "Manage containers",
 		RunE:          unknownSubcommandAction,

diff --git a/cmd/nerdctl/image.go b/cmd/nerdctl/image.go
@@ -22,7 +22,7 @@ import (
 
 func newImageCommand() *cobra.Command {
 	cmd := &cobra.Command{
-		Category:      CategoryManagement,
+		Annotations:   map[string]string{Category: Management},
 		Use:           "image",
 		Short:         "Manage images",
 		RunE:          unknownSubcommandAction,

diff --git a/cmd/nerdctl/main.go b/cmd/nerdctl/main.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"syscall"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/defaults"
@@ -36,12 +37,32 @@ import (
 	"github.com/spf13/pflag"
 )
 
-type Category = string
-
 const (
-	CategoryManagement = Category("Management")
+	Category   = "category"
+	Management = "management"
 )
 
+// mainHelpTemplate was derived from https://github.com/spf13/cobra/blob/v1.2.1/command.go#L491-L514
+const mainHelpTemplate = `Usage:{{if .Runnable}}
+  {{.UseLine}}{{end}}{{if .HasAvailableSubCommands}}
+  {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}}
+Aliases:
+  {{.NameAndAliases}}{{end}}{{if .HasExample}}
+Examples:
+{{.Example}}{{end}}{{if .HasAvailableSubCommands}}
+Management commands:{{range .Commands}}{{if (eq (index .Annotations "category") "management")}}{{if (or .IsAvailableCommand (eq .Name "help"))}}
+  {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}}
+Commands:{{range .Commands}}{{if (eq (index .Annotations "category") "")}}{{if (or .IsAvailableCommand (eq .Name "help"))}}
+  {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}}
+Flags:
+{{.LocalFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}}
+Global Flags:
+{{.InheritedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .HasHelpSubCommands}}
+Additional help topics:{{range .Commands}}{{if .IsAdditionalHelpTopicCommand}}
+  {{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}}{{if .HasAvailableSubCommands}}
+Use "{{.CommandPath}} [command] --help" for more information about a command.{{end}}
+`
+
 func main() {
 	if err := xmain(); err != nil {
 		HandleExitCoder(err)
@@ -50,15 +71,62 @@ func main() {
 }
 
 func xmain() error {
+	logrus.Debugf("PID=%d, os.Args=%v", os.Getpid(), os.Args)
 	if len(os.Args) == 3 && os.Args[1] == logging.MagicArgv1 {
 		// containerd runtime v2 logging plugin mode.
 		// "binary://BIN?KEY=VALUE" URI is parsed into Args {BIN, KEY, VALUE}.
 		return logging.Main(os.Args[2])
 	}
 	// nerdctl CLI mode
+	translateDeprecatedGlobalAliases()
 	return newApp().Execute()
 }
 
+// translateDeprecatedGlobalAliases translates deprecated aliases, e.g., "-a", -> "--address" .
+// These aliases cannot be handled directly in spf13/{cobra, pflag} due to conflicts with child commands.
+// e.g., `nerdctl -a` (`nerdctl --address`) conflicts with `nerdctl images -a` (`nerdctl images --all`)
+func translateDeprecatedGlobalAliases() {
+	if len(os.Args) < 2 {
+		return
+	}
+	dict := map[string]string{
+		"a": "address", // conflicts with `nerdctl (images|ps|stats) --all`, `nerdctl commit --author`
+		"H": "host",
+		"n": "namespace", // conflicts with `nerdctl logs --tail`
+	}
+	var (
+		needsReexec           bool
+		probablySawSubcommand bool
+		args                  = make([]string, len(os.Args))
+	)
+	for i, f := range os.Args {
+		translated := f
+		if i > 0 && !strings.HasPrefix(f, "-") {
+			probablySawSubcommand = true // not robust, but enough for deprecated stuff
+		}
+		if !probablySawSubcommand {
+			for shortName, longName := range dict {
+				switch {
+				case f == "-"+shortName, strings.HasPrefix(f, "-"+shortName+"="):
+					translated = strings.Replace(f, "-"+shortName, "--"+longName, 1)
+					logrus.Warnf("Global alias \"-%s\" is deprecated. Use \"--%s\".", shortName, longName)
+					needsReexec = true
+				}
+			}
+		}
+		args[i] = translated
+	}
+	if needsReexec {
+		logrus.Warnf("Rewriting CLI args to avoid using deprecated aliases: %v -> %v", os.Args, args)
+		arg0, err := os.Executable()
+		if err != nil {
+			panic(err)
+		}
+		reexecErr := syscall.Exec(arg0, args, os.Environ())
+		panic(fmt.Errorf("should not reach here after reexec-ing: %w", reexecErr))
+	}
+}
+
 func newApp() *cobra.Command {
 	var rootCmd = &cobra.Command{
 		Use:           "nerdctl",
@@ -67,75 +135,18 @@ func newApp() *cobra.Command {
 		SilenceUsage:  true,
 		SilenceErrors: true,
 	}
+	rootCmd.SetHelpTemplate(mainHelpTemplate)
 	rootCmd.PersistentFlags().Bool("debug", false, "debug mode")
 	rootCmd.PersistentFlags().Bool("debug-full", false, "debug mode (with full output)")
-	{
-		address := new(string)
-		rootCmd.PersistentFlags().AddFlag(
-			&pflag.Flag{
-				Name:      "address",
-				Shorthand: "a",
-				Usage:     `containerd address, optionally with "unix://" prefix`,
-				EnvVars:   []string{"CONTAINERD_ADDRESS"},
-				Value:     pflag.NewStringValue(defaults.DefaultAddress, address),
-			},
-		)
-		rootCmd.PersistentFlags().AddFlag(
-			&pflag.Flag{
-				Name:      "host",
-				Shorthand: "H",
-				Usage:     `alias of --address`,
-				Value:     pflag.NewStringValue(defaults.DefaultAddress, address),
-			},
-		)
-	}
-	rootCmd.PersistentFlags().AddFlag(
-		&pflag.Flag{
-			Name:      "namespace",
-			Shorthand: "n",
-			Usage:     `containerd namespace, such as "moby" for Docker, "k8s.io" for Kubernetes`,
-			EnvVars:   []string{"CONTAINERD_NAMESPACE"},
-			Value:     pflag.NewStringValue(namespaces.Default, new(string)),
-		},
-	)
+	rootCmdFlags := rootCmd.PersistentFlags()
+	SetStringFlagX(rootCmdFlags, "address", []string{"host"}, defaults.DefaultAddress, "CONTAINERD_ADDRESS", `containerd address, optionally with "unix://" prefix (aliases: -a (deprecated), -H (deprecated))`)
+	SetStringFlagX(rootCmdFlags, "namespace", nil, namespaces.Default, "CONTAINERD_NAMESPACE", `containerd namespace, such as "moby" for Docker, "k8s.io" for Kubernetes (alias: -n (deprecated))`)
 	rootCmd.RegisterFlagCompletionFunc("namespace", shellCompleteNamespaceNames)
-	{
-		snapshotter := new(string)
-		rootCmd.PersistentFlags().AddFlag(
-			&pflag.Flag{
-				Name:    "snapshotter",
-				Usage:   "containerd snapshotter",
-				EnvVars: []string{"CONTAINERD_SNAPSHOTTER"},
-				Value:   pflag.NewStringValue(containerd.DefaultSnapshotter, snapshotter),
-			},
-		)
-		rootCmd.PersistentFlags().AddFlag(
-			&pflag.Flag{
-				Name:  "storage-driver",
-				Usage: "alias of --snapshotter",
-				Value: pflag.NewStringValue(containerd.DefaultSnapshotter, snapshotter),
-			},
-		)
-		rootCmd.RegisterFlagCompletionFunc("snapshotter", shellCompleteSnapshotterNames)
-		rootCmd.RegisterFlagCompletionFunc("storage-driver", shellCompleteSnapshotterNames)
-	}
-
-	rootCmd.PersistentFlags().AddFlag(
-		&pflag.Flag{
-			Name:    "cni-path",
-			Usage:   "Set the cni-plugins binary directory",
-			EnvVars: []string{"CNI_PATH"},
-			Value:   pflag.NewStringValue(ncdefaults.CNIPath(), new(string)),
-		},
-	)
-	rootCmd.PersistentFlags().AddFlag(
-		&pflag.Flag{
-			Name:    "cni-netconfpath",
-			Usage:   "Set the CNI config directory",
-			EnvVars: []string{"NETCONFPATH"},
-			Value:   pflag.NewStringValue(ncdefaults.CNINetConfPath(), new(string)),
-		},
-	)
+	SetStringFlagX(rootCmdFlags, "snapshotter", []string{"storage-driver"}, containerd.DefaultSnapshotter, "CONTAINERD_SNAPSHOTTER", "containerd snapshotter")
+	rootCmd.RegisterFlagCompletionFunc("snapshotter", shellCompleteSnapshotterNames)
+	rootCmd.RegisterFlagCompletionFunc("storage-driver", shellCompleteSnapshotterNames)
+	SetStringFlagX(rootCmdFlags, "cni-path", nil, ncdefaults.CNIPath(), "CNI_PATH", "cni plugins binary directory")
+	SetStringFlagX(rootCmdFlags, "cni-netconfpath", nil, ncdefaults.CNINetConfPath(), "NETCONFPATH", "cni config directory")
 	rootCmd.PersistentFlags().String("data-root", ncdefaults.DataRoot(), "Root directory of persistent nerdctl state (managed by nerdctl, not by containerd)")
 	rootCmd.PersistentFlags().String("cgroup-manager", ncdefaults.CgroupManager(), `Cgroup manager to use ("cgroupfs"|"systemd")`)
 	rootCmd.RegisterFlagCompletionFunc("cgroup-manager", shellCompleteCgroupManagerNames)
@@ -312,3 +323,20 @@ func unknownSubcommandAction(cmd *cobra.Command, args []string) error {
 	}
 	return errors.New(msg)
 }
+
+// SetStringFlagX is similar to f.String but supports aliases and env var
+func SetStringFlagX(f *pflag.FlagSet, name string, aliases []string, value string, env, usage string) {
+	if env != "" {
+		usage = fmt.Sprintf("%s [$%s]", usage, env)
+	}
+	if envV, ok := os.LookupEnv(env); ok {
+		value = envV
+	}
+	p := new(string)
+	f.StringVarP(p, name, "", value, usage)
+
+	aliasesUsage := fmt.Sprintf("Deprecated alias of --%s", name)
+	for _, a := range aliases {
+		f.StringVarP(p, a, "", value, aliasesUsage)
+	}
+}
diff --git a/cmd/nerdctl/namespace.go b/cmd/nerdctl/namespace.go
@@ -29,7 +29,7 @@ import (
 
 func newNamespaceCommand() *cobra.Command {
 	namespaceCommand := &cobra.Command{
-		Category:      CategoryManagement,
+		Annotations:   map[string]string{Category: Management},
 		Use:           "namespace",
 		Short:         "Manage containerd namespaces",
 		Long:          "Unrelated to Linux namespaces and Kubernetes namespaces",

diff --git a/cmd/nerdctl/network.go b/cmd/nerdctl/network.go
@@ -22,7 +22,7 @@ import (
 
 func newNetworkCommand() *cobra.Command {
 	networkCommand := &cobra.Command{
-		Category:      CategoryManagement,
+		Annotations:   map[string]string{Category: Management},
 		Use:           "network",
 		Short:         "Manage networks",
 		RunE:          unknownSubcommandAction,

diff --git a/cmd/nerdctl/run.go b/cmd/nerdctl/run.go
@@ -78,7 +78,7 @@ func newRunCommand() *cobra.Command {
 		longHelp += "WARNING: `nerdctl run` is experimental on FreeBSD and currently requires `--net=none` (https://github.com/containerd/nerdctl/blob/master/docs/freebsd.md)"
 	}
 	var runCommand = &cobra.Command{
-		Use:               "run IMAGE [COMMAND] [ARG...]",
+		Use:               "run [flags] IMAGE [COMMAND] [ARG...]",
 		Args:              cobra.MinimumNArgs(1),
 		Short:             shortHelp,
 		Long:              longHelp,

diff --git a/cmd/nerdctl/system.go b/cmd/nerdctl/system.go
@@ -22,7 +22,7 @@ import (
 
 func newSystemCommand() *cobra.Command {
 	var systemCommand = &cobra.Command{
-		Category:      CategoryManagement,
+		Annotations:   map[string]string{Category: Management},
 		Use:           "system",
 		Short:         "Manage containerd",
 		RunE:          unknownSubcommandAction,

diff --git a/cmd/nerdctl/volume.go b/cmd/nerdctl/volume.go
@@ -23,7 +23,7 @@ import (
 
 func newVolumeCommand() *cobra.Command {
 	volumeCommand := &cobra.Command{
-		Category:      CategoryManagement,
+		Annotations:   map[string]string{Category: Management},
 		Use:           "volume",
 		Short:         "Manage volumes",
 		RunE:          unknownSubcommandAction,

diff --git a/go.mod b/go.mod
@@ -37,8 +37,8 @@ require (
 	github.com/opencontainers/runtime-spec v1.0.3-0.20211101234015-a3c33d663ebc
 	github.com/rootless-containers/rootlesskit v0.14.6
 	github.com/sirupsen/logrus v1.8.1
-	github.com/spf13/cobra v1.2.1 // replaced, see the bottom of this file
-	github.com/spf13/pflag v1.0.5 // replaced, see the bottom of this file
+	github.com/spf13/cobra v1.2.2-0.20211116222018-9e1d6f1c2aa8 // use v1.2.2 (master), as v1.2.1 seems to lack `nerdctl completion bash`
+	github.com/spf13/pflag v1.0.6-0.20210604193023-d5e0c0615ace
 	github.com/tidwall/gjson v1.11.0
 	golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
@@ -47,9 +47,5 @@ require (
 	gotest.tools/v3 v3.0.3
 )
 
-replace (
-	// Temporary fork for avoiding importing patent-protected code: https://github.com/hashicorp/golang-lru/issues/73
-	github.com/hashicorp/golang-lru => github.com/ktock/golang-lru v0.5.5-0.20211029085301-ec551be6f75c
-	github.com/spf13/cobra => github.com/robberphex/cobra v1.2.2-0.20211012081327-8e3ac9400ac4 // https://github.com/spf13/cobra/pull/1503
-	github.com/spf13/pflag => github.com/robberphex/pflag v1.0.6-0.20211014094653-9df3e45100fd // https://github.com/spf13/pflag/pull/333
-)
+// Temporary fork for avoiding importing patent-protected code: https://github.com/hashicorp/golang-lru/issues/73
+replace github.com/hashicorp/golang-lru => github.com/ktock/golang-lru v0.5.5-0.20211029085301-ec551be6f75c