v.io/jiri: extend cl mail to handle multi part CLs and...
- cl mail now handles multi part CLs, it uses a file in
the jiri per-branch metadata dir to store the message for
each repo and then uses jiri runp to run itself in
each repo. Two flags are added to make this work:
--current-project-only disables the multi-part behaviour
--initial-commit-file
- cl mail now ignores error messages for pushing to gerrit
with no new changes..
MultiPart: 1/2
Change-Id: I1424c0a9f21226cdf45991e61dfa922f4291968e
diff --git a/cmd/jiri/cl.go b/cmd/jiri/cl.go
index 2ddc965..023ab18 100644
--- a/cmd/jiri/cl.go
+++ b/cmd/jiri/cl.go
@@ -6,10 +6,13 @@
import (
"fmt"
+ "io"
+ "io/ioutil"
"net/url"
"os"
"path/filepath"
"regexp"
+ "sort"
"strings"
"v.io/jiri"
@@ -22,25 +25,29 @@
)
const (
- commitMessageFileName = ".gerrit_commit_message"
- dependencyPathFileName = ".dependency_path"
+ commitMessageFileName = ".gerrit_commit_message"
+ dependencyPathFileName = ".dependency_path"
+ multiPartMetaDataFileName = "multipart_index"
)
var (
- autosubmitFlag bool
- ccsFlag string
- draftFlag bool
- editFlag bool
- forceFlag bool
- hostFlag string
- messageFlag string
- presubmitFlag string
- remoteBranchFlag string
- reviewersFlag string
- setTopicFlag bool
- topicFlag string
- uncommittedFlag bool
- verifyFlag bool
+ autosubmitFlag bool
+ ccsFlag string
+ draftFlag bool
+ editFlag bool
+ forceFlag bool
+ hostFlag string
+ messageFlag string
+ commitMessageBodyFlag string
+ presubmitFlag string
+ remoteBranchFlag string
+ reviewersFlag string
+ setTopicFlag bool
+ topicFlag string
+ uncommittedFlag bool
+ verifyFlag bool
+ currentProjectFlag bool
+ cleanupMultiPartFlag bool
)
// Special labels stored in the commit message.
@@ -51,9 +58,14 @@
// Change-Ids start with 'I' and are followed by 40 characters of hex.
changeIDRE *regexp.Regexp = regexp.MustCompile("Change-Id: (I[0123456789abcdefABCDEF]{40})")
+ // MultiPart messages are of the form: MultiPart: <n>/<m>
+ multiPartRE *regexp.Regexp = regexp.MustCompile(`(?m)^MultiPart: \d+/\d+$`)
+
// Presubmit test label.
// PresubmitTest: <type>
presubmitTestLabelRE *regexp.Regexp = regexp.MustCompile(`PresubmitTest:\s*(.*)`)
+
+ noChangesRE *regexp.Regexp = regexp.MustCompile(`! \[remote rejected\] HEAD -> refs/(for|drafts)/\S+ \(no new changes\)`)
)
// init carries out the package initialization.
@@ -66,6 +78,7 @@
cmdCLMail.Flags.BoolVar(&editFlag, "edit", true, `Open an editor to edit the CL description.`)
cmdCLMail.Flags.StringVar(&hostFlag, "host", "", `Gerrit host to use. Defaults to gerrit host specified in manifest.`)
cmdCLMail.Flags.StringVar(&messageFlag, "m", "", `CL description.`)
+ cmdCLMail.Flags.StringVar(&commitMessageBodyFlag, "commit-message-body-file", "", `file containing the body of the CL description, that is, text without a ChangeID, MultiPart etc.`)
cmdCLMail.Flags.StringVar(&presubmitFlag, "presubmit", string(gerrit.PresubmitTestTypeAll),
fmt.Sprintf("The type of presubmit tests to run. Valid values: %s.", strings.Join(gerrit.PresubmitTestTypes(), ",")))
cmdCLMail.Flags.StringVar(&remoteBranchFlag, "remote-branch", "master", `Name of the remote branch the CL pertains to, without the leading "origin/".`)
@@ -74,6 +87,8 @@
cmdCLMail.Flags.StringVar(&topicFlag, "topic", "", `CL topic, defaults to <username>-<branchname>.`)
cmdCLMail.Flags.BoolVar(&uncommittedFlag, "check-uncommitted", true, `Check that no uncommitted changes exist.`)
cmdCLMail.Flags.BoolVar(&verifyFlag, "verify", true, `Run pre-push git hooks.`)
+ cmdCLMail.Flags.BoolVar(¤tProjectFlag, "current-project-only", false, `Run mail in the current project only.`)
+ cmdCLMail.Flags.BoolVar(&cleanupMultiPartFlag, "clean-multipart-metadata", false, `Cleanup the metadata associated with multipart CLs pertaining the MultiPart: x/y message without mailing any CLs.`)
cmdCLSync.Flags.StringVar(&remoteBranchFlag, "remote-branch", "master", `Name of the remote branch the CL pertains to, without the leading "origin/".`)
}
@@ -116,8 +131,8 @@
// cmdCL represents the "jiri cl" command.
var cmdCL = &cmdline.Command{
Name: "cl",
- Short: "Manage project changelists",
- Long: "Manage project changelists.",
+ Short: "Manage changelists for multiple projects",
+ Long: "Manage changelists for multiple projects.",
Children: []*cmdline.Command{cmdCLCleanup, cmdCLMail, cmdCLNew, cmdCLSync},
}
@@ -335,19 +350,15 @@
// currentProject returns the Project containing the current working directory.
// The current working directory must be inside JIRI_ROOT.
-func currentProject(jirix *jiri.X, cmd string) (project.Project, error) {
+func currentProject(jirix *jiri.X) (project.Project, error) {
dir, err := os.Getwd()
if err != nil {
return project.Project{}, fmt.Errorf("os.Getwd() failed: %v", err)
}
- // Error if current working dir is not inside jirix.Root.
- if !strings.HasPrefix(dir, jirix.Root) {
- return project.Project{}, fmt.Errorf("'%s' must be run from within a project in JIRI_ROOT", cmd)
- }
-
// Walk up the path until we find a project at that path, or hit the jirix.Root.
- for dir != jirix.Root {
+ // Note that we can't just compare path prefixes because of soft links.
+ for dir != jirix.Root && dir != string(filepath.Separator) {
p, err := project.ProjectAtPath(jirix, dir)
if err != nil {
dir = filepath.Dir(dir)
@@ -358,8 +369,209 @@
return project.Project{}, fmt.Errorf("directory %q is not contained in a project", dir)
}
-// runCLMail is a wrapper that sets up and runs a review instance.
-func runCLMail(jirix *jiri.X, _ []string) error {
+type multiPart struct {
+ clean, current bool
+ currentKey project.ProjectKey
+ states map[project.ProjectKey]*project.ProjectState
+ keys project.ProjectKeys
+}
+
+// initForMultiPart determines the actions to be taken
+// based on command line flags and project state.
+func initForMultiPart(jirix *jiri.X) (*multiPart, error) {
+ mp := &multiPart{}
+ mp.clean = cleanupMultiPartFlag
+ if currentProjectFlag {
+ mp.current = true
+ return mp, nil
+ }
+ if mp.clean {
+ states, keys, err := projectStates(jirix, true)
+ if err != nil {
+ return nil, err
+ }
+ mp.states = states
+ mp.keys = keys
+ return mp, nil
+ }
+ states, keys, err := projectStates(jirix, false)
+ if err != nil {
+ return nil, err
+ }
+ current, err := currentProject(jirix)
+ if err != nil {
+ return nil, err
+ }
+ mp.currentKey = current.Key()
+ if len(keys) == 1 {
+ filename := filepath.Join(states[keys[0]].Project.Path, jiri.ProjectMetaDir, multiPartMetaDataFileName)
+ os.Remove(filename)
+ if mp.currentKey == states[keys[0]].Project.Key() {
+ mp.current = true
+ return mp, nil
+ }
+ }
+ mp.states = states
+ mp.keys = keys
+ return mp, nil
+}
+
+// projectStates returns a map with all projects that are on the same
+// current branch as the current project, as well as a slice of their
+// project keys sorted lexicographically. Unless "allowdirty" is true,
+// an error is returned if any matching project has uncommitted changes.
+// The keys are returned, sorted, to avoid the caller having to recreate
+// the them by iterating over the map.
+func projectStates(jirix *jiri.X, allowdirty bool) (map[project.ProjectKey]*project.ProjectState, project.ProjectKeys, error) {
+ git := gitutil.New(jirix.NewSeq())
+ branch, err := git.CurrentBranchName()
+ if err != nil {
+ return nil, nil, err
+ }
+ states, err := project.GetProjectStates(jirix, false)
+ if err != nil {
+ return nil, nil, err
+ }
+ uncommitted := []string{}
+ var keys project.ProjectKeys
+ for _, s := range states {
+ if s.CurrentBranch == branch {
+ key := s.Project.Key()
+ fullState, err := project.GetProjectState(jirix, key, true)
+ if err != nil {
+ return nil, nil, err
+ }
+ if !allowdirty && fullState.HasUncommitted {
+ uncommitted = append(uncommitted, string(key))
+ } else {
+ keys = append(keys, key)
+ }
+ }
+ }
+ if len(uncommitted) > 0 {
+ return nil, nil, fmt.Errorf("the following projects have uncommitted changes: %s", strings.Join(uncommitted, ", "))
+ }
+ members := map[project.ProjectKey]*project.ProjectState{}
+ for _, key := range keys {
+ members[key] = states[key]
+ }
+ if len(members) == 0 {
+ return nil, nil, nil
+ }
+ sort.Sort(keys)
+ return members, keys, nil
+}
+
+func (mp *multiPart) writeMultiPartMetadata(jirix *jiri.X) error {
+ total := len(mp.states)
+ index := 1
+ s := jirix.NewSeq()
+ for _, key := range mp.keys {
+ state := mp.states[key]
+ filename := filepath.Join(state.Project.Path, jiri.ProjectMetaDir, multiPartMetaDataFileName)
+ if total < 2 {
+ os.Remove(filename)
+ continue
+ }
+ msg := fmt.Sprintf("MultiPart: %d/%d\n", index, total)
+ if err := s.WriteFile(filename, []byte(msg), os.FileMode(0644)).Done(); err != nil {
+ return err
+ }
+ index++
+ }
+ return nil
+}
+
+func (mp *multiPart) cleanMultiPartMetadata(jirix *jiri.X) error {
+ s := jirix.NewSeq()
+ for _, state := range mp.states {
+ filename := filepath.Join(state.Project.Path, jiri.ProjectMetaDir, multiPartMetaDataFileName)
+ ok, err := s.IsFile(filename)
+ if err != nil {
+ return err
+ }
+ if ok {
+ if err := s.Remove(filename).Done(); err != nil {
+ return err
+ }
+ }
+ }
+ return nil
+}
+
+func (mp *multiPart) commandline(excludeKey project.ProjectKey, args []string) []string {
+ keyflag := "--projects="
+ for _, k := range mp.keys {
+ if k == excludeKey {
+ continue
+ }
+ keyflag += string(k) + ","
+ }
+ keyflag = strings.TrimSuffix(keyflag, ",")
+ clargs := []string{
+ "runp",
+ "--interactive",
+ keyflag,
+ "jiri",
+ "cl",
+ "mail",
+ "--current-project-only=true",
+ }
+ return append(clargs, args...)
+}
+
+// runCLMail is a wrapper that sets up and runs a review instance across
+// multiple projects.
+func runCLMail(jirix *jiri.X, args []string) error {
+ mp, err := initForMultiPart(jirix)
+ if err != nil {
+ return err
+ }
+ if mp.clean {
+ if err := mp.cleanMultiPartMetadata(jirix); err != nil {
+ return err
+ }
+ return nil
+ }
+ if mp.current {
+ return runCLMailCurrent(jirix, []string{})
+ }
+ // multipart mode
+ if err := mp.writeMultiPartMetadata(jirix); err != nil {
+ mp.cleanMultiPartMetadata(jirix)
+ return err
+ }
+ if err := runCLMailCurrent(jirix, []string{}); err != nil {
+ return err
+ }
+ git := gitutil.New(jirix.NewSeq())
+ branch, err := git.CurrentBranchName()
+ if err != nil {
+ return err
+ }
+ initialMessage, err := strippedGerritCommitMessage(jirix, branch)
+ if err != nil {
+ return err
+ }
+ s := jirix.NewSeq()
+ tmp, err := s.TempFile("", branch+"-")
+ if err != nil {
+ return err
+ }
+ defer func() {
+ tmp.Close()
+ os.Remove(tmp.Name())
+ }()
+ if _, err := io.WriteString(tmp, initialMessage); err != nil {
+ return err
+ }
+ // Use Capture to make sure that all output from the subcommands is
+ // sent to stdout/stderr.
+ return s.Capture(jirix.Stdout(), jirix.Stderr()).Last("jiri", mp.commandline(mp.currentKey, append([]string{"--edit=false", "--commit-message-body-file=" + tmp.Name()}, args...))...)
+
+}
+
+func runCLMailCurrent(jirix *jiri.X, _ []string) error {
// Check that working dir exist on remote branch. Otherwise checking out
// remote branch will break the users working dir.
git := gitutil.New(jirix.NewSeq())
@@ -385,7 +597,7 @@
strings.Join(gerrit.PresubmitTestTypes(), ","))
}
- p, err := currentProject(jirix, "jiri cl mail")
+ p, err := currentProject(jirix)
if err != nil {
return err
}
@@ -409,7 +621,7 @@
gerritRemote.Path = projectRemoteUrl.Path
// Create and run the review.
- review, err := newReview(jirix, gerrit.CLOpts{
+ review, err := newReview(jirix, p, gerrit.CLOpts{
Autosubmit: autosubmitFlag,
Ccs: parseEmails(ccsFlag),
Draft: draftFlag,
@@ -429,7 +641,13 @@
} else if !confirmed {
return nil
}
- return review.run()
+ err = review.run()
+ // Ignore the error that is returned when there are no differences
+ // between the local and gerrit branches.
+ if err != nil && noChangesRE.MatchString(err.Error()) {
+ return nil
+ }
+ return err
}
// parseEmails input a list of comma separated tokens and outputs a
@@ -488,10 +706,11 @@
type review struct {
jirix *jiri.X
reviewBranch string
+ project project.Project
gerrit.CLOpts
}
-func newReview(jirix *jiri.X, opts gerrit.CLOpts) (*review, error) {
+func newReview(jirix *jiri.X, project project.Project, opts gerrit.CLOpts) (*review, error) {
// Sync all CLs in the sequence of dependent CLs ending in the
// current branch.
if err := syncCL(jirix); err != nil {
@@ -524,6 +743,7 @@
}
return &review{
jirix: jirix,
+ project: project,
reviewBranch: branch + "-REVIEW",
CLOpts: opts,
}, nil
@@ -750,18 +970,59 @@
return nil
}
-// defaultCommitMessage creates the default commit message from the
-// list of commits on the branch.
-func (review *review) defaultCommitMessage() (string, error) {
- commitMessages, err := gitutil.New(review.jirix.NewSeq()).CommitMessages(review.CLOpts.Branch, review.reviewBranch)
+func (review *review) readMultiPart() string {
+ s := review.jirix.NewSeq()
+ filename := filepath.Join(review.project.Path, jiri.ProjectMetaDir, multiPartMetaDataFileName)
+ mpart, err := s.ReadFile(filename)
+ if err != nil {
+ return ""
+ }
+ return strings.TrimSpace(string(mpart))
+}
+
+// strippedGerritCommitMessage returns the commit message stripped of variable
+// meta-data such as multipart messages, or change IDs:
+func strippedGerritCommitMessage(jirix *jiri.X, branch string) (string, error) {
+ filename, err := getCommitMessageFileName(jirix, branch)
if err != nil {
return "", err
}
+ msg, err := jirix.NewSeq().ReadFile(filename)
+ if err != nil {
+ return "", err
+ }
+ // Strip "MultiPart ..." from the commit messages.
+ strippedMessages := multiPartRE.ReplaceAllLiteralString(string(msg), "")
// Strip "Change-Id: ..." from the commit messages.
- strippedMessages := changeIDRE.ReplaceAllLiteralString(commitMessages, "")
+ strippedMessages = changeIDRE.ReplaceAllLiteralString(strippedMessages, "")
+ return strippedMessages, nil
+}
+
+// defaultCommitMessage creates the default commit message from the
+// list of commits on the branch.
+func (review *review) defaultCommitMessage() (string, error) {
+ commitMessages := ""
+ var err error
+ if commitMessageBodyFlag != "" {
+ msg, tmpErr := ioutil.ReadFile(commitMessageBodyFlag)
+ commitMessages = string(msg)
+ err = tmpErr
+ } else {
+ commitMessages, err = gitutil.New(review.jirix.NewSeq()).CommitMessages(review.CLOpts.Branch, review.reviewBranch)
+ }
+ if err != nil {
+ return "", err
+ }
+ // Strip "MultiPart ..." from the commit messages.
+ strippedMessages := multiPartRE.ReplaceAllLiteralString(commitMessages, "")
+ // Strip "Change-Id: ..." from the commit messages.
+ strippedMessages = changeIDRE.ReplaceAllLiteralString(strippedMessages, "")
// Add comment markers (#) to every line.
commentedMessages := "# " + strings.Replace(strippedMessages, "\n", "\n# ", -1)
message := defaultMessageHeader + commentedMessages
+ if multipart := review.readMultiPart(); multipart != "" {
+ message = message + "\n" + multipart + "\n"
+ }
return message, nil
}
@@ -779,15 +1040,24 @@
return nil
}
-// processLabels adds/removes labels for the given commit message.
-func (review *review) processLabels(message string) string {
- // Find the Change-ID line.
+// processLabelsAndCommitFile adds/removes labels for the given commit
+// message and merges in the contents of the initial-message-file.
+func (review *review) processLabelsAndCommitFile(message string) string {
+ // Find the Change-ID and MultiPart lines.
changeIDLine := changeIDRE.FindString(message)
+ multiPartLine := multiPartRE.FindString(message)
+
+ if commitMessageBodyFlag != "" {
+ if msg, err := ioutil.ReadFile(commitMessageBodyFlag); err == nil {
+ message = string(msg)
+ }
+ }
// Strip existing labels and change-ID.
message = autosubmitLabelRE.ReplaceAllLiteralString(message, "")
message = presubmitTestLabelRE.ReplaceAllLiteralString(message, "")
message = changeIDRE.ReplaceAllLiteralString(message, "")
+ message = multiPartRE.ReplaceAllLiteralString(message, "")
// Insert labels and change-ID back.
if review.CLOpts.Autosubmit {
@@ -796,11 +1066,21 @@
if review.CLOpts.Presubmit != gerrit.PresubmitTestTypeAll {
message += fmt.Sprintf("PresubmitTest: %s\n", review.CLOpts.Presubmit)
}
+ if multiPartLine != "" && !strings.HasSuffix(message, "\n") {
+ message += "\n"
+ } else {
+ if multipart := review.readMultiPart(); multipart != "" {
+ if !strings.HasSuffix(message, "\n") {
+ message += "\n"
+ }
+ multiPartLine = multipart
+ }
+ }
+ message += multiPartLine
if changeIDLine != "" && !strings.HasSuffix(message, "\n") {
message += "\n"
}
message += changeIDLine
-
return message
}
@@ -838,10 +1118,12 @@
return err
}
defer collect.Error(func() error { return review.jirix.NewSeq().Chdir(wd).Done() }, &e)
+
file, err := getCommitMessageFileName(review.jirix, review.CLOpts.Branch)
if err != nil {
return err
}
+
message := messageFlag
if message == "" {
// Message was not passed in flag. Attempt to read it from file.
@@ -863,7 +1145,7 @@
// message is edited by users, which happens in the
// updateReviewMessage method.
if message != "" {
- message = review.processLabels(message)
+ message = review.processLabelsAndCommitFile(message)
}
if err := review.createReviewBranch(message); err != nil {
return err
@@ -893,7 +1175,7 @@
return nil
}
-// getChangeID reads the commit message and extracts the change-Id.
+// getChangeID reads the commit message and extracts the change-Id
func (review *review) getChangeID() (string, error) {
file, err := getCommitMessageFileName(review.jirix, review.CLOpts.Branch)
if err != nil {
@@ -922,7 +1204,7 @@
return fmt.Errorf("Cannot set topic for gerrit host %q. Please use a host url with 'https' scheme or run with '--set-topic=false'.", host.String())
}
if err := review.jirix.Gerrit(host).SetTopic(changeID, review.CLOpts); err != nil {
- return err
+ return fmt.Errorf("failed to set topic for %v, %#v: %v", changeID, review.CLOpts, err)
}
return nil
}
@@ -937,6 +1219,9 @@
if err != nil {
return err
}
+ // update MultiPart metadata.
+ mpart := review.readMultiPart()
+ newMessage = multiPartRE.ReplaceAllLiteralString(newMessage, mpart)
s := review.jirix.NewSeq()
// For the initial commit where the commit message file doesn't exist,
// add/remove labels after users finish editing the commit message.
@@ -945,7 +1230,7 @@
// initial commit so we don't confuse users.
if _, err := s.Stat(file); err != nil {
if runutil.IsNotExist(err) {
- newMessage = review.processLabels(newMessage)
+ newMessage = review.processLabelsAndCommitFile(newMessage)
if err := git.CommitAmendWithMessage(newMessage); err != nil {
return err
}
diff --git a/cmd/jiri/cl_test.go b/cmd/jiri/cl_test.go
index 680f0ff..86da915 100644
--- a/cmd/jiri/cl_test.go
+++ b/cmd/jiri/cl_test.go
@@ -6,9 +6,12 @@
import (
"bytes"
+ "fmt"
+ "io/ioutil"
"os"
"path"
"path/filepath"
+ "reflect"
"runtime"
"strings"
"testing"
@@ -17,6 +20,7 @@
"v.io/jiri/gerrit"
"v.io/jiri/gitutil"
"v.io/jiri/jiritest"
+ "v.io/jiri/project"
"v.io/jiri/runutil"
)
@@ -155,10 +159,13 @@
return repoPath
}
-// Simple commit-msg hook that adds a fake Change Id.
+// Simple commit-msg hook that removes any existing Change-Id and adds a
+// fake one.
var commitMsgHook string = `#!/bin/sh
MSG="$1"
-echo "Change-Id: I0000000000000000000000000000000000000000" >> $MSG
+cat $MSG | sed -e "/Change-Id/d" > $MSG.tmp
+echo "Change-Id: I0000000000000000000000000000000000000000" >> $MSG.tmp
+mv $MSG.tmp $MSG
`
// installCommitMsgHook links the gerrit commit-msg hook into a different repo.
@@ -314,7 +321,7 @@
}
files := []string{"file1", "file2", "file3"}
commitFiles(t, fake.X, files)
- review, err := newReview(fake.X, gerrit.CLOpts{})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{})
if err != nil {
t.Fatalf("%v", err)
}
@@ -346,7 +353,7 @@
if err := gitutil.New(fake.X.NewSeq()).CreateAndCheckoutBranch(branch); err != nil {
t.Fatalf("%v", err)
}
- review, err := newReview(fake.X, gerrit.CLOpts{Remote: branch})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: branch})
if err != nil {
t.Fatalf("%v", err)
}
@@ -372,7 +379,7 @@
commitFiles(t, fake.X, files)
{
// Test with draft = false, no reviewiers, and no ccs.
- review, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritPath})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritPath})
if err != nil {
t.Fatalf("%v", err)
}
@@ -384,7 +391,7 @@
}
{
// Test with draft = true, no reviewers, and no ccs.
- review, err := newReview(fake.X, gerrit.CLOpts{
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{
Draft: true,
Remote: gerritPath,
})
@@ -399,7 +406,7 @@
}
{
// Test with draft = false, reviewers, and no ccs.
- review, err := newReview(fake.X, gerrit.CLOpts{
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{
Remote: gerritPath,
Reviewers: parseEmails("reviewer1,reviewer2@example.org"),
})
@@ -414,7 +421,7 @@
}
{
// Test with draft = true, reviewers, and ccs.
- review, err := newReview(fake.X, gerrit.CLOpts{
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{
Ccs: parseEmails("cc1@example.org,cc2"),
Draft: true,
Remote: gerritPath,
@@ -442,7 +449,7 @@
t.Fatalf("%v", err)
}
commitFiles(t, fake.X, []string{"file1"})
- review, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritPath})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritPath})
if err != nil {
t.Fatalf("%v", err)
}
@@ -465,7 +472,7 @@
}
files := []string{"file1", "file2", "file3"}
commitFiles(t, fake.X, files)
- review, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritPath})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritPath})
if err != nil {
t.Fatalf("%v", err)
}
@@ -503,7 +510,7 @@
// Test setting -presubmit=none and autosubmit.
files := []string{"file1", "file2", "file3"}
commitFiles(t, fake.X, files)
- review, err := newReview(fake.X, gerrit.CLOpts{
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{
Autosubmit: true,
Presubmit: gerrit.PresubmitTestTypeNone,
Remote: gerritPath,
@@ -546,7 +553,7 @@
}
// Test setting -presubmit=all but keep autosubmit=true.
- review, err = newReview(fake.X, gerrit.CLOpts{
+ review, err = newReview(fake.X, project.Project{}, gerrit.CLOpts{
Autosubmit: true,
Remote: gerritPath,
Reviewers: parseEmails("run2"),
@@ -575,7 +582,7 @@
}
// Test setting autosubmit=false.
- review, err = newReview(fake.X, gerrit.CLOpts{
+ review, err = newReview(fake.X, project.Project{}, gerrit.CLOpts{
Remote: gerritPath,
Reviewers: parseEmails("run3"),
})
@@ -638,7 +645,7 @@
if err := s.WriteFile(untrackedFile, []byte(untrackedFileContent), 0644).Done(); err != nil {
t.Fatalf("WriteFile(%v, %v) failed: %v", untrackedFile, untrackedFileContent, err)
}
- review, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritPath})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritPath})
if err != nil {
t.Fatalf("%v", err)
}
@@ -684,7 +691,7 @@
files := []string{path.Join(subdir, "file1")}
commitFiles(t, fake.X, files)
chdir(t, fake.X, subdir)
- review, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritPath})
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritPath})
if err != nil {
t.Fatalf("%v", err)
}
@@ -787,14 +794,14 @@
},
}
for _, test := range testCases {
- review, err := newReview(fake.X, gerrit.CLOpts{
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{
Autosubmit: test.autosubmit,
Presubmit: test.presubmitType,
})
if err != nil {
t.Fatalf("%v", err)
}
- if got := review.processLabels(test.originalMessage); got != test.expectedMessage {
+ if got := review.processLabelsAndCommitFile(test.originalMessage); got != test.expectedMessage {
t.Fatalf("want %s, got %s", test.expectedMessage, got)
}
}
@@ -868,7 +875,7 @@
if err := gitutil.New(fake.X.NewSeq()).CommitWithMessage("editing stuff"); err != nil {
t.Fatalf("git commit failed: %v", err)
}
- review, err := newReview(fake.X, gerrit.CLOpts{
+ review, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{
Remote: gerritPath,
Reviewers: parseEmails("run1"), // See hack note about TestLabelsInCommitMessage
})
@@ -889,7 +896,7 @@
if err := gitutil.New(fake.X.NewSeq()).CommitWithMessage("deleting stuff"); err != nil {
t.Fatalf("git commit failed: %v", err)
}
- review, err = newReview(fake.X, gerrit.CLOpts{
+ review, err = newReview(fake.X, project.Project{}, gerrit.CLOpts{
Remote: gerritPath,
Reviewers: parseEmails("run2"),
})
@@ -928,7 +935,7 @@
createCLWithFiles(t, fake.X, "feature1-B", "B")
commitFile(t, fake.X, "A", "Don't tread on me.")
- reviewB, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritBPath})
+ reviewB, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritBPath})
if err != nil {
t.Fatalf("%v", err)
}
@@ -949,7 +956,7 @@
t.Fatalf("%v", err)
}
- reviewA, err := newReview(fake.X, gerrit.CLOpts{Remote: gerritAPath})
+ reviewA, err := newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritAPath})
if err == nil {
t.Fatalf("creating a review did not fail when it should")
}
@@ -979,7 +986,7 @@
}
// Retry review.
- reviewA, err = newReview(fake.X, gerrit.CLOpts{Remote: gerritAPath})
+ reviewA, err = newReview(fake.X, project.Project{}, gerrit.CLOpts{Remote: gerritAPath})
if err != nil {
t.Fatalf("review failed: %v", err)
}
@@ -1031,3 +1038,277 @@
assertFilesExist(t, fake.X, []string{"test"})
}
}
+
+func TestMultiPart(t *testing.T) {
+ fake, cleanup := jiritest.NewFakeJiriRoot(t)
+ defer cleanup()
+ projects := addProjects(t, fake)
+
+ origCleanupFlag, origCurrentProjectFlag := cleanupMultiPartFlag, currentProjectFlag
+ defer func() {
+ cleanupMultiPartFlag, currentProjectFlag = origCleanupFlag, origCurrentProjectFlag
+ }()
+ cleanupMultiPartFlag, currentProjectFlag = false, false
+
+ cwd, err := os.Getwd()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer os.Chdir(cwd)
+
+ relchdir := func(dir string) {
+ chdir(t, fake.X, dir)
+ }
+
+ initMP := func() *multiPart {
+ mp, err := initForMultiPart(fake.X)
+ if err != nil {
+ _, file, line, _ := runtime.Caller(1)
+ t.Fatalf("%s:%d: %v", filepath.Base(file), line, err)
+ }
+ return mp
+ }
+
+ wr := func(mp *multiPart) *multiPart {
+ return mp
+ }
+
+ cleanupMultiPartFlag = true
+ if got, want := initMP(), wr(&multiPart{clean: true}); !reflect.DeepEqual(got, want) {
+ t.Errorf("got %#v, want %#v", got, want)
+ }
+
+ currentProjectFlag = true
+ if got, want := initMP(), wr(&multiPart{clean: true, current: true}); !reflect.DeepEqual(got, want) {
+ t.Errorf("got %#v, want %#v", got, want)
+ }
+ cleanupMultiPartFlag, currentProjectFlag = false, false
+
+ git := func(dir string) *gitutil.Git {
+ return gitutil.New(fake.X.NewSeq(), gitutil.RootDirOpt(dir))
+ }
+
+ // Test metadata generation.
+ ra := projects[0].Path
+ rb := projects[1].Path
+ rc := projects[2].Path
+ t1 := projects[3].Path
+ git(ra).CreateAndCheckoutBranch("a1")
+ relchdir(ra)
+
+ if got, want := initMP(), wr(&multiPart{current: true, currentKey: projects[0].Key()}); !reflect.DeepEqual(got, want) {
+ t.Errorf("got %#v, want %#v", got, want)
+ }
+
+ git(rb).CreateAndCheckoutBranch("a1")
+ mp := initMP()
+ if mp.current != false || mp.clean != false {
+ t.Errorf("current or clean not false: %v, %v", mp.current, mp.clean)
+ }
+ if got, want := len(mp.keys), 2; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ tmp := &multiPart{
+ keys: project.ProjectKeys{projects[0].Key(), projects[1].Key()},
+ }
+ for i, k := range mp.keys {
+ if got, want := k, tmp.keys[i]; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ }
+ if got, want := len(mp.states), 2; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+
+ git(rc).CreateAndCheckoutBranch("a1")
+ git(t1).CreateAndCheckoutBranch("a2")
+ mp = initMP()
+ if got, want := len(mp.keys), 3; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if err := mp.writeMultiPartMetadata(fake.X); err != nil {
+ t.Fatal(err)
+ }
+
+ hasMetaData := func(total int, projectPaths ...string) {
+ for i, dir := range projectPaths {
+ filename := filepath.Join(dir, jiri.ProjectMetaDir, multiPartMetaDataFileName)
+ msg, err := ioutil.ReadFile(filename)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got, want := string(msg), fmt.Sprintf("MultiPart: %d/%d\n", i+1, total); got != want {
+ t.Errorf("%v: got %v, want %v", dir, got, want)
+ }
+ }
+ }
+
+ hasNoMetaData := func(projectPaths ...string) {
+ for _, dir := range projectPaths {
+ filename := filepath.Join(fake.X.Root, dir, jiri.ProjectMetaDir, multiPartMetaDataFileName)
+ _, err := os.Stat(filename)
+ if !os.IsNotExist(err) {
+ t.Fatalf("%s should not exist", filename)
+ }
+ }
+ }
+
+ newFile := func(dir, file string) {
+ testfile := filepath.Join(dir, file)
+ _, err := fake.X.NewSeq().Create(testfile)
+ if err != nil {
+ t.Errorf("failed to create %s: %v", testfile, err)
+ }
+ }
+
+ hasMetaData(len(mp.keys), ra, rb, rc)
+ hasNoMetaData(t1)
+ if err := mp.cleanMultiPartMetadata(fake.X); err != nil {
+ t.Fatal(err)
+ }
+ hasNoMetaData(ra, rb, rc, t1)
+
+ // Test CL messages.
+
+ for _, p := range projects {
+ // Install commit hook so that Change-Id is written.
+ installCommitMsgHook(t, fake.X, p.Path)
+
+ }
+
+ // Create a fake jiri root for the fake gerrit repos.
+ gerritFake, gerritCleanup := jiritest.NewFakeJiriRoot(t)
+ defer gerritCleanup()
+
+ relchdir(ra)
+
+ if err := mp.writeMultiPartMetadata(fake.X); err != nil {
+ t.Fatal(err)
+ }
+ hasMetaData(len(mp.keys), ra, rb, rc)
+
+ gitAddFiles := func(name string, repos ...string) {
+ for _, dir := range repos {
+ newFile(dir, name)
+ if err := git(dir).Add(name); err != nil {
+ t.Error(err)
+ }
+ }
+ }
+
+ gitCommit := func(msg string, repos ...string) {
+ for _, dir := range repos {
+ committer := git(dir).NewCommitter(false)
+ if err := committer.Commit(msg); err != nil {
+ t.Error(err)
+ }
+ }
+ }
+
+ gitAddFiles("new-file", ra, rb, rc)
+ _, err = initForMultiPart(fake.X)
+ if err == nil || !strings.Contains(err.Error(), "uncommitted changes:") {
+ t.Fatalf("expected an error about uncommitted changes: got %v", err)
+ }
+
+ gitCommit("oh multipart test\n", ra, rb, rc)
+ bodyMessage := "xyz\n\na simple message\n"
+ messageFile := filepath.Join(fake.X.Root, jiri.RootMetaDir, "message-body")
+ if err := ioutil.WriteFile(messageFile, []byte(bodyMessage), 0666); err != nil {
+ t.Fatal(err)
+ }
+
+ mp = initMP()
+ setTopicFlag = false
+ commitMessageBodyFlag = messageFile
+
+ testCommitMsgs := func(branch string, cls ...*project.Project) {
+ _, file, line, _ := runtime.Caller(1)
+ loc := fmt.Sprintf("%s:%d", filepath.Base(file), line)
+
+ total := len(cls)
+ for index, p := range cls {
+ // Create a new gerrit repo each time we commit, since we can't
+ // push more than once to the fake gerrit repo without actually
+ // running gerrit.
+ gp := createRepoFromOrigin(t, gerritFake.X, "gerrit", p.Remote)
+ defer os.Remove(gp)
+ relchdir(p.Path)
+ review, err := newReview(fake.X, *p, gerrit.CLOpts{
+ Presubmit: gerrit.PresubmitTestTypeNone,
+ Remote: gp,
+ })
+ if err != nil {
+ t.Fatalf("%v: %v: %v", loc, p.Path, err)
+ }
+ // use the default commit message
+ if err := review.run(); err != nil {
+ t.Fatalf("%v: %v, %v", loc, p.Path, err)
+ }
+ filename, err := getCommitMessageFileName(fake.X, branch)
+ if err != nil {
+ t.Fatalf("%v: %v", loc, err)
+ }
+ msg, err := ioutil.ReadFile(filename)
+ if err != nil {
+ t.Fatalf("%v: %v", loc, err)
+ }
+ if total < 2 {
+ if strings.Contains(string(msg), "MultiPart") {
+ t.Errorf("%v: commit message contains MultiPart when it should not: %v", loc, string(msg))
+ }
+ continue
+ }
+ expected := fmt.Sprintf("\nMultiPart: %d/%d\n", index+1, total)
+ if !strings.Contains(string(msg), expected) {
+ t.Errorf("%v: commit message for %v does not contain %v: %v", loc, p.Path, expected, string(msg))
+ }
+ if got, want := string(msg), bodyMessage+"PresubmitTest: none"+expected+"Change-Id: I0000000000000000000000000000000000000000"; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ }
+ }
+
+ testCommitMsgs("a1", projects[0], projects[1], projects[2])
+ return
+
+ cl := mp.commandline("", []string{"jiri", "cl", "mail"})
+ expected := []string{
+ "runp",
+ "--interactive",
+ "--show-key-prefix",
+ "--projects=" + string(projects[0].Key()) + "," + string(projects[1].Key()) + "," + string(projects[2].Key()),
+ "--current-project-only",
+ "jiri",
+ "cl",
+ "mail",
+ }
+ if got, want := strings.Join(cl, " "), strings.Join(expected, " "); got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ cl = mp.commandline(projects[0].Key(), []string{"jiri", "cl", "mail"})
+ expected[3] = "--projects=" + string(projects[1].Key()) + "," + string(projects[2].Key())
+ if got, want := strings.Join(cl, " "), strings.Join(expected, " "); got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+
+ git(rb).CreateAndCheckoutBranch("a2")
+ gitAddFiles("new-file1", ra, rc)
+ gitCommit("oh multipart test: 2\n", ra, rc)
+
+ mp = initMP()
+ if err := mp.writeMultiPartMetadata(fake.X); err != nil {
+ t.Fatal(err)
+ }
+ hasMetaData(len(mp.keys), ra, rc)
+ testCommitMsgs("a1", projects[0], projects[2])
+
+ git(ra).CreateAndCheckoutBranch("a2")
+
+ mp = initMP()
+ if err := mp.writeMultiPartMetadata(fake.X); err != nil {
+ t.Fatal(err)
+ }
+ hasNoMetaData(rc)
+ testCommitMsgs("a1", projects[2])
+}
diff --git a/cmd/jiri/doc.go b/cmd/jiri/doc.go
index 66c86db..87c2702 100644
--- a/cmd/jiri/doc.go
+++ b/cmd/jiri/doc.go
@@ -12,7 +12,7 @@
jiri [flags] <command>
The jiri commands are:
- cl Manage project changelists
+ cl Manage changelists for multiple projects
import Adds imports to .jiri_manifest file
profile Display information about installed profiles
project Manage the jiri projects
@@ -39,9 +39,9 @@
-time=false
Dump timing information to stderr before exiting the program.
-Jiri cl - Manage project changelists
+Jiri cl - Manage changelists for multiple projects
-Manage project changelists.
+Manage changelists for multiple projects.
Usage:
jiri cl [flags] <command>
@@ -100,6 +100,14 @@
Comma-seperated list of emails or LDAPs to cc.
-check-uncommitted=true
Check that no uncommitted changes exist.
+ -clean-multipart-metadata=false
+ Cleanup the metadata associated with multipart CLs pertaining the MultiPart:
+ x/y message without mailing any CLs.
+ -commit-message-body-file=
+ file containing the body of the CL description, that is, text without a
+ ChangeID, MultiPart etc.
+ -current-project-only=false
+ Run mail in the current project only.
-d=false
Send a draft changelist.
-edit=true
diff --git a/cmd/jiri/runp.go b/cmd/jiri/runp.go
index 8d0eb2b..9708723 100644
--- a/cmd/jiri/runp.go
+++ b/cmd/jiri/runp.go
@@ -312,7 +312,12 @@
var err error
if profilescmdline.IsFlagSet(cmd.ParsedFlags, "projects") {
- keysRE, err = regexp.Compile(runpFlags.projectKeys)
+ re := ""
+ for _, pre := range strings.Split(runpFlags.projectKeys, ",") {
+ re += pre + "|"
+ }
+ re = strings.TrimRight(re, "|")
+ keysRE, err = regexp.Compile(re)
if err != nil {
return fmt.Errorf("failed to compile projects regexp: %q: %v", runpFlags.projectKeys, err)
}
@@ -327,6 +332,9 @@
for _, f := range []string{"show-key-prefix", "show-name-prefix"} {
if profilescmdline.IsFlagSet(cmd.ParsedFlags, f) {
+ if runpFlags.interactive && profilescmdline.IsFlagSet(cmd.ParsedFlags, "interactive") {
+ fmt.Fprintf(jirix.Stderr(), "WARNING: interactive mode being disabled because %s was set\n", f)
+ }
runpFlags.interactive = false
runpFlags.collateOutput = true
break
diff --git a/cmd/jiri/runp_test.go b/cmd/jiri/runp_test.go
index 325bbc8..c1bd932 100644
--- a/cmd/jiri/runp_test.go
+++ b/cmd/jiri/runp_test.go
@@ -48,7 +48,7 @@
}
p := project.Project{
Name: projectPath,
- Path: projectPath,
+ Path: filepath.Join(fake.X.Root, projectPath),
Remote: fake.Projects[projectPath],
RemoteBranch: "master",
}
@@ -97,7 +97,7 @@
defer os.Chdir(cwd)
chdir := func(dir string) {
- if err := os.Chdir(filepath.Join(fake.X.Root, dir)); err != nil {
+ if err := os.Chdir(dir); err != nil {
t.Fatal(err)
}
}
@@ -157,15 +157,15 @@
s := fake.X.NewSeq()
newfile := func(dir, file string) {
- testfile := filepath.Join(fake.X.Root, dir, file)
+ testfile := filepath.Join(dir, file)
_, err := s.Create(testfile)
if err != nil {
t.Errorf("failed to create %s: %v", testfile, err)
}
}
- git := func(root, dir string) *gitutil.Git {
- return gitutil.New(fake.X.NewSeq(), gitutil.RootDirOpt(filepath.Join(fake.X.Root, dir)))
+ git := func(dir string) *gitutil.Git {
+ return gitutil.New(fake.X.NewSeq(), gitutil.RootDirOpt(dir))
}
newfile(rb, "untracked.go")
@@ -182,7 +182,7 @@
newfile(rc, "uncommitted.go")
- if err := git(fake.X.Root, rc).Add("uncommitted.go"); err != nil {
+ if err := git(rc).Add("uncommitted.go"); err != nil {
t.Error(err)
}
@@ -208,10 +208,10 @@
t.Errorf("got %v, want %v", got, want)
}
- git(fake.X.Root, rb).CreateAndCheckoutBranch("a1")
- git(fake.X.Root, rb).CreateAndCheckoutBranch("b2")
- git(fake.X.Root, rc).CreateAndCheckoutBranch("b2")
- git(fake.X.Root, t1).CreateAndCheckoutBranch("a1")
+ git(rb).CreateAndCheckoutBranch("a1")
+ git(rb).CreateAndCheckoutBranch("b2")
+ git(rc).CreateAndCheckoutBranch("b2")
+ git(t1).CreateAndCheckoutBranch("a1")
chdir(rc)
@@ -227,13 +227,13 @@
t.Errorf("got %v, want %v", got, want)
}
- if err := s.MkdirAll(filepath.Join(fake.X.Root, rb, ".jiri", "a1"), os.FileMode(0755)).Done(); err != nil {
+ if err := s.MkdirAll(filepath.Join(rb, ".jiri", "a1"), os.FileMode(0755)).Done(); err != nil {
t.Fatal(err)
}
newfile(rb, filepath.Join(".jiri", "a1", ".gerrit_commit_message"))
- git(fake.X.Root, rb).CheckoutBranch("a1")
- git(fake.X.Root, t1).CheckoutBranch("a1")
+ git(rb).CheckoutBranch("a1")
+ git(t1).CheckoutBranch("a1")
chdir(t1)
got = run(sh, dir, "jiri", "runp", "--has-gerrit-message", "--show-name-prefix", "echo")
