From 426526c692fecd520b1946f362e148d043c61441 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Wed, 29 Oct 2014 17:12:15 +0000 Subject: Echo stderr of subcommands that do network IO when --verbose is used Morph can appear to hang in situations where it is actually waiting on a slow network operation. This change gives users a way to see the output of the subcommands that are doing the network IO (either 'wget', 'git clone' or 'git remote update'). The status information goes onto stderr, because that is where the subcommands write it. Morph tends to put its status output on stdout, but (a) some commands are machine-parsed, such as `serialise-artifact` and (b) it's tricky to get Git to put status output on stdout. --- morphlib/cachedrepo.py | 3 ++- morphlib/git.py | 11 +++++++++++ morphlib/localrepocache.py | 26 +++++++++++++++++++++----- morphlib/localrepocache_tests.py | 11 ++++++++--- 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/morphlib/cachedrepo.py b/morphlib/cachedrepo.py index 30e08ad9..aad3d84e 100644 --- a/morphlib/cachedrepo.py +++ b/morphlib/cachedrepo.py @@ -298,7 +298,8 @@ class CachedRepo(object): def _update(self): # pragma: no cover morphlib.git.gitcmd(self._runcmd, 'remote', 'update', - 'origin', '--prune') + 'origin', '--prune', + echo_stderr=self.app.settings['verbose']) def __str__(self): # pragma: no cover return self.url diff --git a/morphlib/git.py b/morphlib/git.py index d897de3b..70222acb 100644 --- a/morphlib/git.py +++ b/morphlib/git.py @@ -22,6 +22,7 @@ import os import re import string import StringIO +import sys import time @@ -333,6 +334,16 @@ def gitcmd(runcmd, *args, **kwargs): # is enough to say what it contains, so we turn it off by setting # the right flag in an environment variable. kwargs['env']['GIT_NO_REPLACE_OBJECTS'] = '1' + cmdline = ['git'] + + echo_stderr = kwargs.pop('echo_stderr', False) + if echo_stderr: + if 'stderr' not in kwargs: + # Ensure status output is visible. Git will hide it if stderr is + # redirected somewhere else (the --progress flag overrides this + # behaviour for the 'clone' command, but not others). + kwargs['stderr'] = sys.stderr + cmdline.extend(args) return runcmd(cmdline, **kwargs) diff --git a/morphlib/localrepocache.py b/morphlib/localrepocache.py index 002022b9..92c5e763 100644 --- a/morphlib/localrepocache.py +++ b/morphlib/localrepocache.py @@ -20,6 +20,7 @@ import re import urllib2 import urlparse import string +import sys import tempfile import cliapp @@ -102,7 +103,7 @@ class LocalRepoCache(object): self._tarball_base_url = tarball_base_url self._cached_repo_objects = {} - def _git(self, args, cwd=None): # pragma: no cover + def _git(self, args, **kwargs): # pragma: no cover '''Execute git command. This is a method of its own so that unit tests can easily override @@ -110,7 +111,7 @@ class LocalRepoCache(object): ''' - morphlib.git.gitcmd(self._app.runcmd, *args, cwd=cwd) + morphlib.git.gitcmd(self._app.runcmd, *args, **kwargs) def _fetch(self, url, path): # pragma: no cover '''Fetch contents of url into a file. @@ -120,8 +121,20 @@ class LocalRepoCache(object): ''' self._app.status(msg="Trying to fetch %(tarball)s to seed the cache", tarball=url, chatty=True) - self._app.runcmd(['wget', '-q', '-O-', url], - ['tar', 'xf', '-'], cwd=path) + + if self._app.settings['verbose']: + verbosity_flags = [] + kwargs = dict(stderr=sys.stderr) + else: + verbosity_flags = ['--quiet'] + kwargs = dict() + + def wget_command(): + return ['wget'] + verbosity_flags + ['-O-', url] + + self._app.runcmd(wget_command(), + ['tar', 'xf', '-'], + cwd=path, **kwargs) def _mkdtemp(self, dirname): # pragma: no cover '''Creates a temporary directory. @@ -196,9 +209,12 @@ class LocalRepoCache(object): errors.append(error) self._app.status( msg='Using git clone.') + target = self._mkdtemp(self._cachedir) + try: - self._git(['clone', '--mirror', '-n', repourl, target]) + self._git(['clone', '--mirror', '-n', repourl, target], + echo_stderr=self._app.settings['verbose']) except cliapp.AppException, e: errors.append('Unable to clone from %s to %s: %s' % (repourl, target, e)) diff --git a/morphlib/localrepocache_tests.py b/morphlib/localrepocache_tests.py index 22b5ea54..3cc4f07f 100644 --- a/morphlib/localrepocache_tests.py +++ b/morphlib/localrepocache_tests.py @@ -1,4 +1,4 @@ -# Copyright (C) 2012-2013 Codethink Limited +# Copyright (C) 2012-2014 Codethink Limited # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -26,6 +26,11 @@ import morphlib class FakeApplication(object): + def __init__(self): + self.settings = { + 'verbose': True + } + def status(self, msg): pass @@ -53,7 +58,7 @@ class LocalRepoCacheTests(unittest.TestCase): self.lrc._mkdtemp = self.fake_mkdtemp self._mkdtemp_count = 0 - def fake_git(self, args, cwd=None): + def fake_git(self, args, **kwargs): if args[0] == 'clone': self.assertEqual(len(args), 5) remote = args[3] @@ -112,7 +117,7 @@ class LocalRepoCacheTests(unittest.TestCase): self.lrc.cache_repo(self.repourl) def test_fails_to_cache_when_remote_does_not_exist(self): - def fail(args): + def fail(args, **kwargs): self.lrc.fs.makedir(args[4]) raise cliapp.AppException('') self.lrc._git = fail -- cgit v1.2.1