diff --git a/internal/model/model.go b/internal/model/model.go index 4605f9c3..83f289f0 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -1242,8 +1242,16 @@ func (m *Model) ScanFolderSub(folder, sub string) error { "size": f.Size(), }) batch = append(batch, nf) - } else if _, err := os.Lstat(filepath.Join(folderCfg.Path, f.Name)); err != nil && os.IsNotExist(err) { - // File has been deleted + } else if _, err := os.Lstat(filepath.Join(folderCfg.Path, f.Name)); err != nil { + // File has been deleted. + + // We don't specifically verify that the error is + // os.IsNotExist because there is a corner case when a + // directory is suddenly transformed into a file. When that + // happens, files that were in the directory (that is now a + // file) are deleted but will return a confusing error ("not a + // directory") when we try to Lstat() them. + nf := protocol.FileInfo{ Name: f.Name, Flags: f.Flags | protocol.FlagDeleted, diff --git a/internal/model/puller.go b/internal/model/puller.go index 0bc0bbcf..1b4c5426 100644 --- a/internal/model/puller.go +++ b/internal/model/puller.go @@ -614,22 +614,31 @@ func (p *Puller) renameFile(source, target protocol.FileInfo) { err = osutil.TryRename(from, to) } - if err != nil { - l.Infof("Puller (folder %q, file %q): rename from %q: %v", p.folder, target.Name, source.Name, err) - return - } + if err == nil { + // The file was renamed, so we have handled both the necessary delete + // of the source and the creation of the target. Fix-up the metadata, + // and update the local index of the target file. - // Fix-up the metadata, and update the local index of the target file - err = p.shortcutFile(target) - if err != nil { - l.Infof("Puller (folder %q, file %q): rename from %q metadata: %v", p.folder, target.Name, source.Name, err) - return - } + p.model.updateLocal(p.folder, source) - // Source file already has the delete bit set. - // Because we got rid of the file (by renaming it), we just need to update - // the index, and we're done with it. - p.model.updateLocal(p.folder, source) + err = p.shortcutFile(target) + if err != nil { + l.Infof("Puller (folder %q, file %q): rename from %q metadata: %v", p.folder, target.Name, source.Name, err) + return + } + } else { + // We failed the rename so we have a source file that we still need to + // get rid of. Attempt to delete it instead so that we make *some* + // progress. The target is unhandled. + + err = osutil.InWritableDir(os.Remove, from) + if err != nil { + l.Infof("Puller (folder %q, file %q): delete %q after failed rename: %v", p.folder, target.Name, source.Name, err) + return + } + + p.model.updateLocal(p.folder, source) + } } // handleFile queues the copies and pulls as necessary for a single new or diff --git a/test/filetype_test.go b/test/filetype_test.go index bdcb173f..c052628e 100644 --- a/test/filetype_test.go +++ b/test/filetype_test.go @@ -91,10 +91,22 @@ func testFileTypeChange(t *testing.T) { // A directory that we will replace with a file later + err = os.Mkdir("s1/emptyDirToReplace", 0755) + if err != nil { + t.Fatal(err) + } + + // A directory with files that we will replace with a file later + err = os.Mkdir("s1/dirToReplace", 0755) if err != nil { t.Fatal(err) } + fd, err = os.Create("s1/dirToReplace/emptyFile") + if err != nil { + t.Fatal(err) + } + fd.Close() // Verify that the files and directories sync to the other side @@ -165,15 +177,33 @@ func testFileTypeChange(t *testing.T) { // Replace file with directory - os.RemoveAll("s1/fileToReplace") + err = os.RemoveAll("s1/fileToReplace") + if err != nil { + t.Fatal(err) + } err = os.Mkdir("s1/fileToReplace", 0755) if err != nil { t.Fatal(err) } - // Replace directory with file + // Replace empty directory with file - os.RemoveAll("s1/dirToReplace") + err = os.RemoveAll("s1/emptyDirToReplace") + if err != nil { + t.Fatal(err) + } + fd, err = os.Create("s1/emptyDirToReplace") + if err != nil { + t.Fatal(err) + } + fd.Close() + + // Clear directory and replace with file + + err = os.RemoveAll("s1/dirToReplace") + if err != nil { + t.Fatal(err) + } fd, err = os.Create("s1/dirToReplace") if err != nil { t.Fatal(err) diff --git a/test/util.go b/test/util.go index f3c21dfe..2f2cf345 100644 --- a/test/util.go +++ b/test/util.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "log" "math/rand" "os" @@ -113,8 +114,10 @@ func alterFiles(dir string) error { return nil } + info, err = os.Stat(path) if err != nil { - return err + // Something we deleted while walking. Ignore. + return nil } if strings.HasPrefix(filepath.Base(path), "test-") { @@ -128,17 +131,24 @@ func alterFiles(dir string) error { return nil } - r := rand.Float64() + // File structure is base/x/xy/xyz12345... + // comps == 1: base (don't touch) + // comps == 2: base/x (must be dir) + // comps == 3: base/x/xy (must be dir) + // comps > 3: base/x/xy/xyz12345... (can be dir or file) + comps := len(strings.Split(path, string(os.PathSeparator))) + + r := rand.Intn(10) switch { - case r < 0.1 && comps > 2: + case r == 0 && comps > 2: // Delete every tenth file or directory, except top levels err := removeAll(path) if err != nil { return err } - case r < 0.2 && info.Mode().IsRegular(): + case r == 1 && info.Mode().IsRegular(): if info.Mode()&0200 != 0200 { // Not owner writable. Fix. err = os.Chmod(path, 0644) @@ -166,7 +176,31 @@ func alterFiles(dir string) error { if err != nil { return err } - case r < 0.3 && comps > 1 && (info.Mode().IsRegular() || rand.Float64() < 0.2): + + case r == 2 && comps > 3 && rand.Float64() < 0.2: + if !info.Mode().IsRegular() { + err = removeAll(path) + if err != nil { + return err + } + d1 := []byte("I used to be a dir: " + path) + err := ioutil.WriteFile(path, d1, 0644) + if err != nil { + return err + } + } else { + err := os.Remove(path) + if err != nil { + return err + } + err = os.MkdirAll(path, 0755) + if err != nil { + return err + } + generateFiles(path, 10, 20, "../LICENSE") + } + + case r == 3 && comps > 2 && (info.Mode().IsRegular() || rand.Float64() < 0.2): rpath := filepath.Dir(path) if rand.Float64() < 0.2 { for move := rand.Intn(comps - 1); move > 0; move-- { @@ -184,8 +218,7 @@ func alterFiles(dir string) error { return err } - // Create 100 new files - return generateFiles(dir, 100, 20, "../LICENSE") + return generateFiles(dir, 25, 20, "../LICENSE") } func ReadRand(bs []byte) (int, error) {