diff --git a/SPEC.md b/SPEC.md index 9c3ffe1..b2ecdb1 100644 --- a/SPEC.md +++ b/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 diff --git a/cmd/dehub/cmd_commit.go b/cmd/dehub/cmd_commit.go index 862652a..707610e 100644 --- a/cmd/dehub/cmd_commit.go +++ b/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) } } diff --git a/cmd/dehub/tmp_file.go b/cmd/dehub/tmp_file.go index da251c7..59f54a6 100644 --- a/cmd/dehub/tmp_file.go +++ b/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") diff --git a/payload.go b/payload.go index 15d722f..e2bbc4e 100644 --- a/payload.go +++ b/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 diff --git a/payload_change.go b/payload_change.go index c252e3d..d97bb4d 100644 --- a/payload_change.go +++ b/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, }, } diff --git a/payload_credential.go b/payload_credential.go index 9b114d0..ee3817f 100644 --- a/payload_credential.go +++ b/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 }