Drover's back, baby!

go/src/infra/libs/git/commit.go

Index: go/src/infra/libs/git/commit.go
diff --git a/go/src/infra/libs/git/commit.go b/go/src/infra/libs/git/commit.go
new file mode 100644
index 0000000000000000000000000000000000000000..7e182f6f859a01b61960a227601a681e4408becc
--- /dev/null
+++ b/go/src/infra/libs/git/commit.go
@@ -0,0 +1,282 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+package git
+
+import (
+	"bytes"
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+	"time"
+
+	"infra/libs/infra_util"
+)
+
+// Footer represents the Key/Value pair of a single git commit footer.
+type Footer struct {
+	Key   string
+	Value string
+}
+
+// User represents an author/committer line in a Commit

[Vadim Sh., 2014/10/21 15:26:59]

nit: end sentence with dot, same in other places.

+type User struct {
+	Name  string
+	Email string
+	Time  time.Time
+}
+
+// RawString returns a `git hash-object` compatible string for this User
+func (u *User) RawString() string {
+	return fmt.Sprintf("%s <%s> %d %s", u.Name, u.Email, u.Time.Unix(),
+		u.Time.Format("-0700"))

[M-A Ruel, 2014/10/21 00:55:53]

Either use the local time or UTC, but do not hardcode the time zone.

[iannucci, 2016/05/23 21:53:42]

Again, it's not hard coded :) Read Time.Format docs:
http://golang.org/pkg/time/#Time.Format

+}
+
+// Commit represents an immutable git commit.
+//
+// It also will lazily parse the message for footers.
+type Commit struct {
+	id ObjectID
+
+	// TODO(iannucci): Make these real Object's

[agable, 2014/10/21 17:01:35]

nit: Objects

+	tree    ObjectID
+	parents []ObjectID
+
+	author       User
+	committer    User
+	extraHeaders []string
+	messageRaw   string
+
+	// private cache fields
+	messageRawLines []string
+	messageLines    []string
+	footerPairs     []Footer
+	footers         map[string][]string
+}
+
+func (c *Commit) ID() ObjectID     { return c.id }
+func (c *Commit) Type() ObjectType { return CommitType }
+func (c *Commit) Complete() bool   { return c.id != NoID }
+
+func (c *Commit) Tree() ObjectID             { return c.tree }
+func (c *Commit) Parents() (r []ObjectID)    { return append(r, c.parents...) }
+func (c *Commit) Author() User               { return c.author }
+func (c *Commit) Committer() User            { return c.committer }
+func (c *Commit) ExtraHeaders() (r []string) { return append(r, c.extraHeaders...) }
+func (c *Commit) MessageRaw() string         { return c.messageRaw }
+
+// Returns a partial Commit with the id and cache data cleared. This is
+// used by the Set* methods. If this Commit is already unidentified, avoid
+// copying it and return c directly.
+func (c *Commit) partial() *Commit {
+	if c.id != NoID {
+		return &Commit{
+			tree:         c.tree,
+			parents:      c.parents,
+			author:       c.author,
+			committer:    c.committer,
+			extraHeaders: c.extraHeaders,
+			messageRaw:   c.messageRaw,
+		}
+	}
+	return c
+}
+
+func (c *Commit) SetTree(t ObjectID) (ret *Commit) {

[M-A Ruel, 2014/10/21 00:55:53]

That still means that SetTree() modifies the caller if it was not set. I think
it's unexpected behavior.

[iannucci, 2016/05/23 21:53:42]

It modifies the caller only if the caller started with a partial commit (e.g.
wasn't hashed, so id == NoID).

+	ret = c.partial()
+	ret.tree = t
+	return

[agable, 2014/10/21 17:01:35]

Is the explicit return statement necessary for the named output to work, even
though it is the last line of the function?

+}
+
+func (c *Commit) SetParents(ps []ObjectID) (ret *Commit) {
+	ret = c.partial()
+	ret.parents = append([]ObjectID{}, ps...)
+	return
+}
+
+func (c *Commit) SetRawMessage(msg string) (ret *Commit) {
+	ret = c.partial()
+	ret.messageRaw = msg
+	return
+}
+
+// RawString returns a `git hash-object` compatible string for this Commit
+func (c *Commit) RawString() string {
+	buf := &bytes.Buffer{}
+	fmt.Fprintln(buf, "tree", c.Tree())
+	for _, p := range c.parents {
+		fmt.Fprintln(buf, "parent", p)
+	}
+	fmt.Fprintln(buf, "author", c.author.RawString())
+	fmt.Fprintln(buf, "committer", c.committer.RawString())
+	for _, l := range c.extraHeaders {
+		fmt.Fprintln(buf, l)
+	}
+	fmt.Fprintln(buf)
+	fmt.Fprint(buf, c.MessageRaw())
+	if c.id == NoID {
+		c.id = MakeObjectIDForData(CommitType, buf.Bytes())

[agable, 2014/10/21 17:01:35]

Creating a raw string representation of a commit shouldn't modify the commit
struct as a side effect.

+	}
+	return buf.String()
+}
+
+// MessageRawLines returns a cached slice of lines in MessageRaw, which
+// includes all lines in the commit 'message' (body and footers).
+func (c *Commit) MessageRawLines() []string {
+	if c.messageRawLines == nil {
+		c.messageRawLines = strings.Split(strings.TrimRight(c.messageRaw, "\n"), "\n")
+	}
+	return c.messageRawLines
+}
+
+// MessageLines returns a cached slice of lines in MessageRaw, excluding
+// footer lines.
+func (c *Commit) MessageLines() []string {
+	if c.messageLines == nil {
+		c.parseMessage()
+	}
+	return c.messageLines
+}
+
+// FooterPairs returns a cached slice of all Footers found in this Commit.
+//
+// Footers are found in the last paragraph of the Commit message, assuming
+// that each of the lines in the last paragraph looks like:
+//   ([^:]*):\s(.*)

[agable, 2014/10/21 17:01:35]

Doesn't seem quite right. Empty keys and values are allowed? And only one
character of whitespace after the colon? Should be
([^:]+):\s+(.+)
or something. Also, the parsing code actually requires a space, not just a
character of whitespace.

+//
+// The first group is the Key, and the second group is the Value.
+func (c *Commit) FooterPairs() []Footer {
+	if c.footerPairs == nil {
+		c.parseMessage()
+	}
+	return c.footerPairs
+}
+
+// Footers returns a cached map of Key -> []Value. This is often more convenient
+// than iterating through FooterPairs().
+func (c *Commit) Footers() map[string][]string {
+	if c.footers == nil {
+		c.parseMessage()
+	}
+	return c.footers
+}
+
+// CommitFromRaw returns a Commit parsed from the hash-object compatible commit

[agable, 2014/10/21 17:01:35]

nit: I like the earlier "...`git hash-object` compatible..." phrasing better. 

+// text format.
+//
+// This will calculate and fill in the Commit.ID from the actual commit data
+// provided.
+func CommitFromRaw(data []byte) (*Commit, error) {
+	return CommitFromRawWithID(MakeObjectIDForData(CommitType, data), data)
+}
+
+// CommitFromRawWithID returns a Commit parsed from the hash-object compatible
+// commit text format. This assumes that |id| is the correct id for data, and
+// does not hash or verify its correctness. Only use this if you trust the
+// origin of |data|.

[agable, 2014/10/21 17:01:35]

...and of |id|.

+func CommitFromRawWithID(id ObjectID, data []byte) (ret *Commit, err error) {
+	ret = new(Commit)
+	buf := bytes.NewBuffer(data)
+	nom := infra_util.Nom(buf)
+
+	ret.id = id
+	ret.tree = MakeObjectID(strings.Split(nom('\n'), " ")[1])
+	ret.parents = make([]ObjectID, 0, 1)
+	line := nom('\n')
+	for strings.HasPrefix(line, "parent ") {
+		ret.parents = append(ret.parents, MakeObjectID(strings.Split(line, " ")[1]))
+		line = nom('\n')
+	}
+	ret.author, err = decodeUser("author", line)
+	if err != nil {
+		return
+	}
+	ret.committer, err = decodeUser("committer", nom('\n'))
+	if err != nil {
+		return
+	}
+	ret.extraHeaders = make([]string, 0)
+	line = nom('\n')
+	for len(line) != 0 {
+		ret.extraHeaders = append(ret.extraHeaders, line)
+		line = nom('\n')
+	}
+	ret.messageRaw = buf.String()
+	return
+}
+
+// Private functions
+
+// Parses the message for footer_pairs, footers, message_lines

[M-A Ruel, 2014/10/21 00:55:53]

parseMessage parses ...

[iannucci, 2016/05/23 21:53:42]

Done.

+func (c *Commit) parseMessage() {
+	allLines := c.MessageRawLines()
+	i := -1

[M-A Ruel, 2014/10/21 00:55:54]

i := len(allLines)

[iannucci, 2016/05/23 21:53:42]

Done.

+	for i = len(allLines) - 1; i >= 0; i-- {

[M-A Ruel, 2014/10/21 00:55:53]

for ; i>= 0; i-- {

[iannucci, 2016/05/23 21:53:42]

Done.

+		line := allLines[i]
+		if len(line) == 0 {
+			break
+		} else if !strings.Contains(line, ": ") {
+			// invalid footer
+			i = -1
+			break
+		}
+	}
+
+	if i != -1 {
+		c.messageLines = allLines[:i]
+
+		pairs := make([]Footer, 0, len(allLines)-i)
+		footers := map[string][]string{}
+		for i++; i < len(allLines); i++ {
+			line := allLines[i]
+			bits := strings.SplitN(line, ": ", 2)
+			key, value := bits[0], bits[1]

[M-A Ruel, 2014/10/21 00:55:53]

This will crash if there was no ": "

[iannucci, 2016/05/23 21:53:42]

yep, which is impossible due to line 219.

+			pairs = append(pairs, Footer{key, value})
+			footers[key] = append(footers[key], value)
+		}
+		c.footerPairs = pairs
+		c.footers = footers
+	} else {
+		c.messageLines = allLines
+		c.footerPairs = []Footer{}
+	}
+}
+
+// author|committer SP <user_name> SP '<' <user_email> '>' SP timecode +0000
+// I would use named groups, but the interface to access them is terrible
+// in the regexp package...
+var userRegex = regexp.MustCompile(
+	`^(author|committer) ` +
+		`([^<]*) ` + // user_name
+		`<([^>]*)> ` + // user_email
+		`(\d*) ` + // timecode
+		`([+-]\d{4})$`) //timezone
+
+func decodeUser(lineType, line string) (ret User, err error) {
+	fields := userRegex.FindStringSubmatch(line)
+	if len(fields) == 0 {
+		err = fmt.Errorf("Incompatible user line: %#v", line)
+		return
+	}
+	if fields[1] != lineType {
+		err = fmt.Errorf("Expected to parse %s but got %s instead", lineType, fields[1])
+		return
+	}
+	t, err := time.Parse("-0700", fields[5])

[M-A Ruel, 2014/10/21 00:55:54]

I'm not a big fan of named return value. This line is a good example, where it's
ambiguous if err is being aliased or not. I'd recommend against.

[agable, 2014/10/21 17:01:35]

I'm fine with this usage; it's one of the first things pointed out as being
'canonical go' in the Go intro I was reading just a couple weeks ago. I'm not
sure how err could be 'aliased' here -- there's only one scope it could be in.

[iannucci, 2016/05/23 21:53:42]

Done.

+	if err != nil {
+		return
+	}
+
+	timecode, err := strconv.ParseInt(fields[4], 10, 64)
+	if err != nil {
+		return
+	}
+
+	ret = User{
+		Name:  fields[2],
+		Email: fields[3],
+		Time:  time.Unix(timecode, 0).In(t.Location()),
+	}
+	return
+}