Eliminate redundant work and control parallelism.

Eliminate redundant work and control parallelism.

BUG=
R=hinoka@chromium.org,mmoss@chromium.org

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

[Michael Moss, 2014-05-15 16:26:40]

lgtm

https://codereview.chromium.org/281183003/diff/1/git_tools.py
File git_tools.py (left):

https://codereview.chromium.org/281183003/diff/1/git_tools.py#oldcode298
git_tools.py:298: print '%s: %s <-> %s' % (git_repo, output, found_msg)
Why are you getting rid of this? Just to cut down on verbosity?

[szager1, 2014-05-19 20:54:28]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2014-05-19 20:54:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/szager@chromium.org/281183003/1

[commit-bot: I haz the power, 2014-05-19 20:54:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-19 20:54:48]

Failed to apply patch for deps2git/deps2git.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file deps2git/deps2git.py
  Hunk #7 FAILED at 120.
  Hunk #8 FAILED at 234.
  Hunk #9 FAILED at 251.
  Hunk #10 succeeded at 253 (offset -15 lines).
  Hunk #11 succeeded at 336 (offset -15 lines).
  Hunk #12 succeeded at 378 (offset -16 lines).
  Hunk #13 succeeded at 420 (offset -19 lines).
  Hunk #14 FAILED at 454.
  Hunk #15 succeeded at 481 (offset -19 lines).
  4 out of 15 hunks FAILED -- saving rejects to file deps2git/deps2git.py.rej

Patch:       deps2git/deps2git.py
Index: deps2git.py
diff --git deps2git/deps2git.py deps2git/deps2git.py
index
ef1a75fe1eeadddf158e6914daaeddd5aab93645..854dc69c14b4bba00ce7ebe9f8fa3b76eb6f15d7
100755
--- a/deps2git/deps2git.py
+++ b/deps2git/deps2git.py
@@ -6,6 +6,7 @@
 """Convert SVN based DEPS into .DEPS.git for use with NewGit."""
 
 import collections
+from cStringIO import StringIO
 import json
 import optparse
 import os
@@ -13,14 +14,30 @@ import Queue
 import shutil
 import subprocess
 import sys
+import threading
 import time
 
-from multiprocessing.pool import ThreadPool
-
 import deps_utils
 import git_tools
 import svn_to_git_public
 
+try:
+  import git_cache
+except ImportError:
+  for p in os.environ['PATH'].split(os.pathsep):
+    if (os.path.basename(p) == 'depot_tools' and
+        os.path.exists(os.path.join(p, 'git_cache.py'))):
+      sys.path.append(p)
+  import git_cache
+
+Job = collections.namedtuple(
+    'Job',
+    ['dep', 'git_url', 'dep_url', 'path', 'git_host', 'dep_rev', 'svn_branch'])
+
+ConversionResults = collections.namedtuple(
+    'ConversionResults',
+    ['new_deps', 'deps_vars', 'bad_git_urls', 'bad_dep_urls', 'bad_override',
+     'bad_git_hash'])
 
 # This is copied from depot_tools/gclient.py
 DEPS_OS_CHOICES = {
@@ -36,6 +53,7 @@ DEPS_OS_CHOICES = {
     "android": "android",
 }
 
+
 def SplitScmUrl(url):
   """Given a repository, return a set containing the URL and the revision."""
   url_split = url.split('@')
@@ -46,14 +64,15 @@ def SplitScmUrl(url):
   return (scm_url, scm_rev)
 
 
-def SvnRevToGitHash(svn_rev, git_url, repos_path, workspace, dep_path,
-                    git_host, svn_branch_name=None, cache_dir=None):
+def SvnRevToGitHash(
+    svn_rev, git_url, repos_path, workspace, dep_path, git_host,
+    svn_branch_name=None, cache_dir=None, outbuf=None, shallow=None):
   """Convert a SVN revision to a Git commit id."""
   git_repo = None
   if git_url.startswith(git_host):
     git_repo = git_url.replace(git_host, '')
   else:
-    raise Exception('Unknown git server %s, host %s' % (git_url, git_host))
+    raise RuntimeError('Unknown git server %s, host %s' % (git_url, git_host))
   if repos_path is None and workspace is None and cache_dir is None:
     # We're running without a repository directory (i.e. no -r option).
     # We cannot actually find the commit id, but this mode is useful
@@ -64,11 +83,10 @@ def SvnRevToGitHash(svn_rev, git_url, repos_path, workspace,
dep_path,
     mirror = True
     git_repo_path = os.path.join(repos_path, git_repo)
     if not os.path.exists(git_repo_path) or not os.listdir(git_repo_path):
-      git_tools.Clone(git_url, git_repo_path, mirror)
+      git_tools.Clone(git_url, git_repo_path, mirror, outbuf)
   elif cache_dir:
-    mirror = 'bare'
-    git_tools.Clone(git_url, None, mirror, cache_dir=cache_dir)
-    git_repo_path = git_tools.GetCacheRepoDir(git_url, cache_dir)
+    mirror = True
+    git_repo_path = git_tools.PopulateCache(git_url, shallow)
   else:
     mirror = False
     git_repo_path = os.path.join(workspace, dep_path)
@@ -84,7 +102,7 @@ def SvnRevToGitHash(svn_rev, git_url, repos_path, workspace,
dep_path,
       else:
         shutil.rmtree(git_repo_path)
     if not os.path.exists(git_repo_path):
-      git_tools.Clone(git_url, git_repo_path, mirror)
+      git_tools.Clone(git_url, git_repo_path, mirror, outbuf)
 
   if svn_branch_name:
     # svn branches are mirrored with:
@@ -102,112 +120,60 @@ def SvnRevToGitHash(svn_rev, git_url, repos_path,
workspace, dep_path,
   # Work-around for:
   #   http://code.google.com/p/chromium/issues/detail?id=362222
   if (git_url.startswith('https://chromium.googlesource.com/external/pefile')
-      and int(svn_rev) == 63):
+      and int(svn_rev) in (63, 141)):
     return '1ceaa279daa62b71e3431e58f68be6a96dd1519a'
 
-  try:
-    return git_tools.Search(git_repo_path, svn_rev, mirror, refspec, git_url)
-  except Exception:
-    print >> sys.stderr, '%s <-> ERROR' % git_repo_path
-    raise
+  return git_tools.Search(git_repo_path, svn_rev, mirror, refspec, git_url)
 
-def ConvertDepsToGit(deps, options, deps_vars, svn_deps_vars, svn_to_git_objs,
-                     deps_overrides):
-  """Convert a 'deps' section in a DEPS file from SVN to Git."""
-  new_deps = {}
-  bad_git_urls = set([])
-  bad_dep_urls = []
-  bad_override = []
-  bad_git_hash = []
 
-  # Populate our deps list.
-  deps_to_process = {}
-  for dep, dep_url in deps.iteritems():
-    if not dep_url:  # dep is 'None' and emitted to exclude the dep
-      new_deps[dep] = None
+def MessageMain(message_q, threads):
+  while True:
+    try:
+      msg = message_q.get(True, 10)
+    except Queue.Empty:
+      print >> sys.stderr, 'Still working on:'
+      for s in sorted([th.working_on for th in threads if th.working_on]):
+        print >> sys.stderr, '  %s' % s
       continue
+    if msg is Queue.Empty:
+      return
+    if msg:
+      print >> sys.stderr, msg
 
-    # Get the URL and the revision/hash for this dependency.
-    dep_url, dep_rev = SplitScmUrl(deps[dep])
-
-    path = dep
-    git_url = dep_url
-    svn_branch = None
-    git_host = dep_url
-
-    if not dep_url.endswith('.git'):
-      # Convert this SVN URL to a Git URL.
-      for svn_git_converter in svn_to_git_objs:
-        converted_data = svn_git_converter.SvnUrlToGitUrl(dep, dep_url)
-        if converted_data:
-          path, git_url, git_host = converted_data[:3]
-          if len(converted_data) > 3:
-            svn_branch = converted_data[3]
-          break
-      else:
-        # Make all match failures fatal to catch errors early. When a match is
-        # found, we break out of the loop so the exception is not thrown.
-        if options.no_fail_fast:
-          bad_dep_urls.append(dep_url)
-          continue
-        raise Exception('No match found for %s' % dep_url)
-
-    Job = collections.namedtuple('Job', ['git_url', 'dep_url', 'path',
-                                         'git_host', 'dep_rev', 'svn_branch'])
-    deps_to_process[dep] = Job(
-        git_url, dep_url, path, git_host, dep_rev, svn_branch)
-
-  # Lets pre-cache all of the git repos now if we have cache_dir turned on.
-  if options.cache_dir:
-    if not os.path.isdir(options.cache_dir):
-      os.makedirs(options.cache_dir)
-    pool = ThreadPool(processes=len(deps_to_process))
-    output_queue = Queue.Queue()
-    num_threads = 0
-    for git_url, _, _, _, _, _ in deps_to_process.itervalues():
-      print 'Populating cache for %s' % git_url
-      num_threads += 1
-      pool.apply_async(git_tools.Clone, (git_url, None, 'bare',
-                                         output_queue, options.cache_dir,
-                                         options.shallow))
-    pool.close()
-
-    # Stream stdout line by line.
-    sec_since = 0
-    while num_threads > 0:
-      try:
-        line = output_queue.get(block=True, timeout=1)
-        sec_since = 0
-      except Queue.Empty:
-        sec_since += 1
-        line = ('Main> Heartbeat ping. We are still alive!! '
-                'Seconds since last output: %d sec' % sec_since)
-      if line is None:
-        num_threads -= 1
-      else:
-        print line
-    pool.join()
 
+def ConvertDepMain(dep_q, message_q, options, deps_overrides, svn_deps_vars,
+                   results):
+  cur_thread = threading.current_thread()
+  while True:
+    try:
+      job = dep_q.get(False)
+      dep, git_url, dep_url, path, git_host, dep_rev, svn_branch = job
+      cur_thread.working_on = dep
+    except Queue.Empty:
+      cur_thread.working_on = None
+      return
+
+    outbuf = StringIO()
+    def _print(s):
+      for l in s.splitlines():
+        outbuf.write('[%s] %s\n' % (dep, l))
 
-  for dep, items in deps_to_process.iteritems():
-    git_url, dep_url, path, git_host, dep_rev, svn_branch = items
     if options.verify:
       delay = 0.5
       success = False
       for try_index in range(1, 6):
-        print >> sys.stderr, 'checking %s (try #%d) ...' % (git_url,
try_index),
+        _print('checking %s (try #%d) ...' % (git_url, try_index))
         if git_tools.Ping(git_url, verbose=True):
-          print >> sys.stderr, ' success'
+          _print(' success')
           success = True
           break
-
-        print >> sys.stderr, ' failure'
-        print >> sys.stderr, 'sleeping for %.01f seconds ...' % delay
+        _print(' failure')
+        _print('sleeping for %.01f seconds ...' % delay)
         time.sleep(delay)
         delay *= 2
 
       if not success:
-        bad_git_urls.update([git_url])
+        results.bad_git_urls.add(git_url)
 
     # Get the Git hash based off the SVN rev.
     git_hash = ''
@@ -216,11 +182,14 @@ def ConvertDepsToGit(deps, options, deps_vars,
svn_deps_vars, svn_to_git_objs,
         # Transfer any required variables over from SVN DEPS.
         if not deps_overrides[dep] in svn_deps_vars:
           if options.no_fail_fast:
-            bad_override.append(deps_overrides[dep])
+            results.bad_override.append(deps_overrides[dep])
             continue
-          raise Exception('Missing DEPS variable: %s' % deps_overrides[dep])
-        deps_vars[deps_overrides[dep]] = (
-            '@' + svn_deps_vars[deps_overrides[dep]].lstrip('@'))
+          raise RuntimeError(
+              'Missing DEPS variable: %s' % deps_overrides[dep])
+   …
(message too large)

[szager1, 2014-05-19 21:40:07]

The rebase was non-trivial due to:

https://codereview.chromium.org/190853003

PTAL

https://codereview.chromium.org/281183003/diff/1/git_tools.py
File git_tools.py (left):

https://codereview.chromium.org/281183003/diff/1/git_tools.py#oldcode298
git_tools.py:298: print '%s: %s <-> %s' % (git_repo, output, found_msg)
On 2014/05/15 16:26:40, Michael Moss wrote:
> Why are you getting rid of this? Just to cut down on verbosity?

This is the only noise printed to stdout, other than the contents of .DEPS.git,
and it's not really helpful.

[Michael Moss, 2014-05-19 22:09:10]

lgtm

https://codereview.chromium.org/281183003/diff/1/git_tools.py
File git_tools.py (left):

https://codereview.chromium.org/281183003/diff/1/git_tools.py#oldcode298
git_tools.py:298: print '%s: %s <-> %s' % (git_repo, output, found_msg)
On 2014/05/19 21:40:07, szager1 wrote:
> On 2014/05/15 16:26:40, Michael Moss wrote:
> > Why are you getting rid of this? Just to cut down on verbosity?
> 
> This is the only noise printed to stdout, other than the contents of
.DEPS.git,
> and it's not really helpful.

I have actually found it helpful in the past (though not recently) when tracking
down some git conversion issues. Maybe just wrap with "if VERBOSE:"?

[szager1, 2014-05-20 16:13:14]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2014-05-20 16:13:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/szager@chromium.org/281183003/40001

[commit-bot: I haz the power, 2014-05-20 16:13:44]

Change committed as 271672