overwrite lower tree payloads while comparing (fixes #160) (#161)

7 files changed, 74 insertions, 44 deletions

diff --git a/.data/Dockerfile.example b/.data/Dockerfile.example
index 3d47590..eb09f63 100644
--- a/.data/Dockerfile.example
+++ b/.data/Dockerfile.example
@@ -12,3 +12,4 @@ ADD .scripts/ /root/.data/
 RUN cp /root/saved.txt /tmp/saved.again1.txt
 RUN cp /root/saved.txt /root/.data/saved.again2.txt
 RUN chmod +x /root/saved.txt
+RUN chmod 421 /root
diff --git a/filetree/cache.go b/filetree/cache.go
index 0b19fc6..0d69380 100644
--- a/filetree/cache.go
+++ b/filetree/cache.go
@@ -23,9 +23,8 @@ func (cache *TreeCache) Get(bottomTreeStart, bottomTreeStop, topTreeStart, topTr
 
 func (cache *TreeCache) buildTree(key TreeCacheKey) *FileTree {
 	newTree := StackTreeRange(cache.refTrees, key.bottomTreeStart, key.bottomTreeStop)
-
 	for idx := key.topTreeStart; idx <= key.topTreeStop; idx++ {
-		newTree.Compare(cache.refTrees[idx])
+		newTree.CompareAndMark(cache.refTrees[idx])
 	}
 	return newTree
 }
diff --git a/filetree/efficiency.go b/filetree/efficiency.go
index 68ed320..5d02241 100644
--- a/filetree/efficiency.go
+++ b/filetree/efficiency.go
@@ -56,7 +56,10 @@ func Efficiency(trees []*FileTree) (float64, EfficiencySlice) {
 			if err != nil {
 				logrus.Debug(fmt.Sprintf("CurrentTree: %d : %s", currentTree, err))
 			} else if previousTreeNode.Data.FileInfo.IsDir {
-				previousTreeNode.VisitDepthChildFirst(sizer, nil)
+				err = previousTreeNode.VisitDepthChildFirst(sizer, nil)
+				if err != nil {
+					logrus.Errorf("unable to propagate whiteout dir: %+v", err)
+				}
 			}
 
 		} else {
@@ -80,7 +83,10 @@ func Efficiency(trees []*FileTree) (float64, EfficiencySlice) {
 	}
 	for idx, tree := range trees {
 		currentTree = idx
-		tree.VisitDepthChildFirst(visitor, visitEvaluator)
+		err := tree.VisitDepthChildFirst(visitor, visitEvaluator)
+		if err != nil {
+			logrus.Errorf("unable to propagate ref tree: %+v", err)
+		}
 	}
 
 	// calculate the score
diff --git a/filetree/node.go b/filetree/node.go
index 8447602..7a2d2cf 100644
--- a/filetree/node.go
+++ b/filetree/node.go
@@ -3,6 +3,7 @@ package filetree
 import (
 	"archive/tar"
 	"fmt"
+	"github.com/sirupsen/logrus"
 	"sort"
 	"strings"
 
@@ -149,7 +150,10 @@ func (node *FileNode) MetadataString() string {
 			return nil
 		}
 
-		node.VisitDepthChildFirst(sizer, nil)
+		err := node.VisitDepthChildFirst(sizer, nil)
+		if err != nil {
+			logrus.Errorf("unable to propagate node for metadata: %+v", err)
+		}
 	}
 
 	size := humanize.Bytes(uint64(sizeBytes))
@@ -258,7 +262,6 @@ func (node *FileNode) deriveDiffType(diffType DiffType) error {
 	myDiffType := diffType
 	for _, v := range node.Children {
 		myDiffType = myDiffType.merge(v.Data.DiffType)
-
 	}
 
 	return node.AssignDiffType(myDiffType)
diff --git a/filetree/tree.go b/filetree/tree.go
index 4255ef3..b4584ff 100644
--- a/filetree/tree.go
+++ b/filetree/tree.go
@@ -137,11 +137,15 @@ func (tree *FileTree) Copy() *FileTree {
 	newTree.Root = tree.Root.Copy(newTree.Root)
 
 	// update the tree pointers
-	newTree.VisitDepthChildFirst(func(node *FileNode) error {
+	err := newTree.VisitDepthChildFirst(func(node *FileNode) error {
 		node.Tree = newTree
 		return nil
 	}, nil)
 
+	if err != nil {
+		logrus.Errorf("unable to propagate tree on copy(): %+v", err)
+	}
+
 	return newTree
 }
 
@@ -239,13 +243,14 @@ func (tree *FileTree) RemovePath(path string) error {
 }
 
 type compareMark struct {
-	node      *FileNode
+	lowerNode *FileNode
+	upperNode *FileNode
 	tentative DiffType
 	final     DiffType
 }
 
-// Compare marks the FileNodes in the owning (lower) tree with DiffType annotations when compared to the given (upper) tree.
-func (tree *FileTree) Compare(upper *FileTree) error {
+// CompareAndMark marks the FileNodes in the owning (lower) tree with DiffType annotations when compared to the given (upper) tree.
+func (tree *FileTree) CompareAndMark(upper *FileTree) error {
 	// always compare relative to the original, unaltered tree.
 	originalTree := tree
 
@@ -257,28 +262,30 @@ func (tree *FileTree) Compare(upper *FileTree) error {
 			if err != nil {
 				return fmt.Errorf("cannot remove upperNode %s: %v", upperNode.Path(), err.Error())
 			}
-		} else {
-			// note: since we are not comparing against the original tree (copying the tree is expensive) we may mark the parent
-			// of an added node incorrectly as modified. This will be corrected later.
-			originalLowerNode, _ := originalTree.GetNode(upperNode.Path())
-
-			if originalLowerNode == nil {
-				_, newNodes, err := tree.AddPath(upperNode.Path(), upperNode.Data.FileInfo)
-				if err != nil {
-					return fmt.Errorf("cannot add new upperNode %s: %v", upperNode.Path(), err.Error())
-				}
-				for idx := len(newNodes) - 1; idx >= 0; idx-- {
-					newNode := newNodes[idx]
-					modifications = append(modifications, compareMark{node: newNode, tentative: -1, final: Added})
-				}
-
-			} else {
-				// check the tree for comparison markings
-				lowerNode, _ := tree.GetNode(upperNode.Path())
-				diffType := lowerNode.compare(upperNode)
-				modifications = append(modifications, compareMark{node: lowerNode, tentative: diffType, final: -1})
+			return nil
+		}
+
+		// note: since we are not comparing against the original tree (copying the tree is expensive) we may mark the parent
+		// of an added node incorrectly as modified. This will be corrected later.
+		originalLowerNode, _ := originalTree.GetNode(upperNode.Path())
+
+		if originalLowerNode == nil {
+			_, newNodes, err := tree.AddPath(upperNode.Path(), upperNode.Data.FileInfo)
+			if err != nil {
+				return fmt.Errorf("cannot add new upperNode %s: %v", upperNode.Path(), err.Error())
+			}
+			for idx := len(newNodes) - 1; idx >= 0; idx-- {
+				newNode := newNodes[idx]
+				modifications = append(modifications, compareMark{lowerNode: newNode, upperNode: upperNode, tentative: -1, final: Added})
 			}
+			return nil
 		}
+
+		// the file exists in the lower layer
+		lowerNode, _ := tree.GetNode(upperNode.Path())
+		diffType := lowerNode.compare(upperNode)
+		modifications = append(modifications, compareMark{lowerNode: lowerNode, upperNode: upperNode, tentative: diffType, final: -1})
+
 		return nil
 	}
 	// we must visit from the leaves upwards to ensure that diff types can be derived from and assigned to children
@@ -287,15 +294,16 @@ func (tree *FileTree) Compare(upper *FileTree) error {
 		return err
 	}
 
-	// take note of the comparison results on each note in the owning tree
+	// take note of the comparison results on each note in the owning tree.
 	for _, pair := range modifications {
 		if pair.final > 0 {
-			pair.node.AssignDiffType(pair.final)
-		} else {
-			if pair.node.Data.DiffType == Unchanged {
-				pair.node.deriveDiffType(pair.tentative)
-			}
+			pair.lowerNode.AssignDiffType(pair.final)
+		} else if pair.lowerNode.Data.DiffType == Unchanged {
+			pair.lowerNode.deriveDiffType(pair.tentative)
 		}
+
+		// persist the upper's payload on the owning tree
+		pair.lowerNode.Data.FileInfo = *pair.upperNode.Data.FileInfo.Copy()
 	}
 	return nil
 }
@@ -316,7 +324,7 @@ func StackTreeRange(trees []*FileTree, start, stop int) *FileTree {
 	for idx := start; idx <= stop; idx++ {
 		err := tree.Stack(trees[idx])
 		if err != nil {
-			logrus.Debug("could not stack tree range:", err)
+			logrus.Errorf("could not stack tree range: %v", err)
 		}
 	}
 	return tree
diff --git a/filetree/tree_test.go b/filetree/tree_test.go
index 15eb498..b1a2c6b 100644
--- a/filetree/tree_test.go
+++ b/filetree/tree_test.go
@@ -268,7 +268,7 @@ func TestCompareWithNoChanges(t *testing.T) {
 		lowerTree.AddPath(value, fakeData)
 		upperTree.AddPath(value, fakeData)
 	}
-	lowerTree.Compare(upperTree)
+	lowerTree.CompareAndMark(upperTree)
 	asserter := func(n *FileNode) error {
 		if n.Path() == "/" {
 			return nil
@@ -307,7 +307,7 @@ func TestCompareWithAdds(t *testing.T) {
 	}
 
 	failedAssertions := []error{}
-	err := lowerTree.Compare(upperTree)
+	err := lowerTree.CompareAndMark(upperTree)
 	if err != nil {
 		t.Errorf("Expected tree compare to have no errors, got: %v", err)
 	}
@@ -403,7 +403,7 @@ func TestCompareWithChanges(t *testing.T) {
 
 	changedPaths = append(changedPaths, chownPath)
 
-	lowerTree.Compare(upperTree)
+	lowerTree.CompareAndMark(upperTree)
 	failedAssertions := []error{}
 	asserter := func(n *FileNode) error {
 		p := n.Path()
@@ -458,7 +458,7 @@ func TestCompareWithRemoves(t *testing.T) {
 		upperTree.AddPath(value, fakeData)
 	}
 
-	lowerTree.Compare(upperTree)
+	lowerTree.CompareAndMark(upperTree)
 	failedAssertions := []error{}
 	asserter := func(n *FileNode) error {
 		p := n.Path()
diff --git a/ui/filetreeview.go b/ui/filetreeview.go
index 8f42cd4..0e36ccb 100644
--- a/ui/filetreeview.go
+++ b/ui/filetreeview.go
@@ -226,7 +226,10 @@ func (view *FileTreeView) setTreeByLayer(bottomTreeStart, bottomTreeStop, topTre
 		}
 		return nil
 	}
-	view.ModelTree.VisitDepthChildFirst(visitor, nil)
+	err := view.ModelTree.VisitDepthChildFirst(visitor, nil)
+	if err != nil {
+		logrus.Errorf("unable to propagate layer tree: %+v", err)
+	}
 
 	view.resetCursor()
 
@@ -504,12 +507,13 @@ func (view *FileTreeView) Update() error {
 	regex := filterRegex()
 
 	// keep the view selection in parity with the current DiffType selection
-	view.ModelTree.VisitDepthChildFirst(func(node *filetree.FileNode) error {
+	err := view.ModelTree.VisitDepthChildFirst(func(node *filetree.FileNode) error {
 		node.Data.ViewInfo.Hidden = view.HiddenDiffTypes[node.Data.DiffType]
 		visibleChild := false
 		for _, child := range node.Children {
 			if !child.Data.ViewInfo.Hidden {
 				visibleChild = true
+				node.Data.ViewInfo.Hidden = false
 			}
 		}
 		if regex != nil && !visibleChild {
@@ -519,14 +523,23 @@ func (view *FileTreeView) Update() error {
 		return nil
 	}, nil)
 
+	if err != nil {
+		logrus.Errorf("unable to propagate model tree: %+v", err)
+	}
+
 	// make a new tree with only visible nodes
 	view.ViewTree = view.ModelTree.Copy()
-	view.ViewTree.VisitDepthParentFirst(func(node *filetree.FileNode) error {
+	err = view.ViewTree.VisitDepthParentFirst(func(node *filetree.FileNode) error {
 		if node.Data.ViewInfo.Hidden {
 			view.ViewTree.RemovePath(node.Path())
 		}
 		return nil
 	}, nil)
+
+	if err != nil {
+		logrus.Errorf("unable to propagate view tree: %+v", err)
+	}
+
 	return nil
 }