add change_description field to credential commits

---
type: change
description: |-
  add change_description field to credential commits

  This also involved changing how commit range change fingerprints are handled, so
  that the description can be more variable, and using that flexibility to allow
  the cli tool the ability to ask for the change description prior to creating a
  credential commit. Credential commit creation via the cli tool has been further
  improved in that it will check if it's been told to make a cred based on a
  specific credential commit, and if so just append the new credential and
  re-commit, speeding things up a bit for everyone.
fingerprint: AGWXqPM1Lxf3kvV/V3w65jNiUQyrQfemqfCcImSWjHJl
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl68xrgACgkQlcRvpqQRSKx6Ng//Qm/8dCqBe/8R7KgzW2naP352jegW5m9KoIMrVsm1kmgQfAGYs0easnks1+DadD79TEUacTrPYxDR50eScU58uybRUPPbJLXLjdoSbUYCl9bSdr6LVlZF9GVcMbH4iWhTN3YGQFHX7pEqwJ9Qzw/pbheCGPkS3KJFJZ3tC+TVM7QysdkGpxlKn8GiSIfN3d4QaHWFJW9FOc1K4mAFfmM/5QqU5j5OydlLeScJu+na7B8Lcmy4Jz+mdwIZCMG9h4qv1qD0/XcsY8Wv6cptMIBUZgg2SIyrl0KDau8pHX45PHlAaViCjzW2LjU2wMn+zKX6s3hSWje5sE95Z1a7l0ubytHGKUdlfTQDnkVYzs9hCqv3iAERChGl6E1qQWSNQMhhUcwVsZ7+Iw2M6ESrP/MvdsdO+N0t2oakbJnzUliGx8pQcY4DJPaosUadLqwKPMYiBOiSsfN2e+Vw2ZOoiHL2QUEd2VV1JfImZBl5oLFhzuLfR6h+eMkwcHjs+rtgMEh9TnA6qVzn7BzZ9p1ZFBHlw3Yrx22tG2Ehri4eMPa+5+vcuG7xlSlq4FOGvANi3E3lieeIiE61EztoX3iXeY4bdtaFlMEk6i+JU3jHdwXd006Kv0/5jd7n+OAba8aYjthNN2WuhTxj2/VSKn9iPjaCD3GJtkFQajf57v2nbK0QDx4=
  account: mediocregopher

SPEC.md

@@ -174,10 +174,14 @@ credentials: [...]
 
 # This field is not required, but can be helpful in situations where the
 # fingerprint was generated based on multiple change payloads
-fingerprint_commits:
+commits:
     - commit hash
     - commit hash
     - commit hash
+
+# This field is not required, but can be helpful to clarify which description
+# was used when generating a change fingerprint.
+change_description: blah blah blah
 ```
 
 ## Project Configuration

cmd/dehub/cmd_commit.go

@@ -136,25 +136,43 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 					return nil, errors.New("credential commit cannot have staged changes")
 				}
 
-				var credPayUn dehub.PayloadUnion
+				var commits []dehub.Commit
 				if *rev != "" {
 					commit, err := proj.GetCommitByRevision(plumbing.Revision(*rev))
 					if err != nil {
 						return nil, fmt.Errorf("resolving revision %q: %w", *rev, err)
 					}
-
-					if credPayUn, err = proj.NewPayloadCredentialFromChanges([]dehub.Commit{commit}); err != nil {
-						return nil, fmt.Errorf("constructing credential commit: %w", err)
-					}
+					commits = []dehub.Commit{commit}
 				} else {
-					commits, err := proj.GetCommitRangeByRevision(
+					var err error
+					commits, err = proj.GetCommitRangeByRevision(
 						plumbing.Revision(*startRev),
 						plumbing.Revision(*endRev),
 					)
 					if err != nil {
 						return nil, fmt.Errorf("resolving revisions %q to %q: %w",
 							*startRev, *endRev, err)
-					} else if credPayUn, err = proj.NewPayloadCredentialFromChanges(commits); err != nil {
+					}
+				}
+
+				var credPayUn dehub.PayloadUnion
+				if len(commits) == 0 {
+					return nil, errors.New("cannot create credential based on empty range of commits")
+				} else if len(commits) == 1 && commits[0].Payload.Credential != nil {
+					credPayUn = commits[0].Payload
+				} else {
+					lastDescr, err := dehub.LastChangeDescription(commits)
+					if err != nil {
+						return nil, fmt.Errorf("determining change description of commit(s): %w", err)
+					}
+
+					descr, err := tmpFileMsg(defaultCommitFileMsgTpl, lastDescr)
+					if err != nil {
+						return nil, fmt.Errorf("collecting credential description from user: %w", err)
+					}
+
+					credPayUn, err = proj.NewPayloadCredentialFromChanges(descr, commits)
+					if err != nil {
 						return nil, fmt.Errorf("constructing credential commit: %w", err)
 					}
 				}

cmd/dehub/tmp_file.go

@@ -14,8 +14,8 @@ import (
 
 const defaultCommitFileMsgTpl = `%s
 
-# Please enter the commit message for your commit. Lines starting
-# with '#' will be ignored, and an empty message aborts the commit.`
+# Please enter the description for your commit(s). Lines starting with '#' will
+# be ignored, and an empty message aborts the commit.`
 
 func tmpFileMsg(tpl string, args ...interface{}) (string, error) {
 	editor := os.Getenv("EDITOR")

payload.go

@@ -490,12 +490,31 @@ func (proj *Project) verifyCommit(
 	return nil
 }
 
+// LastChangeDescription iterates over the given commits in reverse order and
+// returns the first change description it comes across. A change description
+// may come from a change payload or a credential payload which covers a set of
+// changes.
+//
+// This function will return an error if no given commits contain a change
+// description.
+func LastChangeDescription(commits []Commit) (string, error) {
+	for i := range commits {
+		i = len(commits) - 1 - i
+		payUn := commits[i].Payload
+		if payUn.Change != nil {
+			return payUn.Change.Description, nil
+		} else if payUn.Credential != nil && payUn.Credential.ChangeDescription != "" {
+			return payUn.Credential.ChangeDescription, nil
+		}
+	}
+	return "", errors.New("no commits in range contain a change description")
+}
+
 type changeRangeInfo struct {
 	changeCommits      []Commit
 	authors            map[string]struct{}
-	msg                string
 	startTree, endTree *object.Tree
-	changeFingerprint  []byte
+	changeDescription  string
 }
 
 // changeRangeInfo returns various pieces of information about a range of
@@ -523,20 +542,23 @@ func (proj *Project) changeRangeInfo(commits []Commit) (changeRangeInfo, error)
 	if info.startTree, err = proj.parentTree(commits[0].Object); err != nil {
 		return changeRangeInfo{}, fmt.Errorf("getting tree of parent of %q: %w",
 			commits[0].Hash, err)
+	} else if info.changeDescription, err = LastChangeDescription(commits); err != nil {
+		return changeRangeInfo{}, err
 	}
 
 	lastChangeCommit := info.changeCommits[len(info.changeCommits)-1]
-	info.msg = lastChangeCommit.Payload.Change.Description
 	info.endTree = lastChangeCommit.TreeObject
+	return info, nil
+}
 
+func (info changeRangeInfo) changeFingerprint(descr string) ([]byte, error) {
 	changedFiles, err := ChangedFilesBetweenTrees(info.startTree, info.endTree)
 	if err != nil {
-		return changeRangeInfo{}, fmt.Errorf("calculating diff of commit trees %q and %q: %w",
+		return nil, fmt.Errorf("calculating diff of commit trees %q and %q: %w",
 			info.startTree.Hash, info.endTree.Hash, err)
 	}
 
-	info.changeFingerprint = genChangeFingerprint(nil, info.msg, changedFiles)
-	return info, nil
+	return genChangeFingerprint(nil, descr, changedFiles), nil
 }
 
 // VerifyCanSetBranchHEADTo is used to verify that a branch's HEAD can be set to

payload_change.go

@@ -98,6 +98,11 @@ func (proj *Project) CombinePayloadChanges(commits []Commit, onto plumbing.Refer
 		return Commit{}, err
 	}
 
+	commitsFingerprint, err := info.changeFingerprint(info.changeDescription)
+	if err != nil {
+		return Commit{}, err
+	}
+
 	authors := make([]string, 0, len(info.authors))
 	for author := range info.authors {
 		authors = append(authors, author)
@@ -122,8 +127,8 @@ func (proj *Project) CombinePayloadChanges(commits []Commit, onto plumbing.Refer
 			ontoCommit.Hash, commits[len(commits)-1].Hash, err)
 	}
 
-	ontoEndChangeFingerprint := genChangeFingerprint(nil, info.msg, ontoEndChangedFiles)
-	if !bytes.Equal(ontoEndChangeFingerprint, info.changeFingerprint) {
+	ontoEndChangeFingerprint := genChangeFingerprint(nil, info.changeDescription, ontoEndChangedFiles)
+	if !bytes.Equal(ontoEndChangeFingerprint, commitsFingerprint) {
 		// TODO figure out what files to show as being the "problem files" in
 		// the error message
 		return Commit{}, fmt.Errorf("combining onto %q would produce a different change fingerprint, aborting combine", onto.Short())
@@ -131,7 +136,7 @@ func (proj *Project) CombinePayloadChanges(commits []Commit, onto plumbing.Refer
 
 	var creds []sigcred.CredentialUnion
 	for _, commit := range commits {
-		if bytes.Equal(commit.Payload.Common.Fingerprint, info.changeFingerprint) {
+		if bytes.Equal(commit.Payload.Common.Fingerprint, commitsFingerprint) {
 			creds = append(creds, commit.Payload.Common.Credentials...)
 		}
 	}
@@ -143,10 +148,10 @@ func (proj *Project) CombinePayloadChanges(commits []Commit, onto plumbing.Refer
 
 	payUn := PayloadUnion{
 		Change: &PayloadChange{
-			Description: info.msg,
+			Description: info.changeDescription,
 		},
 		Common: PayloadCommon{
-			Fingerprint: info.changeFingerprint,
+			Fingerprint: commitsFingerprint,
 			Credentials: creds,
 		},
 	}

payload_credential.go

@@ -10,6 +10,11 @@ type PayloadCredential struct {
 	// It is only present for informational purposes, as commits don't not have
 	// any bearing on the CredentialedHash itself.
 	CommitHashes []string `yaml:"commits,omitempty"`
+
+	// ChangeDescription represents the description which has been credentialed.
+	// This field is only relevant if the Credential in the payload is for a
+	// change set.
+	ChangeDescription string `yaml:"change_description"`
 }
 
 var _ Payload = PayloadCredential{}
@@ -26,26 +31,38 @@ func (proj *Project) NewPayloadCredential(fingerprint []byte) (PayloadUnion, err
 
 // NewPayloadCredentialFromChanges constructs and returns a PayloadUnion
 // populated with a PayloadCredential. The fingerprint of the payload will be a
-// change fingerprint encompassing all changes in the given range of Commits.
-// The description of the last change payload in the range is used when
-// generating the fingerprint.
-func (proj *Project) NewPayloadCredentialFromChanges(commits []Commit) (PayloadUnion, error) {
+// change fingerprint generated from the given description and all changes in
+// the given range of Commits.
+//
+// If an empty description is given then the description of the last change
+// payload in the range is used when generating the fingerprint.
+func (proj *Project) NewPayloadCredentialFromChanges(descr string, commits []Commit) (PayloadUnion, error) {
 	info, err := proj.changeRangeInfo(commits)
 	if err != nil {
 		return PayloadUnion{}, err
 	}
 
-	payCred, err := proj.NewPayloadCredential(info.changeFingerprint)
+	if descr == "" {
+		descr = info.changeDescription
+	}
+	fingerprint, err := info.changeFingerprint(descr)
 	if err != nil {
 		return PayloadUnion{}, err
 	}
 
+	payCred, err := proj.NewPayloadCredential(fingerprint)
+	if err != nil {
+		return PayloadUnion{}, err
+	}
+
+	payCred.Credential.ChangeDescription = descr
 	for _, commit := range info.changeCommits {
 		payCred.Credential.CommitHashes = append(
 			payCred.Credential.CommitHashes,
 			commit.Hash.String(),
 		)
 	}
+
 	return payCred, nil
 }