Git Tagger now handles deleted files

Fixes #471

Also strengthened the diff algorithm to handle
more corner cases like renamed files, multiple
deleted files…

Signed-off-by: David Gageot <david@gageot.net>

pkg/skaffold/build/tag/git_commit.go

@@ -21,6 +21,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
+	"sort"
 
 	"github.com/pkg/errors"
 	git "gopkg.in/src-d/go-git.v4"
@@ -62,12 +63,19 @@ func (c *GitCommit) GenerateFullyQualifiedImageName(workingDir string, opts *Tag
 	// The file state is dirty. To generate a unique suffix, let's hash all the modified files.
 	// We add a -dirty-unique-id suffix to work well with local iterations.
 	h := sha256.New()
-	for path, change := range status {
-		if change.Worktree == git.Unmodified {
+	for _, changedPath := range changedPaths(status) {
+		status := status[changedPath].Worktree
+
+		statusLine := fmt.Sprintf("%c %s", status, changedPath)
+		if _, err := h.Write([]byte(statusLine)); err != nil {
+			return "", errors.Wrap(err, "adding deleted file to diff")
+		}
+
+		if status == git.Deleted {
 			continue
 		}
 
-		f, err := w.Filesystem.Open(path)
+		f, err := w.Filesystem.Open(changedPath)
 		if err != nil {
 			return "", errors.Wrap(err, "reading diff")
 		}
@@ -85,3 +93,18 @@ func (c *GitCommit) GenerateFullyQualifiedImageName(workingDir string, opts *Tag
 
 	return fmt.Sprintf("%s-dirty-%s", fqn, shaStr), nil
 }
+
+// changedPaths returns the changed paths in a consistent order.
+// The order is important because we generate a sha256 out of it.
+func changedPaths(status git.Status) []string {
+	var changes []string
+
+	for path, change := range status {
+		if change.Worktree != git.Unmodified {
+			changes = append(changes, path)
+		}
+	}
+
+	sort.Strings(changes)
+	return changes
+}

pkg/skaffold/build/tag/git_commit_test.go

@@ -45,37 +45,95 @@ func TestGitCommit_GenerateFullyQualifiedImageName(t *testing.T) {
 			expectedName: "test:eefe1b9",
 			createGitRepo: func(dir string) {
 				gitInit(t, dir).
-					write(t, "source.go", []byte("code")).
-					add(t, "source.go").
-					commit(t, "initial")
+					write("source.go", []byte("code")).
+					add("source.go").
+					commit("initial")
 			},
 		},
 		{
 			description: "dirty",
 			opts: &TagOptions{
 				ImageName: "test",
 			},
-			expectedName: "test:eefe1b9-dirty-17c3e6fb2811b7af",
+			expectedName: "test:eefe1b9-dirty-af8de1fde8be4367",
 			createGitRepo: func(dir string) {
 				gitInit(t, dir).
-					write(t, "source.go", []byte("code")).
-					add(t, "source.go").
-					commit(t, "initial").
-					write(t, "source.go", []byte("updated code"))
+					write("source.go", []byte("code")).
+					add("source.go").
+					commit("initial").
+					write("source.go", []byte("updated code"))
 			},
 		},
 		{
 			description: "untracked",
 			opts: &TagOptions{
 				ImageName: "test",
 			},
-			expectedName: "test:eefe1b9-dirty-d7bc32e5f6760a99",
+			expectedName: "test:eefe1b9-dirty-bfe9b4566c9d3fec",
 			createGitRepo: func(dir string) {
 				gitInit(t, dir).
-					write(t, "source.go", []byte("code")).
-					add(t, "source.go").
-					commit(t, "initial").
-					write(t, "new.go", []byte("new code"))
+					write("source.go", []byte("code")).
+					add("source.go").
+					commit("initial").
+					write("new.go", []byte("new code"))
+			},
+		},
+		{
+			description: "one file deleted",
+			opts: &TagOptions{
+				ImageName: "test",
+			},
+			expectedName: "test:279d53f-dirty-6a3ce511c689eda7",
+			createGitRepo: func(dir string) {
+				gitInit(t, dir).
+					write("source1.go", []byte("code1")).
+					write("source2.go", []byte("code2")).
+					add("source1.go", "source2.go").
+					commit("initial").
+					delete("source1.go")
+			},
+		},
+		{
+			description: "two files deleted",
+			opts: &TagOptions{
+				ImageName: "test",
+			},
+			expectedName: "test:279d53f-dirty-d48c11ed65c37a09", // Must be <> than when only one file is deleted
+			createGitRepo: func(dir string) {
+				gitInit(t, dir).
+					write("source1.go", []byte("code1")).
+					write("source2.go", []byte("code2")).
+					add("source1.go", "source2.go").
+					commit("initial").
+					delete("source1.go", "source2.go")
+			},
+		},
+		{
+			description: "rename",
+			opts: &TagOptions{
+				ImageName: "test",
+			},
+			expectedName: "test:eefe1b9-dirty-9c858d88cc0bf792",
+			createGitRepo: func(dir string) {
+				gitInit(t, dir).
+					write("source.go", []byte("code")).
+					add("source.go").
+					commit("initial").
+					rename("source.go", "source2.go")
+			},
+		},
+		{
+			description: "rename to different name",
+			opts: &TagOptions{
+				ImageName: "test",
+			},
+			expectedName: "test:eefe1b9-dirty-6534adc17ccd1cf4", // Must be <> each time a new name is used
+			createGitRepo: func(dir string) {
+				gitInit(t, dir).
+					write("source.go", []byte("code")).
+					add("source.go").
+					commit("initial").
+					rename("source.go", "source3.go")
 			},
 		},
 		{
@@ -106,7 +164,7 @@ func TestGenerateFullyQualifiedImageNameFromSubDirectory(t *testing.T) {
 
 	gitInit(t, tmpDir).
 		mkdir("sub/sub").
-		commit(t, "initial")
+		commit("initial")
 
 	opts := &TagOptions{ImageName: "test"}
 	c := &GitCommit{}
@@ -127,6 +185,7 @@ func TestGenerateFullyQualifiedImageNameFromSubDirectory(t *testing.T) {
 type gitRepo struct {
 	dir      string
 	workTree *git.Worktree
+	t        *testing.T
 }
 
 func gitInit(t *testing.T, dir string) *gitRepo {
@@ -139,29 +198,47 @@ func gitInit(t *testing.T, dir string) *gitRepo {
 	return &gitRepo{
 		dir:      dir,
 		workTree: w,
+		t:        t,
 	}
 }
 
 func (g *gitRepo) mkdir(folder string) *gitRepo {
-	os.MkdirAll(filepath.Join(g.dir, folder), os.ModePerm)
+	err := os.MkdirAll(filepath.Join(g.dir, folder), os.ModePerm)
+	failNowIfError(g.t, err)
 	return g
 }
 
-func (g *gitRepo) write(t *testing.T, file string, content []byte) *gitRepo {
+func (g *gitRepo) write(file string, content []byte) *gitRepo {
 	err := ioutil.WriteFile(filepath.Join(g.dir, file), content, os.ModePerm)
-	failNowIfError(t, err)
+	failNowIfError(g.t, err)
 	return g
 }
 
-func (g *gitRepo) add(t *testing.T, file string) *gitRepo {
-	_, err := g.workTree.Add(file)
-	failNowIfError(t, err)
+func (g *gitRepo) rename(file, to string) *gitRepo {
+	err := os.Rename(filepath.Join(g.dir, file), filepath.Join(g.dir, to))
+	failNowIfError(g.t, err)
+	return g
+}
+
+func (g *gitRepo) delete(files ...string) *gitRepo {
+	for _, file := range files {
+		err := os.Remove(filepath.Join(g.dir, file))
+		failNowIfError(g.t, err)
+	}
 	return g
 }
 
-func (g *gitRepo) commit(t *testing.T, msg string) *gitRepo {
+func (g *gitRepo) add(files ...string) *gitRepo {
+	for _, file := range files {
+		_, err := g.workTree.Add(file)
+		failNowIfError(g.t, err)
+	}
+	return g
+}
+
+func (g *gitRepo) commit(msg string) *gitRepo {
 	now, err := time.Parse("Jan 2, 2006 at 15:04:05 -0700 MST", "Feb 3, 2013 at 19:54:00 -0700 MST")
-	failNowIfError(t, err)
+	failNowIfError(g.t, err)
 
 	_, err = g.workTree.Commit(msg, &git.CommitOptions{
 		Author: &object.Signature{
@@ -170,7 +247,7 @@ func (g *gitRepo) commit(t *testing.T, msg string) *gitRepo {
 			When:  now,
 		},
 	})
-	failNowIfError(t, err)
+	failNowIfError(g.t, err)
 
 	return g
 }