[cmd/*] Switch to pflag for all CLI flag parsing (vitessio#10619)

* Switch to `pflag` for all parsing This transparently swaps the cli parsing library used by `internal/flag` from the standard library `flag` package to `spf13/pflag`. It also introduces hook points for packages throughout the vitess codebase to register their flags for either all commands using `servenv` or a particular subset of commands. This allows these packages to continue to define their flag variables in a package-private way, but without polluting the global flagset. Signed-off-by: Andrew Mason <[email protected]> * Workaround exit code difference between stdlib `flag` and `pflag` tl;dr stdlib `flag` has [this][1] and `pflag` does not [1]: golang/go@dcf0929 Signed-off-by: Andrew Mason <[email protected]> * adjust test data for difference in spacing between pflag/stdflag Signed-off-by: Andrew Mason <[email protected]> * update lingering legacy flag tests Signed-off-by: Andrew Mason <[email protected]> * Update vtgate/tabletgateway.go to use new interface to isolate flags, update test data Signed-off-by: Andrew Mason <[email protected]> * update flags in java test code Signed-off-by: Andrew Mason <[email protected]>
slackhq · Aug 28, 2023 · 9bd5467 · 9bd5467
1 parent 772645c
commit 9bd5467
Show file tree

Hide file tree

Showing 27 changed files with 889 additions and 1,515 deletions.
diff --git a/go/cmd/mysqlctl/mysqlctl.go b/go/cmd/mysqlctl/mysqlctl.go
@@ -25,6 +25,8 @@ import (
 	"os"
 	"time"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/cmd"
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/flagutil"
@@ -248,7 +250,7 @@ func main() {
 		exit.Return(1)
 	}
 	dbconfigs.RegisterFlags(dbconfigs.Dba)
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("mysqlctl", pflag.ExitOnError))
 
 	tabletAddr = netutil.JoinHostPort("localhost", int32(*port))
 

diff --git a/go/cmd/query_analyzer/query_analyzer.go b/go/cmd/query_analyzer/query_analyzer.go
@@ -24,6 +24,8 @@ import (
 	"os"
 	"sort"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/sqlparser"
@@ -59,7 +61,7 @@ func (a stats) Less(i, j int) bool { return a[i].Count > a[j].Count }
 
 func main() {
 	defer exit.Recover()
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("query_analyzer", pflag.ExitOnError))
 	for _, filename := range _flag.Args() {
 		fmt.Printf("processing: %s\n", filename)
 		if err := processFile(filename); err != nil {

diff --git a/go/cmd/topo2topo/topo2topo.go b/go/cmd/topo2topo/topo2topo.go
@@ -22,6 +22,8 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -53,7 +55,7 @@ func main() {
 	defer exit.RecoverAll()
 	defer logutil.Flush()
 
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("topo2topo", pflag.ExitOnError))
 	args := _flag.Args()
 	if len(args) != 0 {
 		flag.Usage()

diff --git a/go/cmd/vtbench/vtbench.go b/go/cmd/vtbench/vtbench.go
@@ -23,6 +23,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/log"
@@ -104,7 +106,7 @@ func main() {
 	defer exit.Recover()
 
 	flag.Lookup("logtostderr").Value.Set("true")
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("vtbench", pflag.ExitOnError))
 
 	clientProto := vtbench.MySQL
 	switch *protocol {

diff --git a/go/cmd/vtclient/vtclient.go b/go/cmd/vtclient/vtclient.go
@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"github.com/olekukonko/tablewriter"
+	"github.com/spf13/pflag"
 
 	"vitess.io/vitess/go/vt/concurrency"
 	"vitess.io/vitess/go/vt/log"
@@ -147,7 +148,7 @@ func main() {
 }
 
 func run() (*results, error) {
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("vtclient", pflag.ExitOnError))
 	args := _flag.Args()
 
 	if len(args) == 0 {

diff --git a/go/cmd/vtctlclient/main.go b/go/cmd/vtctlclient/main.go
@@ -25,6 +25,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/trace"
 	"vitess.io/vitess/go/vt/log"
@@ -70,7 +72,7 @@ func checkDeprecations(args []string) {
 func main() {
 	defer exit.Recover()
 
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("vtctlclient", pflag.ExitOnError))
 
 	closer := trace.StartTracing("vtctlclient")
 	defer trace.LogErrorsWhenClosing(closer)

diff --git a/go/cmd/vtgate/vtgate.go b/go/cmd/vtgate/vtgate.go
@@ -142,7 +142,7 @@ func main() {
 		log.Exitf("tablet_types_to_wait should contain at least one serving tablet type")
 	}
 
-	err := CheckCellFlags(context.Background(), resilientServer, *cell, *vtgate.CellsToWatch)
+	err := CheckCellFlags(context.Background(), resilientServer, *cell, vtgate.CellsToWatch)
 	if err != nil {
 		log.Exitf("cells_to_watch validation failed: %v", err)
 	}

diff --git a/go/cmd/vtgr/main.go b/go/cmd/vtgr/main.go
@@ -18,6 +18,8 @@ import (
 	"flag"
 	"strings"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/vt/vtgr"
 
 	// Include deprecation warnings for soon-to-be-unsupported flag invocations.
@@ -26,7 +28,7 @@ import (
 
 func main() {
 	clustersToWatch := flag.String("clusters_to_watch", "", "Comma-separated list of keyspaces or keyspace/shards that this instance will monitor and repair. Defaults to all clusters in the topology. Example: \"ks1,ks2/-80\"")
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("vtgr", pflag.ExitOnError))
 
 	// openTabletDiscovery will open up a connection to topo server
 	// and populate the tablets in memory

diff --git a/go/cmd/vttlstest/vttlstest.go b/go/cmd/vttlstest/vttlstest.go
@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -137,7 +139,7 @@ func main() {
 	_flag.SetUsage(flag.CommandLine, _flag.UsageOptions{
 		Preface: func(w io.Writer) { fmt.Fprint(w, doc) },
 	})
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("vttlstest", pflag.ExitOnError))
 	args := _flag.Args()
 	if len(args) == 0 {
 		flag.Usage()

diff --git a/go/cmd/zk/zkcmd.go b/go/cmd/zk/zkcmd.go
@@ -33,6 +33,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/spf13/pflag"
 	"github.com/z-division/go-zookeeper/zk"
 	"golang.org/x/term"
 
@@ -140,10 +141,11 @@ func main() {
 	defer exit.Recover()
 	defer logutil.Flush()
 
-	_flag.SetUsage(flag.CommandLine, _flag.UsageOptions{
+	fs := pflag.NewFlagSet("zkcmd", pflag.ExitOnError)
+	_flag.SetUsage(flag.CommandLine, _flag.UsageOptions{ // TODO: hmmm
 		Epilogue: func(w io.Writer) { fmt.Fprint(w, doc) },
 	})
-	_flag.Parse()
+	_flag.Parse(fs)
 	args := _flag.Args()
 	if len(args) == 0 {
 		flag.Usage()

diff --git a/go/cmd/zkctl/zkctl.go b/go/cmd/zkctl/zkctl.go
@@ -24,6 +24,8 @@ import (
 	"io"
 	"os"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -60,7 +62,7 @@ func main() {
 	defer exit.Recover()
 	defer logutil.Flush()
 
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("zkctl", pflag.ExitOnError))
 	args := _flag.Args()
 
 	if len(args) == 0 {

diff --git a/go/cmd/zkctld/zkctld.go b/go/cmd/zkctld/zkctld.go
@@ -25,6 +25,8 @@ import (
 	"os/signal"
 	"syscall"
 
+	"github.com/spf13/pflag"
+
 	"vitess.io/vitess/go/exit"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -45,7 +47,7 @@ func main() {
 	defer exit.Recover()
 	defer logutil.Flush()
 
-	_flag.Parse()
+	_flag.Parse(pflag.NewFlagSet("zkctld", pflag.ExitOnError))
 
 	zkConfig := zkctl.MakeZkConfigFromString(*zkCfg, uint32(*myID))
 	zkd := zkctl.NewZkd(zkConfig)