Merge branch 'master' into db/trans

* master:
  lib/model: Handle progress emitter zero interval (fixes syncthing#6281) (syncthing#6282)
  build(deps): bump github.com/pkg/errors from 0.9.0 to 0.9.1 (syncthing#6279)
  cmd/syncthing: Always use monitor process (fixes syncthing#4774, fixes syncthing#5786) (syncthing#6278)
  lib/syncthing: Wait for actual termination on Stop() (syncthing#6277)
  lib/model: Remove legacy handling of symlinks (syncthing#6276)
  lib/model: Return paused summary instead of error on paused folders (syncthing#6272)
  lib/config: Add some info to the folder marker missing (ref syncthing#5207) (syncthing#6270)

cmd/syncthing/main.go

@@ -47,6 +47,7 @@ import (
 const (
 	tlsDefaultCommonName   = "syncthing"
 	deviceCertLifetimeDays = 20 * 365
+	sigTerm                = syscall.Signal(15)
 )
 
 const (
@@ -225,7 +226,7 @@ func parseCommandLineOptions() RuntimeOptions {
 	flag.BoolVar(&options.Verbose, "verbose", false, "Print verbose log output")
 	flag.BoolVar(&options.paused, "paused", false, "Start with all devices and folders paused")
 	flag.BoolVar(&options.unpaused, "unpaused", false, "Start with all devices and folders unpaused")
-	flag.StringVar(&options.logFile, "logfile", options.logFile, "Log file name (still always logs to stdout). Cannot be used together with -no-restart/STNORESTART environment variable.")
+	flag.StringVar(&options.logFile, "logfile", options.logFile, "Log file name (still always logs to stdout).")
 	flag.IntVar(&options.logMaxSize, "log-max-size", options.logMaxSize, "Maximum size of any file (zero to disable log rotation).")
 	flag.IntVar(&options.logMaxFiles, "log-max-old-files", options.logMaxFiles, "Number of old files to keep (zero to keep only current).")
 	flag.StringVar(&options.auditFile, "auditfile", options.auditFile, "Specify audit file (use \"-\" for stdout, \"--\" for stderr)")
@@ -260,15 +261,6 @@ func main() {
 		os.Setenv("STGUIAPIKEY", options.guiAPIKey)
 	}
 
-	// Check for options which are not compatible with each other. We have
-	// to check logfile before it's set to the default below - we only want
-	// to complain if they set -logfile explicitly, not if it's set to its
-	// default location
-	if options.noRestart && (options.logFile != "" && options.logFile != "-") {
-		l.Warnln("-logfile may not be used with -no-restart or STNORESTART")
-		os.Exit(syncthing.ExitError.AsInt())
-	}
-
 	if options.hideConsole {
 		osutil.HideConsole()
 	}
@@ -381,7 +373,7 @@ func main() {
 		return
 	}
 
-	if innerProcess || options.noRestart {
+	if innerProcess {
 		syncthingMain(options)
 	} else {
 		monitorMain(options)
@@ -677,7 +669,6 @@ func setupSignalHandling(app *syncthing.App) {
 	// Exit with "success" code (no restart) on INT/TERM
 
 	stopSign := make(chan os.Signal, 1)
-	sigTerm := syscall.Signal(15)
 	signal.Notify(stopSign, os.Interrupt, sigTerm)
 	go func() {
 		<-stopSign

cmd/syncthing/monitor.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/locations"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
@@ -50,6 +51,9 @@ func monitorMain(runtimeOptions RuntimeOptions) {
 
 	logFile := runtimeOptions.logFile
 	if logFile != "-" {
+		if expanded, err := fs.ExpandTilde(logFile); err == nil {
+			logFile = expanded
+		}
 		var fileDst io.Writer
 		if runtimeOptions.logMaxSize > 0 {
 			open := func(name string) (io.WriteCloser, error) {
@@ -79,7 +83,6 @@ func monitorMain(runtimeOptions RuntimeOptions) {
 	var restarts [countRestarts]time.Time
 
 	stopSign := make(chan os.Signal, 1)
-	sigTerm := syscall.Signal(15)
 	signal.Notify(stopSign, os.Interrupt, sigTerm)
 	restartSign := make(chan os.Signal, 1)
 	sigHup := syscall.Signal(1)
@@ -111,7 +114,7 @@ func monitorMain(runtimeOptions RuntimeOptions) {
 			panic(err)
 		}
 
-		l.Infoln("Starting syncthing")
+		l.Debugln("Starting syncthing")
 		err = cmd.Start()
 		if err != nil {
 			l.Warnln("Error starting the main Syncthing process:", err)
@@ -144,35 +147,47 @@ func monitorMain(runtimeOptions RuntimeOptions) {
 			exit <- cmd.Wait()
 		}()
 
+		stopped := false
 		select {
 		case s := <-stopSign:
 			l.Infof("Signal %d received; exiting", s)
 			cmd.Process.Signal(sigTerm)
-			<-exit
-			return
+			err = <-exit
+			stopped = true
 
 		case s := <-restartSign:
 			l.Infof("Signal %d received; restarting", s)
 			cmd.Process.Signal(sigHup)
 			err = <-exit
 
 		case err = <-exit:
-			if err == nil {
-				// Successful exit indicates an intentional shutdown
-				return
-			} else if exiterr, ok := err.(*exec.ExitError); ok {
-				if exiterr.ExitCode() == syncthing.ExitUpgrade.AsInt() {
-					// Restart the monitor process to release the .old
-					// binary as part of the upgrade process.
-					l.Infoln("Restarting monitor...")
-					if err = restartMonitor(args); err != nil {
-						l.Warnln("Restart:", err)
-					}
-					return
+		}
+
+		if err == nil {
+			// Successful exit indicates an intentional shutdown
+			os.Exit(syncthing.ExitSuccess.AsInt())
+		}
+
+		if exiterr, ok := err.(*exec.ExitError); ok {
+			exitCode := exiterr.ExitCode()
+			if stopped || runtimeOptions.noRestart {
+				os.Exit(exitCode)
+			}
+			if exitCode == syncthing.ExitUpgrade.AsInt() {
+				// Restart the monitor process to release the .old
+				// binary as part of the upgrade process.
+				l.Infoln("Restarting monitor...")
+				if err = restartMonitor(args); err != nil {
+					l.Warnln("Restart:", err)
 				}
+				os.Exit(exitCode)
 			}
 		}
 
+		if runtimeOptions.noRestart {
+			os.Exit(syncthing.ExitError.AsInt())
+		}
+
 		l.Infoln("Syncthing exited:", err)
 		time.Sleep(1 * time.Second)
 

etc/linux-desktop/syncthing-start.desktop

@@ -2,7 +2,7 @@
 Name=Start Syncthing
 GenericName=File synchronization
 Comment=Starts the main syncthing process in the background.
-Exec=/usr/bin/syncthing -no-browser
+Exec=/usr/bin/syncthing -no-browser -logfile=~/.config/syncthing/syncthing.log
 Icon=syncthing
 Terminal=false
 Type=Application

go.mod

@@ -30,7 +30,7 @@ require (
 	github.com/onsi/gomega v1.6.0 // indirect
 	github.com/oschwald/geoip2-golang v1.4.0
 	github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 // indirect
-	github.com/pkg/errors v0.9.0
+	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.2.1
 	github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563
 	github.com/sasha-s/go-deadlock v0.2.0

go.sum

@@ -143,6 +143,8 @@ github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.9.0 h1:J8lpUdobwIeCI7OiSxHqEwJUKvJwicL5+3v1oe2Yb4k=
 github.com/pkg/errors v0.9.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=

lib/config/folderconfiguration.go

@@ -23,7 +23,7 @@ import (
 var (
 	ErrPathNotDirectory = errors.New("folder path not a directory")
 	ErrPathMissing      = errors.New("folder path missing")
-	ErrMarkerMissing    = errors.New("folder marker missing")
+	ErrMarkerMissing    = errors.New("folder marker missing (this indicates potential data loss, search docs/forum to get information about how to proceed)")
 )
 
 const DefaultMarkerName = ".stfolder"

lib/model/folder_summary.go

@@ -83,7 +83,8 @@ func (c *folderSummaryService) Summary(folder string) (map[string]interface{}, e
 	}
 
 	errors, err := c.model.FolderErrors(folder)
-	if err != nil {
+	if err != nil && err != ErrFolderPaused && err != errFolderNotRunning {
+		// Stats from the db can still be obtained if the folder is just paused/being started
 		return nil, err
 	}
 	res["errors"] = len(errors)

lib/model/model.go

@@ -34,7 +34,6 @@ import (
 	"github.com/syncthing/syncthing/lib/scanner"
 	"github.com/syncthing/syncthing/lib/stats"
 	"github.com/syncthing/syncthing/lib/sync"
-	"github.com/syncthing/syncthing/lib/upgrade"
 	"github.com/syncthing/syncthing/lib/util"
 	"github.com/syncthing/syncthing/lib/versioner"
 )
@@ -982,7 +981,6 @@ func (m *model) ClusterConfig(deviceID protocol.DeviceID, cm protocol.ClusterCon
 	m.pmut.RLock()
 	conn, ok := m.conn[deviceID]
 	closed := m.closed[deviceID]
-	hello := m.helloMessages[deviceID]
 	m.pmut.RUnlock()
 	if !ok {
 		panic("bug: ClusterConfig called on closed or nonexistent connection")
@@ -991,14 +989,6 @@ func (m *model) ClusterConfig(deviceID protocol.DeviceID, cm protocol.ClusterCon
 	changed := false
 	deviceCfg := m.cfg.Devices()[deviceID]
 
-	// See issue #3802 - in short, we can't send modern symlink entries to older
-	// clients.
-	dropSymlinks := false
-	if hello.ClientName == m.clientName && upgrade.CompareVersions(hello.ClientVersion, "v0.14.14") < 0 {
-		l.Warnln("Not sending symlinks to old client", deviceID, "- please upgrade to v0.14.14 or newer")
-		dropSymlinks = true
-	}
-
 	// Needs to happen outside of the fmut, as can cause CommitConfiguration
 	if deviceCfg.AutoAcceptFolders {
 		for _, folder := range cm.Folders {
@@ -1119,7 +1109,6 @@ func (m *model) ClusterConfig(deviceID protocol.DeviceID, cm protocol.ClusterCon
 			folder:       folder.ID,
 			fset:         fs,
 			prevSequence: startSequence,
-			dropSymlinks: dropSymlinks,
 			evLogger:     m.evLogger,
 		}
 		is.Service = util.AsService(is.serve, is.String())
@@ -1871,7 +1860,6 @@ type indexSender struct {
 	dev          string
 	fset         *db.FileSet
 	prevSequence int64
-	dropSymlinks bool
 	evLogger     events.Logger
 	connClosed   chan struct{}
 }
@@ -1981,14 +1969,6 @@ func (s *indexSender) sendIndexTo(ctx context.Context) error {
 		}
 		f.LocalFlags = 0 // never sent externally
 
-		if s.dropSymlinks && f.IsSymlink() {
-			// Do not send index entries with symlinks to clients that can't
-			// handle it. Fixes issue #3802. Once both sides are upgraded, a
-			// rescan (i.e., change) of the symlink is required for it to
-			// sync again, due to delta indexes.
-			return true
-		}
-
 		batch.append(f)
 		return true
 	})

lib/model/model_test.go

@@ -1610,7 +1610,7 @@ func TestROScanRecovery(t *testing.T) {
 
 	testOs.Mkdir(fcfg.Path, 0700)
 
-	waitForState(t, sub, "default", "folder marker missing")
+	waitForState(t, sub, "default", config.ErrMarkerMissing.Error())
 
 	fd := testOs.Create(filepath.Join(fcfg.Path, config.DefaultMarkerName))
 	fd.Close()
@@ -1619,7 +1619,7 @@ func TestROScanRecovery(t *testing.T) {
 
 	testOs.Remove(filepath.Join(fcfg.Path, config.DefaultMarkerName))
 
-	waitForState(t, sub, "default", "folder marker missing")
+	waitForState(t, sub, "default", config.ErrMarkerMissing.Error())
 
 	testOs.Remove(fcfg.Path)
 
@@ -1663,7 +1663,7 @@ func TestRWScanRecovery(t *testing.T) {
 
 	testOs.Mkdir(fcfg.Path, 0700)
 
-	waitForState(t, sub, "default", "folder marker missing")
+	waitForState(t, sub, "default", config.ErrMarkerMissing.Error())
 
 	fd := testOs.Create(filepath.Join(fcfg.Path, config.DefaultMarkerName))
 	fd.Close()
@@ -1672,7 +1672,7 @@ func TestRWScanRecovery(t *testing.T) {
 
 	testOs.Remove(filepath.Join(fcfg.Path, config.DefaultMarkerName))
 
-	waitForState(t, sub, "default", "folder marker missing")
+	waitForState(t, sub, "default", config.ErrMarkerMissing.Error())
 
 	testOs.Remove(fcfg.Path)
 

lib/model/progressemitter.go

@@ -197,27 +197,30 @@ func (t *ProgressEmitter) VerifyConfiguration(from, to config.Configuration) err
 }
 
 // CommitConfiguration implements the config.Committer interface
-func (t *ProgressEmitter) CommitConfiguration(from, to config.Configuration) bool {
+func (t *ProgressEmitter) CommitConfiguration(_, to config.Configuration) bool {
 	t.mut.Lock()
 	defer t.mut.Unlock()
 
-	switch {
-	case t.disabled && to.Options.ProgressUpdateIntervalS >= 0:
-		t.disabled = false
-		l.Debugln("progress emitter: enabled")
-		fallthrough
-	case !t.disabled && from.Options.ProgressUpdateIntervalS != to.Options.ProgressUpdateIntervalS:
-		t.interval = time.Duration(to.Options.ProgressUpdateIntervalS) * time.Second
-		if t.interval < time.Second {
-			t.interval = time.Second
+	newInterval := time.Duration(to.Options.ProgressUpdateIntervalS) * time.Second
+	if newInterval > 0 {
+		if t.disabled {
+			t.disabled = false
+			l.Debugln("progress emitter: enabled")
 		}
-		l.Debugln("progress emitter: updated interval", t.interval)
-	case !t.disabled && to.Options.ProgressUpdateIntervalS < 0:
+		if t.interval != newInterval {
+			t.interval = newInterval
+			l.Debugln("progress emitter: updated interval", t.interval)
+		}
+	} else if !t.disabled {
 		t.clearLocked()
 		t.disabled = true
 		l.Debugln("progress emitter: disabled")
 	}
 	t.minBlocks = to.Options.TempIndexMinBlocks
+	if t.interval < time.Second {
+		// can't happen when we're not disabled, but better safe than sorry.
+		t.interval = time.Second
+	}
 
 	return true
 }

lib/model/progressemitter_test.go

@@ -62,7 +62,7 @@ func TestProgressEmitter(t *testing.T) {
 	c := createTmpWrapper(config.Configuration{})
 	defer os.Remove(c.ConfigPath())
 	c.SetOptions(config.OptionsConfiguration{
-		ProgressUpdateIntervalS: 0,
+		ProgressUpdateIntervalS: 60, // irrelevant, but must be positive
 	})
 
 	p := NewProgressEmitter(c, evLogger)
@@ -112,7 +112,7 @@ func TestSendDownloadProgressMessages(t *testing.T) {
 	c := createTmpWrapper(config.Configuration{})
 	defer os.Remove(c.ConfigPath())
 	c.SetOptions(config.OptionsConfiguration{
-		ProgressUpdateIntervalS: 0,
+		ProgressUpdateIntervalS: 60, // irrelevant, but must be positive
 		TempIndexMinBlocks:      10,
 	})
 

lib/syncthing/syncthing.go

@@ -404,6 +404,7 @@ func (a *App) stopWithErr(stopReason ExitStatus, err error) ExitStatus {
 		a.err = err
 		close(a.stop)
 	})
+	<-a.stopped
 	return a.exitStatus
 }