Implement commit combining

---
type: change
message: |-
  Implement commit combining

  Adds a method CombineCommitChanges which takes in a range of commits and a
  branch. It determines what the change_hash of the range is (using the message of
  the last change commit in the range, and the file diff across the full range),
  collects all credentials for that hash, and creates a new change commit with all
  that information. That new change commit is then added onto the given branch
  (but only if the change_hash would remain the same by doing so).

  A corresponding combine command has also been added to the dehub binary.
change_hash: ALpURxN61PHUo5uzN9GTlM5V+4YcirKWpZr4G9llZVdu
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl53+xAACgkQlcRvpqQRSKxbZBAAsnLhOuVQ1YM2xVWVJTJSY09PoyD1EyXMFP/H5gCMvu9wtYHwPO2BOGsmWsmIePzRLH0YHAp3VLBtBiyPy5Xm5e9L54DJc1IvOH7tfP5jyDodExHhhCVvNeGza4ngcyBBi+ywu3SwNq1eYms/A4o8ZxcRRPhEYfGHR2Tob31ntbTM9s1D8cqZzszuj/FJY9AAEWmlBVfJrdPUGZ/DqYplTypEvgfSmbTsKWG6sc3nQu66azDvTz3v6Jn+oLQNdeFAA0gaXvChCWaAtsnDglSgzMTigz4+jY+ruqmvM+J4g7SYiWHrSzABM2QnBBvhMuNcFQJ/o7vQ3sWBlHDEJf+OLlW6APRbPfY/xZn7+TCQSi3UyVuSctqSeQfnCI3pDm2C17mtBeuYLtXDrDsENPOm9SEBz/rCpFVNyVI3TS5o+2EAolFQmEf7tV9ai32v5qqqdxwIS66EwW+eNVwpEpwdJrRRddlQ8Nn49Hgk7NHmniSxKn92BRVjtg/l92tvrNgfopQHRM9rnDcKtd0qvgVbmVOdgx55NnEMMYbOV0ljesmos++x3WpeH35WpOBV60rIbRCVPaSn1SqaEjeLXHKkc35EYgiUoD+F3pP7vfpbUJ5Wq8fIirFXnm6nA2WlBW8GXFEfd5tnjTkPAd9MSyqZz6bu0bDS3l4x7+qPgUlrQDg=
  account: mediocregopher

cmd/dehub/cmd_commit.go

@@ -152,3 +152,33 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 
 	cmd.Run(body)
 }
+
+func cmdCombine(ctx context.Context, cmd *dcmd.Cmd) {
+	flag := cmd.FlagSet()
+	onto := flag.String("onto", "", "Branch the new commit should be put onto")
+	startRev := flag.String("start", "", "Revision of the starting commit to combine")
+	endRev := flag.String("end", "", "Revision of the ending commit to combine")
+
+	cmd.Run(func() (context.Context, error) {
+		if *onto == "" ||
+			*startRev == "" ||
+			*endRev == "" {
+			return nil, errors.New("-onto, -start, and -end are required")
+		}
+
+		repo := ctxRepo(ctx)
+		ontoBranch := plumbing.NewBranchReferenceName(*onto)
+		gitCommit, err := repo.CombineCommitChanges(
+			plumbing.Revision(*startRev),
+			plumbing.Revision(*endRev),
+			ontoBranch,
+		)
+		if err != nil {
+			return nil, err
+		}
+
+		fmt.Printf("new commit %q added to branch %q\n",
+			gitCommit.GitCommit.Hash, ontoBranch.Short())
+		return nil, nil
+	})
+}

cmd/dehub/cmd_hook.go

@@ -48,32 +48,14 @@ func cmdHook(ctx context.Context, cmd *dcmd.Cmd) {
 				return nil, fmt.Errorf("reference %q is not a branch, can't push to it", branchName)
 			}
 
-			var startHash, endHash plumbing.Hash
+			gitCommits, err := repo.GetGitRevisionRange(startRev, endRev)
-			// startRev can be a zero hash if these are the first commits for a
-			// branch being pushed.
-			if startRev != plumbing.Revision(plumbing.ZeroHash.String()) {
-				h, err := repo.GitRepo.ResolveRevision(startRev)
-				if err != nil {
-					return nil, fmt.Errorf("resolving revision %q: %w", startRev, err)
-				}
-				startHash = *h
-			}
-			{
-				h, err := repo.GitRepo.ResolveRevision(endRev)
-				if err != nil {
-					return nil, fmt.Errorf("resolving revision %q: %w", endRev, err)
-				}
-				endHash = *h
-			}
-
-			gitCommits, err := repo.GetGitCommitRange(startHash, endHash)
 			if err != nil {
 				return nil, fmt.Errorf("getting commits from %q to %q: %w",
-					startHash, endHash, err)
+					startRev, endRev, err)
 
 			} else if err := repo.VerifyCommits(branchName, gitCommits); err != nil {
 				return nil, fmt.Errorf("verifying commits from %q to %q: %w",
-					startHash, endHash, err)
+					startRev, endRev, err)
 			}
 		}
 

cmd/dehub/main.go

@@ -30,6 +30,7 @@ func main() {
 	cmd.SubCmd("commit", "commits staged changes to the head of the current branch", cmdCommit)
 	cmd.SubCmd("verify", "verifies one or more commits as having the proper credentials", cmdVerify)
 	cmd.SubCmd("hook", "use dehub as a git hook", cmdHook)
+	cmd.SubCmd("combine", "Combine multiple change and credential commits into a single commit", cmdCombine)
 
 	cmd.Run(func() (context.Context, error) {
 		repo, err := dehub.OpenRepo(".", dehub.OpenBare(*bare))

commit_change.go

@@ -1,11 +1,16 @@
 package dehub
 
 import (
+	"bytes"
 	"dehub/fs"
+	"dehub/sigcred"
 	"dehub/yamlutil"
 	"errors"
 	"fmt"
+	"sort"
+	"strings"
 
+	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/object"
 )
 
@@ -57,3 +62,102 @@ func (cc CommitChange) Hash(parent, this *object.Tree) ([]byte, error) {
 func (cc CommitChange) GetHash() []byte {
 	return cc.ChangeHash
 }
+
+// CombineCommitChanges takes all changes in the given range and combines them
+// into a single change Commit. The resulting Commit will have the same message
+// as the latest change commit in the range, and will contain all Credentials
+// for the resulting change hash that it finds in the range as well.
+//
+// The combined commit is then committed to the repo with the given revision as
+// its parent. If the diff between start/end and onto/end is different then this
+// 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)
+	}
+
+	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
+		}
+	}
+	if !lastChangeCommitOk {
+		return GitCommit{}, fmt.Errorf("no change commits in range %q to %q", startRev, endRev)
+	}
+
+	// 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)
+	if err != nil {
+		return GitCommit{}, fmt.Errorf("getting tree of %q (parent of %q): %w",
+			startRev, startEndCommits[0].GitCommit.Hash, err)
+	}
+
+	msg := lastChangeCommit.Commit.Change.Message
+	endTree := lastChangeCommit.GitTree
+	changeHash := genChangeHash(nil, msg, startTree, endTree)
+
+	ontoBranchName, err := r.ReferenceToBranchName(onto)
+	if err != nil {
+		return GitCommit{}, fmt.Errorf("resolving %q into a branch name: %w", onto, err)
+	}
+
+	// now determine the change hash from onto->end, to ensure that it remains
+	// the same as from start->end
+	ontoCommit, err := r.GetGitRevision(plumbing.Revision(onto))
+	if err != nil {
+		return GitCommit{}, fmt.Errorf("resolving revision %q: %w", onto, err)
+	}
+	ontoTree := ontoCommit.GitTree
+	ontoEndChangeHash := genChangeHash(nil, msg, ontoTree, endTree)
+	if !bytes.Equal(ontoEndChangeHash, changeHash) {
+		// TODO figure out what files to show as being the "problem files" in
+		// the error message
+		return GitCommit{}, fmt.Errorf("rebasing onto %q would cause the change hash to change, aborting combine", onto)
+	}
+
+	var creds []sigcred.Credential
+	for _, commit := range startEndCommits {
+		if bytes.Equal(commit.Interface.GetHash(), changeHash) {
+			creds = append(creds, commit.Commit.Common.Credentials...)
+		}
+	}
+
+	// this is mostly to make tests easier
+	sort.Slice(creds, func(i, j int) bool {
+		return creds[i].AccountID < creds[j].AccountID
+	})
+
+	commit := Commit{
+		Change: &CommitChange{
+			Message:    msg,
+			ChangeHash: changeHash,
+		},
+		Common: CommitCommon{Credentials: creds},
+	}
+	accountID := strings.Join(commit.Common.credAccountIDs(), ",")
+
+	gitCommit, err := r.CommitBare(CommitBareParams{
+		Commit:     commit,
+		AccountID:  accountID,
+		ParentHash: ontoCommit.GitCommit.Hash,
+		GitTree:    endTree,
+	})
+	if err != nil {
+		return GitCommit{}, fmt.Errorf("storing commit: %w", err)
+	}
+
+	// set the onto branch to this new commit
+	newHeadRef := plumbing.NewHashReference(ontoBranchName, gitCommit.GitCommit.Hash)
+	if err := r.GitRepo.Storer.SetReference(newHeadRef); err != nil {
+		return GitCommit{}, fmt.Errorf("setting reference %q to new commit hash %q: %w",
+			ontoBranchName, gitCommit.GitCommit.Hash, err)
+	}
+	return gitCommit, nil
+}

commit_change_test.go

@@ -1,11 +1,14 @@
 package dehub
 
 import (
+	"dehub/sigcred"
 	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/davecgh/go-spew/spew"
+	"gopkg.in/src-d/go-git.v4/plumbing"
+	yaml "gopkg.in/yaml.v2"
 )
 
 func TestChangeCommitVerify(t *testing.T) {
@@ -97,3 +100,93 @@ func TestChangeCommitVerify(t *testing.T) {
 		})
 	}
 }
+
+func TestCombineCommitChanges(t *testing.T) {
+	h := newHarness(t)
+
+	// commit initial config, so the root user can modify it in the next commit
+	h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig)
+
+	// add a toot user and modify the access controls such that both accounts
+	// are required for the main branch
+	tootSig, tootPubKeyBody := sigcred.SignifierPGPTmp("toot", h.rand)
+	h.cfg.Accounts = append(h.cfg.Accounts, Account{
+		ID: "toot",
+		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
+			Body: string(tootPubKeyBody),
+		}}},
+	})
+	err := yaml.Unmarshal([]byte(`
+- action: allow
+  filters:
+  - type: branch
+    pattern: main
+  - type: commit_type
+    commit_type: change
+  - type: signature
+    any_account: true
+    count: 2
+
+- action: allow
+  filters:
+  - type: not
+    filter:
+      type: branch
+      pattern: main
+  - type: signature
+    any_account: true
+    count: 1
+
+- action: deny
+`), &h.cfg.AccessControls)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	h.stageCfg()
+	tootCommit := h.changeCommit("add toot", h.cfg.Accounts[0].ID, h.sig)
+
+	// make a single change commit in another branch using root. Then add a
+	// credential using toot, and combine them onto main.
+	otherBranch := plumbing.NewBranchReferenceName("other")
+	h.checkout(otherBranch)
+	h.stage(map[string]string{"foo": "bar"})
+	fooCommit := h.changeCommit("add foo file", h.cfg.Accounts[0].ID, h.sig)
+
+	// now adding a credential commit from toot should work
+	credCommitObj, err := h.repo.NewCommitCredential(fooCommit.Interface.GetHash())
+	if err != nil {
+		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,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// that new commit should have both credentials
+	creds := combinedCommit.Commit.Common.Credentials
+	if len(creds) != 2 {
+		t.Fatalf("combined commit has %d credentials, not 2", len(creds))
+	} else if creds[0].AccountID != "root" {
+		t.Fatalf("combined commit first credential should be from root, is from %q", creds[0].AccountID)
+	} else if creds[1].AccountID != "toot" {
+		t.Fatalf("combined commit second credential should be from toot, is from %q", creds[1].AccountID)
+	}
+
+	// double check that the HEAD commit of main got properly set
+	h.checkout(MainRefName)
+	mainHead, err := h.repo.GetGitHead()
+	if err != nil {
+		t.Fatal(err)
+	} else if mainHead.GitCommit.Hash != combinedCommit.GitCommit.Hash {
+		t.Fatalf("mainHead's should be pointed at %s but is pointed at %s",
+			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)
+	}
+}

repo.go

@@ -136,10 +136,15 @@ func (r *Repo) TraverseReferenceChain(refName plumbing.ReferenceName, pred func(
 	}
 }
 
-// ReferenceToBranchName traverses a chain of references looking for a branch
+// ReferenceToBranchName traverses a chain of references looking for the first
-// reference, and returns that name, or returns an error if no branch reference
+// branch reference, and returns that name, or returns an error if no branch
-// is part of the chain.
+// reference is part of the chain.
 func (r *Repo) ReferenceToBranchName(refName plumbing.ReferenceName) (plumbing.ReferenceName, error) {
+	// first check if the given refName is a branch, if so just return that.
+	if refName.IsBranch() {
+		return refName, nil
+	}
+
 	ref, err := r.TraverseReferenceChain(refName, func(ref *plumbing.Reference) bool {
 		return ref.Target().IsBranch()
 	})
@@ -301,3 +306,30 @@ func (r *Repo) GetGitCommitRange(start, end plumbing.Hash) ([]GitCommit, error)
 	}
 	return commits, nil
 }
+
+func (r *Repo) resolveRev(rev plumbing.Revision) (plumbing.Hash, error) {
+	if rev == plumbing.Revision(plumbing.ZeroHash.String()) {
+		return plumbing.ZeroHash, nil
+	}
+	h, err := r.GitRepo.ResolveRevision(rev)
+	if err != nil {
+		return plumbing.ZeroHash, fmt.Errorf("resolving revision %q: %w", rev, err)
+	}
+	return *h, nil
+}
+
+// GetGitRevisionRange is like GetGitCommitRange, first resolving the given
+// revisions into hashes before continuing with GetGitCommitRange's behavior.
+func (r *Repo) GetGitRevisionRange(startRev, endRev plumbing.Revision) ([]GitCommit, error) {
+	start, err := r.resolveRev(startRev)
+	if err != nil {
+		return nil, err
+	}
+
+	end, err := r.resolveRev(endRev)
+	if err != nil {
+		return nil, err
+	}
+
+	return r.GetGitCommitRange(start, end)
+}

repo_test.go

@@ -110,8 +110,8 @@ func (h *harness) checkout(branch plumbing.ReferenceName) {
 		h.t.Fatal(err)
 	}
 
-	_, err = h.repo.GitRepo.Branch(branch.Short())
+	_, err = h.repo.GitRepo.Storer.Reference(branch)
-	if errors.Is(err, git.ErrBranchNotFound) {
+	if errors.Is(err, plumbing.ErrReferenceNotFound) {
 		err = w.Checkout(&git.CheckoutOptions{
 			Hash:   head.GitCommit.Hash,
 			Branch: branch,