Fixes #623. LockDiff does not compare hash/solvemeta any more.

LockDiff now uses only project dependencies to identify changes,
thus ignoring any changes to the SolveMeta iformation. If required
changes in the SolveMeta can still be seen using source diffs.
Modified and added tests to check new behaviour.

Author: mikijov

internal/gps/lockdiff.go

@@ -5,7 +5,6 @@
 package gps
 
 import (
-	"encoding/hex"
 	"fmt"
 	"sort"
 	"strings"
@@ -44,10 +43,9 @@ func (diff *StringDiff) String() string {
 // LockDiff is the set of differences between an existing lock file and an updated lock file.
 // Fields are only populated when there is a difference, otherwise they are empty.
 type LockDiff struct {
-	HashDiff *StringDiff
-	Add      []LockedProjectDiff
-	Remove   []LockedProjectDiff
-	Modify   []LockedProjectDiff
+	Add    []LockedProjectDiff
+	Remove []LockedProjectDiff
+	Modify []LockedProjectDiff
 }
 
 // LockedProjectDiff contains the before and after snapshot of a project reference.
@@ -91,12 +89,6 @@ func DiffLocks(l1 Lock, l2 Lock) *LockDiff {
 
 	diff := LockDiff{}
 
-	h1 := hex.EncodeToString(l1.InputHash())
-	h2 := hex.EncodeToString(l2.InputHash())
-	if h1 != h2 {
-		diff.HashDiff = &StringDiff{Previous: h1, Current: h2}
-	}
-
 	var i2next int
 	for i1 := 0; i1 < len(p1); i1++ {
 		lp1 := p1[i1]
@@ -140,7 +132,7 @@ func DiffLocks(l1 Lock, l2 Lock) *LockDiff {
 		diff.Add = append(diff.Add, add)
 	}
 
-	if diff.HashDiff == nil && len(diff.Add) == 0 && len(diff.Remove) == 0 && len(diff.Modify) == 0 {
+	if len(diff.Add) == 0 && len(diff.Remove) == 0 && len(diff.Modify) == 0 {
 		return nil // The locks are the equivalent
 	}
 	return &diff

internal/gps/lockdiff_test.go

@@ -212,15 +212,35 @@ func TestDiffLocks_NoChange(t *testing.T) {
 	}
 }
 
-func TestDiffLocks_AddProjects(t *testing.T) {
+func TestDiffLocks_IgnoreHashChange(t *testing.T) {
 	l1 := safeLock{
 		h: []byte("abc123"),
 		p: []LockedProject{
 			{pi: ProjectIdentifier{ProjectRoot: "github.com/foo/bar"}, v: NewVersion("v1.0.0")},
 		},
 	}
 	l2 := safeLock{
+		h: []byte("def456"),
+		p: []LockedProject{
+			{pi: ProjectIdentifier{ProjectRoot: "github.com/foo/bar"}, v: NewVersion("v1.0.0")},
+		},
+	}
+
+	diff := DiffLocks(l1, l2)
+	if diff != nil {
+		t.Fatal("Expected the diff to be nil")
+	}
+}
+
+func TestDiffLocks_AddProjects(t *testing.T) {
+	l1 := safeLock{
 		h: []byte("abc123"),
+		p: []LockedProject{
+			{pi: ProjectIdentifier{ProjectRoot: "github.com/foo/bar"}, v: NewVersion("v1.0.0")},
+		},
+	}
+	l2 := safeLock{
+		h: []byte("def456"),
 		p: []LockedProject{
 			{
 				pi:   ProjectIdentifier{ProjectRoot: "github.com/baz/qux", Source: "https://github.com/mcfork/bazqux.git"},
@@ -310,7 +330,7 @@ func TestDiffLocks_RemoveProjects(t *testing.T) {
 		},
 	}
 	l2 := safeLock{
-		h: []byte("abc123"),
+		h: []byte("def456"),
 		p: []LockedProject{
 			{pi: ProjectIdentifier{ProjectRoot: "github.com/baz/qux"}, v: NewVersion("v1.0.0")},
 		},
@@ -389,7 +409,7 @@ func TestDiffLocks_ModifyProjects(t *testing.T) {
 		},
 	}
 	l2 := safeLock{
-		h: []byte("abc123"),
+		h: []byte("def456"),
 		p: []LockedProject{
 			{pi: ProjectIdentifier{ProjectRoot: "github.com/baz/qux"}, v: NewVersion("v1.0.0")},
 			{pi: ProjectIdentifier{ProjectRoot: "github.com/foo/bar"}, v: NewVersion("v2.0.0")},
@@ -420,35 +440,6 @@ func TestDiffLocks_ModifyProjects(t *testing.T) {
 	}
 }
 
-func TestDiffLocks_ModifyHash(t *testing.T) {
-	h1, _ := hex.DecodeString("abc123")
-	l1 := safeLock{
-		h: h1,
-		p: []LockedProject{
-			{pi: ProjectIdentifier{ProjectRoot: "github.com/foo/bar"}, v: NewVersion("v1.0.0")},
-		},
-	}
-
-	h2, _ := hex.DecodeString("def456")
-	l2 := safeLock{
-		h: h2,
-		p: []LockedProject{
-			{pi: ProjectIdentifier{ProjectRoot: "github.com/foo/bar"}, v: NewVersion("v1.0.0")},
-		},
-	}
-
-	diff := DiffLocks(l1, l2)
-	if diff == nil {
-		t.Fatal("Expected the diff to be populated")
-	}
-
-	want := "abc123 -> def456"
-	got := diff.HashDiff.String()
-	if got != want {
-		t.Fatalf("Expected diff.HashDiff to be '%s', got '%s'", want, got)
-	}
-}
-
 func TestDiffLocks_EmptyInitialLock(t *testing.T) {
 	h2, _ := hex.DecodeString("abc123")
 	l2 := safeLock{
@@ -460,12 +451,6 @@ func TestDiffLocks_EmptyInitialLock(t *testing.T) {
 
 	diff := DiffLocks(nil, l2)
 
-	wantHash := "+ abc123"
-	gotHash := diff.HashDiff.String()
-	if gotHash != wantHash {
-		t.Fatalf("Expected diff.HashDiff to be '%s', got '%s'", wantHash, gotHash)
-	}
-
 	if len(diff.Add) != 1 {
 		t.Fatalf("Expected diff.Add to contain 1 project, got %d", len(diff.Add))
 	}
@@ -482,12 +467,6 @@ func TestDiffLocks_EmptyFinalLock(t *testing.T) {
 
 	diff := DiffLocks(l1, nil)
 
-	wantHash := "- abc123"
-	gotHash := diff.HashDiff.String()
-	if gotHash != wantHash {
-		t.Fatalf("Expected diff.HashDiff to be '%s', got '%s'", wantHash, gotHash)
-	}
-
 	if len(diff.Remove) != 1 {
 		t.Fatalf("Expected diff.Remove to contain 1 project, got %d", len(diff.Remove))
 	}

testdata/txn_writer/expected_diff_output.txt

@@ -1,5 +1,3 @@
-Memo: 595716d270828e763c811ef79c9c41f85b1d1bfbdfe85280036405c03772206c -> 2252a285ab27944a4d7adcba8dbd03980f59ba652f12db39fa93b927c345593e
-
 Add:
 [[projects]]
   name = "github.com/sdboyer/deptest"

txn_writer.go

@@ -6,7 +6,6 @@ package dep
 
 import (
 	"bytes"
-	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
@@ -176,10 +175,6 @@ func toRawLockedProjectDiffs(diffs []gps.LockedProjectDiff) rawLockedProjectDiff
 func formatLockDiff(diff gps.LockDiff) (string, error) {
 	var buf bytes.Buffer
 
-	if diff.HashDiff != nil {
-		buf.WriteString(fmt.Sprintf("Memo: %s\n\n", diff.HashDiff))
-	}
-
 	writeDiffs := func(diffs []gps.LockedProjectDiff) error {
 		raw := toRawLockedProjectDiffs(diffs)
 		chunk, err := toml.Marshal(raw)

txn_writer_test.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/golang/dep/internal/gps"
 	"github.com/golang/dep/internal/test"
 	"github.com/pkg/errors"
 )
@@ -247,6 +248,50 @@ func TestSafeWriter_ManifestAndUnmodifiedLockWithForceVendor(t *testing.T) {
 	}
 }
 
+func TestSafeWriter_ModifiedLockChangedHash(t *testing.T) {
+	test.NeedsExternalNetwork(t)
+	test.NeedsGit(t)
+
+	h := test.NewHelper(t)
+	defer h.Cleanup()
+
+	pc := NewTestProjectContext(h, safeWriterProject)
+	defer pc.Release()
+	pc.CopyFile(LockName, safeWriterGoldenLock)
+	pc.Load()
+
+	originalLock := new(Lock)
+	*originalLock = *pc.Project.Lock
+	originalLock.SolveMeta.InputsDigest = []byte{} // zero out the input hash to ensure non-equivalency
+	sw, _ := NewSafeWriter(nil, originalLock, pc.Project.Lock, VendorOnChanged)
+
+	// Verify prepared actions
+	if sw.HasManifest() {
+		t.Fatal("Did not expect the manifest to be written")
+	}
+	if !sw.HasLock() {
+		t.Fatal("Expected that the writer should plan to write the lock")
+	}
+	if sw.writeVendor {
+		t.Fatal("Did not expect vendor to be written to")
+	}
+
+	// Write changes
+	err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true)
+	h.Must(errors.Wrap(err, "SafeWriter.Write failed"))
+
+	// Verify file system changes
+	if err := pc.ManifestShouldNotExist(); err != nil {
+		t.Fatal(err)
+	}
+	if err := pc.LockShouldMatchGolden(safeWriterGoldenLock); err != nil {
+		t.Fatal(err)
+	}
+	if err := pc.VendorShouldExist(); err == nil {
+		t.Fatal(err)
+	}
+}
+
 func TestSafeWriter_ModifiedLock(t *testing.T) {
 	test.NeedsExternalNetwork(t)
 	test.NeedsGit(t)
@@ -262,6 +307,12 @@ func TestSafeWriter_ModifiedLock(t *testing.T) {
 	originalLock := new(Lock)
 	*originalLock = *pc.Project.Lock
 	originalLock.SolveMeta.InputsDigest = []byte{} // zero out the input hash to ensure non-equivalency
+	originalLock.P = append(originalLock.P, gps.NewLockedProject(
+		gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/golang/dep")},
+		gps.NewBranch("master").Pair(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")),
+		[]string{"."},
+	))
+
 	sw, _ := NewSafeWriter(nil, originalLock, pc.Project.Lock, VendorOnChanged)
 
 	// Verify prepared actions