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

Commit

Permalink
Merge pull request #879 from erizocosmico/custom-timeouts
Browse files Browse the repository at this point in the history 
use different timeouts for all vcs commands
  • Loading branch information
@sdboyer
sdboyer authored Jul 23, 2017
2 parents 72c2521 + 044b1dd commit 0a63c47
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 28 deletions.
18 changes: 14 additions & 4 deletions internal/gps/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,22 @@ func (e killCmdError) Error() string {
return fmt.Sprintf("error killing command: %s", e.err)
}

func runFromCwd(ctx context.Context, cmd string, args ...string) ([]byte, error) {
c := newMonitoredCmd(exec.Command(cmd, args...), 2*time.Minute)
func runFromCwd(ctx context.Context, timeout time.Duration, cmd string, args ...string) ([]byte, error) {
c := newMonitoredCmd(exec.Command(cmd, args...), timeout)
return c.combinedOutput(ctx)
}

func runFromRepoDir(ctx context.Context, repo vcs.Repo, cmd string, args ...string) ([]byte, error) {
c := newMonitoredCmd(repo.CmdFromDir(cmd, args...), 2*time.Minute)
func runFromRepoDir(ctx context.Context, repo vcs.Repo, timeout time.Duration, cmd string, args ...string) ([]byte, error) {
c := newMonitoredCmd(repo.CmdFromDir(cmd, args...), timeout)
return c.combinedOutput(ctx)
}

const (
// expensiveCmdTimeout is meant to be used in a command that is expensive
// in terms of computation and we know it will take long or one that uses
// the network, such as clones, updates, ....
expensiveCmdTimeout = 2 * time.Minute
// defaultCmdTimeout is just an umbrella value for all other commands that
// should not take much.
defaultCmdTimeout = 10 * time.Second
)
34 changes: 17 additions & 17 deletions internal/gps/vcs_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func newVcsLocalErrorOr(msg string, err error, out string) error {
}

func (r *gitRepo) get(ctx context.Context) error {
out, err := runFromCwd(ctx, "git", "clone", "--recursive", "-v", "--progress", r.Remote(), r.LocalPath())
out, err := runFromCwd(ctx, expensiveCmdTimeout, "git", "clone", "--recursive", "-v", "--progress", r.Remote(), r.LocalPath())
if err != nil {
return newVcsRemoteErrorOr("unable to get repository", err, string(out))
}
Expand All @@ -90,15 +90,15 @@ func (r *gitRepo) get(ctx context.Context) error {

func (r *gitRepo) fetch(ctx context.Context) error {
// Perform a fetch to make sure everything is up to date.
out, err := runFromRepoDir(ctx, r, "git", "fetch", "--tags", "--prune", r.RemoteLocation)
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "git", "fetch", "--tags", "--prune", r.RemoteLocation)
if err != nil {
return newVcsRemoteErrorOr("unable to update repository", err, string(out))
}
return nil
}

func (r *gitRepo) updateVersion(ctx context.Context, v string) error {
out, err := runFromRepoDir(ctx, r, "git", "checkout", v)
out, err := runFromRepoDir(ctx, r, defaultCmdTimeout, "git", "checkout", v)
if err != nil {
return newVcsLocalErrorOr("Unable to update checked out version", err, string(out))
}
Expand All @@ -110,20 +110,20 @@ func (r *gitRepo) updateVersion(ctx context.Context, v string) error {
// submodules. Or nested submodules. What a great idea, submodules.
func (r *gitRepo) defendAgainstSubmodules(ctx context.Context) error {
// First, update them to whatever they should be, if there should happen to be any.
out, err := runFromRepoDir(ctx, r, "git", "submodule", "update", "--init", "--recursive")
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "git", "submodule", "update", "--init", "--recursive")
if err != nil {
return newVcsLocalErrorOr("unexpected error while defensively updating submodules", err, string(out))
}

// Now, do a special extra-aggressive clean in case changing versions caused
// one or more submodules to go away.
out, err = runFromRepoDir(ctx, r, "git", "clean", "-x", "-d", "-f", "-f")
out, err = runFromRepoDir(ctx, r, defaultCmdTimeout, "git", "clean", "-x", "-d", "-f", "-f")
if err != nil {
return newVcsLocalErrorOr("unexpected error while defensively cleaning up after possible derelict submodule directories", err, string(out))
}

// Then, repeat just in case there are any nested submodules that went away.
out, err = runFromRepoDir(ctx, r, "git", "submodule", "foreach", "--recursive", "git", "clean", "-x", "-d", "-f", "-f")
out, err = runFromRepoDir(ctx, r, defaultCmdTimeout, "git", "submodule", "foreach", "--recursive", "git", "clean", "-x", "-d", "-f", "-f")
if err != nil {
return newVcsLocalErrorOr("unexpected error while defensively cleaning up after possible derelict nested submodule directories", err, string(out))
}
Expand All @@ -144,7 +144,7 @@ func (r *bzrRepo) get(ctx context.Context) error {
}
}

out, err := runFromCwd(ctx, "bzr", "branch", r.Remote(), r.LocalPath())
out, err := runFromCwd(ctx, expensiveCmdTimeout, "bzr", "branch", r.Remote(), r.LocalPath())
if err != nil {
return newVcsRemoteErrorOr("unable to get repository", err, string(out))
}
Expand All @@ -153,15 +153,15 @@ func (r *bzrRepo) get(ctx context.Context) error {
}

func (r *bzrRepo) fetch(ctx context.Context) error {
out, err := runFromRepoDir(ctx, r, "bzr", "pull")
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "bzr", "pull")
if err != nil {
return newVcsRemoteErrorOr("unable to update repository", err, string(out))
}
return nil
}

func (r *bzrRepo) updateVersion(ctx context.Context, version string) error {
out, err := runFromRepoDir(ctx, r, "bzr", "update", "-r", version)
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "bzr", "update", "-r", version)
if err != nil {
return newVcsLocalErrorOr("unable to update checked out version", err, string(out))
}
Expand All @@ -173,7 +173,7 @@ type hgRepo struct {
}

func (r *hgRepo) get(ctx context.Context) error {
out, err := runFromCwd(ctx, "hg", "clone", r.Remote(), r.LocalPath())
out, err := runFromCwd(ctx, expensiveCmdTimeout, "hg", "clone", r.Remote(), r.LocalPath())
if err != nil {
return newVcsRemoteErrorOr("unable to get repository", err, string(out))
}
Expand All @@ -182,15 +182,15 @@ func (r *hgRepo) get(ctx context.Context) error {
}

func (r *hgRepo) fetch(ctx context.Context) error {
out, err := runFromRepoDir(ctx, r, "hg", "pull")
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "hg", "pull")
if err != nil {
return newVcsRemoteErrorOr("unable to fetch latest changes", err, string(out))
}
return nil
}

func (r *hgRepo) updateVersion(ctx context.Context, version string) error {
out, err := runFromRepoDir(ctx, r, "hg", "update", version)
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "hg", "update", version)
if err != nil {
return newVcsRemoteErrorOr("unable to update checked out version", err, string(out))
}
Expand All @@ -210,7 +210,7 @@ func (r *svnRepo) get(ctx context.Context) error {
remote = "file:///" + remote
}

out, err := runFromCwd(ctx, "svn", "checkout", remote, r.LocalPath())
out, err := runFromCwd(ctx, expensiveCmdTimeout, "svn", "checkout", remote, r.LocalPath())
if err != nil {
return newVcsRemoteErrorOr("unable to get repository", err, string(out))
}
Expand All @@ -219,7 +219,7 @@ func (r *svnRepo) get(ctx context.Context) error {
}

func (r *svnRepo) update(ctx context.Context) error {
out, err := runFromRepoDir(ctx, r, "svn", "update")
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "svn", "update")
if err != nil {
return newVcsRemoteErrorOr("unable to update repository", err, string(out))
}
Expand All @@ -228,7 +228,7 @@ func (r *svnRepo) update(ctx context.Context) error {
}

func (r *svnRepo) updateVersion(ctx context.Context, version string) error {
out, err := runFromRepoDir(ctx, r, "svn", "update", "-r", version)
out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "svn", "update", "-r", version)
if err != nil {
return newVcsRemoteErrorOr("unable to update checked out version", err, string(out))
}
Expand All @@ -250,7 +250,7 @@ func (r *svnRepo) CommitInfo(id string) (*vcs.CommitInfo, error) {
Commit commit `xml:"entry>commit"`
}

out, err := runFromRepoDir(ctx, r, "svn", "info", "-r", id, "--xml")
out, err := runFromRepoDir(ctx, r, defaultCmdTimeout, "svn", "info", "-r", id, "--xml")
if err != nil {
return nil, newVcsLocalErrorOr("unable to retrieve commit information", err, string(out))
}
Expand All @@ -267,7 +267,7 @@ func (r *svnRepo) CommitInfo(id string) (*vcs.CommitInfo, error) {
}
}

out, err := runFromRepoDir(ctx, r, "svn", "log", "-r", id, "--xml")
out, err := runFromRepoDir(ctx, r, defaultCmdTimeout, "svn", "log", "-r", id, "--xml")
if err != nil {
return nil, newVcsRemoteErrorOr("unable to retrieve commit information", err, string(out))
}
Expand Down
14 changes: 7 additions & 7 deletions internal/gps/vcs_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin
// could have an err here...but it's hard to imagine how?
defer fs.RenameWithFallback(bak, idx)

out, err := runFromRepoDir(ctx, r, "git", "read-tree", rev.String())
out, err := runFromRepoDir(ctx, r, defaultCmdTimeout, "git", "read-tree", rev.String())
if err != nil {
return fmt.Errorf("%s: %s", out, err)
}
Expand All @@ -152,7 +152,7 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin
// though we have a bunch of housekeeping to do to set up, then tear
// down, the sparse checkout controls, as well as restore the original
// index and HEAD.
out, err = runFromRepoDir(ctx, r, "git", "checkout-index", "-a", "--prefix="+to)
out, err = runFromRepoDir(ctx, r, defaultCmdTimeout, "git", "checkout-index", "-a", "--prefix="+to)
if err != nil {
return fmt.Errorf("%s: %s", out, err)
}
Expand Down Expand Up @@ -365,15 +365,15 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
}

// Now, list all the tags
out, err := runFromRepoDir(ctx, r, "bzr", "tags", "--show-ids", "-v")
out, err := runFromRepoDir(ctx, r, defaultCmdTimeout, "bzr", "tags", "--show-ids", "-v")
if err != nil {
return nil, fmt.Errorf("%s: %s", err, string(out))
}

all := bytes.Split(bytes.TrimSpace(out), []byte("\n"))

var branchrev []byte
branchrev, err = runFromRepoDir(ctx, r, "bzr", "version-info", "--custom", "--template={revision_id}", "--revision=branch:.")
branchrev, err = runFromRepoDir(ctx, r, defaultCmdTimeout, "bzr", "version-info", "--custom", "--template={revision_id}", "--revision=branch:.")
br := string(branchrev)
if err != nil {
return nil, fmt.Errorf("%s: %s", err, br)
Expand Down Expand Up @@ -416,7 +416,7 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
}

// Now, list all the tags
out, err := runFromRepoDir(ctx, r, "hg", "tags", "--debug", "--verbose")
out, err := runFromRepoDir(ctx, r, defaultCmdTimeout, "hg", "tags", "--debug", "--verbose")
if err != nil {
return nil, fmt.Errorf("%s: %s", err, string(out))
}
Expand Down Expand Up @@ -450,7 +450,7 @@ 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
out, err = runFromRepoDir(ctx, r, "hg", "bookmarks", "--debug")
out, err = runFromRepoDir(ctx, r, defaultCmdTimeout, "hg", "bookmarks", "--debug")
if err != nil {
// better nothing than partial and misleading
return nil, fmt.Errorf("%s: %s", err, string(out))
Expand Down Expand Up @@ -483,7 +483,7 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
}
}

out, err = runFromRepoDir(ctx, r, "hg", "branches", "-c", "--debug")
out, err = runFromRepoDir(ctx, r, defaultCmdTimeout, "hg", "branches", "-c", "--debug")
if err != nil {
// better nothing than partial and misleading
return nil, fmt.Errorf("%s: %s", err, string(out))
Expand Down

0 comments on commit 0a63c47

Please sign in to comment.