lib/db, lib/fs, lib/model: Introduce fs.MtimeFS, remove VirtualMtimeRepo

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3479

.gitattributes

@@ -6,3 +6,4 @@ vendor/**   -text=auto
 
 # Diffs on these files are meaningless
 *.svg           -diff
+*.pb.go         -diff

lib/db/leveldb_dbinstance.go

@@ -719,13 +719,28 @@ func (db *Instance) indexIDKey(device, folder []byte) []byte {
 	return k
 }
 
+func (db *Instance) mtimesKey(folder []byte) []byte {
+	prefix := make([]byte, 5) // key type + 4 bytes folder idx number
+	prefix[0] = KeyTypeVirtualMtime
+	binary.BigEndian.PutUint32(prefix[1:], db.folderIdx.ID(folder))
+	return prefix
+}
+
 // DropDeltaIndexIDs removes all index IDs from the database. This will
 // cause a full index transmission on the next connection.
 func (db *Instance) DropDeltaIndexIDs() {
+	db.dropPrefix([]byte{KeyTypeIndexID})
+}
+
+func (db *Instance) dropMtimes(folder []byte) {
+	db.dropPrefix(db.mtimesKey(folder))
+}
+
+func (db *Instance) dropPrefix(prefix []byte) {
 	t := db.newReadWriteTransaction()
 	defer t.close()
 
-	dbi := t.NewIterator(util.BytesPrefix([]byte{KeyTypeIndexID}), nil)
+	dbi := t.NewIterator(util.BytesPrefix(prefix), nil)
 	defer dbi.Release()
 
 	for dbi.Next() {

lib/db/set.go

@@ -16,6 +16,7 @@ import (
 	stdsync "sync"
 	"sync/atomic"
 
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/sync"
@@ -283,6 +284,12 @@ func (s *FileSet) SetIndexID(device protocol.DeviceID, id protocol.IndexID) {
 	s.db.setIndexID(device[:], []byte(s.folder), id)
 }
 
+func (s *FileSet) MtimeFS() *fs.MtimeFS {
+	prefix := s.db.mtimesKey([]byte(s.folder))
+	kv := NewNamespacedKV(s.db, string(prefix))
+	return fs.NewMtimeFS(kv)
+}
+
 // maxSequence returns the highest of the Sequence numbers found in
 // the given slice of FileInfos. This should really be the Sequence of
 // the last item, but Syncthing v0.14.0 and other implementations may not
@@ -301,12 +308,12 @@ func maxSequence(fs []protocol.FileInfo) int64 {
 // database.
 func DropFolder(db *Instance, folder string) {
 	db.dropFolder([]byte(folder))
+	db.dropMtimes([]byte(folder))
 	bm := &BlockMap{
 		db:     db,
 		folder: db.folderIdx.ID([]byte(folder)),
 	}
 	bm.Drop()
-	NewVirtualMtimeRepo(db, folder).Drop()
 }
 
 func normalizeFilenames(fs []protocol.FileInfo) {

lib/db/virtualmtime.go

@@ -1,79 +0,0 @@
-// Copyright (C) 2015 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 http://mozilla.org/MPL/2.0/.
-
-package db
-
-import (
-	"encoding/binary"
-	"fmt"
-	"time"
-)
-
-// This type encapsulates a repository of mtimes for platforms where file mtimes
-// can't be set to arbitrary values.  For this to work, we need to store both
-// the mtime we tried to set (the "actual" mtime) as well as the mtime the file
-// has when we're done touching it (the "disk" mtime) so that we can tell if it
-// was changed.  So in GetMtime(), it's not sufficient that the record exists --
-// the argument must also equal the "disk" mtime in the record, otherwise it's
-// been touched locally and the "disk" mtime is actually correct.
-
-type VirtualMtimeRepo struct {
-	ns *NamespacedKV
-}
-
-func NewVirtualMtimeRepo(ldb *Instance, folder string) *VirtualMtimeRepo {
-	var prefix [5]byte // key type + 4 bytes folder idx number
-	prefix[0] = KeyTypeVirtualMtime
-	binary.BigEndian.PutUint32(prefix[1:], ldb.folderIdx.ID([]byte(folder)))
-
-	return &VirtualMtimeRepo{
-		ns: NewNamespacedKV(ldb, string(prefix[:])),
-	}
-}
-
-func (r *VirtualMtimeRepo) UpdateMtime(path string, diskMtime, actualMtime time.Time) {
-	l.Debugf("virtual mtime: storing values for path:%s disk:%v actual:%v", path, diskMtime, actualMtime)
-
-	diskBytes, _ := diskMtime.MarshalBinary()
-	actualBytes, _ := actualMtime.MarshalBinary()
-
-	data := append(diskBytes, actualBytes...)
-
-	r.ns.PutBytes(path, data)
-}
-
-func (r *VirtualMtimeRepo) GetMtime(path string, diskMtime time.Time) time.Time {
-	data, exists := r.ns.Bytes(path)
-	if !exists {
-		// Absence of debug print is significant enough in itself here
-		return diskMtime
-	}
-
-	var mtime time.Time
-	if err := mtime.UnmarshalBinary(data[:len(data)/2]); err != nil {
-		panic(fmt.Sprintf("Can't unmarshal stored mtime at path %s: %v", path, err))
-	}
-
-	if mtime.Equal(diskMtime) {
-		if err := mtime.UnmarshalBinary(data[len(data)/2:]); err != nil {
-			panic(fmt.Sprintf("Can't unmarshal stored mtime at path %s: %v", path, err))
-		}
-
-		l.Debugf("virtual mtime: return %v instead of %v for path: %s", mtime, diskMtime, path)
-		return mtime
-	}
-
-	l.Debugf("virtual mtime: record exists, but mismatch inDisk: %v dbDisk: %v for path: %s", diskMtime, mtime, path)
-	return diskMtime
-}
-
-func (r *VirtualMtimeRepo) DeleteMtime(path string) {
-	r.ns.Delete(path)
-}
-
-func (r *VirtualMtimeRepo) Drop() {
-	r.ns.Reset()
-}

lib/db/virtualmtime_test.go

@@ -1,74 +0,0 @@
-// Copyright (C) 2015 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 http://mozilla.org/MPL/2.0/.
-
-package db
-
-import (
-	"testing"
-	"time"
-)
-
-func TestVirtualMtimeRepo(t *testing.T) {
-	ldb := OpenMemory()
-
-	// A few repos so we can ensure they don't pollute each other
-	repo1 := NewVirtualMtimeRepo(ldb, "folder1")
-	repo2 := NewVirtualMtimeRepo(ldb, "folder2")
-
-	// Since GetMtime() returns its argument if the key isn't found or is outdated, we need a dummy to test with.
-	dummyTime := time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC)
-
-	// Some times to test with
-	time1 := time.Date(2001, time.February, 3, 4, 5, 7, 0, time.UTC)
-	time2 := time.Date(2010, time.February, 3, 4, 5, 6, 0, time.UTC)
-
-	file1 := "file1.txt"
-
-	// Files are not present at the start
-
-	if v := repo1.GetMtime(file1, dummyTime); !v.Equal(dummyTime) {
-		t.Errorf("Mtime should be missing (%v) from repo 1 but it's %v", dummyTime, v)
-	}
-
-	if v := repo2.GetMtime(file1, dummyTime); !v.Equal(dummyTime) {
-		t.Errorf("Mtime should be missing (%v) from repo 2 but it's %v", dummyTime, v)
-	}
-
-	repo1.UpdateMtime(file1, time1, time2)
-
-	// Now it should return time2 only when time1 is passed as the argument
-
-	if v := repo1.GetMtime(file1, time1); !v.Equal(time2) {
-		t.Errorf("Mtime should be %v for disk time %v but we got %v", time2, time1, v)
-	}
-
-	if v := repo1.GetMtime(file1, dummyTime); !v.Equal(dummyTime) {
-		t.Errorf("Mtime should be %v for disk time %v but we got %v", dummyTime, dummyTime, v)
-	}
-
-	// repo2 shouldn't know about this file
-
-	if v := repo2.GetMtime(file1, time1); !v.Equal(time1) {
-		t.Errorf("Mtime should be %v for disk time %v in repo 2 but we got %v", time1, time1, v)
-	}
-
-	repo1.DeleteMtime(file1)
-
-	// Now it should be gone
-
-	if v := repo1.GetMtime(file1, time1); !v.Equal(time1) {
-		t.Errorf("Mtime should be %v for disk time %v but we got %v", time1, time1, v)
-	}
-
-	// Try again but with Drop()
-
-	repo1.UpdateMtime(file1, time1, time2)
-	repo1.Drop()
-
-	if v := repo1.GetMtime(file1, time1); !v.Equal(time1) {
-		t.Errorf("Mtime should be %v for disk time %v but we got %v", time1, time1, v)
-	}
-}

lib/fs/mtimefs.go

@@ -0,0 +1,139 @@
+// Copyright (C) 2016 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 http://mozilla.org/MPL/2.0/.
+
+//go:generate go run ../../script/protofmt.go mtime.proto
+//go:generate protoc --proto_name=../../../../../:../../../../gogo/protobuf/protobuf:. --gogofast_out=. mtime.proto
+
+package fs
+
+import (
+	"os"
+	"time"
+
+	"github.com/syncthing/syncthing/lib/osutil"
+)
+
+// The database is where we store the virtual mtimes
+type database interface {
+	Bytes(key string) (data []byte, ok bool)
+	PutBytes(key string, data []byte)
+	Delete(key string)
+}
+
+// variable so that we can mock it for testing
+var osChtimes = os.Chtimes
+
+// The MtimeFS is a filesystem with nanosecond mtime precision, regardless
+// of what shenanigans the underlying filesystem gets up to.
+type MtimeFS struct {
+	db database
+}
+
+func NewMtimeFS(db database) *MtimeFS {
+	return &MtimeFS{
+		db: db,
+	}
+}
+
+func (f *MtimeFS) Chtimes(name string, atime, mtime time.Time) error {
+	// Do a normal Chtimes call, don't care if it succeeds or not.
+	osChtimes(name, atime, mtime)
+
+	// Stat the file to see what happened. Here we *do* return an error,
+	// because it might be "does not exist" or similar. osutil.Lstat is the
+	// souped up version to account for Android breakage.
+	info, err := osutil.Lstat(name)
+	if err != nil {
+		return err
+	}
+
+	f.save(name, info.ModTime(), mtime)
+	return nil
+}
+
+func (f *MtimeFS) Lstat(name string) (os.FileInfo, error) {
+	info, err := osutil.Lstat(name)
+	if err != nil {
+		return nil, err
+	}
+
+	real, virtual := f.load(name)
+	if real == info.ModTime() {
+		info = mtimeFileInfo{
+			FileInfo: info,
+			mtime:    virtual,
+		}
+	}
+
+	return info, nil
+}
+
+// "real" is the on disk timestamp
+// "virtual" is what want the timestamp to be
+
+func (f *MtimeFS) save(name string, real, virtual time.Time) {
+	if real.Equal(virtual) {
+		// If the virtual time and the real on disk time are equal we don't
+		// need to store anything.
+		f.db.Delete(name)
+		return
+	}
+
+	mtime := dbMtime{
+		real:    real,
+		virtual: virtual,
+	}
+	bs, _ := mtime.Marshal() // Can't fail
+	f.db.PutBytes(name, bs)
+}
+
+func (f *MtimeFS) load(name string) (real, virtual time.Time) {
+	data, exists := f.db.Bytes(name)
+	if !exists {
+		return
+	}
+
+	var mtime dbMtime
+	if err := mtime.Unmarshal(data); err != nil {
+		return
+	}
+
+	return mtime.real, mtime.virtual
+}
+
+// The mtimeFileInfo is an os.FileInfo that lies about the ModTime().
+
+type mtimeFileInfo struct {
+	os.FileInfo
+	mtime time.Time
+}
+
+func (m mtimeFileInfo) ModTime() time.Time {
+	return m.mtime
+}
+
+// The dbMtime is our database representation
+
+type dbMtime struct {
+	real    time.Time
+	virtual time.Time
+}
+
+func (t *dbMtime) Marshal() ([]byte, error) {
+	bs0, _ := t.real.MarshalBinary()
+	bs1, _ := t.virtual.MarshalBinary()
+	return append(bs0, bs1...), nil
+}
+
+func (t *dbMtime) Unmarshal(bs []byte) error {
+	if err := t.real.UnmarshalBinary(bs[:len(bs)/2]); err != nil {
+		return err
+	}
+	if err := t.virtual.UnmarshalBinary(bs[len(bs)/2:]); err != nil {
+		return err
+	}
+	return nil
+}

lib/fs/mtimefs_test.go

@@ -0,0 +1,111 @@
+// Copyright (C) 2016 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 http://mozilla.org/MPL/2.0/.
+
+package fs
+
+import (
+	"errors"
+	"io/ioutil"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/syncthing/syncthing/lib/osutil"
+)
+
+func TestMtimeFS(t *testing.T) {
+	osutil.RemoveAll("testdata")
+	defer osutil.RemoveAll("testdata")
+	os.Mkdir("testdata", 0755)
+	ioutil.WriteFile("testdata/exists0", []byte("hello"), 0644)
+	ioutil.WriteFile("testdata/exists1", []byte("hello"), 0644)
+	ioutil.WriteFile("testdata/exists2", []byte("hello"), 0644)
+
+	// a random time with nanosecond precision
+	testTime := time.Unix(1234567890, 123456789)
+
+	mtimefs := NewMtimeFS(make(mapStore))
+
+	// Do one Chtimes call that will go through to the normal filesystem
+	osChtimes = os.Chtimes
+	if err := mtimefs.Chtimes("testdata/exists0", testTime, testTime); err != nil {
+		t.Error("Should not have failed:", err)
+	}
+
+	// Do one call that gets an error back from the underlying Chtimes
+	osChtimes = failChtimes
+	if err := mtimefs.Chtimes("testdata/exists1", testTime, testTime); err != nil {
+		t.Error("Should not have failed:", err)
+	}
+
+	// Do one call that gets struck by an exceptionally evil Chtimes
+	osChtimes = evilChtimes
+	if err := mtimefs.Chtimes("testdata/exists2", testTime, testTime); err != nil {
+		t.Error("Should not have failed:", err)
+	}
+
+	// All of the calls were successfull, so an Lstat on them should return
+	// the test timestamp.
+
+	for _, file := range []string{"testdata/exists0", "testdata/exists1", "testdata/exists2"} {
+		if info, err := mtimefs.Lstat(file); err != nil {
+			t.Error("Lstat shouldn't fail:", err)
+		} else if !info.ModTime().Equal(testTime) {
+			t.Errorf("Time mismatch; %v != expected %v", info.ModTime(), testTime)
+		}
+	}
+
+	// The two last files should certainly not have the correct timestamp
+	// when looking directly on disk though.
+
+	for _, file := range []string{"testdata/exists1", "testdata/exists2"} {
+		if info, err := os.Lstat(file); err != nil {
+			t.Error("Lstat shouldn't fail:", err)
+		} else if info.ModTime().Equal(testTime) {
+			t.Errorf("Unexpected time match; %v == %v", info.ModTime(), testTime)
+		}
+	}
+
+	// Changing the timestamp on disk should be reflected in a new Lstat
+	// call. Choose a time that is likely to be able to be on all reasonable
+	// filesystems.
+
+	testTime = time.Now().Add(5 * time.Hour).Truncate(time.Minute)
+	os.Chtimes("testdata/exists0", testTime, testTime)
+	if info, err := mtimefs.Lstat("testdata/exists0"); err != nil {
+		t.Error("Lstat shouldn't fail:", err)
+	} else if !info.ModTime().Equal(testTime) {
+		t.Errorf("Time mismatch; %v != expected %v", info.ModTime(), testTime)
+	}
+}
+
+// The mapStore is a simple database
+
+type mapStore map[string][]byte
+
+func (s mapStore) PutBytes(key string, data []byte) {
+	s[key] = data
+}
+
+func (s mapStore) Bytes(key string) (data []byte, ok bool) {
+	data, ok = s[key]
+	return
+}
+
+func (s mapStore) Delete(key string) {
+	delete(s, key)
+}
+
+// failChtimes does nothing, and fails
+func failChtimes(name string, mtime, atime time.Time) error {
+	return errors.New("no")
+}
+
+// evilChtimes will set an mtime that's 300 days in the future of what was
+// asked for, and truncate the time to the closest hour.
+func evilChtimes(name string, mtime, atime time.Time) error {
+	return os.Chtimes(name, mtime.Add(300*time.Hour).Truncate(time.Hour), atime.Add(300*time.Hour).Truncate(time.Hour))
+}

lib/model/model.go

@@ -28,6 +28,7 @@ import (
 	"github.com/syncthing/syncthing/lib/connections"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/ignore"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
@@ -100,7 +101,7 @@ type Model struct {
 	pmut              sync.RWMutex // protects the above
 }
 
-type folderFactory func(*Model, config.FolderConfiguration, versioner.Versioner) service
+type folderFactory func(*Model, config.FolderConfiguration, versioner.Versioner, *fs.MtimeFS) service
 
 var (
 	symlinkWarning  = stdsync.Once{}
@@ -230,7 +231,7 @@ func (m *Model) StartFolder(folder string) {
 		}
 	}
 
-	p := folderFactory(m, cfg, ver)
+	p := folderFactory(m, cfg, ver, fs.MtimeFS())
 	m.folderRunners[folder] = p
 
 	m.warnAboutOverwritingProtectedFiles(folder)
@@ -923,7 +924,7 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset
 		}
 	}
 
-	if info, err := os.Lstat(fn); err == nil && info.Mode()&os.ModeSymlink != 0 {
+	if info, err := osutil.Lstat(fn); err == nil && info.Mode()&os.ModeSymlink != 0 {
 		target, _, err := symlinks.Read(fn)
 		if err != nil {
 			l.Debugln("symlinks.Read:", err)
@@ -1522,6 +1523,7 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error
 	ignores := m.folderIgnores[folder]
 	runner, ok := m.folderRunners[folder]
 	m.fmut.Unlock()
+	mtimefs := fs.MtimeFS()
 
 	// Check if the ignore patterns changed as part of scanning this folder.
 	// If they did we should schedule a pull of the folder so that we
@@ -1579,7 +1581,7 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error
 		TempNamer:             defTempNamer,
 		TempLifetime:          time.Duration(m.cfg.Options().KeepTemporariesH) * time.Hour,
 		CurrentFiler:          cFiler{m, folder},
-		MtimeRepo:             db.NewVirtualMtimeRepo(m.db, folderCfg.ID),
+		Lstater:               mtimefs,
 		IgnorePerms:           folderCfg.IgnorePerms,
 		AutoNormalize:         folderCfg.AutoNormalize,
 		Hashers:               m.numHashers(folder),
@@ -1663,7 +1665,7 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error
 						Version:       f.Version, // The file is still the same, so don't bump version
 					}
 					batch = append(batch, nf)
-				} else if _, err := osutil.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil {
+				} else if _, err := mtimefs.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil {
 					// File has been deleted.
 
 					// We don't specifically verify that the error is

lib/model/rofolder.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 
 	"github.com/syncthing/syncthing/lib/config"
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/versioner"
 )
 
@@ -21,7 +22,7 @@ type roFolder struct {
 	folder
 }
 
-func newROFolder(model *Model, config config.FolderConfiguration, ver versioner.Versioner) service {
+func newROFolder(model *Model, config config.FolderConfiguration, _ versioner.Versioner, _ *fs.MtimeFS) service {
 	return &roFolder{
 		folder: folder{
 			stateTracker: newStateTracker(config.ID),

lib/model/rwfolder.go

@@ -21,6 +21,7 @@ import (
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/ignore"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
@@ -79,17 +80,17 @@ type dbUpdateJob struct {
 type rwFolder struct {
 	folder
 
-	virtualMtimeRepo *db.VirtualMtimeRepo
-	dir              string
-	versioner        versioner.Versioner
-	ignorePerms      bool
-	order            config.PullOrder
-	maxConflicts     int
-	sleep            time.Duration
-	pause            time.Duration
-	allowSparse      bool
-	checkFreeSpace   bool
-	ignoreDelete     bool
+	mtimeFS        *fs.MtimeFS
+	dir            string
+	versioner      versioner.Versioner
+	ignorePerms    bool
+	order          config.PullOrder
+	maxConflicts   int
+	sleep          time.Duration
+	pause          time.Duration
+	allowSparse    bool
+	checkFreeSpace bool
+	ignoreDelete   bool
 
 	copiers int
 	pullers int
@@ -105,7 +106,7 @@ type rwFolder struct {
 	initialScanCompleted chan (struct{}) // exposed for testing
 }
 
-func newRWFolder(model *Model, cfg config.FolderConfiguration, ver versioner.Versioner) service {
+func newRWFolder(model *Model, cfg config.FolderConfiguration, ver versioner.Versioner, mtimeFS *fs.MtimeFS) service {
 	f := &rwFolder{
 		folder: folder{
 			stateTracker: newStateTracker(cfg.ID),
@@ -114,17 +115,17 @@ func newRWFolder(model *Model, cfg config.FolderConfiguration, ver versioner.Ver
 			model:        model,
 		},
 
-		virtualMtimeRepo: db.NewVirtualMtimeRepo(model.db, cfg.ID),
-		dir:              cfg.Path(),
-		versioner:        ver,
-		ignorePerms:      cfg.IgnorePerms,
-		copiers:          cfg.Copiers,
-		pullers:          cfg.Pullers,
-		order:            cfg.Order,
-		maxConflicts:     cfg.MaxConflicts,
-		allowSparse:      !cfg.DisableSparseFiles,
-		checkFreeSpace:   cfg.MinDiskFreePct != 0,
-		ignoreDelete:     cfg.IgnoreDelete,
+		mtimeFS:        mtimeFS,
+		dir:            cfg.Path(),
+		versioner:      ver,
+		ignorePerms:    cfg.IgnorePerms,
+		copiers:        cfg.Copiers,
+		pullers:        cfg.Pullers,
+		order:          cfg.Order,
+		maxConflicts:   cfg.MaxConflicts,
+		allowSparse:    !cfg.DisableSparseFiles,
+		checkFreeSpace: cfg.MinDiskFreePct != 0,
+		ignoreDelete:   cfg.IgnoreDelete,
 
 		queue:       newJobQueue(),
 		pullTimer:   time.NewTimer(time.Second),
@@ -595,7 +596,7 @@ func (f *rwFolder) handleDir(file protocol.FileInfo) {
 		l.Debugf("need dir\n\t%v\n\t%v", file, curFile)
 	}
 
-	info, err := osutil.Lstat(realName)
+	info, err := f.mtimeFS.Lstat(realName)
 	switch {
 	// There is already something under that name, but it's a file/link.
 	// Most likely a file/link is getting replaced with a directory.
@@ -621,7 +622,7 @@ func (f *rwFolder) handleDir(file protocol.FileInfo) {
 			}
 
 			// Stat the directory so we can check its permissions.
-			info, err := osutil.Lstat(path)
+			info, err := f.mtimeFS.Lstat(path)
 			if err != nil {
 				return err
 			}
@@ -696,7 +697,7 @@ func (f *rwFolder) deleteDir(file protocol.FileInfo, matcher *ignore.Matcher) {
 	if err == nil || os.IsNotExist(err) {
 		// It was removed or it doesn't exist to start with
 		f.dbUpdates <- dbUpdateJob{file, dbUpdateDeleteDir}
-	} else if _, serr := os.Lstat(realName); serr != nil && !os.IsPermission(serr) {
+	} else if _, serr := f.mtimeFS.Lstat(realName); serr != nil && !os.IsPermission(serr) {
 		// We get an error just looking at the directory, and it's not a
 		// permission problem. Lets assume the error is in fact some variant
 		// of "file does not exist" (possibly expressed as some parent being a
@@ -745,7 +746,7 @@ func (f *rwFolder) deleteFile(file protocol.FileInfo) {
 	if err == nil || os.IsNotExist(err) {
 		// It was removed or it doesn't exist to start with
 		f.dbUpdates <- dbUpdateJob{file, dbUpdateDeleteFile}
-	} else if _, serr := os.Lstat(realName); serr != nil && !os.IsPermission(serr) {
+	} else if _, serr := f.mtimeFS.Lstat(realName); serr != nil && !os.IsPermission(serr) {
 		// We get an error just looking at the file, and it's not a permission
 		// problem. Lets assume the error is in fact some variant of "file
 		// does not exist" (possibly expressed as some parent being a file and
@@ -923,9 +924,8 @@ func (f *rwFolder) handleFile(file protocol.FileInfo, copyChan chan<- copyBlocks
 		// the database. If there's a mismatch here, there might be local
 		// changes that we don't know about yet and we should scan before
 		// touching the file. If we can't stat the file we'll just pull it.
-		if info, err := osutil.Lstat(realName); err == nil {
-			mtime := f.virtualMtimeRepo.GetMtime(file.Name, info.ModTime())
-			if mtime.Unix() != curFile.Modified || info.Size() != curFile.Size {
+		if info, err := f.mtimeFS.Lstat(realName); err == nil {
+			if info.ModTime().Unix() != curFile.Modified || info.Size() != curFile.Size {
 				l.Debugln("file modified but not rescanned; not pulling:", realName)
 				// Scan() is synchronous (i.e. blocks until the scan is
 				// completed and returns an error), but a scan can't happen
@@ -1045,17 +1045,7 @@ func (f *rwFolder) shortcutFile(file protocol.FileInfo) error {
 	}
 
 	t := time.Unix(file.Modified, 0)
-	if err := os.Chtimes(realName, t, t); err != nil {
-		// Try using virtual mtimes
-		info, err := os.Stat(realName)
-		if err != nil {
-			l.Infof("Puller (folder %q, file %q): shortcut: unable to stat file: %v", f.folderID, file.Name, err)
-			f.newError(file.Name, err)
-			return err
-		}
-
-		f.virtualMtimeRepo.UpdateMtime(file.Name, info.ModTime(), t)
-	}
+	f.mtimeFS.Chtimes(realName, t, t) // never fails
 
 	// This may have been a conflict. We should merge the version vectors so
 	// that our clock doesn't move backwards.
@@ -1258,16 +1248,9 @@ func (f *rwFolder) performFinish(state *sharedPullerState) error {
 
 	// Set the correct timestamp on the new file
 	t := time.Unix(state.file.Modified, 0)
-	if err := os.Chtimes(state.tempName, t, t); err != nil {
-		// Try using virtual mtimes instead
-		info, err := os.Stat(state.tempName)
-		if err != nil {
-			return err
-		}
-		f.virtualMtimeRepo.UpdateMtime(state.file.Name, info.ModTime(), t)
-	}
+	f.mtimeFS.Chtimes(state.tempName, t, t) // never fails
 
-	if stat, err := osutil.Lstat(state.realName); err == nil {
+	if stat, err := f.mtimeFS.Lstat(state.realName); err == nil {
 		// There is an old file or directory already in place. We need to
 		// handle that.
 

lib/scanner/walk.go

@@ -57,9 +57,8 @@ type Config struct {
 	TempLifetime time.Duration
 	// If CurrentFiler is not nil, it is queried for the current file before rescanning.
 	CurrentFiler CurrentFiler
-	// If MtimeRepo is not nil, it is used to provide mtimes on systems that
-	// don't support setting arbitrary mtimes.
-	MtimeRepo MtimeRepo
+	// The Lstater provides reliable mtimes on top of the regular filesystem.
+	Lstater Lstater
 	// If IgnorePerms is true, changes to permission bits will not be
 	// detected. Scanned files will get zero permission bits and the
 	// NoPermissionBits flag set.
@@ -88,10 +87,8 @@ type CurrentFiler interface {
 	CurrentFile(name string) (protocol.FileInfo, bool)
 }
 
-type MtimeRepo interface {
-	// GetMtime returns a (possibly modified) actual mtime given a file name
-	// and its on disk mtime.
-	GetMtime(relPath string, mtime time.Time) time.Time
+type Lstater interface {
+	Lstat(name string) (os.FileInfo, error)
 }
 
 func Walk(cfg Config) (chan protocol.FileInfo, error) {
@@ -103,8 +100,8 @@ func Walk(cfg Config) (chan protocol.FileInfo, error) {
 	if w.TempNamer == nil {
 		w.TempNamer = noTempNamer{}
 	}
-	if w.MtimeRepo == nil {
-		w.MtimeRepo = noMtimeRepo{}
+	if w.Lstater == nil {
+		w.Lstater = defaultLstater{}
 	}
 
 	return w.walk()
@@ -119,8 +116,7 @@ type walker struct {
 func (w *walker) walk() (chan protocol.FileInfo, error) {
 	l.Debugln("Walk", w.Dir, w.Subs, w.BlockSize, w.Matcher)
 
-	err := checkDir(w.Dir)
-	if err != nil {
+	if err := w.checkDir(); err != nil {
 		return nil, err
 	}
 
@@ -245,14 +241,18 @@ func (w *walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 			return nil
 		}
 
-		mtime := w.MtimeRepo.GetMtime(relPath, info.ModTime())
+		info, err = w.Lstater.Lstat(absPath)
+		// An error here would be weird as we've already gotten to this point, but act on it ninetheless
+		if err != nil {
+			return skip
+		}
 
 		if w.TempNamer.IsTemporary(relPath) {
 			// A temporary file
 			l.Debugln("temporary:", relPath)
-			if info.Mode().IsRegular() && mtime.Add(w.TempLifetime).Before(now) {
+			if info.Mode().IsRegular() && info.ModTime().Add(w.TempLifetime).Before(now) {
 				os.Remove(absPath)
-				l.Debugln("removing temporary:", relPath, mtime)
+				l.Debugln("removing temporary:", relPath, info.ModTime())
 			}
 			return nil
 		}
@@ -283,17 +283,17 @@ func (w *walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 			}
 
 		case info.Mode().IsDir():
-			err = w.walkDir(relPath, info, mtime, dchan)
+			err = w.walkDir(relPath, info, dchan)
 
 		case info.Mode().IsRegular():
-			err = w.walkRegular(relPath, info, mtime, fchan)
+			err = w.walkRegular(relPath, info, fchan)
 		}
 
 		return err
 	}
 }
 
-func (w *walker) walkRegular(relPath string, info os.FileInfo, mtime time.Time, fchan chan protocol.FileInfo) error {
+func (w *walker) walkRegular(relPath string, info os.FileInfo, fchan chan protocol.FileInfo) error {
 	curMode := uint32(info.Mode())
 	if runtime.GOOS == "windows" && osutil.IsWindowsExecutable(relPath) {
 		curMode |= 0111
@@ -310,12 +310,12 @@ func (w *walker) walkRegular(relPath string, info os.FileInfo, mtime time.Time,
 	//  - has the same size as previously
 	cf, ok := w.CurrentFiler.CurrentFile(relPath)
 	permUnchanged := w.IgnorePerms || !cf.HasPermissionBits() || PermsEqual(cf.Permissions, curMode)
-	if ok && permUnchanged && !cf.IsDeleted() && cf.Modified == mtime.Unix() && !cf.IsDirectory() &&
+	if ok && permUnchanged && !cf.IsDeleted() && cf.Modified == info.ModTime().Unix() && !cf.IsDirectory() &&
 		!cf.IsSymlink() && !cf.IsInvalid() && cf.Size == info.Size() {
 		return nil
 	}
 
-	l.Debugln("rescan:", cf, mtime.Unix(), info.Mode()&os.ModePerm)
+	l.Debugln("rescan:", cf, info.ModTime().Unix(), info.Mode()&os.ModePerm)
 
 	f := protocol.FileInfo{
 		Name:          relPath,
@@ -323,7 +323,7 @@ func (w *walker) walkRegular(relPath string, info os.FileInfo, mtime time.Time,
 		Version:       cf.Version.Update(w.ShortID),
 		Permissions:   curMode & uint32(maskModePerm),
 		NoPermissions: w.IgnorePerms,
-		Modified:      mtime.Unix(),
+		Modified:      info.ModTime().Unix(),
 		Size:          info.Size(),
 	}
 	l.Debugln("to hash:", relPath, f)
@@ -337,7 +337,7 @@ func (w *walker) walkRegular(relPath string, info os.FileInfo, mtime time.Time,
 	return nil
 }
 
-func (w *walker) walkDir(relPath string, info os.FileInfo, mtime time.Time, dchan chan protocol.FileInfo) error {
+func (w *walker) walkDir(relPath string, info os.FileInfo, dchan chan protocol.FileInfo) error {
 	// A directory is "unchanged", if it
 	//  - exists
 	//  - has the same permissions as previously, unless we are ignoring permissions
@@ -357,7 +357,7 @@ func (w *walker) walkDir(relPath string, info os.FileInfo, mtime time.Time, dcha
 		Version:       cf.Version.Update(w.ShortID),
 		Permissions:   uint32(info.Mode() & maskModePerm),
 		NoPermissions: w.IgnorePerms,
-		Modified:      mtime.Unix(),
+		Modified:      info.ModTime().Unix(),
 	}
 	l.Debugln("dir:", relPath, f)
 
@@ -457,7 +457,7 @@ func (w *walker) normalizePath(absPath, relPath string) (normPath string, skip b
 
 		// We will attempt to normalize it.
 		normalizedPath := filepath.Join(w.Dir, normPath)
-		if _, err := osutil.Lstat(normalizedPath); os.IsNotExist(err) {
+		if _, err := w.Lstater.Lstat(normalizedPath); os.IsNotExist(err) {
 			// Nothing exists with the normalized filename. Good.
 			if err = os.Rename(absPath, normalizedPath); err != nil {
 				l.Infof(`Error normalizing UTF8 encoding of file "%s": %v`, relPath, err)
@@ -475,13 +475,13 @@ func (w *walker) normalizePath(absPath, relPath string) (normPath string, skip b
 	return normPath, false
 }
 
-func checkDir(dir string) error {
-	if info, err := osutil.Lstat(dir); err != nil {
+func (w *walker) checkDir() error {
+	if info, err := w.Lstater.Lstat(w.Dir); err != nil {
 		return err
 	} else if !info.IsDir() {
-		return errors.New(dir + ": not a directory")
+		return errors.New(w.Dir + ": not a directory")
 	} else {
-		l.Debugln("checkDir", dir, info)
+		l.Debugln("checkDir", w.Dir, info)
 	}
 	return nil
 }
@@ -591,10 +591,10 @@ func (noTempNamer) IsTemporary(path string) bool {
 	return false
 }
 
-// A no-op MtimeRepo
+// A no-op Lstater
 
-type noMtimeRepo struct{}
+type defaultLstater struct{}
 
-func (noMtimeRepo) GetMtime(relPath string, mtime time.Time) time.Time {
-	return mtime
+func (defaultLstater) Lstat(name string) (os.FileInfo, error) {
+	return osutil.Lstat(name)
 }