Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
276 changes: 276 additions & 0 deletions .tools/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
package main

import (
"bytes"
"encoding/json"
"flag"
"fmt"
"log"
"os"
"os/exec"
"regexp"
"strings"
)

// DefaultRules are the standard validation to perform on git commits
var DefaultRules = []ValidateRule{
func(c CommitEntry) (vr ValidateResult) {
vr.CommitEntry = c
hasValid := false
for _, line := range strings.Split(c.Body, "\n") {
if validDCO.MatchString(line) {
hasValid = true
}
}
if !hasValid {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<nit>To avoid a double-negative (!hasValid {} else {}), maybe drop the ! and flip the if/else clauses?</nit>

vr.Pass = false
vr.Msg = "does not have a valid DCO"
} else {
vr.Pass = true
vr.Msg = "has a valid DCO"
}

return vr
},
// TODO add something for the cleanliness of the c.Subject
}

var (
flVerbose = flag.Bool("v", false, "verbose")
flCommitRange = flag.String("range", "", "use this commit range instead")

validDCO = regexp.MustCompile(`^Signed-off-by: ([^<]+) <([^<>@]+@[^<>]+)>$`)
)

func main() {
flag.Parse()

var commitrange string
if *flCommitRange != "" {
commitrange = *flCommitRange
} else {
var err error
commitrange, err = GitFetchHeadCommit()
if err != nil {
log.Fatal(err)
}
}

c, err := GitCommits(commitrange)
if err != nil {
log.Fatal(err)
}

results := ValidateResults{}
for _, commit := range c {
fmt.Printf(" * %s %s ... ", commit.AbbreviatedCommit, commit.Subject)
vr := ValidateCommit(commit, DefaultRules)
results = append(results, vr...)
if _, fail := vr.PassFail(); fail == 0 {
fmt.Println("PASS")
if *flVerbose {
for _, r := range vr {
if !r.Pass {
fmt.Printf(" - %s\n", r.Msg)
}
}
}
} else {
fmt.Println("FAIL")
// default, only print out failed validations
for _, r := range vr {
if !r.Pass {
fmt.Printf(" - %s\n", r.Msg)
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe error out if neither TRAVIS_COMMIT_RANGE nor the --range flag are set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps. maybe i'll just make this tool less involved in travis, and the .travis.yml to pass it's args to flags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Sep 09, 2015 at 12:49:38PM -0700, Vincent Batts wrote:

perhaps. maybe i'll just make this tool less involved in travis, and
the .travis.yml to pass it's args to flags.

That sounds easier to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

_, fail := results.PassFail()
if fail > 0 {
fmt.Printf("%d issues to fix\n", fail)
os.Exit(1)
}
}

// ValidateRule will operate over a provided CommitEntry, and return a result.
type ValidateRule func(CommitEntry) ValidateResult

// ValidateCommit processes the given rules on the provided commit, and returns the result set.
func ValidateCommit(c CommitEntry, rules []ValidateRule) ValidateResults {
results := ValidateResults{}
for _, r := range rules {
results = append(results, r(c))
}
return results
}

// ValidateResult is the result for a single validation of a commit.
type ValidateResult struct {
CommitEntry CommitEntry
Pass bool
Msg string
}

// ValidateResults is a set of results. This is type makes it easy for the following function.
type ValidateResults []ValidateResult

// PassFail gives a quick over/under of passes and failures of the results in this set
func (vr ValidateResults) PassFail() (pass int, fail int) {
for _, res := range vr {
if res.Pass {
pass++
} else {
fail++
}
}
return pass, fail
}

// CommitEntry represents a single commit's information from `git`
type CommitEntry struct {
Commit string
AbbreviatedCommit string `json:"abbreviated_commit"`
Tree string
AbbreviatedTree string `json:"abbreviated_tree"`
Parent string
AbbreviatedParent string `json:"abbreviated_parent"`
Refs string
Encoding string
Subject string
SanitizedSubjectLine string `json:"sanitized_subject_line"`
Body string
CommitNotes string `json:"commit_notes"`
VerificationFlag string `json:"verification_flag"`
ShortMsg string
Signer string
SignerKey string `json:"signer_key"`
Author PersonAction `json:"author,omitempty"`
Commiter PersonAction `json:"commiter,omitempty"`
}

// PersonAction is a time and identity of an action on a git commit
type PersonAction struct {
Name string `json:"name"`
Email string `json:"email"`
Date string `json:"date"` // this could maybe be an actual time.Time
}

var (
prettyLogSubject = `--pretty=format:%s`
prettyLogBody = `--pretty=format:%b`
prettyLogCommit = `--pretty=format:%H`
prettyLogAuthorName = `--pretty=format:%aN`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how old Travis' Git is (and I haven't looked up --format in Git's release notes), but at least with v2.1 you can use --format=… instead of --pretty=format:….

I also think it makes sense to drop the leading --pretty=format: (or whatever) here and just list %s, %b, etc. We can inject the option prefix once in the consuming code.

prettyLogAuthorEmail = `--pretty=format:%aE`
prettyLogCommiterName = `--pretty=format:%cN`
prettyLogCommiterEmail = `--pretty=format:%cE`
prettyLogSigner = `--pretty=format:%GS`
prettyLogCommitNotes = `--pretty=format:%N`
prettyLogFormat = `--pretty=format:{"commit": "%H", "abbreviated_commit": "%h", "tree": "%T", "abbreviated_tree": "%t", "parent": "%P", "abbreviated_parent": "%p", "refs": "%D", "encoding": "%e", "sanitized_subject_line": "%f", "verification_flag": "%G?", "signer_key": "%GK", "author": { "date": "%aD" }, "commiter": { "date": "%cD" }}`
)

// GitLogCommit assembles the full information on a commit from its commit hash
func GitLogCommit(commit string) (*CommitEntry, error) {
buf := bytes.NewBuffer([]byte{})
cmd := exec.Command("git", "log", "-1", prettyLogFormat, commit)
cmd.Stdout = buf
cmd.Stderr = os.Stderr

if err := cmd.Run(); err != nil {
log.Println(strings.Join(cmd.Args, " "))
return nil, err
}
c := CommitEntry{}
output := buf.Bytes()
if err := json.Unmarshal(output, &c); err != nil {
fmt.Println(string(output))
return nil, err
}

output, err := exec.Command("git", "log", "-1", prettyLogSubject, commit).Output()
if err != nil {
return nil, err
}
c.Subject = strings.TrimSpace(string(output))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not get the subject from the initial prettyLogFormat output? Is the concern with using %s that there will be double-quotes in the subject, which would need to be escaped in the JSON? It seems like there could be double-quotes in the author name or committer name too, so maybe it's best to pull those out as well? I think a loop over all user-supplied fields with a single log call per-field/commit is probably best. That's what you're doing here for the subject and body (but you've unrolled the loop).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because subject and body can have symbols that --pretty does not escape. :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Sep 09, 2015 at 12:52:00PM -0700, Vincent Batts wrote:

because subject and body can have symbols that --pretty does not
escape. :-)

So can author names:

$ git init
$ git config user.name 'Mua"haha'
$ touch README
$ git add README
$ git commit -m 'Testing crazy author name'
$ git log --format='{"author": "%aN"}'
{"author": "Mua"haha"}

which is why I'm suggesting “a loop over all user-supplied fields with
a single log call per-field/commit” 1 ;).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed this as well.


output, err = exec.Command("git", "log", "-1", prettyLogBody, commit).Output()
if err != nil {
return nil, err
}
c.Body = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogAuthorName, commit).Output()
if err != nil {
return nil, err
}
c.Author.Name = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogAuthorEmail, commit).Output()
if err != nil {
return nil, err
}
c.Author.Email = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogCommiterName, commit).Output()
if err != nil {
return nil, err
}
c.Commiter.Name = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogCommiterEmail, commit).Output()
if err != nil {
return nil, err
}
c.Commiter.Email = strings.TrimSpace(string(output))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Go not have an equivalent of Python's setattr? In Python, I'd make this:

for format, keys in [
        ('%s', ['subject']),
        …
        ('%cE', ['committer', 'email']),
        ]:
    value = …
    obj = commit
    for key in keys[:-1]:
        obj = getattr(obj, key)
    setattr(obj, keys[-1], value)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not without reflect, which is overkill for doing a simple loop.
On Sep 9, 2015 6:02 PM, "W. Trevor King" [email protected] wrote:

In .tools/validate.go
#167 (comment):

  • if err != nil {
  •   return nil, err
    
  • }
  • c.Author.Email = strings.TrimSpace(string(output))
  • output, err = exec.Command("git", "log", "-1", prettyLogCommiterName, commit).Output()
  • if err != nil {
  •   return nil, err
    
  • }
  • c.Commiter.Name = strings.TrimSpace(string(output))
  • output, err = exec.Command("git", "log", "-1", prettyLogCommiterEmail, commit).Output()
  • if err != nil {
  •   return nil, err
    
  • }
  • c.Commiter.Email = strings.TrimSpace(string(output))

Does Go not have an equivalent of Python's setattr? In Python, I'd make
this:

for format, keys in [
('%s', ['subject']),

('%cE', ['committer', 'email']),
]:
value = …
obj = commit
for key in keys[:-1]:
obj = getattr(obj, key)
setattr(obj, keys[-1], value)


Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/167/files#r39102627.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Sep 09, 2015 at 03:08:43PM -0700, Vincent Batts wrote:

Not without reflect, which is overkill for doing a simple loop.

Ah bummer. And I have a friend who would be waving his Lisp-y
parenthesis at this too ;).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the map instead of the structure can avoid reflect. Even using reflect, it is much simpler than current code IMO.


output, err = exec.Command("git", "log", "-1", prettyLogCommitNotes, commit).Output()
if err != nil {
return nil, err
}
c.CommitNotes = strings.TrimSpace(string(output))

output, err = exec.Command("git", "log", "-1", prettyLogSigner, commit).Output()
if err != nil {
return nil, err
}
c.Signer = strings.TrimSpace(string(output))

return &c, nil
}

// GitCommits returns a set of commits.
// If commitrange is a git still range 12345...54321, then it will be isolated set of commits.
// If commitrange is a single commit, all ancestor commits up through the hash provided.
func GitCommits(commitrange string) ([]CommitEntry, error) {
output, err := exec.Command("git", "log", prettyLogCommit, commitrange).Output()
if err != nil {
return nil, err
}
commitHashes := strings.Split(strings.TrimSpace(string(output)), "\n")
commits := make([]CommitEntry, len(commitHashes))
for i, commitHash := range commitHashes {
c, err := GitLogCommit(commitHash)
if err != nil {
return commits, err
}
commits[i] = *c
}
return commits, nil
}

// GitFetchHeadCommit returns the hash of FETCH_HEAD
func GitFetchHeadCommit() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be used anywhere. Maybe leave it out until we need it?

output, err := exec.Command("git", "rev-parse", "--verify", "HEAD").Output()
if err != nil {
return "", err
}
return strings.TrimSpace(string(output)), nil
}

// GitHeadCommit returns the hash of HEAD
func GitHeadCommit() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be used anywhere. Maybe leave it out until we need it?

output, err := exec.Command("git", "rev-parse", "--verify", "HEAD").Output()
if err != nil {
return "", err
}
return strings.TrimSpace(string(output)), nil
}
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ install: true
script:
- go vet -x ./...
- $HOME/gopath/bin/golint ./...
- go run .tools/validate.go -range ${TRAVIS_COMMIT_RANGE}