Merge pull request #138 from vbatts/match_update_behavior

*: update `-u` behavior

cmd/gomtree/main.go

@@ -254,18 +254,36 @@ func app() error {
 	if *flUpdateAttributes && stateDh != nil {
 		// -u
 		// this comes before the next case, intentionally.
-
-		// TODO brainstorm where to allow setting of xattrs. Maybe a new flag that allows a comma delimited list of keywords to update?
-		updateKeywords := []mtree.Keyword{"uid", "gid", "mode"}
-
-		result, err := mtree.Update(rootPath, stateDh, updateKeywords, nil)
+		result, err := mtree.Update(rootPath, specDh, mtree.DefaultUpdateKeywords, nil)
 		if err != nil {
 			return err
 		}
-
-		if result != nil {
+		if result != nil && len(result) > 0 {
 			fmt.Printf("%#v\n", result)
 		}
+
+		var res []mtree.InodeDelta
+		// only check the keywords that we just updated
+		res, err = mtree.Check(rootPath, specDh, mtree.DefaultUpdateKeywords, nil)
+		if err != nil {
+			return err
+		}
+		if res != nil {
+			out := formatFunc(res)
+			if _, err := os.Stdout.Write([]byte(out)); err != nil {
+				return err
+			}
+
+			// TODO: This should be a flag. Allowing files to be added and
+			//       removed and still returning "it's all good" is simply
+			//       unsafe IMO.
+			for _, diff := range res {
+				if diff.Type() == mtree.Modified {
+					return fmt.Errorf("mainfest validation failed")
+				}
+			}
+		}
+
 		return nil
 	}
 
@@ -300,7 +318,6 @@ func app() error {
 	// This is a validation.
 	if specDh != nil && stateDh != nil {
 		var res []mtree.InodeDelta
-
 		res, err = mtree.Compare(specDh, stateDh, currentKeywords)
 		if err != nil {
 			return err

entry.go

@@ -112,6 +112,16 @@ func (e Entry) AllKeys() []KeyVal {
 	return e.Keywords
 }
 
+// IsDir checks the type= value for this entry on whether it is a directory
+func (e Entry) IsDir() bool {
+	for _, kv := range e.AllKeys() {
+		if kv.Keyword().Prefix() == "type" {
+			return kv.Value() == "dir"
+		}
+	}
+	return false
+}
+
 // EntryType are the formats of lines in an mtree spec file
 type EntryType int
 

stat_unix.go

@@ -0,0 +1,18 @@
+// +build !windows
+
+package mtree
+
+import (
+	"os"
+	"syscall"
+)
+
+func statIsUID(stat os.FileInfo, uid int) bool {
+	statT := stat.Sys().(*syscall.Stat_t)
+	return statT.Uid == uint32(uid)
+}
+
+func statIsGID(stat os.FileInfo, gid int) bool {
+	statT := stat.Sys().(*syscall.Stat_t)
+	return statT.Gid == uint32(gid)
+}

stat_windows.go

@@ -0,0 +1,12 @@
+// +build windows
+
+package mtree
+
+import "os"
+
+func statIsUID(stat os.FileInfo, uid int) bool {
+	return false
+}
+func statIsGID(stat os.FileInfo, uid int) bool {
+	return false
+}

test/cli/0009.sh

@@ -7,8 +7,6 @@ gomtree=$(readlink -f ${root}/gomtree)
 t=$(mktemp -t -d go-mtree.XXXXXX)
 
 echo "[${name}] Running in ${t}"
-# This test is for basic running check of manifest, and check against tar and file system
-#
 
 pushd ${root}
 
@@ -17,7 +15,7 @@ mkdir ${t}/root
 echo "some data" > "${t}/root/$(printf 'this file has \u042a some unicode !!')"
 echo "more data" > "${t}/root/$(printf 'even more \x07 unicode \ua4ff characters \udead\ubeef\ucafe')"
 mkdir -p "${t}/root/$(printf '\024 <-- some more weird characters --> \u4f60\u597d\uff0c\u4e16\u754c')"
-ln -s "$(printf '62_\u00c6\u00c62\u00ae\u00b7m\u00db\u00c3r^\u00bfp\u00c6u"q\u00fbc2\u00f0u\u00b8\u00dd\u00e8v\u00ff\u00b0\u00dc\u00c2\u00f53\u00db')" "${t}/root/$(printf '-k\u00f2sd4\\p\u00da\u00a6\u00d3\u00eea<\u00e6s{\u00a0p\u00f0\u00ffj\u00e0\u00e8\u00b8\u00b8\u00bc\u00fcb')"
+ln -s "$(printf '62_\u00c6\u00c62\u00ae\u00b7m\u00db\u00c3r^\u00bfp\u00c6u"q\u00fbc2\u00f0u\u00b8\u00dd\u00e8v\u00ff\u00b0\u00dc\u00c2\u00f53\u00db')" "${t}/root/$(printf 'k\u00f2sd4\\p\u00da\u00a6\u00d3\u00eea<\u00e6s{\u00a0p\u00f0\u00ffj\u00e0\u00e8\u00b8\u00b8\u00bc\u00fcb')"
 printf 'some lovely data 62_\u00c6\u00c62\u00ae\u00b7m\u00db\u00c3r' > "${t}/root/$(printf 'T\u00dcB\u0130TAK_UEKAE_K\u00f6k_Sertifika_Hizmet_Sa\u011flay\u0131c\u0131s\u0131_-_S\u00fcr\u00fcm_3.pem')"
 
 # Create manifest and check it against the same root.

update.go

@@ -1,6 +1,7 @@
 package mtree
 
 import (
+	"container/heap"
 	"os"
 	"sort"
 
@@ -12,8 +13,9 @@ var DefaultUpdateKeywords = []Keyword{
 	"uid",
 	"gid",
 	"mode",
-	"time",
 	"xattr",
+	"link",
+	"time",
 }
 
 // Update attempts to set the attributes of root directory path, given the values of `keywords` in dh DirectoryHierarchy.
@@ -29,6 +31,11 @@ func Update(root string, dh *DirectoryHierarchy, keywords []Keyword, fs FsEval)
 	}
 	sort.Sort(byPos(creator.DH.Entries))
 
+	// This is for deferring the update of mtimes of directories, to unwind them
+	// in a most specific path first
+	h := &pathUpdateHeap{}
+	heap.Init(h)
+
 	results := []InodeDelta{}
 	for i, e := range creator.DH.Entries {
 		switch e.Type {
@@ -62,6 +69,19 @@ func Update(root string, dh *DirectoryHierarchy, keywords []Keyword, fs FsEval)
 					logrus.Debugf("no UpdateKeywordFunc for %s; skipping", kv.Keyword())
 					continue
 				}
+
+				// TODO check for the type=dir of the entry as well
+				if kv.Keyword().Prefix() == "time" && e.IsDir() {
+					heap.Push(h, pathUpdate{
+						Path: pathname,
+						E:    e,
+						KV:   kv,
+						Func: ukFunc,
+					})
+
+					continue
+				}
+
 				if _, err := ukFunc(pathname, kv); err != nil {
 					results = append(results, InodeDelta{
 						diff: ErrorDifference,
@@ -75,16 +95,60 @@ func Update(root string, dh *DirectoryHierarchy, keywords []Keyword, fs FsEval)
 							},
 						}})
 				}
+				// XXX really would be great to have a Check() or Compare() right here,
+				// to compare each entry as it is encountered, rather than just running
+				// Check() on this path after the whole update is finished.
 			}
 		}
 	}
 
+	for h.Len() > 0 {
+		pu := heap.Pop(h).(pathUpdate)
+		if _, err := pu.Func(pu.Path, pu.KV); err != nil {
+			results = append(results, InodeDelta{
+				diff: ErrorDifference,
+				path: pu.Path,
+				old:  pu.E,
+				keys: []KeyDelta{
+					{
+						diff: ErrorDifference,
+						name: pu.KV.Keyword(),
+						err:  err,
+					},
+				}})
+		}
+	}
 	return results, nil
 }
 
-// Result of an "update" returns the produced results
-type Result struct {
-	Path    string
-	Keyword Keyword
-	Got     string
+type pathUpdateHeap []pathUpdate
+
+func (h pathUpdateHeap) Len() int      { return len(h) }
+func (h pathUpdateHeap) Swap(i, j int) { h[i], h[j] = h[j], h[i] }
+
+// This may end up looking backwards, but for container/heap, Less evaluates
+// the negative priority. So when popping members of the array, it will be
+// sorted by least. For this use-case, we want the most-qualified-name popped
+// first (the longest path name), such that "." is the last entry popped.
+func (h pathUpdateHeap) Less(i, j int) bool {
+	return len(h[i].Path) > len(h[j].Path)
+}
+
+func (h *pathUpdateHeap) Push(x interface{}) {
+	*h = append(*h, x.(pathUpdate))
+}
+
+func (h *pathUpdateHeap) Pop() interface{} {
+	old := *h
+	n := len(old)
+	x := old[n-1]
+	*h = old[0 : n-1]
+	return x
+}
+
+type pathUpdate struct {
+	Path string
+	E    Entry
+	KV   KeyVal
+	Func UpdateKeywordFunc
 }

update_test.go

@@ -3,6 +3,7 @@
 package mtree
 
 import (
+	"container/heap"
 	"encoding/json"
 	"io/ioutil"
 	"os"
@@ -11,8 +12,14 @@ import (
 	"strconv"
 	"testing"
 	"time"
+
+	"github.com/Sirupsen/logrus"
 )
 
+func init() {
+	logrus.SetLevel(logrus.DebugLevel)
+}
+
 func TestUpdate(t *testing.T) {
 	content := []byte("I know half of you half as well as I ought to")
 	dir, err := ioutil.TempDir("", "test-check-keywords")
@@ -103,3 +110,27 @@ func TestUpdate(t *testing.T) {
 	}
 
 }
+
+func TestPathUpdateHeap(t *testing.T) {
+	h := &pathUpdateHeap{
+		pathUpdate{Path: "not/the/longest"},
+		pathUpdate{Path: "almost/the/longest"},
+		pathUpdate{Path: "."},
+		pathUpdate{Path: "short"},
+	}
+	heap.Init(h)
+	v := "this/is/one/is/def/the/longest"
+	heap.Push(h, pathUpdate{Path: v})
+
+	longest := len(v)
+	var p string
+	for h.Len() > 0 {
+		p = heap.Pop(h).(pathUpdate).Path
+		if len(p) > longest {
+			t.Errorf("expected next path to be shorter, but it was not %q is longer than %d", p, longest)
+		}
+	}
+	if p != "." {
+		t.Errorf("expected \".\" to be the last, but got %q", p)
+	}
+}

updatefuncs.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/vbatts/go-mtree/pkg/govis"
 )
 
 // UpdateKeywordFunc is the signature for a function that will restore a file's
@@ -24,6 +25,7 @@ var UpdateKeywordFuncs = map[Keyword]UpdateKeywordFunc{
 	"uid":      uidUpdateKeywordFunc,
 	"gid":      gidUpdateKeywordFunc,
 	"xattr":    xattrUpdateKeywordFunc,
+	"link":     linkUpdateKeywordFunc,
 }
 
 func uidUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
@@ -31,6 +33,15 @@ func uidUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	stat, err := os.Lstat(path)
+	if err != nil {
+		return nil, err
+	}
+	if statIsUID(stat, uid) {
+		return stat, nil
+	}
+
 	if err := os.Lchown(path, uid, -1); err != nil {
 		return nil, err
 	}
@@ -42,6 +53,15 @@ func gidUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	stat, err := os.Lstat(path)
+	if err != nil {
+		return nil, err
+	}
+	if statIsGID(stat, gid) {
+		return stat, nil
+	}
+
 	if err := os.Lchown(path, -1, gid); err != nil {
 		return nil, err
 	}
@@ -49,10 +69,28 @@ func gidUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 }
 
 func modeUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
+	info, err := os.Lstat(path)
+	if err != nil {
+		return nil, err
+	}
+
+	// don't set mode on symlinks, as it passes through to the backing file
+	if info.Mode()&os.ModeSymlink != 0 {
+		return info, nil
+	}
 	vmode, err := strconv.ParseInt(kv.Value(), 8, 32)
 	if err != nil {
 		return nil, err
 	}
+
+	stat, err := os.Lstat(path)
+	if err != nil {
+		return nil, err
+	}
+	if stat.Mode() == os.FileMode(vmode) {
+		return stat, nil
+	}
+
 	logrus.Debugf("path: %q, kv.Value(): %q, vmode: %o", path, kv.Value(), vmode)
 	if err := os.Chmod(path, os.FileMode(vmode)); err != nil {
 		return nil, err
@@ -85,7 +123,19 @@ func tartimeUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 	}
 
 	vtime := time.Unix(sec, 0)
-	if err := os.Chtimes(path, vtime, vtime); err != nil {
+
+	// if times are same then don't modify anything
+	// comparing Unix, since it does not include Nano seconds
+	if info.ModTime().Unix() == vtime.Unix() {
+		return info, nil
+	}
+
+	// symlinks are strange and most of the time passes through to the backing file
+	if info.Mode()&os.ModeSymlink != 0 {
+		if err := lchtimes(path, vtime, vtime); err != nil {
+			return nil, err
+		}
+	} else if err := os.Chtimes(path, vtime, vtime); err != nil {
 		return nil, err
 	}
 	return os.Lstat(path)
@@ -93,6 +143,11 @@ func tartimeUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 
 // this is nano second precision
 func timeUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
+	info, err := os.Lstat(path)
+	if err != nil {
+		return nil, err
+	}
+
 	v := strings.SplitN(kv.Value(), ".", 2)
 	if len(v) != 2 {
 		return nil, fmt.Errorf("expected a number like 1469104727.871937272")
@@ -101,11 +156,46 @@ func timeUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 	if err != nil {
 		return nil, fmt.Errorf("expected nano seconds, but got %q", v[0]+v[1])
 	}
-	logrus.Debugf("arg: %q; nsec: %q", v[0]+v[1], nsec)
+	logrus.Debugf("arg: %q; nsec: %d", v[0]+v[1], nsec)
 
 	vtime := time.Unix(0, nsec)
-	if err := os.Chtimes(path, vtime, vtime); err != nil {
+
+	// if times are same then don't modify anything
+	if info.ModTime().Equal(vtime) {
+		return info, nil
+	}
+
+	// symlinks are strange and most of the time passes through to the backing file
+	if info.Mode()&os.ModeSymlink != 0 {
+		if err := lchtimes(path, vtime, vtime); err != nil {
+			return nil, err
+		}
+	} else if err := os.Chtimes(path, vtime, vtime); err != nil {
 		return nil, err
 	}
 	return os.Lstat(path)
 }
+
+func linkUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
+	linkname, err := govis.Unvis(kv.Value(), DefaultVisFlags)
+	if err != nil {
+		return nil, err
+	}
+	got, err := os.Readlink(path)
+	if err != nil {
+		return nil, err
+	}
+	if got == linkname {
+		return os.Lstat(path)
+	}
+
+	logrus.Debugf("linkUpdateKeywordFunc: removing %q to link to %q", path, linkname)
+	if err := os.Remove(path); err != nil {
+		return nil, err
+	}
+	if err := os.Symlink(linkname, path); err != nil {
+		return nil, err
+	}
+
+	return os.Lstat(path)
+}

updatefuncs_linux.go

@@ -5,6 +5,9 @@ package mtree
 import (
 	"encoding/base64"
 	"os"
+	"syscall"
+	"time"
+	"unsafe"
 
 	"github.com/vbatts/go-mtree/xattr"
 )
@@ -19,3 +22,37 @@ func xattrUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 	}
 	return os.Lstat(path)
 }
+
+func lchtimes(name string, atime time.Time, mtime time.Time) error {
+	var utimes [2]syscall.Timespec
+	utimes[0] = syscall.NsecToTimespec(atime.UnixNano())
+	utimes[1] = syscall.NsecToTimespec(mtime.UnixNano())
+	if e := utimensat(atFdCwd, name, (*[2]syscall.Timespec)(unsafe.Pointer(&utimes[0])), atSymlinkNofollow); e != nil {
+		return &os.PathError{Op: "chtimes", Path: name, Err: e}
+	}
+	return nil
+
+}
+
+// from uapi/linux/fcntl.h
+// don't follow symlinks
+const atSymlinkNofollow = 0x100
+
+// special value for utimes as the FD for the current working directory
+const atFdCwd = -0x64
+
+func utimensat(dirfd int, path string, times *[2]syscall.Timespec, flags int) (err error) {
+	if len(times) != 2 {
+		return syscall.EINVAL
+	}
+	var _p0 *byte
+	_p0, err = syscall.BytePtrFromString(path)
+	if err != nil {
+		return
+	}
+	_, _, e1 := syscall.Syscall6(syscall.SYS_UTIMENSAT, uintptr(dirfd), uintptr(unsafe.Pointer(_p0)), uintptr(unsafe.Pointer(times)), uintptr(flags), 0, 0)
+	if e1 != 0 {
+		err = syscall.Errno(e1)
+	}
+	return
+}

updatefuncs_unsupported.go

@@ -2,8 +2,15 @@
 
 package mtree
 
-import "os"
+import (
+	"os"
+	"time"
+)
 
 func xattrUpdateKeywordFunc(path string, kv KeyVal) (os.FileInfo, error) {
 	return os.Lstat(path)
 }
+
+func lchtimes(name string, atime time.Time, mtime time.Time) error {
+	return nil
+}