More work on create branch command
Added logic to check that a branch from a particular version
has not already been created. Also a bunch of small miscellaneous stuff.
TEST=run_tests.sh
BUG=chromium:980346
Change-Id: Ie948c2c5633eafe5a9ab619a2f1a56017dcd977d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/infra/go/+/1690639
Reviewed-by: Sean Abraham <seanabraham@chromium.org>
Reviewed-by: Evan Hernandez <evanhernandez@chromium.org>
Commit-Queue: Jack Neus <jackneus@google.com>
Tested-by: Jack Neus <jackneus@google.com>
diff --git a/cmd/branch_util/checkout.go b/cmd/branch_util/checkout.go
index e051e60..b050d39 100644
--- a/cmd/branch_util/checkout.go
+++ b/cmd/branch_util/checkout.go
@@ -7,13 +7,18 @@
"fmt"
"log"
"os"
+ "path/filepath"
+ "regexp"
+ "go.chromium.org/chromiumos/infra/go/internal/git"
+ "go.chromium.org/chromiumos/infra/go/internal/repo"
"go.chromium.org/chromiumos/infra/go/internal/repo_util"
)
type CrosCheckout struct {
initialized bool
root string
+ Manifest repo.Manifest
}
func (c *CrosCheckout) Initialize(root, manifestUrl string) error {
@@ -46,6 +51,39 @@
}
log.Printf("Syncing checkout %s to manifest %s.", c.root, path)
repository := &repo_util.Repository{Root: c.root}
- err := repository.SyncToFile(path, RepoToolPath)
+ if !skipSync {
+ if err := repository.SyncToFile(path, RepoToolPath); err != nil {
+ return err
+ }
+ }
+ var err error
+ c.Manifest, err = repository.Manifest(RepoToolPath)
return err
}
+
+func (c *CrosCheckout) ReadVersion() repo.VersionInfo {
+ vinfo, err := repo.GetVersionInfoFromRepo(c.root)
+ if err != nil {
+ return repo.VersionInfo{}
+ }
+ return vinfo
+}
+
+// AbsolutePath joins the path components with the repo root.
+func (c *CrosCheckout) AbsolutePath(args ...string) string {
+ args = append([]string{c.root}, args...)
+ return filepath.Join(args...)
+}
+
+// AbsoluteProjectPath joins the path components with the project's root.
+func (c *CrosCheckout) AbsoluteProjectPath(project repo.Project, args ...string) string {
+ args = append([]string{project.Path}, args...)
+ return c.AbsolutePath(args...)
+}
+
+// BranchExists determines if any branch exists in the specified project
+// that matches the specified pattern.
+func (c *CrosCheckout) BranchExists(project repo.Project, pattern *regexp.Regexp) (bool, error) {
+ matches, err := git.MatchBranchName(c.AbsoluteProjectPath(project), pattern)
+ return len(matches) != 0, err
+}
diff --git a/cmd/branch_util/common.go b/cmd/branch_util/common.go
index ac9b793..3a2c34d 100644
--- a/cmd/branch_util/common.go
+++ b/cmd/branch_util/common.go
@@ -1,3 +1,6 @@
+// Copyright 2019 The Chromium OS 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 main
import (
@@ -5,6 +8,7 @@
"io/ioutil"
"github.com/maruel/subcommands"
+ "go.chromium.org/luci/common/errors"
)
type branchCommand interface {
@@ -54,7 +58,7 @@
env subcommands.Env) int {
ok, errMsg := c.validate(args)
if !ok {
- fmt.Fprintf(a.GetErr(), "%s: %s\n", a.GetName(), errMsg)
+ fmt.Fprintf(a.GetErr(), errMsg+"\n")
return 1
}
@@ -64,7 +68,7 @@
root, err = ioutil.TempDir("", "cros-branch-")
// TODO(jackneus): Delete tmp dir at end.
if err != nil {
- fmt.Fprintf(a.GetErr(), "Error. Tmp root could not be created: %s", err)
+ fmt.Fprintf(a.GetErr(), errors.Annotate(err, "tmp root could not be created").Err().Error()+"\n")
return 1
}
}
diff --git a/cmd/branch_util/create.go b/cmd/branch_util/create.go
index b2df0ec..8089c06 100644
--- a/cmd/branch_util/create.go
+++ b/cmd/branch_util/create.go
@@ -1,9 +1,20 @@
+// Copyright 2019 The Chromium OS 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 main
import (
"fmt"
+ "regexp"
+ "strconv"
+ "strings"
"github.com/maruel/subcommands"
+ "go.chromium.org/luci/common/errors"
+)
+
+var (
+ skipSync bool
)
var cmdCreateBranch = &subcommands.Command{
@@ -49,6 +60,13 @@
"names to determine which versions have already been branched. "+
"Version validation is not possible when the naming convention "+
"is broken. Use this at your own risk.")
+ // Dev flags
+ c.Flags.BoolVar(&skipSync, "skip_sync",
+ false,
+ "Used for development purposes. Assumes that you have a properly synced "+
+ "repo at the specified root and does not call `repo sync`. Do not use this "+
+ "unless you know what you're doing! The tool will likely break if you misuse "+
+ "this flag.")
return c
},
}
@@ -64,6 +82,7 @@
firmware bool
stabilize bool
custom string
+ skipSync bool
}
func (c *createBranchRun) getBranchType() (string, bool) {
@@ -98,11 +117,11 @@
func (c *createBranchRun) validate(args []string) (bool, string) {
if c.file == "" {
- return false, "Must set --file."
+ return false, "must set --file."
}
_, ok := c.getBranchType()
if !ok {
- return false, "Must select exactly one branch type " +
+ return false, "must select exactly one branch type " +
"(--release, --factory, --firmware, --stabilize, --custom)."
}
if c.descriptor != "" && c.custom != "" {
@@ -111,6 +130,9 @@
if c.version != "" && c.version[len(c.version)-1] != '0' {
return false, "cannot branch version from nonzero patch number."
}
+ if c.skipSync && c.Root == "" {
+ return false, "cannot use --skip_sync without --root."
+ }
return true, ""
}
@@ -139,5 +161,42 @@
return 1
}
+ // Validate the version.
+ // Double check that the checkout has a zero patch number. Otherwise,
+ // we cannot branch from it.
+ vinfo := checkout.ReadVersion()
+ if vinfo.PatchNumber != 0 {
+ fmt.Fprintf(a.GetErr(), "Cannot branch version with nonzero patch number (version %s).",
+ vinfo.VersionString())
+ return 1
+ }
+
+ // Check that we did not already branch from this version.
+ // manifest-internal serves as the sentinel project.
+ manifestInternal, err := checkout.Manifest.GetUniqueProject("chromeos/manifest-internal")
+ if err != nil {
+ fmt.Fprintf(a.GetErr(),
+ errors.Annotate(err, "Could not get chromeos/manifest-internal project.").Err().Error())
+ return 1
+ }
+ var nonzeroVersionComponents []string
+ for _, component := range vinfo.VersionComponents() {
+ if component == 0 {
+ continue
+ }
+ nonzeroVersionComponents = append(nonzeroVersionComponents, strconv.Itoa(component))
+ }
+ majorVersion := strings.Join(nonzeroVersionComponents, `.`)
+ pattern := regexp.MustCompile(fmt.Sprintf(`.*-%s.B$`, majorVersion))
+ branchExists, err := checkout.BranchExists(manifestInternal, pattern)
+ if err != nil {
+ fmt.Fprintf(a.GetErr(), err.Error())
+ return 1
+ }
+ if branchExists && !c.Force {
+ fmt.Fprintf(a.GetErr(), "Already branched %s. Please rerun with --force if you "+
+ "would like to proceed.", vinfo.VersionString())
+ }
+
return 0
}
diff --git a/cmd/branch_util/delete.go b/cmd/branch_util/delete.go
index 6b1e3b8..06461ea 100644
--- a/cmd/branch_util/delete.go
+++ b/cmd/branch_util/delete.go
@@ -1,3 +1,6 @@
+// Copyright 2019 The Chromium OS 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 main
import (
@@ -23,7 +26,7 @@
func (c *deleteBranchRun) validate(args []string) (bool, string) {
if len(args) < 1 {
- return false, "Missing required argument(s)."
+ return false, "missing required argument(s)."
} else {
c.branch_name = args[0]
}
diff --git a/cmd/branch_util/rename.go b/cmd/branch_util/rename.go
index f5ec538..ab6bb9e 100644
--- a/cmd/branch_util/rename.go
+++ b/cmd/branch_util/rename.go
@@ -1,3 +1,6 @@
+// Copyright 2019 The Chromium OS 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 main
import (
@@ -25,7 +28,7 @@
func (c *renameBranchRun) validate(args []string) (bool, string) {
if len(args) < 2 {
- return false, "Missing required argument(s)."
+ return false, "missing required argument(s)."
} else {
c.old = args[0]
c.new = args[1]
diff --git a/internal/repo/manifest_version.go b/internal/repo/manifest_version.go
index 353a72a..5e143a5 100644
--- a/internal/repo/manifest_version.go
+++ b/internal/repo/manifest_version.go
@@ -10,17 +10,15 @@
"path/filepath"
"regexp"
"strconv"
- "strings"
)
-type IncrType string
+type VersionComponent string
const (
- Unspecified IncrType = ""
- ChromeBranch IncrType = "chrome_branch"
- Build IncrType = "build"
- Branch IncrType = "branch"
- Patch IncrType = "patch"
+ ChromeBranch VersionComponent = "CHROME_BRANCH"
+ Build VersionComponent = "CHROMEOS_BUILD"
+ Branch VersionComponent = "CHROMEOS_BRANCH"
+ Patch VersionComponent = "CHROMEOS_PATCH"
)
// This is a var and not a const for testing purposes.
@@ -30,26 +28,25 @@
)
const (
- keyValueRegex string = `%s=(\d+)\s*$`
+ keyValueRegex string = `%s=(\d+)\b`
)
-var (
- chromeBranchRegex *regexp.Regexp = regexp.MustCompile(fmt.Sprintf(keyValueRegex, "CHROME_BRANCH"))
- chromeosBuildRegex *regexp.Regexp = regexp.MustCompile(fmt.Sprintf(keyValueRegex, "CHROMEOS_BUILD"))
- chromeosBranchRegex *regexp.Regexp = regexp.MustCompile(fmt.Sprintf(keyValueRegex, "CHROMEOS_BRANCH"))
- chromeosPatchRegex *regexp.Regexp = regexp.MustCompile(fmt.Sprintf(keyValueRegex, "CHROMEOS_PATCH"))
-)
-
-type VersionInfo struct {
- BuildNumber string
- BranchBuildNumber string
- PatchNumber string
- ChromeBranch string
- VersionFile string
- incrType IncrType
+var chromeosVersionMapping = map[VersionComponent](*regexp.Regexp){
+ ChromeBranch: regexp.MustCompile(fmt.Sprintf(keyValueRegex, ChromeBranch)),
+ Build: regexp.MustCompile(fmt.Sprintf(keyValueRegex, Build)),
+ Branch: regexp.MustCompile(fmt.Sprintf(keyValueRegex, Branch)),
+ Patch: regexp.MustCompile(fmt.Sprintf(keyValueRegex, Patch)),
}
-func GetVersionInfoFromRepo(sourceRepo string, incrType IncrType) (VersionInfo, error) {
+type VersionInfo struct {
+ ChromeBranch int
+ BuildNumber int
+ BranchBuildNumber int
+ PatchNumber int
+ VersionFile string
+}
+
+func GetVersionInfoFromRepo(sourceRepo string) (VersionInfo, error) {
var v VersionInfo
v.VersionFile = filepath.Join(sourceRepo, versionFilePath)
@@ -58,36 +55,29 @@
return VersionInfo{}, fmt.Errorf("could not read version file %s", v.VersionFile)
}
- for _, line := range strings.Split(string(fileData), "\n") {
- if strings.TrimSpace(line) == "" {
+ for field, pattern := range chromeosVersionMapping {
+ if match := findValue(pattern, string(fileData)); match != "" {
+ num, err := strconv.Atoi(match)
+ if err != nil {
+ log.Fatal(fmt.Sprintf("%s value %s could not be converted to integer.", field, match))
+ }
+ switch field {
+ case ChromeBranch:
+ v.ChromeBranch = num
+ case Build:
+ v.BuildNumber = num
+ case Branch:
+ v.BranchBuildNumber = num
+ case Patch:
+ v.PatchNumber = num
+ default:
+ // This should never happen.
+ log.Fatal("Invalid version component.")
+ }
continue
}
+ }
- if match := findValue(chromeBranchRegex, line); match != "" {
- v.ChromeBranch = match
- log.Printf("Set the Chrome branch number to: %s", v.ChromeBranch)
- continue
- }
- if match := findValue(chromeosBuildRegex, line); match != "" {
- v.BuildNumber = match
- log.Printf("Set the Chrome branch number to: %s", v.BuildNumber)
- continue
- }
- if match := findValue(chromeosBranchRegex, line); match != "" {
- v.BranchBuildNumber = match
- log.Printf("Set the Chrome branch number to: %s", v.BranchBuildNumber)
- continue
- }
- if match := findValue(chromeosPatchRegex, line); match != "" {
- v.PatchNumber = match
- log.Printf("Set the Chrome branch number to: %s", v.PatchNumber)
- continue
- }
- }
- if incrType == Unspecified {
- incrType = Build
- }
- v.incrType = incrType
return v, nil
}
@@ -100,21 +90,21 @@
return string(match[1])
}
-func (v *VersionInfo) IncrementVersion() string {
- if v.incrType == ChromeBranch {
- v.ChromeBranch = incrString(v.ChromeBranch)
+func (v *VersionInfo) IncrementVersion(incrType VersionComponent) string {
+ if incrType == ChromeBranch {
+ v.ChromeBranch += 1
}
// Increment build_number for ChromeBranch incrType to avoid
// crbug.com/213075.
- if v.incrType == ChromeBranch || v.incrType == Build {
- v.BuildNumber = incrString(v.BuildNumber)
- v.BranchBuildNumber = "0"
- v.PatchNumber = "0"
- } else if v.incrType == Branch && v.PatchNumber == "0" {
- v.BranchBuildNumber = incrString(v.BranchBuildNumber)
+ if incrType == ChromeBranch || incrType == Build {
+ v.BuildNumber += 1
+ v.BranchBuildNumber = 0
+ v.PatchNumber = 0
+ } else if incrType == Branch && v.PatchNumber == 0 {
+ v.BranchBuildNumber += 1
} else {
- v.PatchNumber = incrString(v.PatchNumber)
+ v.PatchNumber += 1
}
return v.VersionString()
@@ -129,5 +119,9 @@
}
func (v *VersionInfo) VersionString() string {
- return fmt.Sprintf("%s.%s.%s", v.BuildNumber, v.BranchBuildNumber, v.PatchNumber)
+ return fmt.Sprintf("%d.%d.%d", v.BuildNumber, v.BranchBuildNumber, v.PatchNumber)
+}
+
+func (v *VersionInfo) VersionComponents() []int {
+ return []int{v.BuildNumber, v.BranchBuildNumber, v.PatchNumber}
}
diff --git a/internal/repo/manifest_version_test.go b/internal/repo/manifest_version_test.go
index af46da6..0adcaaf 100644
--- a/internal/repo/manifest_version_test.go
+++ b/internal/repo/manifest_version_test.go
@@ -1,3 +1,6 @@
+// Copyright 2019 The Chromium OS 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 repo
import (
@@ -5,7 +8,7 @@
"testing"
)
-func assertVersionEqual(t *testing.T, v VersionInfo, expected []string) {
+func assertVersionEqual(t *testing.T, v VersionInfo, expected []int) {
assert.Equal(t, v.ChromeBranch, expected[0])
assert.Equal(t, v.BuildNumber, expected[1])
assert.Equal(t, v.BranchBuildNumber, expected[2])
@@ -14,57 +17,67 @@
func TestGetVersionInfoFromRepo_success(t *testing.T) {
versionFilePath = "chromeos_version.sh"
- versionInfo, err := GetVersionInfoFromRepo("test_data", Build)
+ versionInfo, err := GetVersionInfoFromRepo("test_data")
assert.NilError(t, err)
- assertVersionEqual(t, versionInfo, []string{"77", "12302", "1", "0"})
+ assertVersionEqual(t, versionInfo, []int{77, 12302, 1, 0})
}
func TestIncrementVersion_ChromeBranch(t *testing.T) {
versionFilePath = "chromeos_version.sh"
- versionInfo, err := GetVersionInfoFromRepo("test_data", ChromeBranch)
- versionInfo.IncrementVersion()
+ versionInfo, err := GetVersionInfoFromRepo("test_data")
+ versionInfo.IncrementVersion(ChromeBranch)
assert.NilError(t, err)
- assertVersionEqual(t, versionInfo, []string{"78", "12303", "0", "0"})
+ assertVersionEqual(t, versionInfo, []int{78, 12303, 0, 0})
}
func TestIncrementVersion_Build(t *testing.T) {
versionFilePath = "chromeos_version.sh"
- versionInfo, err := GetVersionInfoFromRepo("test_data", Build)
- versionInfo.IncrementVersion()
+ versionInfo, err := GetVersionInfoFromRepo("test_data")
+ versionInfo.IncrementVersion(Build)
assert.NilError(t, err)
- assertVersionEqual(t, versionInfo, []string{"77", "12303", "0", "0"})
+ assertVersionEqual(t, versionInfo, []int{77, 12303, 0, 0})
}
func TestIncrementVersion_Branch(t *testing.T) {
versionFilePath = "chromeos_version.sh"
- versionInfo, err := GetVersionInfoFromRepo("test_data", Branch)
- versionInfo.IncrementVersion()
+ versionInfo, err := GetVersionInfoFromRepo("test_data")
+ versionInfo.IncrementVersion(Branch)
assert.NilError(t, err)
- assertVersionEqual(t, versionInfo, []string{"77", "12302", "2", "0"})
+ assertVersionEqual(t, versionInfo, []int{77, 12302, 2, 0})
}
func TestIncrementVersion_Branch_nonzero(t *testing.T) {
versionFilePath = "chromeos_version.sh"
- versionInfo, err := GetVersionInfoFromRepo("test_data", Branch)
- versionInfo.PatchNumber = "1"
- versionInfo.IncrementVersion()
+ versionInfo, err := GetVersionInfoFromRepo("test_data")
+ versionInfo.PatchNumber = 1
+ versionInfo.IncrementVersion(Branch)
assert.NilError(t, err)
- assertVersionEqual(t, versionInfo, []string{"77", "12302", "1", "2"})
+ assertVersionEqual(t, versionInfo, []int{77, 12302, 1, 2})
}
func TestIncrementVersion_Patch(t *testing.T) {
versionFilePath = "chromeos_version.sh"
- versionInfo, err := GetVersionInfoFromRepo("test_data", Patch)
- versionInfo.IncrementVersion()
+ versionInfo, err := GetVersionInfoFromRepo("test_data")
+ versionInfo.IncrementVersion(Patch)
assert.NilError(t, err)
- assertVersionEqual(t, versionInfo, []string{"77", "12302", "1", "1"})
+ assertVersionEqual(t, versionInfo, []int{77, 12302, 1, 1})
}
func TestVersionString(t *testing.T) {
versionFilePath = "chromeos_version.sh"
var v VersionInfo
- v.BuildNumber = "123"
- v.BranchBuildNumber = "1"
- v.PatchNumber = "0"
+ v.BuildNumber = 123
+ v.BranchBuildNumber = 1
+ v.PatchNumber = 0
assert.Equal(t, v.VersionString(), "123.1.0")
}
+
+func TestVersionComponents(t *testing.T) {
+ versionFilePath = "chromeos_version.sh"
+ var v VersionInfo
+ v.BuildNumber = 123
+ v.BranchBuildNumber = 1
+ v.PatchNumber = 0
+ components := []int{123, 1, 0}
+ assert.DeepEqual(t, v.VersionComponents(), components)
+}
