Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

gps: include output in error #1180

Merged
merged 3 commits into from
Sep 19, 2017
Merged

gps: include output in error #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b5ebb1
gps: include output in error
tamird Sep 18, 2017
a374f1c
gps: DRY
tamird Sep 18, 2017
7d388fa
gps: use commandContext
tamird Sep 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions internal/gps/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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
}

func (c cmd) SetEnv(env []string) {
c.Cmd.Env = env
}
26 changes: 9 additions & 17 deletions internal/gps/cmd_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

@jmank88 jmank88 Sep 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this field being exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that cmd.go can compile. The windows variant embeds this field directly.

}

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
}

Expand All @@ -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()
Expand All @@ -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
Expand Down
8 changes: 0 additions & 8 deletions internal/gps/cmd_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
35 changes: 17 additions & 18 deletions internal/gps/vcs_source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"context"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"

Expand Down Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand All @@ -174,12 +173,12 @@ 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, err
return nil, errors.Wrap(err, string(out))
}

all := bytes.Split(bytes.TrimSpace(out), []byte("\n"))
Expand Down Expand Up @@ -396,17 +395,17 @@ 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))
}

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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down