Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Commit f64e4b8

Browse files
authored
Merge pull request #300 from alcortesm/improvement-difftree-simplify-deprecated
difftree: simplify hash comparison with deprecated files modes
2 parents 199317f + b3aa41a commit f64e4b8

File tree

3 files changed

+31
-51
lines changed

3 files changed

+31
-51
lines changed

plumbing/object/difftree.go

Lines changed: 4 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package object
33
import (
44
"bytes"
55

6-
"srcd.works/go-git.v4/plumbing/filemode"
76
"srcd.works/go-git.v4/utils/merkletrie"
87
"srcd.works/go-git.v4/utils/merkletrie/noder"
98
)
@@ -14,46 +13,14 @@ func DiffTree(a, b *Tree) (Changes, error) {
1413
from := newTreeNoder(a)
1514
to := newTreeNoder(b)
1615

16+
hashEqual := func(a, b noder.Hasher) bool {
17+
return bytes.Equal(a.Hash(), b.Hash())
18+
}
19+
1720
merkletrieChanges, err := merkletrie.DiffTree(from, to, hashEqual)
1821
if err != nil {
1922
return nil, err
2023
}
2124

2225
return newChanges(merkletrieChanges)
2326
}
24-
25-
// check if the hash of the contents is different, if not, check if
26-
// the permissions are different (but taking into account deprecated
27-
// file modes). On a treenoder, the hash of the contents is codified
28-
// in the first 20 bytes of the data returned by Hash() and the last
29-
// 4 bytes is the mode.
30-
func hashEqual(a, b noder.Hasher) bool {
31-
hashA, hashB := a.Hash(), b.Hash()
32-
contentsA, contentsB := hashA[:20], hashB[:20]
33-
34-
sameContents := bytes.Equal(contentsA, contentsB)
35-
if !sameContents {
36-
return false
37-
}
38-
39-
modeA, modeB := hashA[20:], hashB[20:]
40-
41-
return equivalentMode(modeA, modeB)
42-
}
43-
44-
func equivalentMode(a, b []byte) bool {
45-
if isFilish(a) && isFilish(b) {
46-
return true
47-
}
48-
return bytes.Equal(a, b)
49-
}
50-
51-
var (
52-
file = filemode.Regular.Bytes()
53-
fileDeprecated = filemode.Deprecated.Bytes()
54-
)
55-
56-
func isFilish(b []byte) bool {
57-
return bytes.Equal(b, file) ||
58-
bytes.Equal(b, fileDeprecated)
59-
}

plumbing/object/difftree_test.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -358,24 +358,27 @@ func (s *DiffTreeSuite) TestDiffTree(c *C) {
358358
}
359359

360360
func (s *DiffTreeSuite) TestIssue279(c *C) {
361-
// HashEqual should ignore files if the only change is from a 100664
362-
// mode to a 100644 or vice versa.
363-
from := &treeNoder{
364-
hash: plumbing.NewHash("d08e895238bac36d8220586fdc28c27e1a7a76d3"),
361+
// treeNoders should have the same hash when their mode is
362+
// filemode.Deprecated and filemode.Regular.
363+
a := &treeNoder{
364+
hash: plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
365365
mode: filemode.Regular,
366366
}
367-
to := &treeNoder{
368-
hash: plumbing.NewHash("d08e895238bac36d8220586fdc28c27e1a7a76d3"),
369-
mode: filemode.Regular,
367+
b := &treeNoder{
368+
hash: plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
369+
mode: filemode.Deprecated,
370370
}
371-
c.Assert(hashEqual(from, to), Equals, true)
372-
c.Assert(hashEqual(to, from), Equals, true)
371+
c.Assert(a.Hash(), DeepEquals, b.Hash())
373372

374-
// but should detect if the contents of the file also changed.
375-
to = &treeNoder{
376-
hash: plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
373+
// yet, they should have different hashes if their contents change.
374+
aa := &treeNoder{
375+
hash: plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"),
377376
mode: filemode.Regular,
378377
}
379-
c.Assert(hashEqual(from, to), Equals, false)
380-
c.Assert(hashEqual(to, from), Equals, false)
378+
c.Assert(a.Hash(), Not(DeepEquals), aa.Hash())
379+
bb := &treeNoder{
380+
hash: plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"),
381+
mode: filemode.Deprecated,
382+
}
383+
c.Assert(b.Hash(), Not(DeepEquals), bb.Hash())
381384
}

plumbing/object/treenoder.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,17 @@ func (t *treeNoder) String() string {
4545
return "treeNoder <" + t.name + ">"
4646
}
4747

48+
// The hash of a treeNoder is the result of concatenating the hash of
49+
// its contents and its mode; that way the difftree algorithm will
50+
// detect changes in the contents of files and also in their mode.
51+
//
52+
// Files with Regular and Deprecated file modes are considered the same
53+
// for the purpose of difftree, so Regular will be used as the mode for
54+
// Deprecated files here.
4855
func (t *treeNoder) Hash() []byte {
56+
if t.mode == filemode.Deprecated {
57+
return append(t.hash[:], filemode.Regular.Bytes()...)
58+
}
4959
return append(t.hash[:], t.mode.Bytes()...)
5060
}
5161

0 commit comments

Comments
 (0)