From 3e26fdfb67683bd8355d672fc5d6a437cf0620aa Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 3 Dec 2014 11:03:44 +0100 Subject: [PATCH 1/3] Run filetype and symlink integration tests with versioning (ref #1071) --- test/common_test.go | 5 ++++- test/filetype_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++- test/symlink_test.go | 45 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/test/common_test.go b/test/common_test.go index 21023c7c..1474d6f0 100644 --- a/test/common_test.go +++ b/test/common_test.go @@ -379,9 +379,12 @@ func startWalker(dir string, res chan<- fileInfo, abort <-chan struct{}) { if rn == "." || rn == ".stfolder" { return nil } + if rn == ".stversions" { + return filepath.SkipDir + } var f fileInfo - if ok, err := symlinks.IsSymlink(path); err == nil && ok { + if info.Mode()&os.ModeSymlink != 0 { f = fileInfo{ name: rn, mode: os.ModeSymlink, diff --git a/test/filetype_test.go b/test/filetype_test.go index e3f3ddd2..44092163 100644 --- a/test/filetype_test.go +++ b/test/filetype_test.go @@ -24,9 +24,53 @@ import ( "strings" "testing" "time" + + "github.com/syncthing/syncthing/internal/config" + "github.com/syncthing/syncthing/internal/protocol" ) -func TestFiletypeChange(t *testing.T) { +func TestFileTypeChange(t *testing.T) { + // Use no versioning + id, _ := protocol.DeviceIDFromString(id2) + cfg, _ := config.Load("h2/config.xml", id) + fld := cfg.Folders()["default"] + fld.Versioning = config.VersioningConfiguration{} + cfg.SetFolder(fld) + cfg.Save() + + testFileTypeChange(t) +} + +func TestFileTypeChangeSimpleVersioning(t *testing.T) { + // Use simple versioning + id, _ := protocol.DeviceIDFromString(id2) + cfg, _ := config.Load("h2/config.xml", id) + fld := cfg.Folders()["default"] + fld.Versioning = config.VersioningConfiguration{ + Type: "simple", + Params: map[string]string{"keep": "5"}, + } + cfg.SetFolder(fld) + cfg.Save() + + testFileTypeChange(t) +} + +func TestFileTypeChangeStaggeredVersioning(t *testing.T) { + // Use staggered versioning + id, _ := protocol.DeviceIDFromString(id2) + cfg, _ := config.Load("h2/config.xml", id) + fld := cfg.Folders()["default"] + fld.Versioning = config.VersioningConfiguration{ + Type: "staggered", + } + cfg.SetFolder(fld) + cfg.Save() + + testFileTypeChange(t) +} + +func testFileTypeChange(t *testing.T) { log.Println("Cleaning...") err := removeAll("s1", "s2", "h1/index", "h2/index") if err != nil { diff --git a/test/symlink_test.go b/test/symlink_test.go index 8cb9e4b4..025e4fa9 100644 --- a/test/symlink_test.go +++ b/test/symlink_test.go @@ -24,10 +24,53 @@ import ( "testing" "time" + "github.com/syncthing/syncthing/internal/config" + "github.com/syncthing/syncthing/internal/protocol" "github.com/syncthing/syncthing/internal/symlinks" ) func TestSymlinks(t *testing.T) { + // Use no versioning + id, _ := protocol.DeviceIDFromString(id2) + cfg, _ := config.Load("h2/config.xml", id) + fld := cfg.Folders()["default"] + fld.Versioning = config.VersioningConfiguration{} + cfg.SetFolder(fld) + cfg.Save() + + testSymlinks(t) +} + +func TestSymlinksSimpleVersioning(t *testing.T) { + // Use no versioning + id, _ := protocol.DeviceIDFromString(id2) + cfg, _ := config.Load("h2/config.xml", id) + fld := cfg.Folders()["default"] + fld.Versioning = config.VersioningConfiguration{ + Type: "simple", + Params: map[string]string{"keep": "5"}, + } + cfg.SetFolder(fld) + cfg.Save() + + testSymlinks(t) +} + +func TestSymlinksStaggeredVersioning(t *testing.T) { + // Use no versioning + id, _ := protocol.DeviceIDFromString(id2) + cfg, _ := config.Load("h2/config.xml", id) + fld := cfg.Folders()["default"] + fld.Versioning = config.VersioningConfiguration{ + Type: "staggered", + } + cfg.SetFolder(fld) + cfg.Save() + + testSymlinks(t) +} + +func testSymlinks(t *testing.T) { log.Println("Cleaning...") err := removeAll("s1", "s2", "h1/index", "h2/index") if err != nil { @@ -236,7 +279,7 @@ func TestSymlinks(t *testing.T) { log.Fatal(err) } - // Remove a symlink + // Remove a broken symlink err = os.Remove("s1/removeLink") if err != nil { From d38c81fcffe22b906781a6bfa62a6f01a4226ba3 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 3 Dec 2014 11:42:22 +0100 Subject: [PATCH 2/3] Handle broken symlinks, Simple versioner (fixes #1071) --- internal/versioner/simple.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/versioner/simple.go b/internal/versioner/simple.go index 83318b64..e51575fe 100644 --- a/internal/versioner/simple.go +++ b/internal/versioner/simple.go @@ -55,7 +55,7 @@ func NewSimple(folderID, folderPath string, params map[string]string) Versioner // Move away the named file to a version archive. If this function returns // nil, the named file does not exist any more (has been archived). func (v Simple) Archive(filePath string) error { - fileInfo, err := os.Stat(filePath) + fileInfo, err := os.Lstat(filePath) if err != nil { if os.IsNotExist(err) { if debug { From 94ab06e92f213262f48c4252b08ac1cd0b862517 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 3 Dec 2014 11:49:53 +0100 Subject: [PATCH 3/3] Handle symlinks, Staggered versioner (fixes #1071) --- internal/versioner/staggered.go | 125 ++++++++++++++++---------------- 1 file changed, 62 insertions(+), 63 deletions(-) diff --git a/internal/versioner/staggered.go b/internal/versioner/staggered.go index 496858a3..4ae21606 100644 --- a/internal/versioner/staggered.go +++ b/internal/versioner/staggered.go @@ -45,16 +45,6 @@ type Staggered struct { mutex *sync.Mutex } -// Check if file or dir -func isFile(path string) bool { - fileInfo, err := os.Stat(path) - if err != nil { - l.Infoln("versioner isFile:", err) - return false - } - return fileInfo.Mode().IsRegular() -} - // Rename versions with old version format func (v Staggered) renameOld() { err := filepath.Walk(v.versionsPath, func(path string, f os.FileInfo, err error) error { @@ -167,14 +157,16 @@ func (v Staggered) clean() { if err != nil { return err } - switch mode := f.Mode(); { - case mode.IsDir(): + + if f.Mode().IsDir() && f.Mode()&os.ModeSymlink == 0 { filesPerDir[path] = 0 if path != v.versionsPath { dir := filepath.Dir(path) filesPerDir[dir]++ } - case mode.IsRegular(): + } else { + // Regular file, or possibly a symlink. + extension := filenameTag(path) dir := filepath.Dir(path) name := path[:len(path)-len(extension)-1] @@ -227,56 +219,63 @@ func (v Staggered) expire(versions []string) { var prevAge int64 firstFile := true for _, file := range versions { - if isFile(file) { - versionTime, err := time.Parse(TimeFormat, filenameTag(file)) - if err != nil { - l.Infof("Versioner: file name %q is invalid: %v", file, err) - continue - } - age := int64(time.Since(versionTime).Seconds()) - - // If the file is older than the max age of the last interval, remove it - if lastIntv := v.interval[len(v.interval)-1]; lastIntv.end > 0 && age > lastIntv.end { - if debug { - l.Debugln("Versioner: File over maximum age -> delete ", file) - } - err = os.Remove(file) - if err != nil { - l.Warnf("Versioner: can't remove %q: %v", file, err) - } - continue - } - - // If it's the first (oldest) file in the list we can skip the interval checks - if firstFile { - prevAge = age - firstFile = false - continue - } - - // Find the interval the file fits in - var usedInterval Interval - for _, usedInterval = range v.interval { - if age < usedInterval.end { - break - } - } - - if prevAge-age < usedInterval.step { - if debug { - l.Debugln("too many files in step -> delete", file) - } - err = os.Remove(file) - if err != nil { - l.Warnf("Versioner: can't remove %q: %v", file, err) - } - continue - } - - prevAge = age - } else { - l.Infof("non-file %q is named like a file version", file) + fi, err := os.Stat(file) + if err != nil { + l.Warnln("versioner:", err) + continue } + + if fi.IsDir() { + l.Infof("non-file %q is named like a file version", file) + continue + } + + versionTime, err := time.Parse(TimeFormat, filenameTag(file)) + if err != nil { + l.Infof("Versioner: file name %q is invalid: %v", file, err) + continue + } + age := int64(time.Since(versionTime).Seconds()) + + // If the file is older than the max age of the last interval, remove it + if lastIntv := v.interval[len(v.interval)-1]; lastIntv.end > 0 && age > lastIntv.end { + if debug { + l.Debugln("Versioner: File over maximum age -> delete ", file) + } + err = os.Remove(file) + if err != nil { + l.Warnf("Versioner: can't remove %q: %v", file, err) + } + continue + } + + // If it's the first (oldest) file in the list we can skip the interval checks + if firstFile { + prevAge = age + firstFile = false + continue + } + + // Find the interval the file fits in + var usedInterval Interval + for _, usedInterval = range v.interval { + if age < usedInterval.end { + break + } + } + + if prevAge-age < usedInterval.step { + if debug { + l.Debugln("too many files in step -> delete", file) + } + err = os.Remove(file) + if err != nil { + l.Warnf("Versioner: can't remove %q: %v", file, err) + } + continue + } + + prevAge = age } } @@ -289,7 +288,7 @@ func (v Staggered) Archive(filePath string) error { v.mutex.Lock() defer v.mutex.Unlock() - if _, err := os.Stat(filePath); err != nil { + if _, err := os.Lstat(filePath); err != nil { if os.IsNotExist(err) { if debug { l.Debugln("not archiving nonexistent file", filePath)