Refactor combined commits a bit

---
type: change
message: |-
  Refactor combined commits a bit

  This commit makes the combine method not take in revisions, which simplifies
  some things as well as makes it more consistent with other methods. Additionally
  this commit fixes authorship for combined commits so that they only show the
  authors of the change commits involved, not all commits with credentials.
change_hash: APQFD5UML2F+ulO3awYJ3BHqvn05EZSFqmCg2i7vTfgA
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl55PNYACgkQlcRvpqQRSKz51Q//anLA1v9Qqtn0dey24oGngaJnV80fi53CZf57AGulbTj6WWQIlZrH126B/YJgSAFg6EzRZZj8vWW7JTXLXhExVM3vGvGWJWyslW9vadBLXRbK9Uw4RuPxN9hDgnXPYs4TNbIm11q5m8jxrCPAehs4aMebsRB4k//m0ul/17kjWlIOHTJupE/gyg47hjoyD//q2vNi9izkEEtRYDVykxXzy++UtEp7CCt9MO9Tp6H8FRxDH1h5jwWaGbedL/AakLDBAN0DhEZ1tvStpeSwycRiGlbIO3eRpDoLoBgX5yLwwrCqF6acEToJZR+amerKlTa5dNf/cqynS8CzMs35u+ZZx0QKTp7eqn/TEP3LNlAbbzILyrktFVBml47JNjLwS2LVZknJvV3mWI6K9r6yE01m/S+2zbkZz2EoADn0PgA99tKfJoozjFbqRQaTJn/B6/4L38GyVKSW3DXF3BYjz9DrFiEZR95449q9gHmJCVZ7S7wX/KfYvI5K7QMpBhiGUZSsWKc4DX6de2TP1HgXQX/08WJGb5463J6TQoB2LoQbDzOUBli4VblQwpWaREDp0DlPz+BUaFVCAwIXsaYGGrUkKeqFN0MH1OR7Ir5t5uVg8C97RbywAYvdThyOdQLyKQaK4CHwtyHU0BPrKSU13rE6tl66NTqY46MgyBSPEJ4ukvA=
  account: mediocregopher

cmd/dehub/cmd_commit.go

@@ -167,12 +167,17 @@ func cmdCombine(ctx context.Context, cmd *dcmd.Cmd) {
 		}
 
 		repo := ctxRepo(ctx)
-		ontoBranch := plumbing.NewBranchReferenceName(*onto)
-		gitCommit, err := repo.CombineCommitChanges(
+		commits, err := repo.GetGitRevisionRange(
 			plumbing.Revision(*startRev),
 			plumbing.Revision(*endRev),
-			ontoBranch,
 		)
+		if err != nil {
+			return nil, fmt.Errorf("error getting commits %q to %q: %w",
+				*startRev, *endRev, err)
+		}
+
+		ontoBranch := plumbing.NewBranchReferenceName(*onto)
+		gitCommit, err := repo.CombineCommitChanges(commits, ontoBranch)
 		if err != nil {
 			return nil, err
 		}

commit.go

@@ -180,7 +180,7 @@ func (r *Repo) AccreditCommit(commit Commit, sigInt sigcred.SignifierInterface)
 // required, unless otherwise noted.
 type CommitBareParams struct {
 	Commit     Commit
-	AccountID  string        // used as the author
+	Author     string
 	ParentHash plumbing.Hash // can be zero if the commit has no parents (Q_Q)
 	GitTree    *object.Tree
 }
@@ -195,7 +195,7 @@ func (r *Repo) CommitBare(params CommitBareParams) (GitCommit, error) {
 	}
 
 	author := object.Signature{
-		Name: params.AccountID,
+		Name: params.Author,
 		When: time.Now(),
 	}
 	commit := &object.Commit{
@@ -247,7 +247,7 @@ func (r *Repo) Commit(commit Commit, accountID string) (GitCommit, error) {
 
 	gitCommit, err := r.CommitBare(CommitBareParams{
 		Commit:     commit,
-		AccountID:  accountID,
+		Author:     accountID,
 		ParentHash: headHash,
 		GitTree:    stagedTree,
 	})

commit_change.go

@@ -73,30 +73,35 @@ func (cc CommitChange) GetHash() []byte {
 // will return an error, as the change hash which has been accredited in
 // start/end will be different than the one which needs to be accredited in
 // onto/end.
-func (r *Repo) CombineCommitChanges(startRev, endRev plumbing.Revision, onto plumbing.ReferenceName) (GitCommit, error) {
-	startEndCommits, err := r.GetGitRevisionRange(startRev, endRev)
-	if err != nil {
-		return GitCommit{}, fmt.Errorf("retrieving commits %q to %q: %w", startRev, endRev, err)
-	}
-
+func (r *Repo) CombineCommitChanges(commits []GitCommit, onto plumbing.ReferenceName) (GitCommit, error) {
+	authorsSet := map[string]struct{}{}
 	var lastChangeCommit GitCommit
 	var lastChangeCommitOk bool
-	for i := len(startEndCommits) - 1; i >= 0; i-- {
-		if _, lastChangeCommitOk = startEndCommits[i].Interface.(*CommitChange); lastChangeCommitOk {
-			lastChangeCommit = startEndCommits[i]
-			break
+	for _, commit := range commits {
+		if _, ok := commit.Interface.(*CommitChange); ok {
+			lastChangeCommit = commit
+			lastChangeCommitOk = true
+			for _, cred := range commit.Commit.Common.Credentials {
+				authorsSet[cred.AccountID] = struct{}{}
+			}
 		}
 	}
 	if !lastChangeCommitOk {
-		return GitCommit{}, fmt.Errorf("no change commits in range %q to %q", startRev, endRev)
+		return GitCommit{}, errors.New("no change commits in range")
 	}
 
+	authors := make([]string, 0, len(authorsSet))
+	for author := range authorsSet {
+		authors = append(authors, author)
+	}
+	sort.Strings(authors)
+
 	// startTree has to be the tree of startRev, which isn't included in
-	// startEndCommits. Determine it the hard way.
-	startTree, err := r.parentTree(startEndCommits[0].GitCommit)
+	// commits. Determine it the hard way.
+	startTree, err := r.parentTree(commits[0].GitCommit)
 	if err != nil {
-		return GitCommit{}, fmt.Errorf("getting tree of %q (parent of %q): %w",
-			startRev, startEndCommits[0].GitCommit.Hash, err)
+		return GitCommit{}, fmt.Errorf("getting tree of parent of %q: %w",
+			commits[0].GitCommit.Hash, err)
 	}
 
 	msg := lastChangeCommit.Commit.Change.Message
@@ -123,7 +128,7 @@ func (r *Repo) CombineCommitChanges(startRev, endRev plumbing.Revision, onto plu
 	}
 
 	var creds []sigcred.Credential
-	for _, commit := range startEndCommits {
+	for _, commit := range commits {
 		if bytes.Equal(commit.Interface.GetHash(), changeHash) {
 			creds = append(creds, commit.Commit.Common.Credentials...)
 		}
@@ -141,11 +146,10 @@ func (r *Repo) CombineCommitChanges(startRev, endRev plumbing.Revision, onto plu
 		},
 		Common: CommitCommon{Credentials: creds},
 	}
-	accountID := strings.Join(commit.Common.credAccountIDs(), ",")
 
 	gitCommit, err := r.CommitBare(CommitBareParams{
 		Commit:     commit,
-		AccountID:  accountID,
+		Author:     strings.Join(authors, ","),
 		ParentHash: ontoCommit.GitCommit.Hash,
 		GitTree:    endTree,
 	})

commit_change_test.go

@@ -159,11 +159,16 @@ func TestCombineCommitChanges(t *testing.T) {
 		t.Fatal(err)
 	}
 	credCommit := h.tryCommit(true, credCommitObj, h.cfg.Accounts[1].ID, tootSig)
-	combinedCommit, err := h.repo.CombineCommitChanges(
-		plumbing.Revision(tootCommit.GitCommit.Hash.String()),
-		plumbing.Revision(credCommit.GitCommit.Hash.String()),
-		MainRefName,
+
+	allCommits, err := h.repo.GetGitCommitRange(
+		tootCommit.GitCommit.Hash,
+		credCommit.GitCommit.Hash,
 	)
+	if err != nil {
+		t.Fatalf("error getting commits: %v", err)
+	}
+
+	combinedCommit, err := h.repo.CombineCommitChanges(allCommits, MainRefName)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -188,5 +193,7 @@ func TestCombineCommitChanges(t *testing.T) {
 			combinedCommit.GitCommit.Hash, mainHead.GitCommit.Hash)
 	} else if err = h.repo.VerifyCommits(MainRefName, []GitCommit{combinedCommit}); err != nil {
 		t.Fatalf("unable to verify combined commit: %v", err)
+	} else if author := combinedCommit.GitCommit.Author.Name; author != "root" {
+		t.Fatalf("unexpected author value %q", author)
 	}
 }