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

use different timeouts for all vcs commands #879

Merged
merged 1 commit into from
Jul 23, 2017
Merged

use different timeouts for all vcs commands #879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 14 additions & 4 deletions internal/gps/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,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
Open in desktop
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
Open in desktop
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