cmd/syncthing: Add cli as a subcommand (fixes #6566, fixes #4719) (#7364)

* cmd/syncthing: Add cli as a subcommand (fixes #6566, fixes #4719)

* Hijack help

* Add comment

* Revert go.mod/go.sum

cmd/syncthing/cli/client.go

@@ -4,7 +4,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
-package main
+package cli
 
 import (
 	"bytes"

cmd/syncthing/cli/errors.go

@@ -4,7 +4,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
-package main
+package cli
 
 import (
 	"errors"

cmd/syncthing/cli/main.go

@@ -4,13 +4,12 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
-package main
+package cli
 
 import (
 	"bufio"
 	"crypto/tls"
 	"encoding/json"
-	"flag"
 	"log"
 	"os"
 	"reflect"
@@ -19,67 +18,56 @@ import (
 	"github.com/flynn-archive/go-shlex"
 	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
-	"github.com/syncthing/syncthing/lib/build"
+
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/locations"
 	"github.com/syncthing/syncthing/lib/protocol"
+	"github.com/syncthing/syncthing/lib/svcutil"
+
 	"github.com/urfave/cli"
 )
 
-func main() {
-	// This is somewhat a hack around a chicken and egg problem.
-	// We need to set the home directory and potentially other flags to know where the syncthing instance is running
-	// in order to get it's config ... which we then use to construct the actual CLI ... at which point it's too late
-	// to add flags there...
-	homeBaseDir := locations.GetBaseDir(locations.ConfigBaseDir)
-	guiCfg := config.GUIConfiguration{}
+type CLI struct {
+	GUIAddress string   `name:"gui-address" placeholder:"URL" help:"Override GUI address (e.g. \"http://192.0.2.42:8443\")"`
+	GUIAPIKey  string   `name:"gui-apikey" placeholder:"API-KEY" help:"Override GUI API key"`
+	HomeDir    string   `name:"home" placeholder:"PATH" help:"Set configuration and data directory"`
+	ConfDir    string   `name:"conf" placeholder:"PATH" help:"Set configuration directory (config and keys)"`
+	Args       []string `arg:"" optional:""`
+}
 
-	flags := flag.NewFlagSet("", flag.ContinueOnError)
-	flags.StringVar(&guiCfg.RawAddress, "gui-address", guiCfg.RawAddress, "Override GUI address (e.g. \"http://192.0.2.42:8443\")")
-	flags.StringVar(&guiCfg.APIKey, "gui-apikey", guiCfg.APIKey, "Override GUI API key")
-	flags.StringVar(&homeBaseDir, "home", homeBaseDir, "Set configuration directory")
-
-	// Implement the same flags at the lower CLI, with the same default values (pre-parse), but do nothing with them.
-	// This is so that we could reuse os.Args
-	fakeFlags := []cli.Flag{
-		cli.StringFlag{
-			Name:  "gui-address",
-			Value: guiCfg.RawAddress,
-			Usage: "Override GUI address (e.g. \"http://192.0.2.42:8443\")",
-		},
-		cli.StringFlag{
-			Name:  "gui-apikey",
-			Value: guiCfg.APIKey,
-			Usage: "Override GUI API key",
-		},
-		cli.StringFlag{
-			Name:  "home",
-			Value: homeBaseDir,
-			Usage: "Set configuration directory",
-		},
+func (c *CLI) Run() error {
+	// Not set as default above because the strings can be really long.
+	var err error
+	homeSet := c.HomeDir != ""
+	confSet := c.ConfDir != ""
+	switch {
+	case homeSet && confSet:
+		err = errors.New("-home must not be used together with -conf")
+	case homeSet:
+		err = locations.SetBaseDir(locations.ConfigBaseDir, c.HomeDir)
+	case confSet:
+		err = locations.SetBaseDir(locations.ConfigBaseDir, c.ConfDir)
+	}
+	if err != nil {
+		log.Println("Command line options:", err)
+		os.Exit(svcutil.ExitError.AsInt())
+	}
+	guiCfg := config.GUIConfiguration{
+		RawAddress: c.GUIAddress,
+		APIKey:     c.GUIAPIKey,
 	}
-
-	// Do not print usage of these flags, and ignore errors as this can't understand plenty of things
-	flags.Usage = func() {}
-	_ = flags.Parse(os.Args[1:])
 
 	// Now if the API key and address is not provided (we are not connecting to a remote instance),
 	// try to rip it out of the config.
 	if guiCfg.RawAddress == "" && guiCfg.APIKey == "" {
-		// Update the base directory
-		err := locations.SetBaseDir(locations.ConfigBaseDir, homeBaseDir)
-		if err != nil {
-			log.Fatal(errors.Wrap(err, "setting home"))
-		}
-
 		// Load the certs and get the ID
 		cert, err := tls.LoadX509KeyPair(
 			locations.Get(locations.CertFile),
 			locations.Get(locations.KeyFile),
 		)
 		if err != nil {
-			log.Fatal(errors.Wrap(err, "reading device ID"))
+			return errors.Wrap(err, "reading device ID")
 		}
 
 		myID := protocol.NewDeviceID(cert.Certificate[0])
@@ -87,20 +75,20 @@ func main() {
 		// Load the config
 		cfg, _, err := config.Load(locations.Get(locations.ConfigFile), myID, events.NoopLogger)
 		if err != nil {
-			log.Fatalln(errors.Wrap(err, "loading config"))
+			return errors.Wrap(err, "loading config")
 		}
 
 		guiCfg = cfg.GUI()
 	} else if guiCfg.Address() == "" || guiCfg.APIKey == "" {
-		log.Fatalln("Both -gui-address and -gui-apikey should be specified")
+		return errors.New("Both --gui-address and --gui-apikey should be specified")
 	}
 
 	if guiCfg.Address() == "" {
-		log.Fatalln("Could not find GUI Address")
+		return errors.New("Could not find GUI Address")
 	}
 
 	if guiCfg.APIKey == "" {
-		log.Fatalln("Could not find GUI API key")
+		return errors.New("Could not find GUI API key")
 	}
 
 	client := getClient(guiCfg)
@@ -108,7 +96,7 @@ func main() {
 	cfg, err := getConfig(client)
 	original := cfg.Copy()
 	if err != nil {
-		log.Fatalln(errors.Wrap(err, "getting config"))
+		return errors.Wrap(err, "getting config")
 	}
 
 	// Copy the config and set the default flags
@@ -118,16 +106,40 @@ func main() {
 
 	commands, err := recli.New(recliCfg).Construct(&cfg)
 	if err != nil {
-		log.Fatalln(errors.Wrap(err, "config reflect"))
+		return errors.Wrap(err, "config reflect")
+	}
+
+	// Implement the same flags at the upper CLI, but do nothing with them.
+	// This is so that the usage text is the same
+	fakeFlags := []cli.Flag{
+		cli.StringFlag{
+			Name:  "gui-address",
+			Value: "URL",
+			Usage: "Override GUI address (e.g. \"http://192.0.2.42:8443\")",
+		},
+		cli.StringFlag{
+			Name:  "gui-apikey",
+			Value: "API-KEY",
+			Usage: "Override GUI API key",
+		},
+		cli.StringFlag{
+			Name:  "home",
+			Value: "PATH",
+			Usage: "Set configuration and data directory",
+		},
+		cli.StringFlag{
+			Name:  "conf",
+			Value: "PATH",
+			Usage: "Set configuration directory (config and keys)",
+		},
 	}
 
 	// Construct the actual CLI
 	app := cli.NewApp()
-	app.Name = "stcli"
+	app.Name = "syncthing cli"
 	app.HelpName = app.Name
 	app.Author = "The Syncthing Authors"
 	app.Usage = "Syncthing command line interface"
-	app.Version = build.Version
 	app.Flags = fakeFlags
 	app.Metadata = map[string]interface{}{
 		"client": client,
@@ -144,6 +156,9 @@ func main() {
 		errorsCommand,
 	}
 
+	// It expects to be give os.Args which has argv[0] set to executable name, so fake it.
+	c.Args = append([]string{"cli"}, c.Args...)
+
 	tty := isatty.IsTerminal(os.Stdin.Fd()) || isatty.IsCygwinTerminal(os.Stdin.Fd())
 	if !tty {
 		// Not a TTY, consume from stdin
@@ -151,42 +166,43 @@ func main() {
 		for scanner.Scan() {
 			input, err := shlex.Split(scanner.Text())
 			if err != nil {
-				log.Fatalln(errors.Wrap(err, "parsing input"))
+				return errors.Wrap(err, "parsing input")
 			}
 			if len(input) == 0 {
 				continue
 			}
-			err = app.Run(append(os.Args, input...))
+			err = app.Run(append(c.Args, input...))
 			if err != nil {
-				log.Fatalln(err)
+				return err
 			}
 		}
 		err = scanner.Err()
 		if err != nil {
-			log.Fatalln(err)
+			return err
 		}
 	} else {
-		err = app.Run(os.Args)
+		err = app.Run(c.Args)
 		if err != nil {
-			log.Fatalln(err)
+			return err
 		}
 	}
 
 	if !reflect.DeepEqual(cfg, original) {
 		body, err := json.MarshalIndent(cfg, "", "  ")
 		if err != nil {
-			log.Fatalln(err)
+			return err
 		}
 		resp, err := client.Post("system/config", string(body))
 		if err != nil {
-			log.Fatalln(err)
+			return err
 		}
 		if resp.StatusCode != 200 {
 			body, err := responseToBArray(resp)
 			if err != nil {
-				log.Fatalln(err)
+				return err
 			}
-			log.Fatalln(string(body))
+			return errors.New(string(body))
 		}
 	}
+	return nil
 }

cmd/syncthing/cli/operations.go

@@ -4,7 +4,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
-package main
+package cli
 
 import (
 	"fmt"

cmd/syncthing/cli/show.go

@@ -4,7 +4,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
-package main
+package cli
 
 import (
 	"github.com/urfave/cli"

cmd/syncthing/cli/utils.go

@@ -4,7 +4,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
-package main
+package cli
 
 import (
 	"encoding/json"

cmd/syncthing/main.go

@@ -31,6 +31,9 @@ import (
 	"time"
 
 	"github.com/alecthomas/kong"
+	"github.com/thejerf/suture/v4"
+
+	"github.com/syncthing/syncthing/cmd/syncthing/cli"
 	"github.com/syncthing/syncthing/cmd/syncthing/decrypt"
 	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/config"
@@ -48,7 +51,6 @@ import (
 	"github.com/syncthing/syncthing/lib/upgrade"
 
 	"github.com/pkg/errors"
-	"github.com/thejerf/suture/v4"
 )
 
 const (
@@ -127,11 +129,12 @@ var (
 	errTooEarlyUpgrade      = fmt.Errorf("last upgrade happened less than %v ago, skipping", upgradeRetryInterval)
 )
 
-// The cli struct is the main entry point for the command line parser. The
+// The entrypoint struct is the main entry point for the command line parser. The
 // commands and options here are top level commands to syncthing.
-var cli struct {
+var entrypoint struct {
 	Serve   serveOptions `cmd:"" help:"Run Syncthing"`
 	Decrypt decrypt.CLI  `cmd:"" help:"Decrypt or verify an encrypted folder"`
+	Cli     cli.CLI      `cmd:"" help:"Command line interface for Syncthing"`
 }
 
 // serveOptions are the options for the `syncthing serve` command.
@@ -227,11 +230,11 @@ func main() {
 		args = append([]string{"serve"}, convertLegacyArgs(args)...)
 	}
 
-	cli.Serve.setDefaults()
+	entrypoint.Serve.setDefaults()
 
 	// Create a parser with an overridden help function to print our extra
 	// help info.
-	parser, err := kong.New(&cli, kong.Help(extraHelpPrinter))
+	parser, err := kong.New(&entrypoint, kong.Help(helpHandler))
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -242,7 +245,11 @@ func main() {
 	parser.FatalIfErrorf(err)
 }
 
-func extraHelpPrinter(options kong.HelpOptions, ctx *kong.Context) error {
+func helpHandler(options kong.HelpOptions, ctx *kong.Context) error {
+	// If we're looking for CLI help, pass the arguments down to the CLI library to print it's own help.
+	if ctx.Command() == "cli" {
+		return ctx.Run()
+	}
 	if err := kong.DefaultHelpPrinter(options, ctx); err != nil {
 		return err
 	}
@@ -282,12 +289,12 @@ func (options serveOptions) Run() error {
 	case homeSet && dataSet:
 		err = errors.New("-home must not be used together with -conf and -data")
 	case homeSet:
-		if err = setLocation(locations.ConfigBaseDir, options.HomeDir); err == nil {
-			err = setLocation(locations.DataBaseDir, options.HomeDir)
+		if err = locations.SetBaseDir(locations.ConfigBaseDir, options.HomeDir); err == nil {
+			err = locations.SetBaseDir(locations.DataBaseDir, options.HomeDir)
 		}
 	case dataSet:
-		if err = setLocation(locations.ConfigBaseDir, options.ConfDir); err == nil {
-			err = setLocation(locations.DataBaseDir, options.DataDir)
+		if err = locations.SetBaseDir(locations.ConfigBaseDir, options.ConfDir); err == nil {
+			err = locations.SetBaseDir(locations.DataBaseDir, options.DataDir)
 		}
 	}
 	if err != nil {
@@ -1004,17 +1011,6 @@ func exitCodeForUpgrade(err error) int {
 	return svcutil.ExitError.AsInt()
 }
 
-func setLocation(enum locations.BaseDirEnum, loc string) error {
-	if !filepath.IsAbs(loc) {
-		var err error
-		loc, err = filepath.Abs(loc)
-		if err != nil {
-			return err
-		}
-	}
-	return locations.SetBaseDir(enum, loc)
-}
-
 // convertLegacyArgs returns the slice of arguments with single dash long
 // flags converted to double dash long flags.
 func convertLegacyArgs(args []string) []string {

lib/locations/locations.go

@@ -68,6 +68,13 @@ func init() {
 }
 
 func SetBaseDir(baseDirName BaseDirEnum, path string) error {
+	if !filepath.IsAbs(path) {
+		var err error
+		path, err = filepath.Abs(path)
+		if err != nil {
+			return err
+		}
+	}
 	_, ok := baseDirs[baseDirName]
 	if !ok {
 		return fmt.Errorf("unknown base dir: %s", baseDirName)