Incremental changes towards more efficient refactoring of gclient.py

Fix gclient diff & pack to not throw uncatched exceptions when a directory is missing.

Reorder things in gclient.py for easier diff later
Update revinfo help
Add GetScmName(), it will be needed later.

TEST=unit tests
BUG=none

Review URL: http://codereview.chromium.org/2786013

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@49565 0039d316-1c4b-4281-b951-d872f2087c98

gclient.py

@@ -157,6 +157,7 @@ class GClientKeywords(object):
 
 class GClient(GClientKeywords):
   """Object that represent a gclient checkout."""
+  DEPS_FILE = 'DEPS'
 
   SUPPORTED_COMMANDS = [
     'cleanup', 'diff', 'export', 'pack', 'revert', 'status', 'update',
@@ -174,8 +175,6 @@ class GClient(GClientKeywords):
     "linux2": "unix",
   }
 
-  DEPS_FILE = 'DEPS'
-
   DEFAULT_CLIENT_FILE_TEXT = ("""\
 solutions = [
   { "name"        : "%(solution_name)s",
@@ -1040,14 +1039,22 @@ def CMDrunhooks(parser, args):
 
 
 def CMDrevinfo(parser, args):
-  """Outputs details for every dependencies."""
-  parser.add_option("--deps", dest="deps_os", metavar="OS_LIST",
-                    help="override deps for the specified (comma-separated) "
-                         "platform(s); 'all' will process all deps_os "
-                         "references")
-  parser.add_option("-s", "--snapshot", action="store_true",
-                    help="create a snapshot file of the current "
-                         "version of all repositories")
+  """Output revision info mapping for the client and its dependencies.
+
+  This allows the capture of an overall "revision" for the source tree that
+  can be used to reproduce the same tree in the future. It is only useful for
+  "unpinned dependencies", i.e. DEPS/deps references without a svn revision
+  number or a git hash. A git branch name isn't "pinned" since the actual
+  commit can change.
+  """
+  parser.add_option('--deps', dest='deps_os', metavar='OS_LIST',
+                    help='override deps for the specified (comma-separated) '
+                         'platform(s); \'all\' will process all deps_os '
+                         'references')
+  parser.add_option('-s', '--snapshot', action='store_true',
+                    help='creates a snapshot .gclient file of the current '
+                         'version of all repositories to reproduce the tree, '
+                         'implies -a')
   (options, args) = parser.parse_args(args)
   client = GClient.LoadCurrentConfig(options)
   if not client:

gclient_scm.py

@@ -55,22 +55,28 @@ class DiffFilterer(object):
 
 # Factory Method for SCM wrapper creation
 
-def CreateSCM(url=None, root_dir=None, relpath=None, scm_name='svn'):
-  scm_map = {
+def GetScmName(url):
+  if url:
+    url, _ = gclient_utils.SplitUrlRevision(url)
+    if (url.startswith('git://') or url.startswith('ssh://') or
+        url.endswith('.git')):
+      return 'git'
+    elif (url.startswith('http://') or url.startswith('svn://') or
+          url.startswith('ssh+svn://')):
+      return 'svn'
+  return None
+
+
+def CreateSCM(url, root_dir=None, relpath=None):
+  SCM_MAP = {
     'svn' : SVNWrapper,
     'git' : GitWrapper,
   }
 
-  orig_url = url
-
-  if url:
-    url, _ = gclient_utils.SplitUrlRevision(url)
-    if url.startswith('git:') or url.startswith('ssh:') or url.endswith('.git'):
-      scm_name = 'git'
-
-  if not scm_name in scm_map:
-    raise gclient_utils.Error('Unsupported scm %s' % scm_name)
-  return scm_map[scm_name](orig_url, root_dir, relpath, scm_name)
+  scm_name = GetScmName(url)
+  if not scm_name in SCM_MAP:
+    raise gclient_utils.Error('No SCM found for url %s' % url)
+  return SCM_MAP[scm_name](url, root_dir, relpath)
 
 
 # SCMWrapper base class
@@ -80,9 +86,7 @@ class SCMWrapper(object):
 
   This is the abstraction layer to bind to different SCM.
   """
-  def __init__(self, url=None, root_dir=None, relpath=None,
-               scm_name='svn'):
-    self.scm_name = scm_name
+  def __init__(self, url=None, root_dir=None, relpath=None):
     self.url = url
     self._root_dir = root_dir
     if self._root_dir:
@@ -106,7 +110,7 @@ class SCMWrapper(object):
 
     if not command in dir(self):
       raise gclient_utils.Error('Command %s not implemented in %s wrapper' % (
-          command, self.scm_name))
+          command, self.__class__.__name__))
 
     return getattr(self, command)(options, args, file_list)
 
@@ -671,9 +675,12 @@ class SVNWrapper(SCMWrapper):
 
   def diff(self, options, args, file_list):
     # NOTE: This function does not currently modify file_list.
+    path = os.path.join(self._root_dir, self.relpath)
+    if not os.path.isdir(path):
+      raise gclient_utils.Error('Directory %s is not present.' % path)
     command = ['diff']
     command.extend(args)
-    scm.SVN.Run(command, os.path.join(self._root_dir, self.relpath))
+    scm.SVN.Run(command, path)
 
   def export(self, options, args, file_list):
     """Export a clean directory tree into the given path."""
@@ -692,6 +699,8 @@ class SVNWrapper(SCMWrapper):
     """Generates a patch file which can be applied to the root of the
     repository."""
     path = os.path.join(self._root_dir, self.relpath)
+    if not os.path.isdir(path):
+      raise gclient_utils.Error('Directory %s is not present.' % path)
     command = ['diff']
     command.extend(args)
 

gclient_utils.py

@@ -301,10 +301,10 @@ def IsUsingGit(root, paths):
       return True
   return False
 
-def FindGclientRoot(from_dir):
+def FindGclientRoot(from_dir, filename='.gclient'):
   """Tries to find the gclient root."""
   path = os.path.realpath(from_dir)
-  while not os.path.exists(os.path.join(path, '.gclient')):
+  while not os.path.exists(os.path.join(path, filename)):
     next = os.path.split(path)
     if not next[1]:
       return None
@@ -312,6 +312,7 @@ def FindGclientRoot(from_dir):
   logging.info('Found gclient root at ' + path)
   return path
 
+
 def PathDifference(root, subpath):
   """Returns the difference subpath minus root."""
   root = os.path.realpath(root)

tests/gclient_scm_test.py

@@ -70,18 +70,17 @@ class SVNWrapperTestCase(BaseTestCase):
     members = [
         'AddAdditionalFlags', 'FullUrlForRelativeUrl', 'RunCommand',
         'cleanup', 'diff', 'export', 'pack', 'relpath', 'revert',
-        'revinfo', 'runhooks', 'scm_name', 'status', 'update',
+        'revinfo', 'runhooks', 'status', 'update',
         'updatesingle', 'url',
     ]
 
     # If you add a member, be sure to add the relevant test!
-    self.compareMembers(self._scm_wrapper(), members)
+    self.compareMembers(self._scm_wrapper('svn://a'), members)
 
   def testUnsupportedSCM(self):
-    args = [self.url, self.root_dir, self.relpath]
-    kwargs = {'scm_name' : 'foo'}
-    exception_msg = 'Unsupported scm %(scm_name)s' % kwargs
-    self.assertRaisesError(exception_msg, self._scm_wrapper, *args, **kwargs)
+    args = ['gopher://foo', self.root_dir, self.relpath]
+    exception_msg = 'No SCM found for url gopher://foo'
+    self.assertRaisesError(exception_msg, self._scm_wrapper, *args)
 
   def testSVNFullUrlForRelativeUrl(self):
     self.url = 'svn://a/b/c/d'
@@ -528,7 +527,7 @@ from :3
     members = [
         'FullUrlForRelativeUrl', 'RunCommand',
         'cleanup', 'diff', 'export', 'pack', 'relpath', 'revert',
-        'revinfo', 'runhooks', 'scm_name', 'status', 'update', 'url',
+        'revinfo', 'runhooks', 'status', 'update', 'url',
     ]
 
     # If you add a member, be sure to add the relevant test!