branch_util: Fix up logging
Stdout was being captured in a weird way so informational messages
were being logged to stderr. This change better organizes log
messages.
BUG=b:177903295
TEST=none
Change-Id: I3117e878972683fdd543bc242d6d0cc3deccb743
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/infra/go/+/2698795
Reviewed-by: Julio Hurtado <juahurta@google.com>
Commit-Queue: Jack Neus <jackneus@google.com>
Tested-by: Jack Neus <jackneus@google.com>
diff --git a/cmd/branch_util/cli.go b/cmd/branch_util/cli.go
index 380f658..2452722 100644
--- a/cmd/branch_util/cli.go
+++ b/cmd/branch_util/cli.go
@@ -6,14 +6,15 @@
import (
"context"
"fmt"
+ "log"
+ "os"
+
"github.com/maruel/subcommands"
"go.chromium.org/chromiumos/infra/go/internal/branch"
"go.chromium.org/luci/auth"
"go.chromium.org/luci/auth/client/authcli"
"go.chromium.org/luci/common/api/gerrit"
"go.chromium.org/luci/hardcoded/chromeinfra"
- "log"
- "os"
)
var (
@@ -81,10 +82,6 @@
func Run(c branchCommand, a subcommands.Application, args []string, env subcommands.Env) int {
branch.StdoutLog = a.(*branchApplication).stdoutLog
branch.StderrLog = a.(*branchApplication).stderrLog
- // Set output of standard log in case any packages use it.
- if branch.StdoutLog != nil {
- log.SetOutput(branch.StdoutLog.Writer())
- }
// Validate flags/arguments.
ok, errMsg := c.validate(args)
diff --git a/cmd/branch_util/create.go b/cmd/branch_util/create.go
index bec6f6f..4a865ee 100644
--- a/cmd/branch_util/create.go
+++ b/cmd/branch_util/create.go
@@ -5,6 +5,9 @@
import (
"context"
+ "io/ioutil"
+ "os"
+
"github.com/maruel/subcommands"
"go.chromium.org/chromiumos/infra/go/internal/branch"
mv "go.chromium.org/chromiumos/infra/go/internal/chromeos_version"
@@ -13,8 +16,6 @@
"go.chromium.org/chromiumos/infra/go/internal/repo"
"go.chromium.org/luci/auth"
"go.chromium.org/luci/common/errors"
- "io/ioutil"
- "os"
)
const (
@@ -110,7 +111,6 @@
func (c *createBranch) Run(a subcommands.Application, args []string,
env subcommands.Env) int {
// Common setup (argument validation, repo init, etc.)
-
ret := Run(c, a, args, env)
if ret != 0 {
return ret
@@ -149,7 +149,7 @@
branch.LogErr(errors.Annotate(err, "Error: Failed to load manifest from file ").Err().Error())
return 1
}
- branch.LogErr("Got manifest from filepath %v", c.file)
+ branch.LogOut("Got manifest from filepath %v", c.file)
branch.WorkingManifest = *file
} else {
file, err := gerrit.DownloadFileFromGitiles(authedClient, ctx, "chrome-internal.googlesource.com",
@@ -158,7 +158,7 @@
branch.LogErr(errors.Annotate(err, "failed to fetch buildspec %v", c.buildSpecManifest).Err().Error())
return 1
}
- branch.LogErr("Got %v from Gitiles", c.buildSpecManifest)
+ branch.LogOut("Got %v from Gitiles", c.buildSpecManifest)
wm, err := ioutil.TempFile("", "working-manifest.xml")
if err != nil {
branch.LogErr("%s\n", err.Error())
@@ -175,7 +175,7 @@
branch.LogErr("%s\n", err.Error())
return 1
}
- branch.LogErr("Fetched working manifest.\n")
+ branch.LogOut("Fetched working manifest.\n")
}
// Use manifest-internal as a sentinel repository to get the appropriate branch name.
@@ -196,8 +196,8 @@
sourceUpstream = "main"
}
- branch.LogErr("Using sourceRevision %s for manifestInternal", sourceRevision)
- branch.LogErr("Using sourceUpstream %s for manifestInternal", sourceUpstream)
+ branch.LogOut("Using sourceRevision %s for manifestInternal", sourceRevision)
+ branch.LogOut("Using sourceUpstream %s for manifestInternal", sourceUpstream)
// Validate the version.
// Double check that the checkout has a zero patch number. Otherwise we cannot branch from it.
@@ -228,9 +228,9 @@
vinfo.VersionString())
return 1
}
- branch.LogErr("Version found: %s.\n", vinfo.VersionString())
+ branch.LogOut("Version found: %s.\n", vinfo.VersionString())
- branch.LogErr("Have manifest = %v", manifestInternal)
+ branch.LogOut("Have manifest = %v", manifestInternal)
branchType := ""
@@ -262,6 +262,7 @@
// Generate git branch names.
branches := branch.ProjectBranches(branchName, git.StripRefs(sourceRevision))
+ // Do not change the format of this string, it is parsed by the brancher recipe.
branch.LogOut("Creating branch: %s\n", branchName)
projectBranches, err := branch.GerritProjectBranches(branches)
@@ -289,7 +290,7 @@
}
if !c.Push {
- branch.LogErr("Dry run (no --push): completed successfully")
+ branch.LogOut("Dry run (no --push): completed successfully")
}
return 0
}
diff --git a/internal/branch/branch_api.go b/internal/branch/branch_api.go
index b2611a1..9890b14 100644
--- a/internal/branch/branch_api.go
+++ b/internal/branch/branch_api.go
@@ -6,14 +6,14 @@
import (
"fmt"
+ "io/ioutil"
+ "net/http"
+ "time"
+
gerritapi "github.com/andygrunwald/go-gerrit"
"go.chromium.org/luci/common/errors"
"go.uber.org/atomic"
"golang.org/x/sync/errgroup"
- "io/ioutil"
- "log"
- "net/http"
- "time"
)
// GerritProjectBranch contains all the details for creating a new Gerrit branch
@@ -28,7 +28,7 @@
func qpsToPeriod(qps float64) time.Duration {
if qps <= 0 {
// some very generous default duration
- LogErr("Got qps %v, <= 0. Using a default duration instead.", qps)
+ LogOut("Got qps %v, <= 0. Using a default duration instead.", qps)
return time.Second * 10
}
periodSec := float64(time.Second) / qps
@@ -64,10 +64,10 @@
// for the specified inputs using the Gerrit API.
func CreateRemoteBranchesApi(authedClient *http.Client, branches []GerritProjectBranch, dryRun bool, gerritQps float64) error {
if dryRun {
- log.Printf("Dry run (no --push): would create remote branches for %v Gerrit repos", len(branches))
+ LogOut("Dry run (no --push): would create remote branches for %v Gerrit repos", len(branches))
return nil
}
- log.Printf("Creating remote branches for %v Gerrit repos. This will take a few minutes, since otherwise Gerrit would throttle us.", len(branches))
+ LogOut("Creating remote branches for %v Gerrit repos. This will take a few minutes, since otherwise Gerrit would throttle us.", len(branches))
var g errgroup.Group
throttle := time.Tick(qpsToPeriod(gerritQps))
createCount := atomic.Int64{}
@@ -81,13 +81,13 @@
}
count := createCount.Inc()
if count%10 == 0 {
- log.Printf("Created %v of %v remote branches", count, len(branches))
+ LogOut("Created %v of %v remote branches", count, len(branches))
}
return nil
})
}
err := g.Wait()
- log.Printf("Successfully created %v of %v remote branches", createCount.Load(), len(branches))
+ LogOut("Successfully created %v of %v remote branches", createCount.Load(), len(branches))
return err
}
diff --git a/internal/branch/bump.go b/internal/branch/bump.go
index fdf5818..ba255f6 100644
--- a/internal/branch/bump.go
+++ b/internal/branch/bump.go
@@ -6,10 +6,11 @@
import (
"fmt"
+ "os"
+
"go.chromium.org/chromiumos/infra/go/internal/chromeos_version"
"go.chromium.org/chromiumos/infra/go/internal/git"
"go.chromium.org/luci/common/errors"
- "os"
)
func bumpVersion(
@@ -72,7 +73,7 @@
// source branch for a branch creation command.
func BumpForCreate(componentToBump chromeos_version.VersionComponent, release, push bool, branchName, sourceUpstream string) error {
commitMsg := fmt.Sprintf("Bump %s number after creating branch %s", componentToBump, branchName)
- LogErr(commitMsg)
+ LogOut(commitMsg)
if err := bumpVersion(componentToBump, branchName, commitMsg, !push); err != nil {
return err
}
@@ -80,14 +81,14 @@
if release {
// Bump milestone after creating release branch.
commitMsg = fmt.Sprintf("Bump milestone after creating release branch %s", branchName)
- LogErr(commitMsg)
+ LogOut(commitMsg)
if err := bumpVersion(chromeos_version.ChromeBranch, sourceUpstream, commitMsg, !push); err != nil {
return err
}
// Also need to bump the build number, otherwise two release will have conflicting versions.
// See crbug.com/213075.
commitMsg = fmt.Sprintf("Bump build number after creating release branch %s", branchName)
- LogErr(commitMsg)
+ LogOut(commitMsg)
if err := bumpVersion(chromeos_version.Build, sourceUpstream, commitMsg, !push); err != nil {
return err
}
@@ -109,7 +110,7 @@
}
commitMsg = fmt.Sprintf("Bump %s number for source branch %s after creating branch %s",
sourceComponentToBump, sourceUpstream, branchName)
- LogErr(commitMsg)
+ LogOut(commitMsg)
if err := bumpVersion(sourceComponentToBump, sourceUpstream, commitMsg, !push); err != nil {
return err
}
diff --git a/internal/branch/common.go b/internal/branch/common.go
index b28c65c..555e4e3 100644
--- a/internal/branch/common.go
+++ b/internal/branch/common.go
@@ -6,9 +6,6 @@
import (
"fmt"
- "go.chromium.org/chromiumos/infra/go/internal/git"
- "go.chromium.org/chromiumos/infra/go/internal/repo"
- "go.chromium.org/luci/common/errors"
"io/ioutil"
"log"
"net/url"
@@ -16,6 +13,10 @@
"path/filepath"
"strconv"
"strings"
+
+ "go.chromium.org/chromiumos/infra/go/internal/git"
+ "go.chromium.org/chromiumos/infra/go/internal/repo"
+ "go.chromium.org/luci/common/errors"
)
const VersionFileProjectPath = "src/third_party/chromiumos-overlay"
@@ -44,7 +45,7 @@
}
}
-// LogOut logs to stderr.
+// LogErr logs to stderr.
func LogErr(format string, a ...interface{}) {
if StderrLog != nil {
StderrLog.Printf(format, a...)
diff --git a/internal/branch/helper.go b/internal/branch/helper.go
index c6bace9..958619d 100644
--- a/internal/branch/helper.go
+++ b/internal/branch/helper.go
@@ -531,10 +531,10 @@
return fmt.Errorf("already branched %s. Please rerun with --force if you "+
"would like to proceed", vinfo.VersionString())
} else {
- LogErr("Overwriting branch with version %s (--force was set).\n", vinfo.VersionString())
+ LogOut("Overwriting branch with version %s (--force was set).\n", vinfo.VersionString())
}
} else {
- LogErr("No branch exists for version %s. Continuing...\n", vinfo.VersionString())
+ LogOut("No branch exists for version %s. Continuing...\n", vinfo.VersionString())
}
return nil
}
diff --git a/internal/branch/manifest_repo.go b/internal/branch/manifest_repo.go
index c96d237..741ffaf 100644
--- a/internal/branch/manifest_repo.go
+++ b/internal/branch/manifest_repo.go
@@ -8,7 +8,6 @@
"encoding/xml"
"fmt"
"io/ioutil"
- "log"
"path/filepath"
"regexp"
"strings"
@@ -228,7 +227,7 @@
// RepairManifestsOnDisk repairs the revision and upstream attributes of
// manifest elements on disk for the given projects.
func (m *ManifestRepo) RepairManifestsOnDisk(branchesByPath map[string]string) error {
- log.Printf("Repairing manifest project %s", m.Project.Name)
+ LogOut("Repairing manifest project %s", m.Project.Name)
manifestPaths, err := m.listManifests([]string{defaultManifest, officialManifest})
if err != nil {
