From 2b5ebb1cede44f7092aab3be73097f349e8fde24 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 18 Sep 2017 19:43:41 -0400 Subject: [PATCH 1/3] gps: include output in error --- internal/gps/vcs_source.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gps/vcs_source.go b/internal/gps/vcs_source.go index d2984d7c..7140cfc1 100644 --- a/internal/gps/vcs_source.go +++ b/internal/gps/vcs_source.go @@ -179,7 +179,7 @@ func (s *gitSource) listVersions(ctx context.Context) (vlist []PairedVersion, er cmd.Env = append([]string{"GIT_ASKPASS=", "GIT_TERMINAL_PROMPT=0"}, os.Environ()...) out, err := cmd.CombinedOutput() if err != nil { - return nil, err + return nil, errors.Wrap(err, string(out)) } all := bytes.Split(bytes.TrimSpace(out), []byte("\n")) From a374f1cebd94a915125015dca1d352c6ceb0cce2 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 18 Sep 2017 19:44:44 -0400 Subject: [PATCH 2/3] gps: DRY --- internal/gps/cmd.go | 13 +++++++++++++ internal/gps/cmd_unix.go | 26 +++++++++----------------- internal/gps/cmd_windows.go | 8 -------- 3 files changed, 22 insertions(+), 25 deletions(-) create mode 100644 internal/gps/cmd.go diff --git a/internal/gps/cmd.go b/internal/gps/cmd.go new file mode 100644 index 00000000..76a372ed --- /dev/null +++ b/internal/gps/cmd.go @@ -0,0 +1,13 @@ +// Copyright 2017 The Go 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 gps + +func (c cmd) Args() []string { + return c.Cmd.Args +} + +func (c cmd) SetDir(dir string) { + c.Cmd.Dir = dir +} diff --git a/internal/gps/cmd_unix.go b/internal/gps/cmd_unix.go index d5df8193..f548e319 100644 --- a/internal/gps/cmd_unix.go +++ b/internal/gps/cmd_unix.go @@ -21,41 +21,33 @@ type cmd struct { ctx context.Context // cancel is called when the graceful shutdown timeout expires. cancel context.CancelFunc - cmd *exec.Cmd + Cmd *exec.Cmd } func commandContext(ctx context.Context, name string, arg ...string) cmd { // Grab the caller's context and pass a derived one to CommandContext. c := cmd{ctx: ctx} ctx, cancel := context.WithCancel(ctx) - c.cmd = exec.CommandContext(ctx, name, arg...) + c.Cmd = exec.CommandContext(ctx, name, arg...) c.cancel = cancel return c } -func (c cmd) Args() []string { - return c.cmd.Args -} - -func (c cmd) SetDir(dir string) { - c.cmd.Dir = dir -} - // CombinedOutput is like (*os/exec.Cmd).CombinedOutput except that it // terminates subprocesses gently (via os.Interrupt), but resorts to Kill if // the subprocess fails to exit after 1 minute. func (c cmd) CombinedOutput() ([]byte, error) { // Adapted from (*os/exec.Cmd).CombinedOutput - if c.cmd.Stdout != nil { + if c.Cmd.Stdout != nil { return nil, errors.New("exec: Stdout already set") } - if c.cmd.Stderr != nil { + if c.Cmd.Stderr != nil { return nil, errors.New("exec: Stderr already set") } var b bytes.Buffer - c.cmd.Stdout = &b - c.cmd.Stderr = &b - if err := c.cmd.Start(); err != nil { + c.Cmd.Stdout = &b + c.Cmd.Stderr = &b + if err := c.Cmd.Start(); err != nil { return nil, err } @@ -71,7 +63,7 @@ func (c cmd) CombinedOutput() ([]byte, error) { go func() { select { case <-c.ctx.Done(): - if err := c.cmd.Process.Signal(os.Interrupt); err != nil { + if err := c.Cmd.Process.Signal(os.Interrupt); err != nil { // If an error comes back from attempting to signal, proceed // immediately to hard kill. c.cancel() @@ -82,7 +74,7 @@ func (c cmd) CombinedOutput() ([]byte, error) { } }() - if err := c.cmd.Wait(); err != nil { + if err := c.Cmd.Wait(); err != nil { return nil, err } return b.Bytes(), nil diff --git a/internal/gps/cmd_windows.go b/internal/gps/cmd_windows.go index 943925e7..ce1a0347 100644 --- a/internal/gps/cmd_windows.go +++ b/internal/gps/cmd_windows.go @@ -16,11 +16,3 @@ type cmd struct { func commandContext(ctx context.Context, name string, arg ...string) cmd { return cmd{Cmd: exec.CommandContext(ctx, name, arg...)} } - -func (c cmd) Args() []string { - return c.Cmd.Args -} - -func (c cmd) SetDir(dir string) { - c.Cmd.Dir = dir -} From 7d388fa7470b47d3dc903d728a436c6fb13bbbc9 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 18 Sep 2017 19:45:25 -0400 Subject: [PATCH 3/3] gps: use commandContext --- internal/gps/cmd.go | 4 ++++ internal/gps/vcs_source.go | 33 ++++++++++++++++----------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/internal/gps/cmd.go b/internal/gps/cmd.go index 76a372ed..1166cb9c 100644 --- a/internal/gps/cmd.go +++ b/internal/gps/cmd.go @@ -11,3 +11,7 @@ func (c cmd) Args() []string { func (c cmd) SetDir(dir string) { c.Cmd.Dir = dir } + +func (c cmd) SetEnv(env []string) { + c.Cmd.Env = env +} diff --git a/internal/gps/vcs_source.go b/internal/gps/vcs_source.go index 7140cfc1..d71941ee 100644 --- a/internal/gps/vcs_source.go +++ b/internal/gps/vcs_source.go @@ -9,7 +9,6 @@ import ( "context" "fmt" "os" - "os/exec" "path/filepath" "strings" @@ -142,8 +141,8 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin defer fs.RenameWithFallback(bak, idx) { - cmd := exec.CommandContext(ctx, "git", "read-tree", rev.String()) - cmd.Dir = r.LocalPath() + cmd := commandContext(ctx, "git", "read-tree", rev.String()) + cmd.SetDir(r.LocalPath()) if out, err := cmd.CombinedOutput(); err != nil { return errors.Wrap(err, string(out)) } @@ -161,8 +160,8 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin // down, the sparse checkout controls, as well as restore the original // index and HEAD. { - cmd := exec.CommandContext(ctx, "git", "checkout-index", "-a", "--prefix="+to) - cmd.Dir = r.LocalPath() + cmd := commandContext(ctx, "git", "checkout-index", "-a", "--prefix="+to) + cmd.SetDir(r.LocalPath()) if out, err := cmd.CombinedOutput(); err != nil { return errors.Wrap(err, string(out)) } @@ -174,9 +173,9 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin func (s *gitSource) listVersions(ctx context.Context) (vlist []PairedVersion, err error) { r := s.repo - cmd := exec.CommandContext(ctx, "git", "ls-remote", r.Remote()) + cmd := commandContext(ctx, "git", "ls-remote", r.Remote()) // Ensure no prompting for PWs - cmd.Env = append([]string{"GIT_ASKPASS=", "GIT_TERMINAL_PROMPT=0"}, os.Environ()...) + cmd.SetEnv(append([]string{"GIT_ASKPASS=", "GIT_TERMINAL_PROMPT=0"}, os.Environ()...)) out, err := cmd.CombinedOutput() if err != nil { return nil, errors.Wrap(err, string(out)) @@ -396,8 +395,8 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) { } // Now, list all the tags - tagsCmd := exec.CommandContext(ctx, "bzr", "tags", "--show-ids", "-v") - tagsCmd.Dir = r.LocalPath() + tagsCmd := commandContext(ctx, "bzr", "tags", "--show-ids", "-v") + tagsCmd.SetDir(r.LocalPath()) out, err := tagsCmd.CombinedOutput() if err != nil { return nil, errors.Wrap(err, string(out)) @@ -405,8 +404,8 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) { all := bytes.Split(bytes.TrimSpace(out), []byte("\n")) - viCmd := exec.CommandContext(ctx, "bzr", "version-info", "--custom", "--template={revision_id}", "--revision=branch:.") - viCmd.Dir = r.LocalPath() + viCmd := commandContext(ctx, "bzr", "version-info", "--custom", "--template={revision_id}", "--revision=branch:.") + viCmd.SetDir(r.LocalPath()) branchrev, err := viCmd.CombinedOutput() if err != nil { return nil, errors.Wrap(err, string(branchrev)) @@ -478,8 +477,8 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { } // Now, list all the tags - tagsCmd := exec.CommandContext(ctx, "hg", "tags", "--debug", "--verbose") - tagsCmd.Dir = r.LocalPath() + tagsCmd := commandContext(ctx, "hg", "tags", "--debug", "--verbose") + tagsCmd.SetDir(r.LocalPath()) out, err := tagsCmd.CombinedOutput() if err != nil { return nil, errors.Wrap(err, string(out)) @@ -514,8 +513,8 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { // bookmarks next, because the presence of the magic @ bookmark has to // determine how we handle the branches var magicAt bool - bookmarksCmd := exec.CommandContext(ctx, "hg", "bookmarks", "--debug") - bookmarksCmd.Dir = r.LocalPath() + bookmarksCmd := commandContext(ctx, "hg", "bookmarks", "--debug") + bookmarksCmd.SetDir(r.LocalPath()) out, err = bookmarksCmd.CombinedOutput() if err != nil { // better nothing than partial and misleading @@ -549,8 +548,8 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { } } - cmd := exec.CommandContext(ctx, "hg", "branches", "-c", "--debug") - cmd.Dir = r.LocalPath() + cmd := commandContext(ctx, "hg", "branches", "-c", "--debug") + cmd.SetDir(r.LocalPath()) out, err = cmd.CombinedOutput() if err != nil { // better nothing than partial and misleading