From d10e81fb3d2c69b7c8595773c3aba9c3ea624c29 Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Thu, 17 Dec 2015 17:40:46 -0500 Subject: [PATCH 1/5] Remove most global variables from main.go This takes advantage of the newly created parseCommandLineOptions() function and makes it work so that it now returns a nice struct of options rather than relying on global variables. There are a few global variables left, but they will take a bit more refactoring in order to be removed, so it'll happen in later commits. --- cmd/syncthing/main.go | 169 +++++++++++++++++++++------------------ cmd/syncthing/monitor.go | 3 +- 2 files changed, 93 insertions(+), 79 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 27b7923c7..f75fbdfa8 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -118,7 +118,6 @@ func init() { var ( myID protocol.DeviceID - confDir string logFlags = log.Ltime stop = make(chan int) cert tls.Certificate @@ -190,8 +189,14 @@ The following are valid values for the STTRACE variable: %s` ) -// Command line and environment options +// Environment options var ( + noUpgrade = os.Getenv("STNOUPGRADE") != "" + innerProcess = os.Getenv("STNORESTART") != "" || os.Getenv("STMONITORED") != "" +) + +type RuntimeOptions struct { + confDir string reset bool showVersion bool doUpgrade bool @@ -207,132 +212,140 @@ var ( guiAddress string guiAPIKey string generateDir string - noRestart = os.Getenv("STNORESTART") != "" - noUpgrade = os.Getenv("STNOUPGRADE") != "" - profiler = os.Getenv("STPROFILER") - guiAssets = os.Getenv("STGUIASSETS") - cpuProfile = os.Getenv("STCPUPROFILE") != "" - stRestarting = os.Getenv("STRESTART") != "" - innerProcess = os.Getenv("STNORESTART") != "" || os.Getenv("STMONITORED") != "" -) + noRestart bool + profiler string + guiAssets string + cpuProfile bool + stRestarting bool +} -func parseCommandLineOptions() { - if runtime.GOOS == "windows" { - // On Windows, we use a log file by default. Setting the -logfile flag - // to "-" disables this behavior. - flag.StringVar(&logFile, "logfile", "", "Log file name (use \"-\" for stdout)") - - // We also add an option to hide the console window - flag.BoolVar(&noConsole, "no-console", false, "Hide console window") - } else { - flag.StringVar(&logFile, "logfile", "-", "Log file name (use \"-\" for stdout)") +func defaultRuntimeOptions() RuntimeOptions { + options := RuntimeOptions{ + noRestart: os.Getenv("STNORESTART") != "", + profiler: os.Getenv("STPROFILER"), + guiAssets: os.Getenv("STGUIASSETS"), + cpuProfile: os.Getenv("STCPUPROFILE") != "", + stRestarting: os.Getenv("STRESTART") != "", + logFile: "-", // Output to stdout } - flag.StringVar(&generateDir, "generate", "", "Generate key and config in specified dir, then exit") - flag.StringVar(&guiAddress, "gui-address", guiAddress, "Override GUI address (e.g. \"http://192.0.2.42:8443\")") - flag.StringVar(&guiAPIKey, "gui-apikey", guiAPIKey, "Override GUI API key") - flag.StringVar(&confDir, "home", "", "Set configuration directory") + if options.guiAssets != "" { + options.guiAssets = locations[locGUIAssets] + } + + if runtime.GOOS == "windows" { + options.logFile = locations[locLogFile] + } + + return options +} + +func parseCommandLineOptions() RuntimeOptions { + options := defaultRuntimeOptions() + + flag.StringVar(&options.generateDir, "generate", "", "Generate key and config in specified dir, then exit") + flag.StringVar(&options.guiAddress, "gui-address", options.guiAddress, "Override GUI address (e.g. \"http://192.0.2.42:8443\")") + flag.StringVar(&options.guiAPIKey, "gui-apikey", options.guiAPIKey, "Override GUI API key") + flag.StringVar(&options.confDir, "home", "", "Set configuration directory") flag.IntVar(&logFlags, "logflags", logFlags, "Select information in log line prefix (see below)") - flag.BoolVar(&noBrowser, "no-browser", false, "Do not start browser") - flag.BoolVar(&browserOnly, "browser-only", false, "Open GUI in browser") - flag.BoolVar(&noRestart, "no-restart", noRestart, "Do not restart; just exit") - flag.BoolVar(&reset, "reset", false, "Reset the database") - flag.BoolVar(&doUpgrade, "upgrade", false, "Perform upgrade") - flag.BoolVar(&doUpgradeCheck, "upgrade-check", false, "Check for available upgrade") - flag.BoolVar(&showVersion, "version", false, "Show version") - flag.StringVar(&upgradeTo, "upgrade-to", upgradeTo, "Force upgrade directly from specified URL") - flag.BoolVar(&auditEnabled, "audit", false, "Write events to audit file") - flag.BoolVar(&verbose, "verbose", false, "Print verbose log output") - flag.BoolVar(&paused, "paused", false, "Start with all devices paused") + flag.BoolVar(&options.noBrowser, "no-browser", false, "Do not start browser") + flag.BoolVar(&options.browserOnly, "browser-only", false, "Open GUI in browser") + flag.BoolVar(&options.noRestart, "no-restart", options.noRestart, "Do not restart; just exit") + flag.BoolVar(&options.reset, "reset", false, "Reset the database") + flag.BoolVar(&options.doUpgrade, "upgrade", false, "Perform upgrade") + flag.BoolVar(&options.doUpgradeCheck, "upgrade-check", false, "Check for available upgrade") + flag.BoolVar(&options.showVersion, "version", false, "Show version") + flag.StringVar(&options.upgradeTo, "upgrade-to", options.upgradeTo, "Force upgrade directly from specified URL") + flag.BoolVar(&options.auditEnabled, "audit", false, "Write events to audit file") + flag.BoolVar(&options.verbose, "verbose", false, "Print verbose log output") + flag.BoolVar(&options.paused, "paused", false, "Start with all devices paused") + flag.StringVar(&options.logFile, "logfile", options.logFile, "Log file name (use \"-\" for stdout)") + if runtime.GOOS == "windows" { + // Allow user to hide the console window + flag.BoolVar(&options.noConsole, "no-console", false, "Hide console window") + } longUsage := fmt.Sprintf(extraUsage, baseDirs["config"], debugFacilities()) flag.Usage = usageFor(flag.CommandLine, usage, longUsage) flag.Parse() + + return options } func main() { - parseCommandLineOptions() + options := parseCommandLineOptions() - if guiAddress != "" { + if options.guiAddress != "" { // The config picks this up from the environment. - os.Setenv("STGUIADDRESS", guiAddress) + os.Setenv("STGUIADDRESS", options.guiAddress) } - if guiAPIKey != "" { + if options.guiAPIKey != "" { // The config picks this up from the environment. - os.Setenv("STGUIAPIKEY", guiAPIKey) + os.Setenv("STGUIAPIKEY", options.guiAPIKey) } - if noConsole { + if options.noConsole { osutil.HideConsole() } - if confDir != "" { + if options.confDir != "" { // Not set as default above because the string can be really long. - baseDirs["config"] = confDir + baseDirs["config"] = options.confDir } if err := expandLocations(); err != nil { l.Fatalln(err) } - if guiAssets == "" { - guiAssets = locations[locGUIAssets] - } - - if logFile == "" { - // Use the default log file location - logFile = locations[locLogFile] - } - - if showVersion { + if options.showVersion { fmt.Println(LongVersion) return } - if browserOnly { + if options.browserOnly { openGUI() return } l.SetFlags(logFlags) - if generateDir != "" { - generate(generateDir) + if options.generateDir != "" { + generate(options.generateDir) return } // Ensure that our home directory exists. ensureDir(baseDirs["config"], 0700) - if upgradeTo != "" { - err := upgrade.ToURL(upgradeTo) + if options.upgradeTo != "" { + err := upgrade.ToURL(options.upgradeTo) if err != nil { l.Fatalln("Upgrade:", err) // exits 1 } - l.Okln("Upgraded from", upgradeTo) + l.Okln("Upgraded from", options.upgradeTo) return } - if doUpgradeCheck { + if options.doUpgradeCheck { checkUpgrade() return } - if doUpgrade { + if options.doUpgrade { release := checkUpgrade() performUpgrade(release) return } - if reset { + if options.reset { resetDB() return } - if noRestart { - syncthingMain() + if options.noRestart { + syncthingMain(options) } else { - monitorMain() + monitorMain(options) } } @@ -494,7 +507,7 @@ func upgradeViaRest() error { return err } -func syncthingMain() { +func syncthingMain(runtimeOptions RuntimeOptions) { setupSignalHandling() // Create a main service manager. We'll add things to this as we go along. @@ -510,11 +523,11 @@ func syncthingMain() { // lines look ugly. l.SetPrefix("[start] ") - if auditEnabled { + if runtimeOptions.auditEnabled { startAuditing(mainSvc) } - if verbose { + if runtimeOptions.verbose { mainSvc.Add(newVerboseSvc()) } @@ -594,11 +607,11 @@ func syncthingMain() { l.Fatalln("Short device IDs are in conflict. Unlucky!\n Regenerate the device ID of one if the following:\n ", err) } - if len(profiler) > 0 { + if len(runtimeOptions.profiler) > 0 { go func() { - l.Debugln("Starting profiler on", profiler) + l.Debugln("Starting profiler on", runtimeOptions.profiler) runtime.SetBlockProfileRate(1) - err := http.ListenAndServe(profiler, nil) + err := http.ListenAndServe(runtimeOptions.profiler, nil) if err != nil { l.Fatalln(err) } @@ -693,7 +706,7 @@ func syncthingMain() { m.StartDeadlockDetector(20 * time.Minute) } - if paused { + if runtimeOptions.paused { for device := range cfg.Devices() { m.PauseDevice(device) } @@ -798,14 +811,14 @@ func syncthingMain() { // GUI - setupGUI(mainSvc, cfg, m, apiSub, cachedDiscovery, relaySvc, errors, systemLog) + setupGUI(mainSvc, cfg, m, apiSub, cachedDiscovery, relaySvc, errors, systemLog, runtimeOptions) // Start connection management connectionSvc := connections.NewConnectionSvc(cfg, myID, m, tlsCfg, cachedDiscovery, relaySvc, bepProtocolName, tlsDefaultCommonName, lans) mainSvc.Add(connectionSvc) - if cpuProfile { + if runtimeOptions.cpuProfile { f, err := os.Create(fmt.Sprintf("cpu-%d.pprof", os.Getpid())) if err != nil { log.Fatal(err) @@ -867,7 +880,7 @@ func syncthingMain() { l.Okln("Exiting") - if cpuProfile { + if runtimeOptions.cpuProfile { pprof.StopCPUProfile() } @@ -952,7 +965,7 @@ func startAuditing(mainSvc *suture.Supervisor) { l.Infoln("Audit log in", auditFile) } -func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, apiSub *events.BufferedSubscription, discoverer *discover.CachingMux, relaySvc *relay.Svc, errors, systemLog *logger.Recorder) { +func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, apiSub *events.BufferedSubscription, discoverer *discover.CachingMux, relaySvc *relay.Svc, errors, systemLog *logger.Recorder, runtimeOptions RuntimeOptions) { guiCfg := cfg.GUI() if !guiCfg.Enabled { @@ -963,14 +976,14 @@ func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, a l.Warnln("Insecure admin access is enabled.") } - api, err := newAPISvc(myID, cfg, guiAssets, m, apiSub, discoverer, relaySvc, errors, systemLog) + api, err := newAPISvc(myID, cfg, runtimeOptions.guiAssets, m, apiSub, discoverer, relaySvc, errors, systemLog) if err != nil { l.Fatalln("Cannot start GUI:", err) } cfg.Subscribe(api) mainSvc.Add(api) - if cfg.Options().StartBrowser && !noBrowser && !stRestarting { + if cfg.Options().StartBrowser && !runtimeOptions.noBrowser && !runtimeOptions.stRestarting { // Can potentially block if the utility we are invoking doesn't // fork, and just execs, hence keep it in it's own routine. go openURL(guiCfg.URL()) diff --git a/cmd/syncthing/monitor.go b/cmd/syncthing/monitor.go index b56bb2bee..2c10bf003 100644 --- a/cmd/syncthing/monitor.go +++ b/cmd/syncthing/monitor.go @@ -34,13 +34,14 @@ const ( logFileMaxOpenTime = time.Minute ) -func monitorMain() { +func monitorMain(runtimeOptions RuntimeOptions) { os.Setenv("STNORESTART", "yes") os.Setenv("STMONITORED", "yes") l.SetPrefix("[monitor] ") var dst io.Writer = os.Stdout + logFile := runtimeOptions.logFile if logFile != "-" { var fileDst io.Writer = newAutoclosedFile(logFile, logFileAutoCloseDelay, logFileMaxOpenTime) From 490962ccdb10f54746dcd97985fbbb6ea19b826f Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Fri, 18 Dec 2015 11:34:39 -0500 Subject: [PATCH 2/5] Move logFlags into RuntimeOptions --- cmd/syncthing/main.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index f75fbdfa8..f5afae76f 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -110,18 +110,13 @@ func init() { date := BuildDate.UTC().Format("2006-01-02 15:04:05 MST") LongVersion = fmt.Sprintf(`syncthing %s "%s" (%s %s-%s) %s@%s %s`, Version, Codename, runtime.Version(), runtime.GOOS, runtime.GOARCH, BuildUser, BuildHost, date) - - if os.Getenv("STTRACE") != "" { - logFlags = log.Ltime | log.Ldate | log.Lmicroseconds | log.Lshortfile - } } var ( - myID protocol.DeviceID - logFlags = log.Ltime - stop = make(chan int) - cert tls.Certificate - lans []*net.IPNet + myID protocol.DeviceID + stop = make(chan int) + cert tls.Certificate + lans []*net.IPNet ) const ( @@ -217,6 +212,7 @@ type RuntimeOptions struct { guiAssets string cpuProfile bool stRestarting bool + logFlags int } func defaultRuntimeOptions() RuntimeOptions { @@ -227,12 +223,17 @@ func defaultRuntimeOptions() RuntimeOptions { cpuProfile: os.Getenv("STCPUPROFILE") != "", stRestarting: os.Getenv("STRESTART") != "", logFile: "-", // Output to stdout + logFlags: log.Ltime, } if options.guiAssets != "" { options.guiAssets = locations[locGUIAssets] } + if os.Getenv("STTRACE") != "" { + options.logFlags = log.Ltime | log.Ldate | log.Lmicroseconds | log.Lshortfile + } + if runtime.GOOS == "windows" { options.logFile = locations[locLogFile] } @@ -247,7 +248,7 @@ func parseCommandLineOptions() RuntimeOptions { flag.StringVar(&options.guiAddress, "gui-address", options.guiAddress, "Override GUI address (e.g. \"http://192.0.2.42:8443\")") flag.StringVar(&options.guiAPIKey, "gui-apikey", options.guiAPIKey, "Override GUI API key") flag.StringVar(&options.confDir, "home", "", "Set configuration directory") - flag.IntVar(&logFlags, "logflags", logFlags, "Select information in log line prefix (see below)") + flag.IntVar(&options.logFlags, "logflags", options.logFlags, "Select information in log line prefix (see below)") flag.BoolVar(&options.noBrowser, "no-browser", false, "Do not start browser") flag.BoolVar(&options.browserOnly, "browser-only", false, "Open GUI in browser") flag.BoolVar(&options.noRestart, "no-restart", options.noRestart, "Do not restart; just exit") @@ -274,6 +275,7 @@ func parseCommandLineOptions() RuntimeOptions { func main() { options := parseCommandLineOptions() + l.SetFlags(options.logFlags) if options.guiAddress != "" { // The config picks this up from the environment. @@ -307,8 +309,6 @@ func main() { return } - l.SetFlags(logFlags) - if options.generateDir != "" { generate(options.generateDir) return From b01496755095d2832a5d0409da67df197d49e33e Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Fri, 18 Dec 2015 12:00:59 -0500 Subject: [PATCH 3/5] Rename guiAssets to assetDir for consistency --- cmd/syncthing/main.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index f5afae76f..860ce1949 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -209,7 +209,7 @@ type RuntimeOptions struct { generateDir string noRestart bool profiler string - guiAssets string + assetDir string cpuProfile bool stRestarting bool logFlags int @@ -219,15 +219,15 @@ func defaultRuntimeOptions() RuntimeOptions { options := RuntimeOptions{ noRestart: os.Getenv("STNORESTART") != "", profiler: os.Getenv("STPROFILER"), - guiAssets: os.Getenv("STGUIASSETS"), + assetDir: os.Getenv("STGUIASSETS"), cpuProfile: os.Getenv("STCPUPROFILE") != "", stRestarting: os.Getenv("STRESTART") != "", logFile: "-", // Output to stdout logFlags: log.Ltime, } - if options.guiAssets != "" { - options.guiAssets = locations[locGUIAssets] + if options.assetDir != "" { + options.assetDir = locations[locGUIAssets] } if os.Getenv("STTRACE") != "" { @@ -976,7 +976,7 @@ func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, a l.Warnln("Insecure admin access is enabled.") } - api, err := newAPISvc(myID, cfg, runtimeOptions.guiAssets, m, apiSub, discoverer, relaySvc, errors, systemLog) + api, err := newAPISvc(myID, cfg, runtimeOptions.assetDir, m, apiSub, discoverer, relaySvc, errors, systemLog) if err != nil { l.Fatalln("Cannot start GUI:", err) } From 1dc894087c636a3d0e329f8b637b5bf62d34168d Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Fri, 18 Dec 2015 12:28:02 -0500 Subject: [PATCH 4/5] Rename noConsole to hideConsole This avoids the double negative of having noConsole = false to represent not hiding the console. It is also consistent with the action performed by osutils. --- cmd/syncthing/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 860ce1949..1ea2880ee 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -199,7 +199,7 @@ type RuntimeOptions struct { upgradeTo string noBrowser bool browserOnly bool - noConsole bool + hideConsole bool logFile string auditEnabled bool verbose bool @@ -263,7 +263,7 @@ func parseCommandLineOptions() RuntimeOptions { flag.StringVar(&options.logFile, "logfile", options.logFile, "Log file name (use \"-\" for stdout)") if runtime.GOOS == "windows" { // Allow user to hide the console window - flag.BoolVar(&options.noConsole, "no-console", false, "Hide console window") + flag.BoolVar(&options.hideConsole, "no-console", false, "Hide console window") } longUsage := fmt.Sprintf(extraUsage, baseDirs["config"], debugFacilities()) @@ -286,7 +286,7 @@ func main() { os.Setenv("STGUIAPIKEY", options.guiAPIKey) } - if options.noConsole { + if options.hideConsole { osutil.HideConsole() } From e54036be2509b1d860180be4eedbd039e3ce9952 Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Fri, 18 Dec 2015 12:47:26 -0500 Subject: [PATCH 5/5] Reuse existing ensureDir function --- cmd/syncthing/main.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 1ea2880ee..29adaf0ef 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -366,17 +366,7 @@ func generate(generateDir string) { if err != nil { l.Fatalln("generate:", err) } - - info, err := os.Stat(dir) - if err == nil && !info.IsDir() { - l.Fatalln(dir, "is not a directory") - } - if err != nil && os.IsNotExist(err) { - err = osutil.MkdirAll(dir, 0700) - if err != nil { - l.Fatalln("generate:", err) - } - } + ensureDir(dir, 0700) certFile, keyFile := filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem") cert, err := tls.LoadX509KeyPair(certFile, keyFile)