fix(syncthing): make directory flags global for all commands (#10028)

The home/config/data flags and end vars apply equally to all subcommands

cmd/syncthing/cli/main.go

@@ -14,13 +14,10 @@ import (
 	"github.com/alecthomas/kong"
 	"github.com/kballard/go-shellquote"
 
-	"github.com/syncthing/syncthing/cmd/syncthing/cmdutil"
 	"github.com/syncthing/syncthing/lib/config"
 )
 
 type CLI struct {
-	cmdutil.DirOptions
-
 	GUIAddress string `name:"gui-address" env:"STGUIADDRESS"`
 	GUIAPIKey  string `name:"gui-apikey" env:"STGUIAPIKEY"`
 
@@ -37,11 +34,6 @@ type Context struct {
 }
 
 func (cli CLI) AfterApply(kongCtx *kong.Context) error {
-	err := cmdutil.SetConfigDataLocationsFromFlags(cli.HomeDir, cli.ConfDir, cli.DataDir)
-	if err != nil {
-		return fmt.Errorf("command line options: %w", err)
-	}
-
 	clientFactory := &apiClientFactory{
 		cfg: config.GUIConfiguration{
 			RawAddress: cli.GUIAddress,

cmd/syncthing/cmdutil/diroptions.go

@@ -1,14 +0,0 @@
-// Copyright (C) 2021 The Syncthing Authors.
-//
-// This Source Code Form is subject to the terms of the Mozilla Public
-// 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 cmdutil
-
-// DirOptions are reused among several subcommands
-type DirOptions struct {
-	ConfDir string `name:"config" short:"C" placeholder:"PATH" env:"STCONFDIR" help:"Set configuration directory (config and keys)"`
-	DataDir string `name:"data" short:"D" placeholder:"PATH" env:"STDATADIR" help:"Set data directory (database and logs)"`
-	HomeDir string `name:"home" short:"H" placeholder:"PATH" env:"STHOMEDIR" help:"Set configuration and data directory"`
-}

cmd/syncthing/cmdutil/util.go

@@ -1,35 +0,0 @@
-// Copyright (C) 2014 The Syncthing Authors.
-//
-// This Source Code Form is subject to the terms of the Mozilla Public
-// 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 cmdutil
-
-import (
-	"errors"
-
-	"github.com/syncthing/syncthing/lib/locations"
-)
-
-func SetConfigDataLocationsFromFlags(homeDir, confDir, dataDir string) error {
-	homeSet := homeDir != ""
-	confSet := confDir != ""
-	dataSet := dataDir != ""
-	switch {
-	case dataSet != confSet:
-		return errors.New("either both or none of --config and --data must be given, use --home to set both at once")
-	case homeSet && dataSet:
-		return errors.New("--home must not be used together with --config and --data")
-	case homeSet:
-		confDir = homeDir
-		dataDir = homeDir
-		fallthrough
-	case dataSet:
-		if err := locations.SetBaseDir(locations.ConfigBaseDir, confDir); err != nil {
-			return err
-		}
-		return locations.SetBaseDir(locations.DataBaseDir, dataDir)
-	}
-	return nil
-}

cmd/syncthing/generate/generate.go

@@ -11,11 +11,9 @@ import (
 	"bufio"
 	"context"
 	"crypto/tls"
-	"errors"
 	"fmt"
 	"os"
 
-	"github.com/syncthing/syncthing/cmd/syncthing/cmdutil"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/fs"
@@ -26,7 +24,6 @@ import (
 )
 
 type CLI struct {
-	cmdutil.DirOptions
 	GUIUser         string `placeholder:"STRING" help:"Specify new GUI authentication user name"`
 	GUIPassword     string `placeholder:"STRING" help:"Specify new GUI authentication password (use - to read from standard input)"`
 	NoDefaultFolder bool   `help:"Don't create the \"default\" folder on first startup" env:"STNODEFAULTFOLDER"`
@@ -34,16 +31,6 @@ type CLI struct {
 }
 
 func (c *CLI) Run(l logger.Logger) error {
-	if c.HomeDir != "" {
-		if c.ConfDir != "" {
-			return errors.New("--home must not be used together with --config")
-		}
-		c.ConfDir = c.HomeDir
-	}
-	if c.ConfDir == "" {
-		c.ConfDir = locations.GetBaseDir(locations.ConfigBaseDir)
-	}
-
 	// Support reading the password from a pipe or similar
 	if c.GUIPassword == "-" {
 		reader := bufio.NewReader(os.Stdin)
@@ -54,7 +41,7 @@ func (c *CLI) Run(l logger.Logger) error {
 		c.GUIPassword = string(password)
 	}
 
-	if err := Generate(l, c.ConfDir, c.GUIUser, c.GUIPassword, c.NoDefaultFolder, c.NoPortProbing); err != nil {
+	if err := Generate(l, locations.GetBaseDir(locations.ConfigBaseDir), c.GUIUser, c.GUIPassword, c.NoDefaultFolder, c.NoPortProbing); err != nil {
 		return fmt.Errorf("failed to generate config and keys: %w", err)
 	}
 	return nil

cmd/syncthing/main.go

@@ -35,7 +35,6 @@ import (
 	"github.com/willabides/kongplete"
 
 	"github.com/syncthing/syncthing/cmd/syncthing/cli"
-	"github.com/syncthing/syncthing/cmd/syncthing/cmdutil"
 	"github.com/syncthing/syncthing/cmd/syncthing/decrypt"
 	"github.com/syncthing/syncthing/cmd/syncthing/generate"
 	"github.com/syncthing/syncthing/internal/db"
@@ -128,9 +127,17 @@ var (
 // The entrypoint struct is the main entry point for the command line parser. The
 // commands and options here are top level commands to syncthing.
 // Cli is just a placeholder for the help text (see main).
-var entrypoint struct {
+type CLI struct {
-	Serve serveOptions `cmd:"" help:"Run Syncthing (default)" default:"withargs"`
+	// The directory options are defined at top level and available for all
-	CLI   cli.CLI      `cmd:"" help:"Command line interface for Syncthing"`
+	// subcommands. Their settings take effect on the `locations` package by
+	// way of the command line parser, so anything using `locations.Get` etc
+	// will be doing the right thing.
+	ConfDir string `name:"config" short:"C" placeholder:"PATH" env:"STCONFDIR" help:"Set configuration directory (config and keys)"`
+	DataDir string `name:"data" short:"D" placeholder:"PATH" env:"STDATADIR" help:"Set data directory (database and logs)"`
+	HomeDir string `name:"home" short:"H" placeholder:"PATH" env:"STHOMEDIR" help:"Set configuration and data directory"`
+
+	Serve serveCmd `cmd:"" help:"Run Syncthing (default)" default:"withargs"`
+	CLI   cli.CLI  `cmd:"" help:"Command line interface for Syncthing"`
 
 	Browser  browserCmd   `cmd:"" help:"Open GUI in browser, then exit"`
 	Decrypt  decrypt.CLI  `cmd:"" help:"Decrypt or verify an encrypted folder"`
@@ -144,9 +151,14 @@ var entrypoint struct {
 	InstallCompletions kongplete.InstallCompletions `cmd:"" help:"Print commands to install shell completions"`
 }
 
-// serveOptions are the options for the `syncthing serve` command.
+func (c *CLI) AfterApply() error {
-type serveOptions struct {
+	// Executed after parsing command line options but before running actual
-	cmdutil.DirOptions
+	// subcommands
+	return setConfigDataLocationsFromFlags(c.HomeDir, c.ConfDir, c.DataDir)
+}
+
+// serveCmd are the options for the `syncthing serve` command.
+type serveCmd struct {
 	buildSpecificOptions
 
 	AllowNewerConfig      bool          `help:"Allow loading newer than current config version" env:"STALLOWNEWERCONFIG"`
@@ -209,6 +221,7 @@ func defaultVars() kong.Vars {
 func main() {
 	// Create a parser with an overridden help function to print our extra
 	// help info.
+	var entrypoint CLI
 	parser, err := kong.New(
 		&entrypoint,
 		kong.ConfigureHelp(kong.HelpOptions{
@@ -242,46 +255,39 @@ func helpHandler(options kong.HelpOptions, ctx *kong.Context) error {
 	return nil
 }
 
-// serveOptions.Run() is the entrypoint for `syncthing serve`
+// serveCmd.Run() is the entrypoint for `syncthing serve`
-func (options serveOptions) Run() error {
+func (c *serveCmd) Run() error {
-	l.SetFlags(options.LogFlags)
+	l.SetFlags(c.LogFlags)
 
-	if options.GUIAddress != "" {
+	if c.GUIAddress != "" {
 		// The config picks this up from the environment.
-		os.Setenv("STGUIADDRESS", options.GUIAddress)
+		os.Setenv("STGUIADDRESS", c.GUIAddress)
 	}
-	if options.GUIAPIKey != "" {
+	if c.GUIAPIKey != "" {
 		// The config picks this up from the environment.
-		os.Setenv("STGUIAPIKEY", options.GUIAPIKey)
+		os.Setenv("STGUIAPIKEY", c.GUIAPIKey)
 	}
 
-	if options.HideConsole {
+	if c.HideConsole {
 		osutil.HideConsole()
 	}
 
-	// Not set as default above because the strings can be really long.
-	err := cmdutil.SetConfigDataLocationsFromFlags(options.HomeDir, options.ConfDir, options.DataDir)
-	if err != nil {
-		l.Warnln("Command line options:", err)
-		os.Exit(svcutil.ExitError.AsInt())
-	}
-
 	// Treat an explicitly empty log file name as no log file
-	if options.LogFile == "" {
+	if c.LogFile == "" {
-		options.LogFile = "-"
+		c.LogFile = "-"
 	}
-	if options.LogFile != "default" {
+	if c.LogFile != "default" {
 		// We must set this *after* expandLocations above.
-		if err := locations.Set(locations.LogFile, options.LogFile); err != nil {
+		if err := locations.Set(locations.LogFile, c.LogFile); err != nil {
 			l.Warnln("Setting log file path:", err)
 			os.Exit(svcutil.ExitError.AsInt())
 		}
 	}
 
-	if options.DebugGUIAssetsDir != "" {
+	if c.DebugGUIAssetsDir != "" {
 		// The asset dir is blank if STGUIASSETS wasn't set, in which case we
 		// should look for extra assets in the default place.
-		if err := locations.Set(locations.GUIAssets, options.DebugGUIAssetsDir); err != nil {
+		if err := locations.Set(locations.GUIAssets, c.DebugGUIAssetsDir); err != nil {
 			l.Warnln("Setting GUI assets path:", err)
 			os.Exit(svcutil.ExitError.AsInt())
 		}
@@ -293,10 +299,10 @@ func (options serveOptions) Run() error {
 		os.Exit(svcutil.ExitError.AsInt())
 	}
 
-	if options.InternalInnerProcess {
+	if c.InternalInnerProcess {
-		syncthingMain(options)
+		c.syncthingMain()
 	} else {
-		monitorMain(options)
+		c.monitorMain()
 	}
 	return nil
 }
@@ -405,14 +411,14 @@ func upgradeViaRest() error {
 	return err
 }
 
-func syncthingMain(options serveOptions) {
+func (c *serveCmd) syncthingMain() {
-	if options.DebugProfileBlock {
+	if c.DebugProfileBlock {
 		startBlockProfiler()
 	}
-	if options.DebugProfileHeap {
+	if c.DebugProfileHeap {
 		startHeapProfiler()
 	}
-	if options.DebugPerfStats {
+	if c.DebugPerfStats {
 		startPerfStats()
 	}
 
@@ -457,7 +463,7 @@ func syncthingMain(options serveOptions) {
 	evLogger := events.NewLogger()
 	earlyService.Add(evLogger)
 
-	cfgWrapper, err := syncthing.LoadConfigAtStartup(locations.Get(locations.ConfigFile), cert, evLogger, options.AllowNewerConfig, options.NoDefaultFolder, options.NoPortProbing)
+	cfgWrapper, err := syncthing.LoadConfigAtStartup(locations.Get(locations.ConfigFile), cert, evLogger, c.AllowNewerConfig, c.NoDefaultFolder, c.NoPortProbing)
 	if err != nil {
 		l.Warnln("Failed to initialize config:", err)
 		os.Exit(svcutil.ExitError.AsInt())
@@ -468,7 +474,7 @@ func syncthingMain(options serveOptions) {
 	// unless we are in a build where it's disabled or the STNOUPGRADE
 	// environment variable is set.
 
-	if build.IsCandidate && !upgrade.DisabledByCompilation && !options.NoUpgrade {
+	if build.IsCandidate && !upgrade.DisabledByCompilation && !c.NoUpgrade {
 		cfgWrapper.Modify(func(cfg *config.Configuration) {
 			l.Infoln("Automatic upgrade is always enabled for candidate releases.")
 			if cfg.Options.AutoUpgradeIntervalH == 0 || cfg.Options.AutoUpgradeIntervalH > 24 {
@@ -495,7 +501,7 @@ func syncthingMain(options serveOptions) {
 	// Check if auto-upgrades is possible, and if yes, and it's enabled do an initial
 	// upgrade immediately. The auto-upgrade routine can only be started
 	// later after App is initialised.
-	autoUpgradePossible := autoUpgradePossible(options)
+	autoUpgradePossible := c.autoUpgradePossible()
 	if autoUpgradePossible && cfgWrapper.Options().AutoUpgradeEnabled() {
 		// try to do upgrade directly and log the error if relevant.
 		miscDB := db.NewMiscDB(sdb)
@@ -515,21 +521,21 @@ func syncthingMain(options serveOptions) {
 		}
 	}
 
-	if options.Unpaused {
+	if c.Unpaused {
 		setPauseState(cfgWrapper, false)
-	} else if options.Paused {
+	} else if c.Paused {
 		setPauseState(cfgWrapper, true)
 	}
 
 	appOpts := syncthing.Options{
-		NoUpgrade:             options.NoUpgrade,
+		NoUpgrade:             c.NoUpgrade,
-		ProfilerAddr:          options.DebugProfilerListen,
+		ProfilerAddr:          c.DebugProfilerListen,
-		ResetDeltaIdxs:        options.DebugResetDeltaIdxs,
+		ResetDeltaIdxs:        c.DebugResetDeltaIdxs,
-		Verbose:               options.Verbose,
+		Verbose:               c.Verbose,
-		DBMaintenanceInterval: options.DBMaintenanceInterval,
+		DBMaintenanceInterval: c.DBMaintenanceInterval,
 	}
-	if options.Audit {
+	if c.Audit {
-		appOpts.AuditWriter = auditWriter(options.AuditFile)
+		appOpts.AuditWriter = auditWriter(c.AuditFile)
 	}
 
 	app, err := syncthing.New(cfgWrapper, sdb, evLogger, cert, appOpts)
@@ -544,7 +550,7 @@ func syncthingMain(options serveOptions) {
 
 	setupSignalHandling(app)
 
-	if options.DebugProfileCPU {
+	if c.DebugProfileCPU {
 		f, err := os.Create(fmt.Sprintf("cpu-%d.pprof", os.Getpid()))
 		if err != nil {
 			l.Warnln("Creating profile:", err)
@@ -562,7 +568,7 @@ func syncthingMain(options serveOptions) {
 
 	cleanConfigDirectory()
 
-	if cfgWrapper.Options().StartBrowser && !options.NoBrowser && !options.InternalRestarting {
+	if cfgWrapper.Options().StartBrowser && !c.NoBrowser && !c.InternalRestarting {
 		// Can potentially block if the utility we are invoking doesn't
 		// fork, and just execs, hence keep it in its own routine.
 		go func() { _ = openURL(cfgWrapper.GUI().URL()) }()
@@ -574,7 +580,7 @@ func syncthingMain(options serveOptions) {
 		l.Warnln("Syncthing stopped with error:", app.Error())
 	}
 
-	if options.DebugProfileCPU {
+	if c.DebugProfileCPU {
 		pprof.StopCPUProfile()
 	}
 
@@ -648,15 +654,11 @@ func auditWriter(auditFile string) io.Writer {
 	return fd
 }
 
-func resetDB() error {
+func (c *serveCmd) autoUpgradePossible() bool {
-	return os.RemoveAll(locations.Get(locations.Database))
-}
-
-func autoUpgradePossible(options serveOptions) bool {
 	if upgrade.DisabledByCompilation {
 		return false
 	}
-	if options.NoUpgrade {
+	if c.NoUpgrade {
 		l.Infof("No automatic upgrades; STNOUPGRADE environment variable defined.")
 		return false
 	}
@@ -921,10 +923,33 @@ type debugCmd struct {
 type resetDatabaseCmd struct{}
 
 func (resetDatabaseCmd) Run() error {
-	if err := resetDB(); err != nil {
+	l.Infoln("Removing database in", locations.Get(locations.Database))
+	if err := os.RemoveAll(locations.Get(locations.Database)); err != nil {
 		l.Warnln("Resetting database:", err)
 		os.Exit(svcutil.ExitError.AsInt())
 	}
 	l.Infoln("Successfully reset database - it will be rebuilt after next start.")
 	return nil
 }
+
+func setConfigDataLocationsFromFlags(homeDir, confDir, dataDir string) error {
+	homeSet := homeDir != ""
+	confSet := confDir != ""
+	dataSet := dataDir != ""
+	switch {
+	case dataSet != confSet:
+		return errors.New("either both or none of --config and --data must be given, use --home to set both at once")
+	case homeSet && dataSet:
+		return errors.New("--home must not be used together with --config and --data")
+	case homeSet:
+		confDir = homeDir
+		dataDir = homeDir
+		fallthrough
+	case dataSet:
+		if err := locations.SetBaseDir(locations.ConfigBaseDir, confDir); err != nil {
+			return err
+		}
+		return locations.SetBaseDir(locations.DataBaseDir, dataDir)
+	}
+	return nil
+}

cmd/syncthing/monitor.go

@@ -43,7 +43,7 @@ const (
 	panicUploadNoticeWait = 10 * time.Second
 )
 
-func monitorMain(options serveOptions) {
+func (c *serveCmd) monitorMain() {
 	l.SetPrefix("[monitor] ")
 
 	var dst io.Writer = os.Stdout
@@ -58,8 +58,8 @@ func monitorMain(options serveOptions) {
 		open := func(name string) (io.WriteCloser, error) {
 			return newAutoclosedFile(name, logFileAutoCloseDelay, logFileMaxOpenTime)
 		}
-		if options.LogMaxSize > 0 {
+		if c.LogMaxSize > 0 {
-			fileDst, err = newRotatedFile(logFile, open, int64(options.LogMaxSize), options.LogMaxFiles)
+			fileDst, err = newRotatedFile(logFile, open, int64(c.LogMaxSize), c.LogMaxFiles)
 		} else {
 			fileDst, err = open(logFile)
 		}
@@ -178,7 +178,7 @@ func monitorMain(options serveOptions) {
 
 		if exiterr, ok := err.(*exec.ExitError); ok {
 			exitCode := exiterr.ExitCode()
-			if stopped || options.NoRestart {
+			if stopped || c.NoRestart {
 				os.Exit(exitCode)
 			}
 			if exitCode == svcutil.ExitUpgrade.AsInt() {
@@ -192,7 +192,7 @@ func monitorMain(options serveOptions) {
 			}
 		}
 
-		if options.NoRestart {
+		if c.NoRestart {
 			os.Exit(svcutil.ExitError.AsInt())
 		}