lib/scanner: Support walking a symlink root (ref #4353)

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4666
LGTM: AudriusButkevicius, imsodin

AUTHORS

@@ -90,6 +90,7 @@ Michael Jephcote (Rewt0r) <rewt0r@gmx.com> <Rewt0r@users.noreply.github.com>
 Michael Ploujnikov (plouj) <ploujj@gmail.com>
 Michael Tilli (pyfisch) <pyfisch@gmail.com>
 Nate Morrison (nrm21) <natemorrison@gmail.com>
+Nicholas Rishel (PrototypeNM1) <PrototypeNM1@users.noreply.github.com>
 Niels Peter Roest (Niller303) <nielsproest@hotmail.com> <seje.niels@hotmail.com>
 Pascal Jungblut (pascalj) <github@pascalj.com> <mail@pascal-jungblut.com>
 Pawel Palenica (qepasa) <pawelpalenica11@gmail.com>

lib/fs/walkfs.go

@@ -46,7 +46,7 @@ func (f *walkFilesystem) walk(path string, info FileInfo, walkFn WalkFunc) error
 		return err
 	}
 
-	if !info.IsDir() {
+	if !info.IsDir() && path != "." {
 		return nil
 	}
 

lib/model/model.go

@@ -1954,7 +1954,7 @@ func (m *Model) internalScanFolderSubdirs(ctx context.Context, folder string, su
 
 	runner.setState(FolderScanning)
 
-	fchan, err := scanner.Walk(ctx, scanner.Config{
+	fchan := scanner.Walk(ctx, scanner.Config{
 		Folder:                folderCfg.ID,
 		Subs:                  subDirs,
 		Matcher:               ignores,
@@ -1970,14 +1970,7 @@ func (m *Model) internalScanFolderSubdirs(ctx context.Context, folder string, su
 		UseWeakHashes:         weakhash.Enabled,
 	})
 
-	if err != nil {
+	if err := runner.CheckHealth(); err != nil {
-		// The error we get here is likely an OS level error, which might not be
-		// as readable as our health check errors. Check if we can get a health
-		// check error first, and use that if it's available.
-		if ferr := runner.CheckHealth(); ferr != nil {
-			err = ferr
-		}
-		runner.setError(err)
 		return err
 	}
 
@@ -2082,7 +2075,7 @@ func (m *Model) internalScanFolderSubdirs(ctx context.Context, folder string, su
 		})
 
 		if iterError != nil {
-			l.Debugln("Stopping scan of folder %s due to: %s", folderCfg.Description(), err)
+			l.Debugln("Stopping scan of folder %s due to: %s", folderCfg.Description(), iterError)
 			return iterError
 		}
 	}

lib/scanner/infinitefs_test.go

@@ -31,6 +31,10 @@ func (i infiniteFS) Lstat(name string) (fs.FileInfo, error) {
 	return fakeInfo{name, i.filesize}, nil
 }
 
+func (i infiniteFS) Stat(name string) (fs.FileInfo, error) {
+	return fakeInfo{name, i.filesize}, nil
+}
+
 func (i infiniteFS) DirNames(name string) ([]string, error) {
 	// Returns a list of fake files and directories. Names are such that
 	// files appear before directories - this makes it so the scanner will

lib/scanner/walk.go

@@ -8,7 +8,6 @@ package scanner
 
 import (
 	"context"
-	"errors"
 	"runtime"
 	"sync/atomic"
 	"time"
@@ -76,7 +75,7 @@ type CurrentFiler interface {
 	CurrentFile(name string) (protocol.FileInfo, bool)
 }
 
-func Walk(ctx context.Context, cfg Config) (chan protocol.FileInfo, error) {
+func Walk(ctx context.Context, cfg Config) chan protocol.FileInfo {
 	w := walker{cfg}
 
 	if w.CurrentFiler == nil {
@@ -98,13 +97,9 @@ type walker struct {
 
 // Walk returns the list of files found in the local folder by scanning the
 // file system. Files are blockwise hashed.
-func (w *walker) walk(ctx context.Context) (chan protocol.FileInfo, error) {
+func (w *walker) walk(ctx context.Context) chan protocol.FileInfo {
 	l.Debugln("Walk", w.Subs, w.BlockSize, w.Matcher)
 
-	if err := w.checkDir(); err != nil {
-		return nil, err
-	}
-
 	toHashChan := make(chan protocol.FileInfo)
 	finishedChan := make(chan protocol.FileInfo)
 
@@ -126,7 +121,7 @@ func (w *walker) walk(ctx context.Context) (chan protocol.FileInfo, error) {
 	// and feed inputs directly from the walker.
 	if w.ProgressTickIntervalS < 0 {
 		newParallelHasher(ctx, w.Filesystem, w.BlockSize, w.Hashers, finishedChan, toHashChan, nil, nil, w.UseWeakHashes)
-		return finishedChan, nil
+		return finishedChan
 	}
 
 	// Defaults to every 2 seconds.
@@ -198,7 +193,7 @@ func (w *walker) walk(ctx context.Context) (chan protocol.FileInfo, error) {
 		close(realToHashChan)
 	}()
 
-	return finishedChan, nil
+	return finishedChan
 }
 
 func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protocol.FileInfo) fs.WalkFunc {
@@ -480,21 +475,6 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string,
 	return normPath, false
 }
 
-func (w *walker) checkDir() error {
-	info, err := w.Filesystem.Lstat(".")
-	if err != nil {
-		return err
-	}
-
-	if !info.IsDir() {
-		return errors.New(w.Filesystem.URI() + ": not a directory")
-	}
-
-	l.Debugln("checkDir", w.Filesystem.Type(), w.Filesystem.URI(), info)
-
-	return nil
-}
-
 func PermsEqual(a, b uint32) bool {
 	switch runtime.GOOS {
 	case "windows":

lib/scanner/walk_test.go

@@ -12,6 +12,7 @@ import (
 	"crypto/rand"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -61,7 +62,7 @@ func TestWalkSub(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	fchan, err := Walk(context.TODO(), Config{
+	fchan := Walk(context.TODO(), Config{
 		Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata"),
 		Subs:       []string{"dir2"},
 		BlockSize:  128 * 1024,
@@ -72,9 +73,6 @@ func TestWalkSub(t *testing.T) {
 	for f := range fchan {
 		files = append(files, f)
 	}
-	if err != nil {
-		t.Fatal(err)
-	}
 
 	// The directory contains two files, where one is ignored from a higher
 	// level. We should see only the directory and one of the files.
@@ -98,17 +96,13 @@ func TestWalk(t *testing.T) {
 	}
 	t.Log(ignores)
 
-	fchan, err := Walk(context.TODO(), Config{
+	fchan := Walk(context.TODO(), Config{
 		Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata"),
 		BlockSize:  128 * 1024,
 		Matcher:    ignores,
 		Hashers:    2,
 	})
 
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	var tmp []protocol.FileInfo
 	for f := range fchan {
 		tmp = append(tmp, f)
@@ -121,27 +115,6 @@ func TestWalk(t *testing.T) {
 	}
 }
 
-func TestWalkError(t *testing.T) {
-	_, err := Walk(context.TODO(), Config{
-		Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata-missing"),
-		BlockSize:  128 * 1024,
-		Hashers:    2,
-	})
-
-	if err == nil {
-		t.Error("no error from missing directory")
-	}
-
-	_, err = Walk(context.TODO(), Config{
-		Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata/bar"),
-		BlockSize:  128 * 1024,
-	})
-
-	if err == nil {
-		t.Error("no error from non-directory")
-	}
-}
-
 func TestVerify(t *testing.T) {
 	blocksize := 16
 	// data should be an even multiple of blocksize long
@@ -292,39 +265,25 @@ func TestWalkSymlinkUnix(t *testing.T) {
 	}
 
 	// Create a folder with a symlink in it
-
 	os.RemoveAll("_symlinks")
-	defer os.RemoveAll("_symlinks")
-
 	os.Mkdir("_symlinks", 0755)
-	os.Symlink("destination", "_symlinks/link")
+	defer os.RemoveAll("_symlinks")
+	os.Symlink("../testdata", "_symlinks/link")
 
-	// Scan it
+	for _, path := range []string{".", "link"} {
+		// Scan it
+		files, _ := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks"), path)
 
-	fchan, err := Walk(context.TODO(), Config{
+		// Verify that we got one symlink and with the correct attributes
-		Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks"),
+		if len(files) != 1 {
-		BlockSize:  128 * 1024,
+			t.Errorf("expected 1 symlink, not %d", len(files))
-	})
+		}
-
+		if len(files[0].Blocks) != 0 {
-	if err != nil {
+			t.Errorf("expected zero blocks for symlink, not %d", len(files[0].Blocks))
-		t.Fatal(err)
+		}
-	}
+		if files[0].SymlinkTarget != "../testdata" {
-
+			t.Errorf("expected symlink to have target destination, not %q", files[0].SymlinkTarget)
-	var files []protocol.FileInfo
+		}
-	for f := range fchan {
-		files = append(files, f)
-	}
-
-	// Verify that we got one symlink and with the correct attributes
-
-	if len(files) != 1 {
-		t.Errorf("expected 1 symlink, not %d", len(files))
-	}
-	if len(files[0].Blocks) != 0 {
-		t.Errorf("expected zero blocks for symlink, not %d", len(files[0].Blocks))
-	}
-	if files[0].SymlinkTarget != "destination" {
-		t.Errorf("expected symlink to have target destination, not %q", files[0].SymlinkTarget)
 	}
 }
 
@@ -334,41 +293,57 @@ func TestWalkSymlinkWindows(t *testing.T) {
 	}
 
 	// Create a folder with a symlink in it
-
 	os.RemoveAll("_symlinks")
-	defer os.RemoveAll("_symlinks")
-
 	os.Mkdir("_symlinks", 0755)
-	if err := osutil.DebugSymlinkForTestsOnly("destination", "_symlinks/link"); err != nil {
+	defer os.RemoveAll("_symlinks")
+	if err := osutil.DebugSymlinkForTestsOnly("../testdata", "_symlinks/link"); err != nil {
 		// Probably we require permissions we don't have.
 		t.Skip(err)
 	}
 
-	// Scan it
+	for _, path := range []string{".", "link"} {
+		// Scan it
+		files, _ := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks"), path)
 
-	fchan, err := Walk(context.TODO(), Config{
+		// Verify that we got zero symlinks
-		Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks"),
+		if len(files) != 0 {
-		BlockSize:  128 * 1024,
+			t.Errorf("expected zero symlinks, not %d", len(files))
-	})
+		}
+	}
+}
 
+func TestWalkRootSymlink(t *testing.T) {
+	// Create a folder with a symlink in it
+	tmp, err := ioutil.TempDir("", "")
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer os.RemoveAll(tmp)
 
-	var files []protocol.FileInfo
+	link := tmp + "/link"
-	for f := range fchan {
+	dest, _ := filepath.Abs("testdata/dir1")
-		files = append(files, f)
+	if err := osutil.DebugSymlinkForTestsOnly(dest, link); err != nil {
+		if runtime.GOOS == "windows" {
+			// Probably we require permissions we don't have.
+			t.Skip("Need admin permissions or developer mode to run symlink test on Windows: " + err.Error())
+		} else {
+			t.Fatal(err)
+		}
 	}
 
-	// Verify that we got zero symlinks
+	// Scan it
-
+	files, err := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, link), ".")
-	if len(files) != 0 {
+	if err != nil {
-		t.Errorf("expected zero symlinks, not %d", len(files))
+		t.Fatal("Expected no error when root folder path is provided via a symlink: " + err.Error())
+	}
+	// Verify that we got two files
+	if len(files) != 2 {
+		t.Errorf("expected two files, not %d", len(files))
 	}
 }
 
 func walkDir(fs fs.Filesystem, dir string) ([]protocol.FileInfo, error) {
-	fchan, err := Walk(context.TODO(), Config{
+	fchan := Walk(context.TODO(), Config{
 		Filesystem:    fs,
 		Subs:          []string{dir},
 		BlockSize:     128 * 1024,
@@ -376,10 +351,6 @@ func walkDir(fs fs.Filesystem, dir string) ([]protocol.FileInfo, error) {
 		Hashers:       2,
 	})
 
-	if err != nil {
-		return nil, err
-	}
-
 	var tmp []protocol.FileInfo
 	for f := range fchan {
 		tmp = append(tmp, f)
@@ -478,17 +449,13 @@ func TestStopWalk(t *testing.T) {
 
 	const numHashers = 4
 	ctx, cancel := context.WithCancel(context.Background())
-	fchan, err := Walk(ctx, Config{
+	fchan := Walk(ctx, Config{
 		Filesystem:            fs,
 		BlockSize:             128 * 1024,
 		Hashers:               numHashers,
 		ProgressTickIntervalS: -1, // Don't attempt to build the full list of files before starting to scan...
 	})
 
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	// Receive a few entries to make sure the walker is up and running,
 	// scanning both files and dirs. Do some quick sanity tests on the
 	// returned file entries to make sure we are not just reading crap from