Rework gclient 'recurse' command to use a WorkQueue.

Rework gclient 'recurse' command to use a WorkQueue.

Support --jobs in 'fetch' and 'recurse' commands. 

BUG=115840
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124657

[davidbarr, 2012-03-01 05:33:15]

This is a first-cut of parallelizing 'gclient recurse' in support of 'gclient
fetch'.

https://chromiumcodereview.appspot.com/9560010/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9560010/diff/1/gclient.py#newcode1146
gclient.py:1146: subprocess2.call(args, cwd=self._cwd, env=env)
Maybe this should be a wrapper method provided by WorkQueue?

https://chromiumcodereview.appspot.com/9560010/diff/1/gclient.py#newcode1149
gclient.py:1149: 
Whitespace nit.

https://chromiumcodereview.appspot.com/9560010/diff/1/gclient.py#newcode1197
gclient.py:1197: args = ['-j%d' % options.jobs, '-s', 'git', 'git', 'fetch'] +
args
Is this necessary? Will the parser accumulate options when it is reinvoked?

[M-A Ruel, 2012-03-01 14:12:12]

https://chromiumcodereview.appspot.com/9560010/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9560010/diff/1/gclient.py#newcode1124
gclient.py:1124: class Entry(gclient_utils.WorkItem):
I'm a tad uncomfortable with this class. I'd rather you to use the "normal way"
to use the work queue and change Dependency.run() to be able to run an arbitrary
functor. This means adding a new "command". I understand that it's more
refactoring but it's easier on the long run.

In particular, you could use command = None but use args. Right now "args" is
ignored with command == None. Just make sure to document the new behavior. In
particular, the diff would look like:


diff --git a/gclient.py b/gclient.py
index 9931ad7..49de5bf 100644
--- a/gclient.py
+++ b/gclient.py
@@ -565,6 +565,23 @@ class Dependency(gclient_utils.WorkItem,
DependencySettings):
         # Strip any leading path separators.
         while file_list[i].startswith(('\\', '/')):
           file_list[i] = file_list[i][1:]
+    elif command is None and args:
+      # Used by CMDrecurse to run an arbitrary command.
+      if not instance(parsed_url, self.FileImpl):
+        # Skip file only checkout.
+        scm = gclient_scm.GetScmName(parsed_url)
+        # TODO(davidbarr): Convert options.scm as a set()
+        if options.scm and scm not in options.scm:
+          continue
+        cwd = os.path.normpath(os.path.join(self.root.root_dir, self.name))
+        if scm:
+          env['GCLIENT_SCM'] = scm
+        if parsed_url:
+          env['GCLIENT_URL'] = parsed_url
+        if os.path.isdir(cwd):
+          subprocess2.call(args, cwd=cwd, env=env)
+        else:
+          print >> sys.stderr, 'Skipped missing %s' % cwd

     # Always parse the DEPS file.
     self.ParseDepsFile()


Then you can remove the corresponding conditions in CMDrecurse.

[davidbarr, 2012-03-02 01:23:05]

This is starting to look acceptable.

Fetching from my local mirror is now awesome fast:
$ time DEPOT_TOOLS_UPDATE=0 gclient fetch -j69
real	0m2.012s
user	0m0.930s
sys	0m1.541s

https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py#newcode582
gclient.py:582: gclient_utils.CheckCallAndFilterAndHeader(args, cwd=cwd,
env=env)
Now the output is handled nicely.

https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py#newcode1187
gclient.py:1187: args = ['-j%d' % options.jobs, '-s', 'git', 'git', 'fetch'] +
args
This still makes me uncomfortable, but there isn't a cleaner means of performing
this mapping yet.

[M-A Ruel, 2012-03-02 01:40:37]

https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py#newcode1169
gclient.py:1169: pm = None
Replace lines 1168-1176 with:
options.nohooks = True
client = GClient.LoadCurrentConfig(options)
return client.RunOnDeps(None, args)

In theory, it should just work. :) In practice, you'll want to tweak the
Progress object. I'm fine with an additional argument to RunOnDeps().

[davidbarr, 2012-03-02 03:12:55]

On 2012/03/02 01:40:37, Marc-Antoine Ruel wrote:
> https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py
> File gclient.py (right):
> 
>
https://chromiumcodereview.appspot.com/9560010/diff/7001/gclient.py#newcode1169
> gclient.py:1169: pm = None
> Replace lines 1168-1176 with:
> options.nohooks = True
> client = GClient.LoadCurrentConfig(options)
> return client.RunOnDeps(None, args)
> 
> In theory, it should just work. :) In practice, you'll want to tweak the
> Progress object. I'm fine with an additional argument to RunOnDeps().

I've pushed the differences in behavior down to RunOnDeps, gated on the recurse
command being run. Also, I've opted to create a command 'recurse' rather than
rely on (command is None and args) as that is sure to be confusing to readers.

[davidbarr, 2012-03-02 03:21:55]

CMDrecurse is exclusively option parsing and precondition checking now!

https://chromiumcodereview.appspot.com/9560010/diff/9002/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9560010/diff/9002/gclient.py#newcode528
gclient.py:528: run_scm = command not in ('runhooks', 'recurse', None)
It felt a bit too tricky to implicitly rely on this value.

https://chromiumcodereview.appspot.com/9560010/diff/9002/gclient.py#newcode986
gclient.py:986: revision_overrides = self._EnforceRevisions()
Is _EnforceRevisions() necessary in the 'recurse' case? It adds about 6 seconds
to the startup time.

[M-A Ruel, 2012-03-02 14:25:30]

lgtm, thanks!

https://chromiumcodereview.appspot.com/9560010/diff/9002/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9560010/diff/9002/gclient.py#newcode986
gclient.py:986: revision_overrides = self._EnforceRevisions()
On 2012/03/02 03:21:55, davidbarr wrote:
> Is _EnforceRevisions() necessary in the 'recurse' case? It adds about 6
seconds
> to the startup time.

Just add a comment that it's not strictly necessary in that case and that it's
an optimisation to skip that.

[commit-bot: I haz the power, 2012-03-02 15:05:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidbarr@chromium.org/9560010/12001

[commit-bot: I haz the power, 2012-03-02 15:11:52]

Presubmit check for 9560010-12001 failed and returned exit status 1.

warning: code.google.com certificate with fingerprint
b6:0d:69:6d:47:4e:86:41:f4:a7:31:8f:64:d5:bd:6f:bb:e8:a3:cd not verified (check
hostfingerprints or web.cacerts config setting)
Running presubmit commit checks ...
Checking out rietveld...
Running push-basic.sh
Running save-description-on-failure.sh
Running basic.sh
Running upload-stale.sh
Running upload-local-tracking-branch.sh
Running patch.sh
Running submit-from-new-dir.sh
Running post-dcommit-hook-test.sh
Running hooks.sh
Running abandon.sh
Running upstream.sh

** Presubmit Messages **
You should install python 2.5 and run ln -s $(which python2.5) python.
A great place to put this symlink is in depot_tools.
Otherwise, you break depot_tools on python 2.5, you get to keep the pieces.

** Presubmit ERRORS **
tests/gclient_smoketest.py failed!
Command tests/gclient_smoketest.py returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/tools/depot_tools
.........F..............................
======================================================================
FAIL: testRecurse (__main__.GClientSmokeBoth)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1181, in testRecurse
    self.assertEquals(sorted(entries), sorted(expected))
AssertionError: [("1>________ running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL',
"`pwd`' in '/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src'"),
('1>svn', 'svn://127.0.0.1:10000/svn/trunk/src/',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src'), ("2>________
running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src-git'"), ('2>git',
'git://127.0.0.1:20000/git/repo_1',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src-git'), ("4>________
running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/other'"), ('4>svn',
'svn://127.0.0.1:10000/svn/trunk/other',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/other'),
("5>________ running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/repo2'"), ('5>git',
'git://127.0.0.1:20000/git/repo_2@471ea20',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/repo2'),
("6>________ running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/third_party/foo'"),
('6>svn', 'svn://127.0.0.1:10000/svn/trunk/third_party/foo@1',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/third_party/foo'),
("7>________ running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/repo2/repo_renamed'"),
('7>git', 'git://127.0.0.1:20000/git/repo_3',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/repo2/repo_renamed')]
!= [('git', 'git://127.0.0.1:20000/git/repo_1',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src-git'), ('git',
'git://127.0.0.1:20000/git/repo_2@471ea20',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/repo2'), ('git',
'git://127.0.0.1:20000/git/repo_3',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/repo2/repo_renamed'),
('svn', 'svn://127.0.0.1:10000/svn/trunk/other',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/other'), ('svn',
'svn://127.0.0.1:10000/svn/trunk/src/',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src'), ('svn',
'svn://127.0.0.1:10000/svn/trunk/third_party/foo@1',
'/tmp/trialy3yef9/__main__.GClientSmokeBoth.testRecurse/src/third_party/foo')]

----------------------------------------------------------------------
Ran 40 tests in 164.851s

FAILED (failures=1)


Presubmit checks took 366.2s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[M-A Ruel, 2012-03-02 15:12:43]

On 2012/03/02 15:11:52, I haz the power (commit-bot) wrote:
> FAIL: testRecurse (__main__.GClientSmokeBoth)

Eh, I wrote tests. :)

[commit-bot: I haz the power, 2012-03-02 15:22:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidbarr@chromium.org/9560010/15001

[commit-bot: I haz the power, 2012-03-02 15:28:34]

Presubmit check for 9560010-15001 failed and returned exit status 1.

warning: code.google.com certificate with fingerprint
b6:0d:69:6d:47:4e:86:41:f4:a7:31:8f:64:d5:bd:6f:bb:e8:a3:cd not verified (check
hostfingerprints or web.cacerts config setting)
Running presubmit commit checks ...
Checking out rietveld...
Running push-basic.sh
Running save-description-on-failure.sh
Running basic.sh
Running upload-stale.sh
Running upload-local-tracking-branch.sh
Running patch.sh
Running submit-from-new-dir.sh
Running post-dcommit-hook-test.sh
Running hooks.sh
Running abandon.sh
Running upstream.sh

** Presubmit Messages **
You should install python 2.5 and run ln -s $(which python2.5) python.
A great place to put this symlink is in depot_tools.
Otherwise, you break depot_tools on python 2.5, you get to keep the pieces.

** Presubmit ERRORS **
tests/gclient_smoketest.py failed!
Command tests/gclient_smoketest.py returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/tools/depot_tools
.........F..............................
======================================================================
FAIL: testRecurse (__main__.GClientSmokeBoth)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1181, in testRecurse
    self.assertEquals(sorted(entries), sorted(expected))
AssertionError: [('',), ('',), ('',), ('',), ('',), ("________ running 'sh -c
echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src'"), ("________
running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src-git'"), ("________
running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/other'"), ("________
running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/repo2'"), ("________
running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/repo2/repo_renamed'"),
("________ running 'sh -c echo $GCLIENT_SCM", '$GCLIENT_URL', "`pwd`' in
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/third_party/foo'"),
('git', 'git://127.0.0.1:20000/git/repo_1',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src-git'), ('git',
'git://127.0.0.1:20000/git/repo_2@2589780',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/repo2'), ('git',
'git://127.0.0.1:20000/git/repo_3',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/repo2/repo_renamed'),
('svn', 'svn://127.0.0.1:10000/svn/trunk/other',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/other'), ('svn',
'svn://127.0.0.1:10000/svn/trunk/src/',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src'), ('svn',
'svn://127.0.0.1:10000/svn/trunk/third_party/foo@1',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/third_party/foo')]
!= [('git', 'git://127.0.0.1:20000/git/repo_1',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src-git'), ('git',
'git://127.0.0.1:20000/git/repo_2@2589780',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/repo2'), ('git',
'git://127.0.0.1:20000/git/repo_3',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/repo2/repo_renamed'),
('svn', 'svn://127.0.0.1:10000/svn/trunk/other',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/other'), ('svn',
'svn://127.0.0.1:10000/svn/trunk/src/',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src'), ('svn',
'svn://127.0.0.1:10000/svn/trunk/third_party/foo@1',
'/tmp/trialFTt0tB/__main__.GClientSmokeBoth.testRecurse/src/third_party/foo')]

----------------------------------------------------------------------
Ran 40 tests in 161.413s

FAILED (failures=1)


Presubmit checks took 364.9s to calculate.

[commit-bot: I haz the power, 2012-03-02 15:48:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidbarr@chromium.org/9560010/11004

[commit-bot: I haz the power, 2012-03-02 15:54:17]

Presubmit check for 9560010-11004 failed and returned exit status 1.

warning: code.google.com certificate with fingerprint
b6:0d:69:6d:47:4e:86:41:f4:a7:31:8f:64:d5:bd:6f:bb:e8:a3:cd not verified (check
hostfingerprints or web.cacerts config setting)
Running presubmit commit checks ...
************* Module gclient
C0301:581,0: Line too long (87/80)
Checking out rietveld...
Running push-basic.sh
Running save-description-on-failure.sh
Running basic.sh
Running upload-stale.sh
Running upload-local-tracking-branch.sh
Running patch.sh
Running submit-from-new-dir.sh
Running post-dcommit-hook-test.sh
Running hooks.sh
Running abandon.sh
Running upstream.sh

** Presubmit Messages **
You should install python 2.5 and run ln -s $(which python2.5) python.
A great place to put this symlink is in depot_tools.
Otherwise, you break depot_tools on python 2.5, you get to keep the pieces.

** Presubmit ERRORS **
Fix pylint errors first.

Presubmit checks took 355.0s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[commit-bot: I haz the power, 2012-03-02 15:59:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidbarr@chromium.org/9560010/15003

[commit-bot: I haz the power, 2012-03-02 16:06:12]

Change committed as 124657