From 4c9a6b0951a78cca8737099331401e2101b830ad Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 12 Apr 2022 16:16:01 +0000 Subject: [PATCH] tools: Minor improvements to the git package Rename 'Auth' to 'Credentials' to match the gerrit package. Add a couple more helpers, and add more comments. Bug: dawn:1342 Change-Id: Ieb6d12d23bb71678e04ff694ab9085a8f5367437 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/86525 Reviewed-by: Corentin Wallez Commit-Queue: Ben Clayton Kokoro: Kokoro --- tools/src/cmd/perfmon/main.go | 30 +++++++++++++------- tools/src/git/git.go | 53 ++++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/tools/src/cmd/perfmon/main.go b/tools/src/cmd/perfmon/main.go index 7521f1f33a..672c24eb9d 100644 --- a/tools/src/cmd/perfmon/main.go +++ b/tools/src/cmd/perfmon/main.go @@ -141,9 +141,9 @@ type Config struct { // GitConfig holds the configuration options for accessing a git repo type GitConfig struct { - URL string - Branch string - Auth git.Auth + URL string + Branch string + Credentials git.Credentials } // GerritConfig holds the configuration options for accessing gerrit @@ -298,7 +298,9 @@ func (e env) doSomeWork() (bool, error) { // benchmark results, which should be benchmarked. func (e env) changesToBenchmark() ([]git.Hash, error) { log.Println("syncing tint repo...") - latest, err := e.tintRepo.Fetch(e.cfg.Tint.Branch, &git.FetchOptions{Auth: e.cfg.Tint.Auth}) + latest, err := e.tintRepo.Fetch(e.cfg.Tint.Branch, &git.FetchOptions{ + Credentials: e.cfg.Tint.Credentials, + }) if err != nil { return nil, err } @@ -481,7 +483,9 @@ func (e env) pushUpdatedResults(res CommitResults) error { // Push the change log.Println("pushing updated results to results repo...") - if err := e.resultsRepo.Push(hash.String(), e.cfg.Results.Branch, &git.PushOptions{Auth: e.cfg.Results.Auth}); err != nil { + if err := e.resultsRepo.Push(hash.String(), e.cfg.Results.Branch, &git.PushOptions{ + Credentials: e.cfg.Results.Credentials, + }); err != nil { return fmt.Errorf("failed to push updated results file '%v':\n %w", absPath, err) } @@ -700,7 +704,9 @@ func (e env) findGerritChangeToBenchmark() (*gerrit.ChangeInfo, error) { func (e env) benchmarkGerritChange(change gerrit.ChangeInfo) error { current := change.Revisions[change.CurrentRevision] log.Printf("fetching '%v'...", current.Ref) - currentHash, err := e.tintRepo.Fetch(current.Ref, &git.FetchOptions{Auth: e.cfg.Tint.Auth}) + currentHash, err := e.tintRepo.Fetch(current.Ref, &git.FetchOptions{ + Credentials: e.cfg.Tint.Credentials, + }) if err != nil { return err } @@ -730,7 +736,9 @@ func (e env) benchmarkGerritChange(change gerrit.ChangeInfo) error { } return err } - if _, err := e.tintRepo.Fetch(parent, &git.FetchOptions{Auth: e.cfg.Tint.Auth}); err != nil { + if _, err := e.tintRepo.Fetch(parent, &git.FetchOptions{ + Credentials: e.cfg.Tint.Credentials, + }); err != nil { return err } parentRun, err := e.benchmarkTintChange(parentHash) @@ -786,8 +794,8 @@ func createOrOpenGitRepo(g *git.Git, filepath string, cfg GitConfig) (*git.Repos if errors.Is(err, git.ErrRepositoryDoesNotExist) { log.Printf("cloning '%v' branch '%v' to '%v'...", cfg.URL, cfg.Branch, filepath) repo, err = g.Clone(filepath, cfg.URL, &git.CloneOptions{ - Branch: cfg.Branch, - Auth: cfg.Auth, + Branch: cfg.Branch, + Credentials: cfg.Credentials, }) } if err != nil { @@ -832,7 +840,9 @@ func makeWorkingDirs(cfg Config) (tintDir, resultsDir string, err error) { // fetchAndCheckoutLatest calls fetch(cfg.Branch) followed by checkoutLatest(). func fetchAndCheckoutLatest(repo *git.Repository, cfg GitConfig) error { - hash, err := repo.Fetch(cfg.Branch, &git.FetchOptions{Auth: cfg.Auth}) + hash, err := repo.Fetch(cfg.Branch, &git.FetchOptions{ + Credentials: cfg.Credentials, + }) if err != nil { return err } diff --git a/tools/src/git/git.go b/tools/src/git/git.go index 4745929e57..04ed43a6f4 100644 --- a/tools/src/git/git.go +++ b/tools/src/git/git.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package git provides helpers for interfacing with the git tool package git import ( @@ -68,17 +69,30 @@ func New(exe string) (*Git, error) { return &Git{exe: exe}, nil } -// Auth holds git authentication credentials -type Auth struct { +// Credentials holds the user name and password used to perform git operations. +type Credentials struct { Username string Password string } // Empty return true if there's no username or password for authentication -func (a Auth) Empty() bool { +func (a Credentials) Empty() bool { return a.Username == "" && a.Password == "" } +// addToURL returns the url with the credentials appended +func (c Credentials) addToURL(u string) (string, error) { + if !c.Empty() { + modified, err := url.Parse(u) + if err != nil { + return "", fmt.Errorf("failed to parse url '%v': %v", u, err) + } + modified.User = url.UserPassword(c.Username, c.Password) + u = modified.String() + } + return u, nil +} + // ErrRepositoryDoesNotExist indicates that a repository does not exist var ErrRepositoryDoesNotExist = errors.New("repository does not exist") @@ -99,7 +113,7 @@ type CloneOptions struct { // Timeout for the operation Timeout time.Duration // Authentication for the clone - Auth Auth + Credentials Credentials } // Clone performs a clone of the repository at url to path. @@ -110,7 +124,7 @@ func (g Git) Clone(path, url string, opt *CloneOptions) (*Repository, error) { if opt == nil { opt = &CloneOptions{} } - url, err := opt.Auth.addToURL(url) + url, err := opt.Credentials.addToURL(url) if err != nil { return nil, err } @@ -140,7 +154,7 @@ type FetchOptions struct { // Timeout for the operation Timeout time.Duration // Git authentication for the remote - Auth Auth + Credentials Credentials } // Fetch performs a fetch of a reference from the remote, returning the Hash of @@ -169,7 +183,7 @@ type PushOptions struct { // Timeout for the operation Timeout time.Duration // Git authentication for the remote - Auth Auth + Credentials Credentials } // Push performs a push of the local reference to the remote reference. @@ -184,7 +198,7 @@ func (r Repository) Push(localRef, remoteRef string, opt *PushOptions) error { if err != nil { return err } - url, err = opt.Auth.addToURL(url) + url, err = opt.Credentials.addToURL(url) if err != nil { return err } @@ -199,7 +213,7 @@ type AddOptions struct { // Timeout for the operation Timeout time.Duration // Git authentication for the remote - Auth Auth + Credentials Credentials } // Add stages the listed files @@ -221,6 +235,8 @@ type CommitOptions struct { AuthorName string // Author email address AuthorEmail string + // Amend last commit? + Amend bool } // Commit commits the staged files with the given message, returning the hash of @@ -229,7 +245,12 @@ func (r Repository) Commit(msg string, opt *CommitOptions) (Hash, error) { if opt == nil { opt = &CommitOptions{} } - args := []string{"commit", "-m", msg} + args := []string{"commit"} + if opt.Amend { + args = append(args, "--amend") + } else { + args = append(args, "-m", msg) + } if opt.AuthorName != "" || opt.AuthorEmail != "" { args = append(args, "--author", fmt.Sprintf("%v <%v>", opt.AuthorName, opt.AuthorEmail)) } @@ -368,18 +389,6 @@ func (g Git) run(dir string, timeout time.Duration, args ...string) (string, err return strings.TrimSpace(string(out)), nil } -func (a Auth) addToURL(u string) (string, error) { - if !a.Empty() { - modified, err := url.Parse(u) - if err != nil { - return "", fmt.Errorf("failed to parse url '%v': %v", u, err) - } - modified.User = url.UserPassword(a.Username, a.Password) - u = modified.String() - } - return u, nil -} - func parseLog(str string) ([]CommitInfo, error) { msgs := strings.Split(str, "ǁ") cls := make([]CommitInfo, 0, len(msgs))