From f3226d6171cd5bf972043630c0677201e50162b1 Mon Sep 17 00:00:00 2001 From: mediocregopher <> Date: Mon, 23 Mar 2020 16:49:01 -0600 Subject: [PATCH] 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 | 11 ++++++++--- commit.go | 6 +++--- commit_change.go | 40 ++++++++++++++++++++++------------------ commit_change_test.go | 15 +++++++++++---- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/cmd/dehub/cmd_commit.go b/cmd/dehub/cmd_commit.go index f02139c..790d6db 100644 --- a/cmd/dehub/cmd_commit.go +++ b/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 } diff --git a/commit.go b/commit.go index e853e35..b8dd8eb 100644 --- a/commit.go +++ b/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, }) diff --git a/commit_change.go b/commit_change.go index 116b38a..8b005f0 100644 --- a/commit_change.go +++ b/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, }) diff --git a/commit_change_test.go b/commit_change_test.go index 032d576..7c12107 100644 --- a/commit_change_test.go +++ b/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) } }