lib: Folder marker is now a folder

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4341
LGTM: calmh

cmd/stcompdirs/main.go

@@ -88,10 +88,10 @@ func startWalker(dir string, res chan<- fileInfo, abort <-chan struct{}) chan er
 		}
 
 		rn, _ := filepath.Rel(dir, path)
-		if rn == "." || rn == ".stfolder" {
+		if rn == "." {
 			return nil
 		}
-		if rn == ".stversions" {
+		if rn == ".stversions" || rn == ".stfolder" {
 			return filepath.SkipDir
 		}
 

lib/config/config.go

@@ -32,7 +32,7 @@ import (
 
 const (
 	OldestHandledVersion = 10
-	CurrentVersion       = 22
+	CurrentVersion       = 23
 	MaxRescanIntervalS   = 365 * 24 * 60 * 60
 )
 
@@ -323,6 +323,9 @@ func (cfg *Configuration) clean() error {
 	if cfg.Version == 21 {
 		convertV21V22(cfg)
 	}
+	if cfg.Version == 22 {
+		convertV22V23(cfg)
+	}
 
 	// Build a list of available devices
 	existingDevices := make(map[protocol.DeviceID]bool)
@@ -372,6 +375,33 @@ func (cfg *Configuration) clean() error {
 	return nil
 }
 
+func convertV22V23(cfg *Configuration) {
+	permBits := fs.FileMode(0777)
+	if runtime.GOOS == "windows" {
+		// Windows has no umask so we must chose a safer set of bits to
+		// begin with.
+		permBits = 0700
+	}
+	for i := range cfg.Folders {
+		fs := cfg.Folders[i].Filesystem()
+		// Invalid config posted, or tests.
+		if fs == nil {
+			continue
+		}
+		if stat, err := fs.Stat(".stfolder"); err == nil && !stat.IsDir() {
+			err = fs.Remove(".stfolder")
+			if err == nil {
+				err = fs.Mkdir(".stfolder", permBits)
+			}
+			if err != nil {
+				l.Fatalln("failed to upgrade folder marker:", err)
+			}
+		}
+	}
+
+	cfg.Version = 23
+}
+
 func convertV21V22(cfg *Configuration) {
 	for i := range cfg.Folders {
 		cfg.Folders[i].FilesystemType = fs.FilesystemTypeBasic

lib/config/config_test.go

@@ -86,7 +86,7 @@ func TestDefaultValues(t *testing.T) {
 
 func TestDeviceConfig(t *testing.T) {
 	for i := OldestHandledVersion; i <= CurrentVersion; i++ {
-		os.Remove("testdata/.stfolder")
+		os.RemoveAll("testdata/.stfolder")
 		wr, err := Load(fmt.Sprintf("testdata/v%d.xml", i), device1)
 		if err != nil {
 			t.Fatal(err)

lib/config/folderconfiguration.go

@@ -81,12 +81,17 @@ func (f FolderConfiguration) Filesystem() fs.Filesystem {
 
 func (f *FolderConfiguration) CreateMarker() error {
 	if !f.HasMarker() {
+		permBits := fs.FileMode(0777)
+		if runtime.GOOS == "windows" {
+			// Windows has no umask so we must chose a safer set of bits to
+			// begin with.
+			permBits = 0700
+		}
 		fs := f.Filesystem()
-		fd, err := fs.Create(".stfolder")
+		err := fs.Mkdir(".stfolder", permBits)
 		if err != nil {
 			return err
 		}
-		fd.Close()
 		if dir, err := fs.Open("."); err == nil {
 			if serr := dir.Sync(); err != nil {
 				l.Infof("fsync %q failed: %v", ".", serr)

lib/config/testdata/v23.xml

@@ -0,0 +1,16 @@
+<configuration version="22">
+    <folder id="test" path="testdata" type="readonly" ignorePerms="false" rescanIntervalS="600" autoNormalize="true">
+        <filesystemType>basic</filesystemType>
+        <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR"></device>
+        <device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2"></device>
+        <minDiskFree unit="%">1</minDiskFree>
+        <maxConflicts>-1</maxConflicts>
+        <fsync>true</fsync>
+    </folder>
+    <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR" name="node one" compression="metadata">
+        <address>tcp://a</address>
+    </device>
+    <device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2" name="node two" compression="metadata">
+        <address>tcp://b</address>
+    </device>
+</configuration>

lib/fs/filesystem.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 )
 
@@ -133,3 +134,20 @@ func NewFilesystem(fsType FilesystemType, uri string) Filesystem {
 	}
 	return fs
 }
+
+// IsInternal returns true if the file, as a path relative to the folder
+// root, represents an internal file that should always be ignored. The file
+// path must be clean (i.e., in canonical shortest form).
+func IsInternal(file string) bool {
+	internals := []string{".stfolder", ".stignore", ".stversions"}
+	pathSep := string(PathSeparator)
+	for _, internal := range internals {
+		if file == internal {
+			return true
+		}
+		if strings.HasPrefix(file, internal+pathSep) {
+			return true
+		}
+	}
+	return false
+}

lib/fs/filesystem_test.go

@@ -0,0 +1,43 @@
+// Copyright (C) 2017 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 fs
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func TestIsInternal(t *testing.T) {
+	cases := []struct {
+		file     string
+		internal bool
+	}{
+		{".stfolder", true},
+		{".stignore", true},
+		{".stversions", true},
+		{".stfolder/foo", true},
+		{".stignore/foo", true},
+		{".stversions/foo", true},
+
+		{".stfolderfoo", false},
+		{".stignorefoo", false},
+		{".stversionsfoo", false},
+		{"foo.stfolder", false},
+		{"foo.stignore", false},
+		{"foo.stversions", false},
+		{"foo/.stfolder", false},
+		{"foo/.stignore", false},
+		{"foo/.stversions", false},
+	}
+
+	for _, tc := range cases {
+		res := IsInternal(filepath.FromSlash(tc.file))
+		if res != tc.internal {
+			t.Errorf("Unexpected result: IsInteral(%q): %v should be %v", tc.file, res, tc.internal)
+		}
+	}
+}

lib/ignore/tempname.go → lib/fs/tempname.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 ignore
+package fs
 
 import (
 	"crypto/md5"

lib/ignore/tempname_test.go → lib/fs/tempname_test.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 ignore
+package fs
 
 import (
 	"strings"

lib/ignore/ignore.go

@@ -278,10 +278,10 @@ func (m *Matcher) clean(d time.Duration) {
 // ShouldIgnore returns true when a file is temporary, internal or ignored
 func (m *Matcher) ShouldIgnore(filename string) bool {
 	switch {
-	case IsTemporary(filename):
+	case fs.IsTemporary(filename):
 		return true
 
-	case IsInternal(filename):
+	case fs.IsInternal(filename):
 		return true
 
 	case m.Match(filename).IsIgnored():
@@ -458,23 +458,6 @@ func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd Chan
 	return lines, patterns, nil
 }
 
-// IsInternal returns true if the file, as a path relative to the folder
-// root, represents an internal file that should always be ignored. The file
-// path must be clean (i.e., in canonical shortest form).
-func IsInternal(file string) bool {
-	internals := []string{".stfolder", ".stignore", ".stversions"}
-	pathSep := string(fs.PathSeparator)
-	for _, internal := range internals {
-		if file == internal {
-			return true
-		}
-		if strings.HasPrefix(file, internal+pathSep) {
-			return true
-		}
-	}
-	return false
-}
-
 // WriteIgnores is a convenience function to avoid code duplication
 func WriteIgnores(filesystem fs.Filesystem, path string, content []string) error {
 	fd, err := osutil.CreateAtomicFilesystem(filesystem, path)

lib/ignore/ignore_test.go

@@ -837,37 +837,6 @@ func TestGobwasGlobIssue18(t *testing.T) {
 	}
 }
 
-func TestIsInternal(t *testing.T) {
-	cases := []struct {
-		file     string
-		internal bool
-	}{
-		{".stfolder", true},
-		{".stignore", true},
-		{".stversions", true},
-		{".stfolder/foo", true},
-		{".stignore/foo", true},
-		{".stversions/foo", true},
-
-		{".stfolderfoo", false},
-		{".stignorefoo", false},
-		{".stversionsfoo", false},
-		{"foo.stfolder", false},
-		{"foo.stignore", false},
-		{"foo.stversions", false},
-		{"foo/.stfolder", false},
-		{"foo/.stignore", false},
-		{"foo/.stversions", false},
-	}
-
-	for _, tc := range cases {
-		res := IsInternal(filepath.FromSlash(tc.file))
-		if res != tc.internal {
-			t.Errorf("Unexpected result: IsInteral(%q): %v should be %v", tc.file, res, tc.internal)
-		}
-	}
-}
-
 func TestRoot(t *testing.T) {
 	stignore := `
 	!/a

lib/model/model.go

@@ -336,7 +336,7 @@ func (m *Model) RemoveFolder(folder string) {
 	// Delete syncthing specific files
 	folderCfg := m.folderCfgs[folder]
 	fs := folderCfg.Filesystem()
-	fs.Remove(".stfolder")
+	fs.RemoveAll(".stfolder")
 
 	m.tearDownFolderLocked(folder)
 	// Remove it from the database
@@ -1156,7 +1156,7 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset
 	// acceptable relative to "folderPath" and in canonical form, so we can
 	// trust it.
 
-	if ignore.IsInternal(name) {
+	if fs.IsInternal(name) {
 		l.Debugf("%v REQ(in) for internal file: %s: %q / %q o=%d s=%d", m, deviceID, folder, name, offset, len(buf))
 		return protocol.ErrNoSuchFile
 	}
@@ -1174,7 +1174,7 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset
 	// Only check temp files if the flag is set, and if we are set to advertise
 	// the temp indexes.
 	if fromTemporary && !folderCfg.DisableTempIndexes {
-		tempFn := ignore.TempName(name)
+		tempFn := fs.TempName(name)
 
 		if info, err := folderFs.Lstat(tempFn); err != nil || !info.IsRegular() {
 			// Reject reads for anything that doesn't exist or is something
@@ -2552,7 +2552,7 @@ func unifySubs(dirs []string, exists func(dir string) bool) []string {
 func trimUntilParentKnown(dirs []string, exists func(dir string) bool) []string {
 	var subs []string
 	for _, sub := range dirs {
-		for sub != "" && !ignore.IsInternal(sub) {
+		for sub != "" && !fs.IsInternal(sub) {
 			sub = filepath.Clean(sub)
 			parent := filepath.Dir(sub)
 			if parent == "." || exists(parent) {

lib/model/model_test.go

@@ -1026,7 +1026,8 @@ func changeIgnores(t *testing.T, m *Model, expected []string) {
 
 func TestIgnores(t *testing.T) {
 	// Assure a clean start state
-	ioutil.WriteFile("testdata/.stfolder", nil, 0644)
+	os.RemoveAll("testdata/.stfolder")
+	os.MkdirAll("testdata/.stfolder", 0644)
 	ioutil.WriteFile("testdata/.stignore", []byte(".*\nquux\n"), 0644)
 
 	db := db.OpenMemory()

lib/model/rwfolder.go

@@ -760,7 +760,7 @@ func (f *sendReceiveFolder) deleteDir(file protocol.FileInfo, matcher *ignore.Ma
 	files, _ := f.fs.DirNames(file.Name)
 	for _, dirFile := range files {
 		fullDirFile := filepath.Join(file.Name, dirFile)
-		if ignore.IsTemporary(dirFile) || (matcher != nil && matcher.Match(fullDirFile).IsDeletable()) {
+		if fs.IsTemporary(dirFile) || (matcher != nil && matcher.Match(fullDirFile).IsDeletable()) {
 			f.fs.RemoveAll(fullDirFile)
 		}
 	}
@@ -988,7 +988,7 @@ func (f *sendReceiveFolder) handleFile(file protocol.FileInfo, copyChan chan<- c
 		return
 	}
 
-	tempName := ignore.TempName(file.Name)
+	tempName := fs.TempName(file.Name)
 
 	scanner.PopulateOffsets(file.Blocks)
 

lib/model/rwfolder_test.go

@@ -17,7 +17,6 @@ import (
 
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/fs"
-	"github.com/syncthing/syncthing/lib/ignore"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/scanner"
 	"github.com/syncthing/syncthing/lib/sync"
@@ -27,15 +26,15 @@ func TestMain(m *testing.M) {
 	// We do this to make sure that the temp file required for the tests
 	// does not get removed during the tests. Also set the prefix so it's
 	// found correctly regardless of platform.
-	if ignore.TempPrefix != ignore.WindowsTempPrefix {
-		originalPrefix := ignore.TempPrefix
-		ignore.TempPrefix = ignore.WindowsTempPrefix
+	if fs.TempPrefix != fs.WindowsTempPrefix {
+		originalPrefix := fs.TempPrefix
+		fs.TempPrefix = fs.WindowsTempPrefix
 		defer func() {
-			ignore.TempPrefix = originalPrefix
+			fs.TempPrefix = originalPrefix
 		}()
 	}
 	future := time.Now().Add(time.Hour)
-	err := os.Chtimes(filepath.Join("testdata", ignore.TempName("file")), future, future)
+	err := os.Chtimes(filepath.Join("testdata", fs.TempName("file")), future, future)
 	if err != nil {
 		panic(err)
 	}
@@ -191,14 +190,14 @@ func TestCopierFinder(t *testing.T) {
 	// After dropping out blocks found locally:
 	// Pull: 1, 5, 6, 8
 
-	tempFile := filepath.Join("testdata", ignore.TempName("file2"))
+	tempFile := filepath.Join("testdata", fs.TempName("file2"))
 	err := os.Remove(tempFile)
 	if err != nil && !os.IsNotExist(err) {
 		t.Error(err)
 	}
 
 	existingBlocks := []int{0, 2, 3, 4, 0, 0, 7, 0}
-	existingFile := setUpFile(ignore.TempName("file"), existingBlocks)
+	existingFile := setUpFile(fs.TempName("file"), existingBlocks)
 	requiredFile := existingFile
 	requiredFile.Blocks = blocks[1:]
 	requiredFile.Name = "file2"
@@ -261,7 +260,7 @@ func TestCopierFinder(t *testing.T) {
 }
 
 func TestWeakHash(t *testing.T) {
-	tempFile := filepath.Join("testdata", ignore.TempName("weakhash"))
+	tempFile := filepath.Join("testdata", fs.TempName("weakhash"))
 	var shift int64 = 10
 	var size int64 = 1 << 20
 	expectBlocks := int(size / protocol.BlockSize)
@@ -466,12 +465,12 @@ func TestLastResortPulling(t *testing.T) {
 	}
 
 	(<-finisherChan).fd.Close()
-	os.Remove(filepath.Join("testdata", ignore.TempName("newfile")))
+	os.Remove(filepath.Join("testdata", fs.TempName("newfile")))
 }
 
 func TestDeregisterOnFailInCopy(t *testing.T) {
 	file := setUpFile("filex", []int{0, 2, 0, 0, 5, 0, 0, 8})
-	defer os.Remove("testdata/" + ignore.TempName("filex"))
+	defer os.Remove("testdata/" + fs.TempName("filex"))
 
 	db := db.OpenMemory()
 
@@ -545,7 +544,7 @@ func TestDeregisterOnFailInCopy(t *testing.T) {
 
 func TestDeregisterOnFailInPull(t *testing.T) {
 	file := setUpFile("filex", []int{0, 2, 0, 0, 5, 0, 0, 8})
-	defer os.Remove("testdata/" + ignore.TempName("filex"))
+	defer os.Remove("testdata/" + fs.TempName("filex"))
 
 	db := db.OpenMemory()
 	m := NewModel(defaultConfig, protocol.LocalDeviceID, "syncthing", "dev", db, nil)

lib/scanner/walk.go

@@ -230,7 +230,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 			return skip
 		}
 
-		if ignore.IsTemporary(path) {
+		if fs.IsTemporary(path) {
 			l.Debugln("temporary:", path)
 			if info.IsRegular() && info.ModTime().Add(w.TempLifetime).Before(now) {
 				w.Filesystem.Remove(path)
@@ -239,7 +239,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 			return nil
 		}
 
-		if ignore.IsInternal(path) {
+		if fs.IsInternal(path) {
 			l.Debugln("ignored (internal):", path)
 			return skip
 		}