Merge pull request #2231 from calmh/symtype

lib/symlinks need not depend on protocol

lib/model/rwfolder.go

@@ -1080,7 +1080,11 @@ func (p *rwFolder) shortcutFile(file protocol.FileInfo) error {
 
 // shortcutSymlink changes the symlinks type if necessary.
 func (p *rwFolder) shortcutSymlink(file protocol.FileInfo) (err error) {
-	err = symlinks.ChangeType(filepath.Join(p.dir, file.Name), file.Flags)
+	tt := symlinks.TargetFile
+	if file.IsDirectory() {
+		tt = symlinks.TargetDirectory
+	}
+	err = symlinks.ChangeType(filepath.Join(p.dir, file.Name), tt)
 	if err != nil {
 		l.Infof("Puller (folder %q, file %q): symlink shortcut: %v", p.folder, file.Name, err)
 		p.newError(file.Name, err)
@@ -1316,7 +1320,11 @@ func (p *rwFolder) performFinish(state *sharedPullerState) error {
 		// Remove the file, and replace it with a symlink.
 		err = osutil.InWritableDir(func(path string) error {
 			os.Remove(path)
-			return symlinks.Create(path, string(content), state.file.Flags)
+			tt := symlinks.TargetFile
+			if state.file.IsDirectory() {
+				tt = symlinks.TargetDirectory
+			}
+			return symlinks.Create(path, string(content), tt)
 		}, state.realName)
 		if err != nil {
 			return err

lib/scanner/walk.go

@@ -311,8 +311,7 @@ func (w *Walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 			// checking that their existing blocks match with the blocks in
 			// the index.
 
-			target, flags, err := symlinks.Read(p)
-			flags = flags & protocol.SymlinkTypeMask
+			target, targetType, err := symlinks.Read(p)
 			if err != nil {
 				if debug {
 					l.Debugln("readlink error:", p, err)
@@ -337,7 +336,7 @@ func (w *Walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 				//  - the symlink type (file/dir) was the same
 				//  - the block list (i.e. hash of target) was the same
 				cf, ok = w.CurrentFiler.CurrentFile(rn)
-				if ok && !cf.IsDeleted() && cf.IsSymlink() && !cf.IsInvalid() && SymlinkTypeEqual(flags, cf.Flags) && BlocksEqual(cf.Blocks, blocks) {
+				if ok && !cf.IsDeleted() && cf.IsSymlink() && !cf.IsInvalid() && SymlinkTypeEqual(targetType, cf) && BlocksEqual(cf.Blocks, blocks) {
 					return skip
 				}
 			}
@@ -345,7 +344,7 @@ func (w *Walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 			f := protocol.FileInfo{
 				Name:     rn,
 				Version:  cf.Version.Update(w.ShortID),
-				Flags:    protocol.FlagSymlink | flags | protocol.FlagNoPermBits | 0666,
+				Flags:    uint32(protocol.FlagSymlink | protocol.FlagNoPermBits | 0666 | SymlinkFlags(targetType)),
 				Modified: 0,
 				Blocks:   blocks,
 			}
@@ -467,7 +466,7 @@ func PermsEqual(a, b uint32) bool {
 	}
 }
 
-func SymlinkTypeEqual(disk, index uint32) bool {
+func SymlinkTypeEqual(disk symlinks.TargetType, f protocol.FileInfo) bool {
 	// If the target is missing, Unix never knows what type of symlink it is
 	// and Windows always knows even if there is no target. Which means that
 	// without this special check a Unix node would be fighting with a Windows
@@ -476,8 +475,25 @@ func SymlinkTypeEqual(disk, index uint32) bool {
 	// know means you are on Unix, and on Unix you don't really care what the
 	// target type is. The moment you do know, and if something doesn't match,
 	// that will propagate through the cluster.
-	if disk&protocol.FlagSymlinkMissingTarget != 0 && index&protocol.FlagSymlinkMissingTarget == 0 {
+	switch disk {
+	case symlinks.TargetUnknown:
 		return true
+	case symlinks.TargetDirectory:
+		return f.IsDirectory() && f.Flags&protocol.FlagSymlinkMissingTarget == 0
+	case symlinks.TargetFile:
+		return !f.IsDirectory() && f.Flags&protocol.FlagSymlinkMissingTarget == 0
 	}
-	return disk&protocol.SymlinkTypeMask == index&protocol.SymlinkTypeMask
+	panic("unknown symlink TargetType")
+}
+
+func SymlinkFlags(t symlinks.TargetType) uint32 {
+	switch t {
+	case symlinks.TargetFile:
+		return 0
+	case symlinks.TargetDirectory:
+		return protocol.FlagDirectory
+	case symlinks.TargetUnknown:
+		return protocol.FlagSymlinkMissingTarget
+	}
+	panic("unknown symlink TargetType")
 }

lib/scanner/walk_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/syncthing/protocol"
 	"github.com/syncthing/syncthing/lib/ignore"
 	"github.com/syncthing/syncthing/lib/osutil"
+	"github.com/syncthing/syncthing/lib/symlinks"
 	"golang.org/x/text/unicode/norm"
 )
 
@@ -343,3 +344,31 @@ func (l testfileList) String() string {
 	b.WriteString("}")
 	return b.String()
 }
+
+func TestSymlinkTypeEqual(t *testing.T) {
+	testcases := []struct {
+		onDiskType   symlinks.TargetType
+		inIndexFlags uint32
+		equal        bool
+	}{
+		// File is only equal to file
+		{symlinks.TargetFile, 0, true},
+		{symlinks.TargetFile, protocol.FlagDirectory, false},
+		{symlinks.TargetFile, protocol.FlagSymlinkMissingTarget, false},
+		// Directory is only equal to directory
+		{symlinks.TargetDirectory, 0, false},
+		{symlinks.TargetDirectory, protocol.FlagDirectory, true},
+		{symlinks.TargetDirectory, protocol.FlagSymlinkMissingTarget, false},
+		// Unknown is equal to anything
+		{symlinks.TargetUnknown, 0, true},
+		{symlinks.TargetUnknown, protocol.FlagDirectory, true},
+		{symlinks.TargetUnknown, protocol.FlagSymlinkMissingTarget, true},
+	}
+
+	for _, tc := range testcases {
+		res := SymlinkTypeEqual(tc.onDiskType, protocol.FileInfo{Flags: tc.inIndexFlags})
+		if res != tc.equal {
+			t.Errorf("Incorrect result %v for %v, %v", res, tc.onDiskType, tc.inIndexFlags)
+		}
+	}
+}

lib/symlinks/symlink_unix.go

@@ -11,7 +11,6 @@ package symlinks
 import (
 	"os"
 
-	"github.com/syncthing/protocol"
 	"github.com/syncthing/syncthing/lib/osutil"
 )
 
@@ -19,23 +18,24 @@ var (
 	Supported = true
 )
 
-func Read(path string) (string, uint32, error) {
-	var mode uint32
-	stat, err := os.Stat(path)
-	if err != nil {
-		mode = protocol.FlagSymlinkMissingTarget
-	} else if stat.IsDir() {
-		mode = protocol.FlagDirectory
+func Read(path string) (string, TargetType, error) {
+	tt := TargetUnknown
+	if stat, err := os.Stat(path); err == nil {
+		if stat.IsDir() {
+			tt = TargetDirectory
+		} else {
+			tt = TargetFile
+		}
 	}
-	path, err = os.Readlink(path)
+	path, err := os.Readlink(path)
 
-	return osutil.NormalizedFilename(path), mode, err
+	return osutil.NormalizedFilename(path), tt, err
 }
 
-func Create(source, target string, flags uint32) error {
+func Create(source, target string, tt TargetType) error {
 	return os.Symlink(osutil.NativeFilename(target), source)
 }
 
-func ChangeType(path string, flags uint32) error {
+func ChangeType(path string, tt TargetType) error {
 	return nil
 }

lib/symlinks/symlink_windows.go

@@ -101,14 +101,14 @@ func (r *reparseData) SubstituteName() string {
 	return string(utf16.Decode(r.buffer[offset : offset+length]))
 }
 
-func Read(path string) (string, uint32, error) {
+func Read(path string) (string, TargetType, error) {
 	ptr, err := syscall.UTF16PtrFromString(path)
 	if err != nil {
-		return "", protocol.FlagSymlinkMissingTarget, err
+		return "", TargetUnknown, err
 	}
 	handle, err := syscall.CreateFile(ptr, 0, syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE, nil, syscall.OPEN_EXISTING, syscall.FILE_FLAG_BACKUP_SEMANTICS|Win32FileFlagOpenReparsePoint, 0)
 	if err != nil || handle == syscall.InvalidHandle {
-		return "", protocol.FlagSymlinkMissingTarget, err
+		return "", TargetUnknown, err
 	}
 	defer syscall.Close(handle)
 	var ret uint16
@@ -116,21 +116,22 @@ func Read(path string) (string, uint32, error) {
 
 	r1, _, err := syscall.Syscall9(procDeviceIoControl.Addr(), 8, uintptr(handle), Win32FsctlGetReparsePoint, 0, 0, uintptr(unsafe.Pointer(&data)), unsafe.Sizeof(data), uintptr(unsafe.Pointer(&ret)), 0, 0)
 	if r1 == 0 {
-		return "", protocol.FlagSymlinkMissingTarget, err
+		return "", TargetUnknown, err
 	}
 
-	var flags uint32
-	attr, err := syscall.GetFileAttributes(ptr)
-	if err != nil {
-		flags = protocol.FlagSymlinkMissingTarget
-	} else if attr&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
-		flags = protocol.FlagDirectory
+	tt := TargetUnknown
+	if attr, err := syscall.GetFileAttributes(ptr); err == nil {
+		if attr&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
+			tt = TargetDirectory
+		} else {
+			tt = TargetFile
+		}
 	}
 
-	return osutil.NormalizedFilename(data.PrintName()), flags, nil
+	return osutil.NormalizedFilename(data.PrintName()), tt, nil
 }
 
-func Create(source, target string, flags uint32) error {
+func Create(source, target string, tt TargetType) error {
 	srcp, err := syscall.UTF16PtrFromString(source)
 	if err != nil {
 		return err
@@ -146,7 +147,7 @@ func Create(source, target string, flags uint32) error {
 	// If the flags doesn't reveal the target type, try to evaluate it
 	// ourselves, and worst case default to the symlink pointing to a file.
 	mode := 0
-	if flags&protocol.FlagSymlinkMissingTarget != 0 {
+	if tt == TargetUnknown {
 		path := target
 		if !filepath.IsAbs(target) {
 			path = filepath.Join(filepath.Dir(source), target)
@@ -156,7 +157,7 @@ func Create(source, target string, flags uint32) error {
 		if err == nil && stat.IsDir() {
 			mode = Win32SymbolicLinkFlagDirectory
 		}
-	} else if flags&protocol.FlagDirectory != 0 {
+	} else if tt == TargetDirectory {
 		mode = Win32SymbolicLinkFlagDirectory
 	}
 
@@ -167,24 +168,24 @@ func Create(source, target string, flags uint32) error {
 	return err
 }
 
-func ChangeType(path string, flags uint32) error {
-	target, cflags, err := Read(path)
+func ChangeType(path string, tt TargetType) error {
+	target, exTt, err := Read(path)
 	if err != nil {
 		return err
 	}
 	// If it's the same type, nothing to do.
-	if cflags&protocol.SymlinkTypeMask == flags&protocol.SymlinkTypeMask {
+	if tt == exTt {
 		return nil
 	}
 
 	// If the actual type is unknown, but the new type is file, nothing to do
-	if cflags&protocol.FlagSymlinkMissingTarget != 0 && flags&protocol.FlagDirectory == 0 {
+	if exTt == TargetUnknown && tt != TargetDirectory {
 		return nil
 	}
 	return osutil.InWritableDir(func(path string) error {
 		// It should be a symlink as well hence no need to change permissions on
 		// the file.
 		os.Remove(path)
-		return Create(path, target, flags)
+		return Create(path, target, tt)
 	}, path)
 }

lib/symlinks/targets.go

@@ -0,0 +1,15 @@
+// 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 symlinks
+
+type TargetType int
+
+const (
+	TargetFile TargetType = iota
+	TargetDirectory
+	TargetUnknown
+)