From 9943096f6a53fb670c36a27af0d133c6acc944fc Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Mon, 18 Feb 2019 16:21:43 -0500 Subject: [PATCH] overwrite lower tree payloads while comparing (fixes #160) (#161) --- .data/Dockerfile.example | 1 + filetree/cache.go | 3 +- filetree/efficiency.go | 10 ++++-- filetree/node.go | 7 ++-- filetree/tree.go | 72 ++++++++++++++++++++++------------------ filetree/tree_test.go | 8 ++--- ui/filetreeview.go | 19 +++++++++-- 7 files changed, 75 insertions(+), 45 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 }