Repository.ConfigScoped and Repository.Commit with empty author support

8 files changed, 117 insertions, 27 deletions

diff --git a/go.mod b/go.mod
index 6d8da35..0c9cfd2 100644
--- a/go.mod
+++ b/go.mod
@@ -10,6 +10,7 @@ require (
 	github.com/go-git/go-billy/v5 v5.0.0
 	github.com/go-git/go-git-fixtures/v4 v4.0.1
 	github.com/google/go-cmp v0.3.0
+	github.com/imdario/mergo v0.3.9
 	github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99
 	github.com/jessevdk/go-flags v1.4.0
 	github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd
diff --git a/go.sum b/go.sum
index a73585e..e14e29a 100644
--- a/go.sum
+++ b/go.sum
@@ -22,6 +22,8 @@ github.com/go-git/go-git-fixtures/v4 v4.0.1 h1:q+IFMfLx200Q3scvt2hN79JsEzy4AmBTp
 github.com/go-git/go-git-fixtures/v4 v4.0.1/go.mod h1:m+ICp2rF3jDhFgEZ/8yziagdT1C+ZpZcrJjappBCDSw=
 github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/imdario/mergo v0.3.9 h1:UauaLniWCFHWd+Jp9oCEkTBj8VO/9DKg3PV3VCNMDIg=
+github.com/imdario/mergo v0.3.9/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
 github.com/jessevdk/go-flags v1.4.0 h1:4IU2WS7AumrZ/40jfhf4QVDMsQwqA7VEHozFRrGARJA=
diff --git a/options.go b/options.go
index a147f58..93fe684 100644
--- a/options.go
+++ b/options.go
@@ -6,12 +6,12 @@ import (
 	"strings"
 	"time"
 
-	"golang.org/x/crypto/openpgp"
 	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/plumbing/protocol/packp/sideband"
 	"github.com/go-git/go-git/v5/plumbing/transport"
+	"golang.org/x/crypto/openpgp"
 )
 
 // SubmoduleRescursivity defines how depth will affect any submodule recursive
@@ -375,7 +375,8 @@ type CommitOptions struct {
 	// All automatically stage files that have been modified and deleted, but
 	// new files you have not told Git about are not affected.
 	All bool
-	// Author is the author's signature of the commit.
+	// Author is the author's signature of the commit. If Author is empty the
+	// Name and Email is read from the config, and time.Now it's used as When.
 	Author *object.Signature
 	// Committer is the committer's signature of the commit. If Committer is
 	// nil the Author signature is used.
@@ -392,7 +393,9 @@ type CommitOptions struct {
 // Validate validates the fields and sets the default values.
 func (o *CommitOptions) Validate(r *Repository) error {
 	if o.Author == nil {
-		return ErrMissingAuthor
+		if err := o.loadConfigAuthorAndCommitter(r); err != nil {
+			return err
+		}
 	}
 
 	if o.Committer == nil {
@@ -413,6 +416,43 @@ func (o *CommitOptions) Validate(r *Repository) error {
 	return nil
 }
 
+func (o *CommitOptions) loadConfigAuthorAndCommitter(r *Repository) error {
+	cfg, err := r.ConfigScoped(config.SystemScope)
+	if err != nil {
+		return err
+	}
+
+	if o.Author == nil && cfg.Author.Email != "" && cfg.Author.Name != "" {
+		o.Author = &object.Signature{
+			Name:  cfg.Author.Name,
+			Email: cfg.Author.Email,
+			When:  time.Now(),
+		}
+	}
+
+	if o.Committer == nil && cfg.Committer.Email != "" && cfg.Committer.Name != "" {
+		o.Committer = &object.Signature{
+			Name:  cfg.Committer.Name,
+			Email: cfg.Committer.Email,
+			When:  time.Now(),
+		}
+	}
+
+	if o.Author == nil && cfg.User.Email != "" && cfg.User.Name != "" {
+		o.Author = &object.Signature{
+			Name:  cfg.User.Name,
+			Email: cfg.User.Email,
+			When:  time.Now(),
+		}
+	}
+
+	if o.Author == nil {
+		return ErrMissingAuthor
+	}
+
+	return nil
+}
+
 var (
 	ErrMissingName    = errors.New("name field is required")
 	ErrMissingTagger  = errors.New("tagger field is required")
diff --git a/options_test.go b/options_test.go
index b9e9096..aa36dab 100644
--- a/options_test.go
+++ b/options_test.go
@@ -1,8 +1,8 @@
 package git
 
 import (
-	. "gopkg.in/check.v1"
 	"github.com/go-git/go-git/v5/plumbing/object"
+	. "gopkg.in/check.v1"
 )
 
 type OptionsSuite struct {
@@ -18,12 +18,6 @@ func (s *OptionsSuite) TestCommitOptionsParentsFromHEAD(c *C) {
 	c.Assert(o.Parents, HasLen, 1)
 }
 
-func (s *OptionsSuite) TestCommitOptionsMissingAuthor(c *C) {
-	o := CommitOptions{}
-	err := o.Validate(s.Repository)
-	c.Assert(err, Equals, ErrMissingAuthor)
-}
-
 func (s *OptionsSuite) TestCommitOptionsCommitter(c *C) {
 	sig := &object.Signature{}
 
diff --git a/repository.go b/repository.go
index c83a136..60baf85 100644
--- a/repository.go
+++ b/repository.go
@@ -13,7 +13,6 @@ import (
 	"strings"
 	"time"
 
-	"golang.org/x/crypto/openpgp"
 	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/internal/revision"
 	"github.com/go-git/go-git/v5/plumbing"
@@ -24,6 +23,8 @@ import (
 	"github.com/go-git/go-git/v5/storage"
 	"github.com/go-git/go-git/v5/storage/filesystem"
 	"github.com/go-git/go-git/v5/utils/ioutil"
+	"github.com/imdario/mergo"
+	"golang.org/x/crypto/openpgp"
 
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/osfs"
@@ -155,7 +156,7 @@ func setConfigWorktree(r *Repository, worktree, storage billy.Filesystem) error
 		return nil
 	}
 
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return err
 	}
@@ -439,9 +440,42 @@ func (r *Repository) Config() (*config.Config, error) {
 	return r.Storer.Config()
 }
 
+// ConfigScoped returns the repository config, merged with requested scope and
+// lower. For example if, config.GlobalScope is given the local and global config
+// are returned merged in one config value.
+func (r *Repository) ConfigScoped(scope config.Scope) (*config.Config, error) {
+	// TODO(mcuadros): v6, add this as ConfigOptions.Scoped
+
+	var err error
+	system := config.NewConfig()
+	if scope >= config.SystemScope {
+		system, err = config.LoadConfig(config.SystemScope)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	global := config.NewConfig()
+	if scope >= config.GlobalScope {
+		global, err = config.LoadConfig(config.GlobalScope)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	local, err := r.Storer.Config()
+	if err != nil {
+		return nil, err
+	}
+
+	_ = mergo.Merge(global, system)
+	_ = mergo.Merge(local, global)
+	return local, nil
+}
+
 // Remote return a remote if exists
 func (r *Repository) Remote(name string) (*Remote, error) {
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return nil, err
 	}
@@ -456,7 +490,7 @@ func (r *Repository) Remote(name string) (*Remote, error) {
 
 // Remotes returns a list with all the remotes
 func (r *Repository) Remotes() ([]*Remote, error) {
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return nil, err
 	}
@@ -480,7 +514,7 @@ func (r *Repository) CreateRemote(c *config.RemoteConfig) (*Remote, error) {
 
 	remote := NewRemote(r.Storer, c)
 
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return nil, err
 	}
@@ -511,7 +545,7 @@ func (r *Repository) CreateRemoteAnonymous(c *config.RemoteConfig) (*Remote, err
 
 // DeleteRemote delete a remote from the repository and delete the config
 func (r *Repository) DeleteRemote(name string) error {
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return err
 	}
@@ -526,7 +560,7 @@ func (r *Repository) DeleteRemote(name string) error {
 
 // Branch return a Branch if exists
 func (r *Repository) Branch(name string) (*config.Branch, error) {
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return nil, err
 	}
@@ -545,7 +579,7 @@ func (r *Repository) CreateBranch(c *config.Branch) error {
 		return err
 	}
 
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return err
 	}
@@ -560,7 +594,7 @@ func (r *Repository) CreateBranch(c *config.Branch) error {
 
 // DeleteBranch delete a Branch from the repository and delete the config
 func (r *Repository) DeleteBranch(name string) error {
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return err
 	}
@@ -835,7 +869,7 @@ func (r *Repository) cloneRefSpec(o *CloneOptions) []config.RefSpec {
 }
 
 func (r *Repository) setIsBare(isBare bool) error {
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return err
 	}
@@ -851,7 +885,7 @@ func (r *Repository) updateRemoteConfigIfNeeded(o *CloneOptions, c *config.Remot
 
 	c.Fetch = r.cloneRefSpec(o)
 
-	cfg, err := r.Storer.Config()
+	cfg, err := r.Config()
 	if err != nil {
 		return err
 	}
@@ -1541,7 +1575,7 @@ func (r *Repository) createNewObjectPack(cfg *RepackConfig) (h plumbing.Hash, er
 		return h, err
 	}
 	defer ioutil.CheckClose(wc, &err)
-	scfg, err := r.Storer.Config()
+	scfg, err := r.Config()
 	if err != nil {
 		return h, err
 	}
diff --git a/repository_test.go b/repository_test.go
index e366373..37cd914 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -1870,6 +1870,21 @@ func (s *RepositorySuite) TestLogLimitWithOtherParamsPass(c *C) {
 	c.Assert(iterErr, Equals, io.EOF)
 }
 
+func (s *RepositorySuite) TestConfigScoped(c *C) {
+	r, _ := Init(memory.NewStorage(), nil)
+	err := r.clone(context.Background(), &CloneOptions{
+		URL: s.GetBasicLocalRepositoryURL(),
+	})
+
+	cfg, err := r.ConfigScoped(config.LocalScope)
+	c.Assert(err, IsNil)
+	c.Assert(cfg.User.Email, Equals, "")
+
+	cfg, err = r.ConfigScoped(config.SystemScope)
+	c.Assert(err, IsNil)
+	c.Assert(cfg.User.Email, Not(Equals), "")
+}
+
 func (s *RepositorySuite) TestCommit(c *C) {
 	r, _ := Init(memory.NewStorage(), nil)
 	err := r.clone(context.Background(), &CloneOptions{
diff --git a/submodule.go b/submodule.go
index 92ccdb1..dff26b0 100644
--- a/submodule.go
+++ b/submodule.go
@@ -35,7 +35,7 @@ func (s *Submodule) Config() *config.Submodule {
 // Init initialize the submodule reading the recorded Entry in the index for
 // the given submodule
 func (s *Submodule) Init() error {
-	cfg, err := s.w.r.Storer.Config()
+	cfg, err := s.w.r.Config()
 	if err != nil {
 		return err
 	}
diff --git a/worktree_commit_test.go b/worktree_commit_test.go
index aeb4a9a..6eafb15 100644
--- a/worktree_commit_test.go
+++ b/worktree_commit_test.go
@@ -24,16 +24,20 @@ import (
 	. "gopkg.in/check.v1"
 )
 
-func (s *WorktreeSuite) TestCommitInvalidOptions(c *C) {
+func (s *WorktreeSuite) TestCommitEmptyOptions(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("", &CommitOptions{})
-	c.Assert(err, Equals, ErrMissingAuthor)
-	c.Assert(hash.IsZero(), Equals, true)
+	hash, err := w.Commit("foo", &CommitOptions{})
+	c.Assert(err, IsNil)
+	c.Assert(hash.IsZero(), Equals, false)
+
+	commit, err := r.CommitObject(hash)
+	c.Assert(err, IsNil)
+	c.Assert(commit.Author.Name, Not(Equals), "")
 }
 
 func (s *WorktreeSuite) TestCommitInitial(c *C) {