Merge pull request #11 from fluxcd/flux-empty-commit

Return error instead of creating empty commits

worktree_commit.go

@@ -2,6 +2,7 @@ package git
 
 import (
 	"bytes"
+	"errors"
 	"path"
 	"sort"
 	"strings"
@@ -16,6 +17,12 @@ import (
 	"github.com/go-git/go-billy/v5"
 )
 
+var (
+	// ErrEmptyCommit occurs when a commit is attempted using a clean
+	// working tree, with no changes to be committed.
+	ErrEmptyCommit = errors.New("cannot create empty commit: clean working tree")
+)
+
 // Commit stores the current contents of the index in a new commit along with
 // a log message from the user describing the changes.
 func (w *Worktree) Commit(msg string, opts *CommitOptions) (plumbing.Hash, error) {
@@ -146,6 +153,10 @@ type buildTreeHelper struct {
 // BuildTree builds the tree objects and push its to the storer, the hash
 // of the root tree is returned.
 func (h *buildTreeHelper) BuildTree(idx *index.Index) (plumbing.Hash, error) {
+	if len(idx.Entries) == 0 {
+		return plumbing.ZeroHash, ErrEmptyCommit
+	}
+
 	const rootNode = ""
 	h.trees = map[string]*object.Tree{rootNode: {}}
 	h.entries = map[string]*object.TreeEntry{}

worktree_commit_test.go

@@ -26,12 +26,18 @@ import (
 )
 
 func (s *WorktreeSuite) TestCommitEmptyOptions(c *C) {
-	r, err := Init(memory.NewStorage(), memfs.New())
+	fs := memfs.New()
+	r, err := Init(memory.NewStorage(), fs)
 	c.Assert(err, IsNil)
 
 	w, err := r.Worktree()
 	c.Assert(err, IsNil)
 
+	util.WriteFile(fs, "foo", []byte("foo"), 0644)
+
+	_, err = w.Add("foo")
+	c.Assert(err, IsNil)
+
 	hash, err := w.Commit("foo", &CommitOptions{})
 	c.Assert(err, IsNil)
 	c.Assert(hash.IsZero(), Equals, false)
@@ -65,6 +71,18 @@ func (s *WorktreeSuite) TestCommitInitial(c *C) {
 	assertStorageStatus(c, r, 1, 1, 1, expected)
 }
 
+func (s *WorktreeSuite) TestNothingToCommit(c *C) {
+	r, err := Init(memory.NewStorage(), memfs.New())
+	c.Assert(err, IsNil)
+
+	w, err := r.Worktree()
+	c.Assert(err, IsNil)
+
+	hash, err := w.Commit("empty commit\n", &CommitOptions{Author: defaultSignature()})
+	c.Assert(hash, Equals, plumbing.ZeroHash)
+	c.Assert(err, Equals, ErrEmptyCommit)
+}
+
 func (s *WorktreeSuite) TestCommitParent(c *C) {
 	expected := plumbing.NewHash("ef3ca05477530b37f48564be33ddd48063fc7a22")
 

worktree_test.go

@@ -3,7 +3,6 @@ package git
 import (
 	"bytes"
 	"context"
-	"errors"
 	"io"
 	"io/ioutil"
 	"os"
@@ -2167,6 +2166,8 @@ func (s *WorktreeSuite) TestGrep(c *C) {
 }
 
 func (s *WorktreeSuite) TestAddAndCommit(c *C) {
+	expectedFiles := 2
+
 	dir, clean := s.TemporalDir()
 	defer clean()
 
@@ -2176,29 +2177,58 @@ func (s *WorktreeSuite) TestAddAndCommit(c *C) {
 	w, err := repo.Worktree()
 	c.Assert(err, IsNil)
 
+	os.WriteFile(filepath.Join(dir, "foo"), []byte("bar"), 0o644)
+	os.WriteFile(filepath.Join(dir, "bar"), []byte("foo"), 0o644)
+
 	_, err = w.Add(".")
 	c.Assert(err, IsNil)
 
-	w.Commit("Test Add And Commit", &CommitOptions{Author: &object.Signature{
+	_, err = w.Commit("Test Add And Commit", &CommitOptions{Author: &object.Signature{
 		Name:  "foo",
 		Email: "foo@foo.foo",
 		When:  time.Now(),
 	}})
+	c.Assert(err, IsNil)
 
 	iter, err := w.r.Log(&LogOptions{})
 	c.Assert(err, IsNil)
+
+	filesFound := 0
 	err = iter.ForEach(func(c *object.Commit) error {
 		files, err := c.Files()
 		if err != nil {
 			return err
 		}
 
 		err = files.ForEach(func(f *object.File) error {
-			return errors.New("Expected no files, got at least 1")
+			filesFound++
+			return nil
 		})
 		return err
 	})
 	c.Assert(err, IsNil)
+	c.Assert(filesFound, Equals, expectedFiles)
+}
+
+func (s *WorktreeSuite) TestAddAndCommitEmpty(c *C) {
+	dir, clean := s.TemporalDir()
+	defer clean()
+
+	repo, err := PlainInit(dir, false)
+	c.Assert(err, IsNil)
+
+	w, err := repo.Worktree()
+	c.Assert(err, IsNil)
+
+	_, err = w.Add(".")
+	c.Assert(err, IsNil)
+
+	_, err = w.Commit("Test Add And Commit", &CommitOptions{Author: &object.Signature{
+		Name:  "foo",
+		Email: "foo@foo.foo",
+		When:  time.Now(),
+	}})
+	c.Assert(err, Equals, ErrEmptyCommit)
 }
 
 func (s *WorktreeSuite) TestLinkedWorktree(c *C) {