Merge pull request #90 from vmarkovtsev/master

Survive git/git again

internal/burndown/file.go

@@ -2,8 +2,8 @@ package burndown
 
 import (
 	"fmt"
+	"log"
 
-	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/hercules.v4/internal"
 	"gopkg.in/src-d/hercules.v4/internal/rbtree"
 )
@@ -23,9 +23,6 @@ type Updater = func(currentTime, previousTime, delta int)
 //
 // Dump() writes the tree to a string and Validate() checks the tree integrity.
 type File struct {
-	// Git hash of the contents.
-	Hash     plumbing.Hash
-
 	tree     *rbtree.RBTree
 	updaters []Updater
 }
@@ -59,8 +56,8 @@ func (file *File) updateTime(currentTime, previousTime, delta int) {
 // last node);
 //
 // updaters are the attached interval length mappings.
-func NewFile(hash plumbing.Hash, time int, length int, updaters ...Updater) *File {
-	file := &File{Hash: hash, tree: new(rbtree.RBTree), updaters: updaters}
+func NewFile(time int, length int, updaters ...Updater) *File {
+	file := &File{tree: new(rbtree.RBTree), updaters: updaters}
 	file.updateTime(time, time, length)
 	if length > 0 {
 		file.tree.Insert(rbtree.Item{Key: 0, Value: time})
@@ -77,8 +74,8 @@ func NewFile(hash plumbing.Hash, time int, length int, updaters ...Updater) *Fil
 // vals is a slice with the starting tree values. Must match the size of keys.
 //
 // updaters are the attached interval length mappings.
-func NewFileFromTree(hash plumbing.Hash, keys []int, vals []int, updaters ...Updater) *File {
-	file := &File{Hash: hash, tree: new(rbtree.RBTree), updaters: updaters}
+func NewFileFromTree(keys []int, vals []int, updaters ...Updater) *File {
+	file := &File{tree: new(rbtree.RBTree), updaters: updaters}
 	if len(keys) != len(vals) {
 		panic("keys and vals must be of equal length")
 	}
@@ -93,7 +90,7 @@ func NewFileFromTree(hash plumbing.Hash, keys []int, vals []int, updaters ...Upd
 // depending on `clearStatuses` the original updaters are removed or not.
 // Any new `updaters` are appended.
 func (file *File) Clone(clearStatuses bool, updaters ...Updater) *File {
-	clone := &File{Hash: file.Hash, tree: file.tree.Clone(), updaters: file.updaters}
+	clone := &File{tree: file.tree.Clone(), updaters: file.updaters}
 	if clearStatuses {
 		clone.updaters = []Updater{}
 	}
@@ -235,27 +232,17 @@ func (file *File) Update(time int, pos int, insLength int, delLength int) {
 	}
 }
 
-// Merge combines several prepared File-s together. Returns the value
-// indicating whether at least one File required merging.
-func (file *File) Merge(day int, others... *File) bool {
-	dirty := false
+// Merge combines several prepared File-s together.
+func (file *File) Merge(day int, others... *File) {
+	myself := file.flatten()
 	for _, other := range others {
 		if other == nil {
-			panic("merging File with nil")
-		}
-		if file.Hash != other.Hash || other.Hash == plumbing.ZeroHash {
-			dirty = true
-			break
+			log.Panic("merging with a nil file")
 		}
-	}
-	if !dirty {
-		return false
-	}
-	myself := file.flatten()
-	for _, other := range others {
 		lines := other.flatten()
 		if len(myself) != len(lines) {
-			panic("file corruption, lines number mismatch during merge")
+			log.Panicf("file corruption, lines number mismatch during merge %d != %d",
+				len(myself), len(lines))
 		}
 		for i, l := range myself {
 			ol := lines[i]
@@ -293,7 +280,6 @@ func (file *File) Merge(day int, others... *File) bool {
 	}
 	tree.Insert(rbtree.Item{Key: len(myself), Value: TreeEnd})
 	file.tree = tree
-	return true
 }
 
 // Dump formats the underlying line interval tree into a string.

internal/burndown/file_test.go

@@ -6,7 +6,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/src-d/hercules.v4/internal/rbtree"
 	"fmt"
-	"gopkg.in/src-d/go-git.v4/plumbing"
 )
 
 func updateStatusFile(status map[int]int64, _, previousTime, delta int) {
@@ -15,7 +14,7 @@ func updateStatusFile(status map[int]int64, _, previousTime, delta int) {
 
 func fixtureFile() (*File, map[int]int64) {
 	status := map[int]int64{}
-	file := NewFile(plumbing.ZeroHash, 0, 100, func(a, b, c int) {
+	file := NewFile(0, 100, func(a, b, c int) {
 		updateStatusFile(status, a, b, c)
 	})
 	return file, status
@@ -171,7 +170,7 @@ func TestInsertFile(t *testing.T) {
 
 func TestZeroInitializeFile(t *testing.T) {
 	status := map[int]int64{}
-	file := NewFile(plumbing.ZeroHash, 0, 0, func(a, b, c int) {
+	file := NewFile(0, 0, func(a, b, c int) {
 		updateStatusFile(status, a, b, c)
 	})
 	assert.Contains(t, status, 0)
@@ -424,7 +423,7 @@ func TestBug3File(t *testing.T) {
 
 func TestBug4File(t *testing.T) {
 	status := map[int]int64{}
-	file := NewFile(plumbing.ZeroHash, 0, 10, func(a, b, c int) {
+	file := NewFile(0, 10, func(a, b, c int) {
 		updateStatusFile(status, a, b, c)
 	})
 	file.Update(125, 0, 20, 9)
@@ -456,7 +455,7 @@ func TestBug5File(t *testing.T) {
 	status := map[int]int64{}
 	keys := []int{0, 2, 4, 7, 10}
 	vals := []int{24, 28, 24, 28, -1}
-	file := NewFileFromTree(plumbing.ZeroHash, keys, vals, func(a, b, c int) {
+	file := NewFileFromTree(keys, vals, func(a, b, c int) {
 		updateStatusFile(status, a, b, c)
 	})
 	file.Update(28, 0, 1, 3)
@@ -465,7 +464,7 @@ func TestBug5File(t *testing.T) {
 
 	keys = []int{0, 1, 16, 18}
 	vals = []int{305, 0, 157, -1}
-	file = NewFileFromTree(plumbing.ZeroHash, keys, vals, func(a, b, c int) {
+	file = NewFileFromTree(keys, vals, func(a, b, c int) {
 		updateStatusFile(status, a, b, c)
 	})
 	file.Update(310, 0, 0, 2)
@@ -476,13 +475,13 @@ func TestBug5File(t *testing.T) {
 func TestNewFileFromTreeInvalidSize(t *testing.T) {
 	keys := [...]int{1, 2, 3}
 	vals := [...]int{4, 5}
-	assert.Panics(t, func() { NewFileFromTree(plumbing.ZeroHash, keys[:], vals[:]) })
+	assert.Panics(t, func() { NewFileFromTree(keys[:], vals[:]) })
 }
 
 func TestUpdatePanic(t *testing.T) {
 	keys := [...]int{0}
 	vals := [...]int{-1}
-	file := NewFileFromTree(plumbing.ZeroHash, keys[:], vals[:])
+	file := NewFileFromTree(keys[:], vals[:])
 	file.tree.DeleteWithKey(0)
 	file.tree.Insert(rbtree.Item{Key: -1, Value: -1})
 	var paniced interface{}
@@ -498,7 +497,7 @@ func TestUpdatePanic(t *testing.T) {
 func TestFileValidate(t *testing.T) {
 	keys := [...]int{0}
 	vals := [...]int{-1}
-	file := NewFileFromTree(plumbing.ZeroHash, keys[:], vals[:])
+	file := NewFileFromTree(keys[:], vals[:])
 	file.tree.DeleteWithKey(0)
 	file.tree.Insert(rbtree.Item{Key: -1, Value: -1})
 	assert.Panics(t, func() { file.Validate() })
@@ -572,7 +571,6 @@ func TestFileMergeMark(t *testing.T) {
 
 func TestFileMerge(t *testing.T) {
 	file1, status := fixtureFile()
-	file1.Hash = plumbing.NewHash("0b7101095af6f90a3a2f3941fdf82563a83ce4db")
 	// 0 0 | 100 -1                             [0]: 100
 	file1.Update(1, 20, 30, 0)
 	// 0 0 | 20 1 | 50 0 | 130 -1               [0]: 100, [1]: 30
@@ -583,7 +581,6 @@ func TestFileMerge(t *testing.T) {
 	file1.Update(4, 20, 10, 0)
 	// 0 0 | 20 4 | 30 1 | 50 0 | 130 -1        [0]: 100, [1]: 20, [4]: 10
 	file2 := file1.Clone(false)
-	assert.Equal(t, file1.Hash, file2.Hash)
 	file1.Update(TreeMergeMark, 60, 30, 30)
 	// 0 0 | 20 4 | 30 1 | 50 0 | 60 M | 90 0 | 130 -1
 	// [0]: 70, [1]: 20, [4]: 10
@@ -596,9 +593,7 @@ func TestFileMerge(t *testing.T) {
 	file2.Update(6, 0, 10, 10)
 	// 0 6 | 10 0 | 20 4 | 30 1 | 50 0 | 60 5 | 80 M | 90 0 | 130 -1
 	// [0]: 60, [1]: 20, [4]: 10, [5]: 20, [6]: 10
-	file2.Hash = plumbing.ZeroHash
-	dirty := file1.Merge(7, file2)
-	assert.True(t, dirty)
+	file1.Merge(7, file2)
 	// 0 0 | 20 4 | 30 1 | 50 0 | 60 5 | 80 7 | 90 0 | 130 -1
 	// [0]: 70, [1]: 20, [4]: 10, [5]: 20, [6]: 0, [7]: 10
 	dump := file1.Dump()
@@ -614,47 +609,9 @@ func TestFileMerge(t *testing.T) {
 }
 
 func TestFileMergeNoop(t *testing.T) {
-	file1, status := fixtureFile()
+	file1, _ := fixtureFile()
 	// 0 0 | 100 -1                             [0]: 100
-	file1.Update(1, 20, 30, 0)
-	// 0 0 | 20 1 | 50 0 | 130 -1               [0]: 100, [1]: 30
-	file1.Update(2, 20, 0, 5)
-	// 0 0 | 20 1 | 45 0 | 125 -1               [0]: 100, [1]: 25
-	file1.Update(3, 20, 0, 5)
-	// 0 0 | 20 1 | 40 0 | 120 -1               [0]: 100, [1]: 20
-	file1.Update(4, 20, 10, 0)
-	// 0 0 | 20 4 | 30 1 | 50 0 | 130 -1        [0]: 100, [1]: 20, [4]: 10
-	file2 := file1.Clone(false)
-
-	dirty := file1.Merge(7, file2)
-	assert.True(t, dirty)
-	dirty = file1.Merge(7, file2)
-	assert.True(t, dirty)
-	file1.Hash = plumbing.NewHash("ffffffffffffffffffffffffffffffffffffffff")
-	file2.Hash = plumbing.NewHash("ffffffffffffffffffffffffffffffffffffffff")
-	dirty = file1.Merge(7, file2)
-	assert.False(t, dirty)
-	file2.Hash = plumbing.ZeroHash
-	dirty = file1.Merge(7, file2)
-	assert.True(t, dirty)
-
-	dump1 := file1.Dump()
-	dump2 := file2.Dump()
-	assert.Equal(t, dump1, dump2)
-	assert.Equal(t, dump1, "0 0\n20 4\n30 1\n50 0\n130 -1\n")
-	assert.Equal(t, int64(100), status[0])
-	assert.Equal(t, int64(20), status[1])
-	assert.Equal(t, int64(0), status[2])
-	assert.Equal(t, int64(0), status[3])
-	assert.Equal(t, int64(10), status[4])
-	file1.Update(TreeMergeMark, 60, 30, 30)
-	// 0 0 | 20 4 | 30 1 | 50 0 | 60 M | 90 0 | 130 -1
-	// [0]: 70, [1]: 20, [4]: 10
-	file1.Hash = plumbing.NewHash("ffffffffffffffffffffffffffffffffffffffff")
-	file2.Hash = plumbing.NewHash("ffffffffffffffffffffffffffffffffffffffff")
-	dirty = file1.Merge(7, file2)
-	// because the hashes are the same
-	assert.False(t, dirty)
+	assert.Panics(t, func() { file1.Merge(3, nil) })
 }
 
 func TestFileMergeNil(t *testing.T) {

leaves/burndown.go

@@ -67,7 +67,7 @@ type BurndownAnalysis struct {
 	// files is the mapping <file path> -> *File.
 	files map[string]*burndown.File
 	// mergedFiles is used during merges to record the real file hashes
-	mergedFiles map[*burndown.File]plumbing.Hash
+	mergedFiles map[string]bool
 	// renames is a quick and dirty solution for the "future branch renames" problem.
 	renames map[string]string
 	// matrix is the mutual deletions and self insertions.
@@ -242,7 +242,7 @@ func (analyser *BurndownAnalysis) Initialize(repository *git.Repository) {
 	analyser.fileHistories = map[string]sparseHistory{}
 	analyser.peopleHistories = make([]sparseHistory, analyser.PeopleNumber)
 	analyser.files = map[string]*burndown.File{}
-	analyser.mergedFiles = map[*burndown.File]plumbing.Hash{}
+	analyser.mergedFiles = map[string]bool{}
 	analyser.renames = map[string]string{}
 	analyser.matrix = make([]map[int]int64, analyser.PeopleNumber)
 	analyser.day = 0
@@ -264,6 +264,7 @@ func (analyser *BurndownAnalysis) Consume(deps map[string]interface{}) (map[stri
 		// effectively disables the status updates if the commit is a merge
 		// we will analyse the conflicts resolution in Merge()
 		analyser.day = burndown.TreeMergeMark
+		analyser.mergedFiles = map[string]bool{}
 	}
 	cache := deps[items.DependencyBlobCache].(map[plumbing.Hash]*object.Blob)
 	treeDiffs := deps[items.DependencyTreeChanges].(object.Changes)
@@ -305,35 +306,47 @@ func (analyser *BurndownAnalysis) Fork(n int) []core.PipelineItem {
 
 // Merge combines several items together. We apply the special file merging logic here.
 func (analyser *BurndownAnalysis) Merge(branches []core.PipelineItem) {
-	for key, file := range analyser.files {
-		others := make([]*burndown.File, 0, len(branches))
-		for _, branch := range branches {
-			otherFile := branch.(*BurndownAnalysis).files[key]
-			if otherFile != nil {
-				// otherFile can be nil if it is considered binary in the other branch
-				others = append(others, otherFile)
-			}
+	all := make([]*BurndownAnalysis, len(branches) + 1)
+	all[0] = analyser
+	for i, branch := range branches {
+		all[i+1] = branch.(*BurndownAnalysis)
+	}
+	keys := map[string]bool{}
+	for _, burn := range all {
+		for key, val := range burn.mergedFiles {
+			// (*)
+			// there can be contradicting flags,
+			// e.g. item was renamed and a new item written on its place
+			// this may be not exactly accurate
+			keys[key] = keys[key] || val
 		}
-		// don't worry, we compare the hashes first before heavy-lifting
-		if file.Merge(analyser.day, others...) {
-			hash := analyser.mergedFiles[file]
-			if hash != plumbing.ZeroHash {
-				delete(analyser.mergedFiles, file)
+	}
+	for key, val := range keys {
+		if !val {
+			for _, burn := range all {
+				delete(burn.files, key)
 			}
-			for _, branch := range branches {
-				otherAnalyser := branch.(*BurndownAnalysis)
-				otherHash := otherAnalyser.mergedFiles[otherAnalyser.files[key]]
-				if otherHash != plumbing.ZeroHash {
-					delete(otherAnalyser.mergedFiles, otherAnalyser.files[key])
-					if hash != plumbing.ZeroHash && hash != otherHash {
-						log.Panicf("merged hash mismatch: %s", key)
-					}
-					hash = otherHash
-				}
+			continue
+		}
+		files := make([]*burndown.File, 0, len(all))
+		for _, burn := range all {
+			file := burn.files[key]
+			if file != nil {
+				// file can be nil if it is considered binary in this branch
+				files = append(files, file)
 			}
-			file.Hash = hash
-			for _, branch := range branches {
-				branch.(*BurndownAnalysis).files[key] = file.Clone(false)
+		}
+		if len(files) == 0 {
+			// so we could be wrong in (*) and there is no such file eventually
+			// it could be also removed in the merge commit itself
+			continue
+		}
+		if len(files) > 1 {
+			files[0].Merge(analyser.day, files[1:]...)
+		}
+		for _, burn := range all {
+			if burn.files[key] != files[0] {
+				burn.files[key] = files[0].Clone(false)
 			}
 		}
 	}
@@ -977,7 +990,7 @@ func (analyser *BurndownAnalysis) newFile(
 		updaters = append(updaters, analyser.updateMatrix)
 		day = analyser.packPersonWithDay(author, day)
 	}
-	return burndown.NewFile(hash, day, size, updaters...), nil
+	return burndown.NewFile(day, size, updaters...), nil
 }
 
 func (analyser *BurndownAnalysis) handleInsertion(
@@ -1002,7 +1015,7 @@ func (analyser *BurndownAnalysis) handleInsertion(
 	file, err = analyser.newFile(hash, name, author, analyser.day, lines)
 	analyser.files[name] = file
 	if analyser.day == burndown.TreeMergeMark {
-		analyser.mergedFiles[file] = blob.Hash
+		analyser.mergedFiles[name] = true
 	}
 	return err
 }
@@ -1021,10 +1034,12 @@ func (analyser *BurndownAnalysis) handleDeletion(
 	name := change.From.Name
 	file := analyser.files[name]
 	file.Update(analyser.packPersonWithDay(author, analyser.day), 0, 0, lines)
-	file.Hash = plumbing.NewHash("ffffffffffffffffffffffffffffffffffffffff")
 	delete(analyser.files, name)
 	delete(analyser.fileHistories, name)
 	analyser.renames[name] = ""
+	if analyser.day == burndown.TreeMergeMark {
+		analyser.mergedFiles[name] = false
+	}
 	return nil
 }
 
@@ -1032,18 +1047,14 @@ func (analyser *BurndownAnalysis) handleModification(
 	change *object.Change, author int, cache map[plumbing.Hash]*object.Blob,
 	diffs map[string]items.FileDiffData) error {
 
+	if analyser.day == burndown.TreeMergeMark {
+		analyser.mergedFiles[change.To.Name] = true
+	}
 	file, exists := analyser.files[change.From.Name]
 	if !exists {
 		// this indeed may happen
 		return analyser.handleInsertion(change, author, cache)
 	}
-	var hash plumbing.Hash
-	if analyser.day != burndown.TreeMergeMark {
-		hash = change.To.TreeEntry.Hash
-	} else {
-		analyser.mergedFiles[file] = change.To.TreeEntry.Hash
-	}
-	file.Hash = hash
 
 	// possible rename
 	if change.To.Name != change.From.Name {
@@ -1149,6 +1160,9 @@ func (analyser *BurndownAnalysis) handleRename(from, to string) error {
 	}
 	analyser.files[to] = file
 	delete(analyser.files, from)
+	if analyser.day == burndown.TreeMergeMark {
+		analyser.mergedFiles[from] = false
+	}
 
 	if analyser.TrackFiles {
 		history := analyser.fileHistories[from]

leaves/burndown_test.go

@@ -210,9 +210,9 @@ func TestBurndownConsumeFinalize(t *testing.T) {
 	deps[items.DependencyDay] = file.TreeMergeMark
 	_, err = burndown3.Consume(deps)
 	assert.Nil(t, err)
-	assert.Equal(t, burndown3.files["cmd/hercules/main.go"].Hash, plumbing.ZeroHash)
-	assert.Equal(t, burndown3.files["analyser.go"].Hash, plumbing.ZeroHash)
-	assert.Equal(t, burndown3.files[".travis.yml"].Hash, plumbing.ZeroHash)
+	assert.True(t, burndown3.mergedFiles["cmd/hercules/main.go"])
+	assert.True(t, burndown3.mergedFiles["analyser.go"], plumbing.ZeroHash)
+	assert.True(t, burndown3.mergedFiles[".travis.yml"], plumbing.ZeroHash)
 
 	// stage 2
 	// 2b1ed978194a94edeabbca6de7ff3b5771d4d665