use different timeouts for all vcs commands

both `runFromRepoDir` and `runFromCwd` accept now a timeout to control
the timeouts of the underlying `monitoredCmd`, instead of having a
hardcoded timeout of 2 minutes for all commands.
This will allow better tuning of the timeouts on a per-command basis.

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
This commit is contained in:
Miguel Molina 2017-07-22 00:42:24 +02:00
Родитель bcfa7ee917
Коммит 044b1dd0f2
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: D8826D1B86D051EA
3 изменённых файлов: 38 добавлений и 28 удалений

Просмотреть файл

@ -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
)

Просмотреть файл

@ -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))
}
@ -90,7 +90,7 @@ 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))
}
@ -98,7 +98,7 @@ func (r *gitRepo) fetch(ctx context.Context) error {
}
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))
}
@ -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))
}
@ -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))
}
@ -153,7 +153,7 @@ 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))
}
@ -161,7 +161,7 @@ func (r *bzrRepo) fetch(ctx context.Context) error {
}
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))
}
@ -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))
}
@ -182,7 +182,7 @@ 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))
}
@ -190,7 +190,7 @@ func (r *hgRepo) fetch(ctx context.Context) error {
}
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))
}
@ -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))
}
@ -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))
}
@ -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))
}
@ -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))
}
@ -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))
}

Просмотреть файл

@ -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)
}
@ -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)
}
@ -365,7 +365,7 @@ 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))
}
@ -373,7 +373,7 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) {
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)
@ -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))
}
@ -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))
@ -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))