From 37e89873a4049cd3a7c5e9333cd1dbc20875938b Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 7 Sep 2010 16:11:33 +0000 Subject: [PATCH] Add options argument to _Run() to redirect output. Add automatic 'header' on command execution BUG=54084 TEST=none Review URL: http://codereview.chromium.org/3319008 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@58700 0039d316-1c4b-4281-b951-d872f2087c98 --- gclient_scm.py | 24 ++++++------ tests/gclient_smoketest.py | 78 ++++++++++++++------------------------ trychange.py | 2 +- 3 files changed, 40 insertions(+), 64 deletions(-) diff --git a/gclient_scm.py b/gclient_scm.py index b6ee8a08ac..3909dd750d 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -128,7 +128,7 @@ class GitWrapper(SCMWrapper): def diff(self, options, args, file_list): merge_base = self._Capture(['merge-base', 'HEAD', 'origin']) - self._Run(['diff', merge_base]) + self._Run(['diff', merge_base], options) def export(self, options, args, file_list): """Export a clean directory tree into the given path. @@ -140,7 +140,8 @@ class GitWrapper(SCMWrapper): export_path = os.path.abspath(os.path.join(args[0], self.relpath)) if not os.path.exists(export_path): os.makedirs(export_path) - self._Run(['checkout-index', '-a', '--prefix=%s/' % export_path]) + self._Run(['checkout-index', '-a', '--prefix=%s/' % export_path], + options) def pack(self, options, args, file_list): """Generates a patch file which can be applied to the root of the @@ -282,7 +283,7 @@ class GitWrapper(SCMWrapper): # This is a big hammer, debatable if it should even be here... if options.force or options.reset: - self._Run(['reset', '--hard', 'HEAD']) + self._Run(['reset', '--hard', 'HEAD'], options) if current_type == 'detached': # case 0 @@ -426,7 +427,7 @@ class GitWrapper(SCMWrapper): deps_revision = deps_revision.replace('refs/heads/', 'origin/') files = self._Capture(['diff', deps_revision, '--name-only']).split() - self._Run(['reset', '--hard', deps_revision]) + self._Run(['reset', '--hard', deps_revision], options) file_list.extend([os.path.join(self.checkout_path, f) for f in files]) def revinfo(self, options, args, file_list): @@ -444,7 +445,7 @@ class GitWrapper(SCMWrapper): 'does not exist.\n') % self.checkout_path) else: merge_base = self._Capture(['merge-base', 'HEAD', 'origin']) - self._Run(['diff', '--name-status', merge_base]) + self._Run(['diff', '--name-status', merge_base], options) files = self._Capture(['diff', '--name-only', merge_base]).split() file_list.extend([os.path.join(self.checkout_path, f) for f in files]) @@ -480,7 +481,7 @@ class GitWrapper(SCMWrapper): for _ in range(3): try: - self._Run(clone_cmd, cwd=self._root_dir) + self._Run(clone_cmd, options, cwd=self._root_dir) break except gclient_utils.Error, e: # TODO(maruel): Hackish, should be fixed by moving _Run() to @@ -544,7 +545,7 @@ class GitWrapper(SCMWrapper): "work in your current branch!" " (y)es / (q)uit / (s)how : ")) if re.match(r'yes|y', rebase_action, re.I): - self._Run(['reset', '--hard', 'HEAD']) + self._Run(['reset', '--hard', 'HEAD'], options) # Should this be recursive? rebase_output, rebase_err = scm.GIT.Capture(rebase_cmd, self.checkout_path) @@ -655,13 +656,10 @@ class GitWrapper(SCMWrapper): return gclient_utils.CheckCall(['git'] + args, cwd=self.checkout_path)[0].strip() - def _Run(self, args, **kwargs): + def _Run(self, args, options, **kwargs): kwargs.setdefault('cwd', self.checkout_path) - try: - gclient_utils.Popen(['git'] + args, **kwargs).communicate() - except OSError: - raise gclient_utils.Error("git command '%s' failed to run." % - ' '.join(cmd) + "\nCheck that you have git installed.") + gclient_utils.CheckCallAndFilterAndHeader(['git'] + args, + always=options.verbose, stdout=options.stdout, **kwargs) class SVNWrapper(SCMWrapper): diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index 2ebbc335f1..bb877cd5f1 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -49,10 +49,10 @@ class GClientSmokeBase(FakeReposTestBase): return (stdout.replace('\r\n', '\n'), stderr.replace('\r\n', '\n'), process.returncode) - def parseGclient(self, cmd, items): + def parseGclient(self, cmd, items, expected_stderr=''): """Parse gclient's output to make it easier to test.""" (stdout, stderr, returncode) = self.gclient(cmd) - self.checkString('', stderr) + self.checkString(expected_stderr, stderr) self.assertEquals(0, returncode) return self.checkBlock(stdout, items) @@ -103,7 +103,7 @@ class GClientSmokeBase(FakeReposTestBase): path = self.root_dir self.checkString(results[i][0][0], verb, (i, results[i][0][0], verb)) self.checkString(results[i][0][2], path, (i, results[i][0][2], path)) - self.assertEquals(len(results), len(items), (stdout, items)) + self.assertEquals(len(results), len(items), (stdout, items, len(results))) return results def svnBlockCleanup(self, out): @@ -473,12 +473,10 @@ class GClientSmokeGIT(GClientSmokeBase): # TODO(maruel): safesync. self.gclient(['config', self.git_base + 'repo_1', '--name', 'src']) # Test unversioned checkout. - results = self.gclient(['sync', '--deps', 'mac']) - self.checkBlock(results[0], ['running', 'running']) + self.parseGclient(['sync', '--deps', 'mac'], + ['running', 'running', 'running', 'running', 'running']) # TODO(maruel): http://crosbug.com/3582 hooks run even if not matching, must # add sync parsing to get the list of updated files. - #self.assertTrue(results[1].startswith('Switched to a new branch \'')) - self.assertEquals(0, results[2]) tree = self.mangle_git_tree(('repo_1@2', 'src'), ('repo_2@1', 'src/repo2'), ('repo_3@2', 'src/repo2/repo_renamed')) @@ -525,18 +523,12 @@ class GClientSmokeGIT(GClientSmokeBase): if not self.enabled: return self.gclient(['config', self.git_base + 'repo_1', '--name', 'src']) - results = self.gclient(['sync', '--deps', 'mac', '--revision', - 'invalid@' + self.githash('repo_1', 1)]) - self.checkBlock(results[0], ['running', 'running']) - # TODO(maruel): git shouldn't output to stderr... - self.checkString('Please fix your script, having invalid --revision flags ' - 'will soon considered an error.\n', - results[1]) - #self.checkString('Please fix your script, having invalid --revision flags ' - # 'will soon considered an error.\nSwitched to a new branch \'%s\'\n' % - # self.githash('repo_2', 1)[:7], - # results[1]) - self.assertEquals(0, results[2]) + self.parseGclient( + ['sync', '--deps', 'mac', + '--revision', 'invalid@' + self.githash('repo_1', 1)], + ['running', 'running', 'running', 'running', 'running'], + 'Please fix your script, having invalid --revision flags ' + 'will soon considered an error.\n') tree = self.mangle_git_tree(('repo_1@2', 'src'), ('repo_2@1', 'src/repo2'), ('repo_3@2', 'src/repo2/repo_renamed')) @@ -549,14 +541,9 @@ class GClientSmokeGIT(GClientSmokeBase): return # When no solution name is provided, gclient uses the first solution listed. self.gclient(['config', self.git_base + 'repo_1', '--name', 'src']) - results = self.gclient(['sync', '--deps', 'mac', - '--revision', self.githash('repo_1', 1)]) - self.checkBlock(results[0], []) - # TODO(maruel): git shouldn't output to stderr... - #self.checkString('Switched to a new branch \'%s\'\n' - # % self.githash('repo_1', 1), results[1]) - self.checkString('', results[1]) - self.assertEquals(0, results[2]) + self.parseGclient(['sync', '--deps', 'mac', + '--revision', self.githash('repo_1', 1)], + ['running', 'running', 'running', 'running']) tree = self.mangle_git_tree(('repo_1@1', 'src'), ('repo_2@2', 'src/repo2'), ('repo_3@1', 'src/repo2/repo3'), @@ -572,8 +559,7 @@ class GClientSmokeGIT(GClientSmokeBase): self.gclient(['sync', '--deps', 'mac']) write(join(self.root_dir, 'src', 'repo2', 'hi'), 'Hey!') - results = self.gclient(['status', '--deps', 'mac']) - out = results[0].splitlines(False) + out = self.parseGclient(['status', '--deps', 'mac'], []) # TODO(maruel): http://crosbug.com/3584 It should output the unversioned # files. self.assertEquals(0, len(out)) @@ -583,7 +569,7 @@ class GClientSmokeGIT(GClientSmokeBase): results = self.gclient(['revert', '--deps', 'mac']) out = results[0].splitlines(False) # TODO(maruel): http://crosbug.com/3583 It just runs the hooks right now. - self.assertEquals(7, len(out)) + self.assertEquals(13, len(out)) self.checkString('', results[1]) self.assertEquals(0, results[2]) tree = self.mangle_git_tree(('repo_1@2', 'src'), @@ -671,19 +657,13 @@ class GClientSmokeBoth(GClientSmokeBase): ' "url": "' + self.svn_base + 'trunk/src/"},' '{"name": "src-git",' '"url": "' + self.git_base + 'repo_1"}]']) - results = self.gclient(['sync', '--deps', 'mac']) - # 3x svn checkout, 3x run hooks - self.checkBlock(results[0], - ['running', 'running', - # This is due to the way svn update is called for a single - # file when File() is used in a DEPS file. - ('running', self.root_dir + '/src/file/other'), - 'running', 'running', 'running', 'running', 'running', - 'running']) - # TODO(maruel): Something's wrong here. git outputs to stderr 'Switched to - # new branch \'hash\''. - #self.checkString('', results[1]) - self.assertEquals(0, results[2]) + self.parseGclient(['sync', '--deps', 'mac'], + ['running', 'running', 'running', + # This is due to the way svn update is called for a single + # file when File() is used in a DEPS file. + ('running', self.root_dir + '/src/file/other'), + 'running', 'running', 'running', 'running', 'running', 'running', + 'running', 'running']) tree = self.mangle_git_tree(('repo_1@2', 'src-git'), ('repo_2@1', 'src/repo2'), ('repo_3@2', 'src/repo2/repo_renamed')) @@ -707,13 +687,11 @@ class GClientSmokeBoth(GClientSmokeBase): ' "url": "' + self.svn_base + 'trunk/src/"},' '{"name": "src-git",' '"url": "' + self.git_base + 'repo_1"}]']) - results = self.gclient(['sync', '--deps', 'mac', '--revision', '1', - '-r', 'src-git@' + self.githash('repo_1', 1)]) - self.checkBlock(results[0], ['running', 'running', 'running', 'running']) - # TODO(maruel): Something's wrong here. git outputs to stderr 'Switched to - # new branch \'hash\''. - #self.checkString('', results[1]) - self.assertEquals(0, results[2]) + self.parseGclient( + ['sync', '--deps', 'mac', '--revision', '1', + '-r', 'src-git@' + self.githash('repo_1', 1)], + ['running', 'running', 'running', 'running', + 'running', 'running', 'running', 'running']) tree = self.mangle_git_tree(('repo_1@1', 'src-git'), ('repo_2@2', 'src/repo2'), ('repo_3@1', 'src/repo2/repo3'), diff --git a/trychange.py b/trychange.py index f1ea12ddcc..289b0fe4ea 100755 --- a/trychange.py +++ b/trychange.py @@ -433,7 +433,7 @@ def GuessVCS(options, path): if e.returncode != errno.ENOENT and e.returncode != 128: # ENOENT == 2 = they don't have git installed. # 128 = git error code when not in a repo. - logging.warn(e.returncode) + logging.warn('Unexpected error code: %s' % e.returncode) raise raise NoTryServerAccess("Could not guess version control system. " "Are you in a working copy directory?")