Reapply 32057, 32058, 32059, 32062 and fixes problems introduced by these changes.

Noteworthy change is scm.SVN.GetFileProperty calls Capture instead of Run.

TEST=unit tests
BUG=none

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

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

gcl.py

@@ -19,10 +19,10 @@ import upload
 import urllib2
 
 # gcl now depends on gclient.
-import gclient_scm
+from scm import SVN
 import gclient_utils
 
-__version__ = '1.1.1'
+__version__ = '1.1.2'
 
 
 CODEREVIEW_SETTINGS = {
@@ -46,43 +46,13 @@ MISSING_TEST_MSG = "Change contains new or modified methods, but no new tests!"
 FILES_CACHE = {}
 
 
-### SVN Functions
-
-def IsSVNMoved(filename):
-  """Determine if a file has been added through svn mv"""
-  info = gclient_scm.CaptureSVNInfo(filename)
-  return (info.get('Copied From URL') and
-          info.get('Copied From Rev') and
-          info.get('Schedule') == 'add')
-
-
-def GetSVNFileProperty(file, property_name):
-  """Returns the value of an SVN property for the given file.
-
-  Args:
-    file: The file to check
-    property_name: The name of the SVN property, e.g. "svn:mime-type"
-
-  Returns:
-    The value of the property, which will be the empty string if the property
-    is not set on the file.  If the file is not under version control, the
-    empty string is also returned.
-  """
-  output = RunShell(["svn", "propget", property_name, file])
-  if (output.startswith("svn: ") and
-      output.endswith("is not under version control")):
-    return ""
-  else:
-    return output
-
-
 def UnknownFiles(extra_args):
-  """Runs svn status and prints unknown files.
+  """Runs svn status and returns unknown files.
 
   Any args in |extra_args| are passed to the tool to support giving alternate
   code locations.
   """
-  return [item[1] for item in gclient_scm.CaptureSVNStatus(extra_args)
+  return [item[1] for item in SVN.CaptureStatus(extra_args)
           if item[0][0] == '?']
 
 
@@ -93,7 +63,7 @@ def GetRepositoryRoot():
   """
   global REPOSITORY_ROOT
   if not REPOSITORY_ROOT:
-    infos = gclient_scm.CaptureSVNInfo(os.getcwd(), print_error=False)
+    infos = SVN.CaptureInfo(os.getcwd(), print_error=False)
     cur_dir_repo_root = infos.get("Repository Root")
     if not cur_dir_repo_root:
       raise gclient_utils.Error("gcl run outside of repository")
@@ -101,7 +71,7 @@ def GetRepositoryRoot():
     REPOSITORY_ROOT = os.getcwd()
     while True:
       parent = os.path.dirname(REPOSITORY_ROOT)
-      if (gclient_scm.CaptureSVNInfo(parent, print_error=False).get(
+      if (SVN.CaptureInfo(parent, print_error=False).get(
               "Repository Root") != cur_dir_repo_root):
         break
       REPOSITORY_ROOT = parent
@@ -146,7 +116,7 @@ def GetCachedFile(filename, max_age=60*60*24*3, use_root=False):
         os.stat(cached_file).st_mtime > max_age):
       local_dir = os.path.dirname(os.path.abspath(filename))
       local_base = os.path.basename(filename)
-      dir_info = gclient_scm.CaptureSVNInfo(".")
+      dir_info = SVN.CaptureInfo(".")
       repo_root = dir_info["Repository Root"]
       if use_root:
         url_path = repo_root
@@ -158,7 +128,7 @@ def GetCachedFile(filename, max_age=60*60*24*3, use_root=False):
         r = ""
         if not use_root:
           local_path = os.path.join(local_dir, local_base)
-          r = gclient_scm.CaptureSVNStatus((local_path,))
+          r = SVN.CaptureStatus((local_path,))
         rc = -1
         if r:
           status = r[0][0]
@@ -478,7 +448,7 @@ class ChangeInfo(object):
     if update_status:
       for item in files:
         filename = os.path.join(local_root, item[1])
-        status_result = gclient_scm.CaptureSVNStatus(filename)
+        status_result = SVN.CaptureStatus(filename)
         if not status_result or not status_result[0][0]:
           # File has been reverted.
           save = True
@@ -562,7 +532,7 @@ def GetModifiedFiles():
       files_in_cl[filename] = change_info.name
 
   # Get all the modified files.
-  status_result = gclient_scm.CaptureSVNStatus(None)
+  status_result = SVN.CaptureStatus(None)
   for line in status_result:
     status = line[0]
     filename = line[1]
@@ -749,10 +719,10 @@ def GenerateDiff(files, root=None):
 
   diff = []
   for filename in files:
+    # TODO(maruel): Use SVN.DiffItem().
     # Use svn info output instead of os.path.isdir because the latter fails
     # when the file is deleted.
-    if gclient_scm.CaptureSVNInfo(filename).get("Node Kind") in ("dir",
-                                                                 "directory"):
+    if SVN.CaptureInfo(filename).get('Node Kind') == 'directory':
       continue
     # If the user specified a custom diff command in their svn config file,
     # then it'll be used when we do svn diff, which we don't want to happen
@@ -770,7 +740,7 @@ def GenerateDiff(files, root=None):
     output = RunShell(["svn", "diff", "--config-dir", bogus_dir, filename])
     if output:
       diff.append(output)
-    elif IsSVNMoved(filename):
+    elif SVN.IsMoved(filename):
       #  svn diff on a mv/cp'd file outputs nothing.
       # We put in an empty Index entry so upload.py knows about them.
       diff.append("\nIndex: %s\n" % filename)
@@ -996,7 +966,7 @@ def Change(change_info, args):
   silent = FilterFlag(args, "--silent")
 
   # Verify the user is running the change command from a read-write checkout.
-  svn_info = gclient_scm.CaptureSVNInfo('.')
+  svn_info = SVN.CaptureInfo('.')
   if not svn_info:
     ErrorExit("Current checkout is unversioned.  Please retry with a versioned "
               "directory.")
@@ -1008,7 +978,7 @@ def Change(change_info, args):
     f.close()
   else:
     override_description = None
-      
+
   if change_info.issue:
     try:
       description = GetIssueDescription(change_info.issue)

gclient.py

@@ -66,7 +66,7 @@ Hooks
 """
 
 __author__ = "darinf@gmail.com (Darin Fisher)"
-__version__ = "0.3.3"
+__version__ = "0.3.4"
 
 import errno
 import logging
@@ -747,9 +747,9 @@ class GClient(object):
         # Use entry and not entry_fixed there.
         if entry not in entries and os.path.exists(e_dir):
           modified_files = False
-          if isinstance(prev_entries,list):
+          if isinstance(prev_entries, list):
             # old .gclient_entries format was list, now dict
-            modified_files = gclient_scm.CaptureSVNStatus(e_dir)
+            modified_files = gclient_scm.scm.SVN.CaptureStatus(e_dir)
           else:
             file_list = []
             scm = gclient_scm.CreateSCM(prev_entries[entry], self._root_dir,
@@ -830,7 +830,7 @@ class GClient(object):
       (url, rev) = GetURLAndRev(name, solution["url"])
       entries[name] = "%s@%s" % (url, rev)
       # TODO(aharper): SVN/SCMWrapper cleanup (non-local commandset)
-      entries_deps_content[name] = gclient_scm.CaptureSVN(
+      entries_deps_content[name] = gclient_scm.scm.SVN.Capture(
                                      ["cat",
                                       "%s/%s@%s" % (url,
                                                     self._options.deps_file,

gclient_utils.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""Generic utils."""
+
 import errno
 import os
 import re
@@ -22,8 +24,6 @@ import time
 import xml.dom.minidom
 import xml.parsers.expat
 
-## Generic utils
-
 
 def SplitUrlRevision(url):
   """Splits url and returns a two-tuple: url, rev"""
@@ -76,9 +76,9 @@ class PrintableObject(object):
     return output
 
 
-def FileRead(filename):
+def FileRead(filename, mode='rU'):
   content = None
-  f = open(filename, "rU")
+  f = open(filename, mode)
   try:
     content = f.read()
   finally:
@@ -86,8 +86,8 @@ def FileRead(filename):
   return content
 
 
-def FileWrite(filename, content):
-  f = open(filename, "w")
+def FileWrite(filename, content, mode='w'):
+  f = open(filename, mode)
   try:
     f.write(content)
   finally:
@@ -201,9 +201,9 @@ def SubprocessCallAndFilter(command,
   only if we actually need to print something else as well, so you can
   get the context of the output. If print_messages is false and print_stdout
   is false, no output at all is generated.
-  
-  Also, if print_stdout is true, the command's stdout is also forwarded 
-  to stdout. 
+
+  Also, if print_stdout is true, the command's stdout is also forwarded
+  to stdout.
 
   If a filter function is specified, it is expected to take a single
   string argument, and it will be called with each line of the
@@ -223,7 +223,7 @@ def SubprocessCallAndFilter(command,
   # executable, but shell=True makes subprocess on Linux fail when it's called
   # with a list because it only tries to execute the first item in the list.
   kid = subprocess.Popen(command, bufsize=0, cwd=in_directory,
-      shell=(sys.platform == 'win32'), stdout=subprocess.PIPE, 
+      shell=(sys.platform == 'win32'), stdout=subprocess.PIPE,
       stderr=subprocess.STDOUT)
 
   # Also, we need to forward stdout to prevent weird re-ordering of output.
@@ -238,7 +238,7 @@ def SubprocessCallAndFilter(command,
         if not print_messages:
           print("\n________ running \'%s\' in \'%s\'"
               % (' '.join(command), in_directory))
-          print_messages = True 
+          print_messages = True
         sys.stdout.write(in_byte)
       if in_byte != "\n":
         in_line += in_byte

git_cl_hooks.py

@@ -7,9 +7,8 @@ import re
 import subprocess
 import sys
 
-# Imported from depot_tools.
-import gclient_scm
 import presubmit_support
+import scm
 
 def Backquote(cmd, cwd=None):
   """Like running `cmd` in a shell script."""
@@ -35,7 +34,7 @@ class ChangeOptions:
       raise Exception("Could not parse log message: %s" % log)
     name = m.group(1)
     description = m.group(2)
-    files = gclient_scm.CaptureGitStatus([root], upstream_branch)
+    files = scm.GIT.CaptureStatus([root], upstream_branch)
     issue = Backquote(['git', 'cl', 'status', '--field=id'])
     patchset = None
     self.change = presubmit_support.GitChange(name, description, root, files,

presubmit_support.py

@@ -6,7 +6,7 @@
 """Enables directory-specific presubmit checks to run at upload and/or commit.
 """
 
-__version__ = '1.3.3'
+__version__ = '1.3.4'
 
 # TODO(joi) Add caching where appropriate/needed. The API is designed to allow
 # caching (between all different invocations of presubmit scripts for a given
@@ -35,11 +35,10 @@ import urllib2  # Exposed through the API.
 import warnings
 
 # Local imports.
-# TODO(joi) Would be cleaner to factor out utils in gcl to separate module, but
-# for now it would only be a couple of functions so hardly worth it.
 import gcl
-import gclient_scm
+import gclient_utils
 import presubmit_canned_checks
+import scm
 
 
 # Ask for feedback only once in program lifetime.
@@ -241,7 +240,7 @@ class InputApi(object):
 
       Remember to check for the None case and show an appropriate error!
     """
-    local_path = gclient_scm.CaptureSVNInfo(depot_path).get('Path')
+    local_path = scm.SVN.CaptureInfo(depot_path).get('Path')
     if local_path:
       return local_path
 
@@ -254,7 +253,7 @@ class InputApi(object):
     Returns:
       The depot path (SVN URL) of the file if mapped, otherwise None.
     """
-    depot_path = gclient_scm.CaptureSVNInfo(local_path).get('URL')
+    depot_path = scm.SVN.CaptureInfo(local_path).get('URL')
     if depot_path:
       return depot_path
 
@@ -354,7 +353,7 @@ class InputApi(object):
       file_item = file_item.AbsoluteLocalPath()
     if not file_item.startswith(self.change.RepositoryRoot()):
       raise IOError('Access outside the repository root is denied.')
-    return gcl.ReadFile(file_item, mode)
+    return gclient_utils.FileRead(file_item, mode)
 
   @staticmethod
   def _RightHandSideLinesImpl(affected_files):
@@ -432,7 +431,8 @@ class AffectedFile(object):
     if self.IsDirectory():
       return []
     else:
-      return gcl.ReadFile(self.AbsoluteLocalPath()).splitlines()
+      return gclient_utils.FileRead(self.AbsoluteLocalPath(),
+                                    'rU').splitlines()
 
   def OldContents(self):
     """Returns an iterator over the lines in the old version of file.
@@ -464,7 +464,7 @@ class SvnAffectedFile(AffectedFile):
 
   def ServerPath(self):
     if self._server_path is None:
-      self._server_path = gclient_scm.CaptureSVNInfo(
+      self._server_path = scm.SVN.CaptureInfo(
           self.AbsoluteLocalPath()).get('URL', '')
     return self._server_path
 
@@ -476,13 +476,13 @@ class SvnAffectedFile(AffectedFile):
         # querying subversion, especially on Windows.
         self._is_directory = os.path.isdir(path)
       else:
-        self._is_directory = gclient_scm.CaptureSVNInfo(
+        self._is_directory = scm.SVN.CaptureInfo(
             path).get('Node Kind') in ('dir', 'directory')
     return self._is_directory
 
   def Property(self, property_name):
     if not property_name in self._properties:
-      self._properties[property_name] = gcl.GetSVNFileProperty(
+      self._properties[property_name] = scm.SVN.GetFileProperty(
           self.AbsoluteLocalPath(), property_name).rstrip()
     return self._properties[property_name]
 
@@ -494,8 +494,8 @@ class SvnAffectedFile(AffectedFile):
       elif self.IsDirectory():
         self._is_text_file = False
       else:
-        mime_type = gcl.GetSVNFileProperty(self.AbsoluteLocalPath(),
-                                           'svn:mime-type')
+        mime_type = scm.SVN.GetFileProperty(self.AbsoluteLocalPath(),
+                                            'svn:mime-type')
         self._is_text_file = (not mime_type or mime_type.startswith('text/'))
     return self._is_text_file
 
@@ -809,7 +809,7 @@ def DoGetTrySlaves(changed_files,
     if verbose:
       output_stream.write("Running %s\n" % filename)
     # Accept CRLF presubmit script.
-    presubmit_script = gcl.ReadFile(filename, 'rU')
+    presubmit_script = gclient_utils.FileRead(filename, 'rU')
     results += executer.ExecPresubmitScript(presubmit_script)
 
   slaves = list(set(results))
@@ -925,7 +925,7 @@ def DoPresubmitChecks(change,
     if verbose:
       output_stream.write("Running %s\n" % filename)
     # Accept CRLF presubmit script.
-    presubmit_script = gcl.ReadFile(filename, 'rU')
+    presubmit_script = gclient_utils.FileRead(filename, 'rU')
     results += executer.ExecPresubmitScript(presubmit_script, filename)
 
   errors = []
@@ -1022,7 +1022,7 @@ def Main(argv):
         options.files = ParseFiles(args, options.recursive)
       else:
         # Grab modified files.
-        options.files = gclient_scm.CaptureGitStatus([options.root])
+        options.files = scm.GIT.CaptureStatus([options.root])
   elif os.path.isdir(os.path.join(options.root, '.svn')):
     change_class = SvnChange
     if not options.files:
@@ -1030,7 +1030,7 @@ def Main(argv):
         options.files = ParseFiles(args, options.recursive)
       else:
         # Grab modified files.
-        options.files = gclient_scm.CaptureSVNStatus([options.root])
+        options.files = scm.SVN.CaptureStatus([options.root])
   else:
     # Doesn't seem under source control.
     change_class = Change

revert.py

@@ -146,7 +146,7 @@ def Revert(revisions, force=False, commit=True, send_email=True, message=None,
   print ""
 
   # Make sure these files are unmodified with svn status.
-  status = gclient_scm.CaptureSVNStatus(files)
+  status = gclient_scm.scm.SVN.CaptureStatus(files)
   if status:
     if force:
       # TODO(maruel): Use the tool to correctly revert '?' files.

tests/gcl_unittest.py

@@ -16,7 +16,7 @@ class GclTestsBase(SuperMoxTestBase):
     SuperMoxTestBase.setUp(self)
     self.fake_root_dir = self.RootDir()
     self.mox.StubOutWithMock(gcl, 'RunShell')
-    self.mox.StubOutWithMock(gcl.gclient_scm, 'CaptureSVNInfo')
+    self.mox.StubOutWithMock(gcl.SVN, 'CaptureInfo')
     self.mox.StubOutWithMock(gcl, 'tempfile')
     self.mox.StubOutWithMock(gcl.upload, 'RealMain')
     # These are not tested.
@@ -29,25 +29,21 @@ class GclUnittest(GclTestsBase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-      'CODEREVIEW_SETTINGS', 'CODEREVIEW_SETTINGS_FILE',
-      'Change', 'ChangeInfo', 'Changes', 'Commit',
-      'DEFAULT_LINT_IGNORE_REGEX', 'DEFAULT_LINT_REGEX',
-      'DeleteEmptyChangeLists', 'DoPresubmitChecks',
-      'ErrorExit', 'FILES_CACHE', 'FilterFlag', 'GenerateChangeName',
-      'GenerateDiff',
-      'GetCacheDir', 'GetCachedFile', 'GetChangesDir', 'GetCLs',
-      'GetChangelistInfoFile', 'GetCodeReviewSetting', 'GetEditor',
-      'GetFilesNotInCL', 'GetInfoDir', 'GetIssueDescription',
-      'GetModifiedFiles', 'GetRepositoryRoot',
-      'GetSVNFileProperty', 'Help', 'IsSVNMoved',
-      'Lint', 'LoadChangelistInfoForMultiple',
-      'MISSING_TEST_MSG', 'Opened', 'OptionallyDoPresubmitChecks',
-      'PresubmitCL', 'ReadFile', 'REPOSITORY_ROOT', 'RunShell',
-      'RunShellWithReturnCode', 'SendToRietveld', 'TryChange',
-      'UnknownFiles', 'UploadCL', 'Warn', 'WriteFile',
-      'gclient_scm', 'gclient_utils', 'getpass', 'main', 'os', 'random', 're',
-      'shutil', 'string', 'subprocess', 'sys', 'tempfile',
-      'upload', 'urllib2',
+        'CODEREVIEW_SETTINGS', 'CODEREVIEW_SETTINGS_FILE', 'Change',
+        'ChangeInfo', 'Changes', 'Commit', 'DEFAULT_LINT_IGNORE_REGEX',
+        'DEFAULT_LINT_REGEX', 'DeleteEmptyChangeLists', 'DoPresubmitChecks',
+        'ErrorExit', 'FILES_CACHE', 'FilterFlag', 'GenerateChangeName',
+        'GenerateDiff', 'GetCLs', 'GetCacheDir', 'GetCachedFile',
+        'GetChangelistInfoFile', 'GetChangesDir', 'GetCodeReviewSetting',
+        'GetEditor', 'GetFilesNotInCL', 'GetInfoDir', 'GetIssueDescription',
+        'GetModifiedFiles', 'GetRepositoryRoot', 'Help', 'Lint',
+        'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG', 'Opened',
+        'OptionallyDoPresubmitChecks', 'PresubmitCL', 'REPOSITORY_ROOT',
+        'ReadFile', 'RunShell', 'RunShellWithReturnCode', 'SVN',
+        'SendToRietveld', 'TryChange', 'UnknownFiles', 'UploadCL', 'Warn',
+        'WriteFile', 'gclient_utils', 'getpass', 'main', 'os', 'random', 're',
+        'shutil', 'string', 'subprocess', 'sys', 'tempfile', 'upload',
+        'urllib2',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(gcl, members)
@@ -70,8 +66,7 @@ class GclUnittest(GclTestsBase):
     result = {
       "Repository Root": ""
     }
-    gcl.gclient_scm.CaptureSVNInfo("/bleh/prout", print_error=False).AndReturn(
-        result)
+    gcl.SVN.CaptureInfo("/bleh/prout", print_error=False).AndReturn(result)
     self.mox.ReplayAll()
     self.assertRaises(Exception, gcl.GetRepositoryRoot)
 
@@ -80,13 +75,11 @@ class GclUnittest(GclTestsBase):
     root_path = gcl.os.path.join('bleh', 'prout', 'pouet')
     gcl.os.getcwd().AndReturn(root_path)
     result1 = { "Repository Root": "Some root" }
-    gcl.gclient_scm.CaptureSVNInfo(root_path,
-                                   print_error=False).AndReturn(result1)
+    gcl.SVN.CaptureInfo(root_path, print_error=False).AndReturn(result1)
     gcl.os.getcwd().AndReturn(root_path)
     results2 = { "Repository Root": "A different root" }
-    gcl.gclient_scm.CaptureSVNInfo(
-        gcl.os.path.dirname(root_path),
-        print_error=False).AndReturn(results2)
+    gcl.SVN.CaptureInfo(gcl.os.path.dirname(root_path),
+                        print_error=False).AndReturn(results2)
     self.mox.ReplayAll()
     self.assertEquals(gcl.GetRepositoryRoot(), root_path)
 

tests/gclient_scm_test.py

@@ -33,12 +33,12 @@ class BaseTestCase(GCBaseTestCase):
     self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'FileWrite')
     self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'SubprocessCall')
     self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'RemoveDirectory')
-    self._CaptureSVNInfo = gclient_scm.CaptureSVNInfo
-    self.mox.StubOutWithMock(gclient_scm, 'CaptureSVN')
-    self.mox.StubOutWithMock(gclient_scm, 'CaptureSVNInfo')
-    self.mox.StubOutWithMock(gclient_scm, 'CaptureSVNStatus')
-    self.mox.StubOutWithMock(gclient_scm, 'RunSVN')
-    self.mox.StubOutWithMock(gclient_scm, 'RunSVNAndGetFileList')
+    self._CaptureSVNInfo = gclient_scm.scm.SVN.CaptureInfo
+    self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'Capture')
+    self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'CaptureInfo')
+    self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'CaptureStatus')
+    self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'Run')
+    self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'RunAndGetFileList')
     self._scm_wrapper = gclient_scm.CreateSCM
 
 
@@ -64,9 +64,11 @@ class SVNWrapperTestCase(BaseTestCase):
 
   def testDir(self):
     members = [
-      'FullUrlForRelativeUrl', 'RunCommand', 'cleanup', 'diff', 'export',
-      'pack', 'relpath', 'revert', 'revinfo', 'runhooks', 'scm_name', 'status',
-      'update', 'url',
+        'COMMAND', 'Capture', 'CaptureHeadRevision', 'CaptureInfo',
+        'CaptureStatus', 'DiffItem', 'FullUrlForRelativeUrl', 'GetFileProperty',
+        'IsMoved', 'Run', 'RunAndFilterOutput', 'RunAndGetFileList',
+        'RunCommand', 'cleanup', 'diff', 'export', 'pack', 'relpath', 'revert',
+        'revinfo', 'runhooks', 'scm_name', 'status', 'update', 'url',
     ]
 
     # If you add a member, be sure to add the relevant test!
@@ -113,8 +115,9 @@ class SVNWrapperTestCase(BaseTestCase):
     # Checkout.
     gclient_scm.os.path.exists(base_path).AndReturn(False)
     files_list = self.mox.CreateMockAnything()
-    gclient_scm.RunSVNAndGetFileList(options, ['checkout', self.url, base_path],
-                                     self.root_dir, files_list)
+    gclient_scm.scm.SVN.RunAndGetFileList(options,
+                                          ['checkout', self.url, base_path],
+                                          self.root_dir, files_list)
 
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -125,9 +128,10 @@ class SVNWrapperTestCase(BaseTestCase):
     options = self.Options(verbose=True)
     base_path = gclient_scm.os.path.join(self.root_dir, self.relpath)
     gclient_scm.os.path.isdir(base_path).AndReturn(True)
-    gclient_scm.CaptureSVNStatus(base_path).AndReturn([])
-    gclient_scm.RunSVNAndGetFileList(options, ['update', '--revision', 'BASE'],
-                                     base_path, mox.IgnoreArg())
+    gclient_scm.scm.SVN.CaptureStatus(base_path).AndReturn([])
+    gclient_scm.scm.SVN.RunAndGetFileList(options,
+                                          ['update', '--revision', 'BASE'],
+                                          base_path, mox.IgnoreArg())
 
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -145,15 +149,16 @@ class SVNWrapperTestCase(BaseTestCase):
     ]
     file_path1 = gclient_scm.os.path.join(base_path, 'a')
     file_path2 = gclient_scm.os.path.join(base_path, 'b')
-    gclient_scm.CaptureSVNStatus(base_path).AndReturn(items)
+    gclient_scm.scm.SVN.CaptureStatus(base_path).AndReturn(items)
     gclient_scm.os.path.exists(file_path1).AndReturn(True)
     gclient_scm.os.path.isfile(file_path1).AndReturn(True)
     gclient_scm.os.remove(file_path1)
     gclient_scm.os.path.exists(file_path2).AndReturn(True)
     gclient_scm.os.path.isfile(file_path2).AndReturn(True)
     gclient_scm.os.remove(file_path2)
-    gclient_scm.RunSVNAndGetFileList(options, ['update', '--revision', 'BASE'],
-                                     base_path, mox.IgnoreArg())
+    gclient_scm.scm.SVN.RunAndGetFileList(options,
+                                          ['update', '--revision', 'BASE'],
+                                          base_path, mox.IgnoreArg())
     print(gclient_scm.os.path.join(base_path, 'a'))
     print(gclient_scm.os.path.join(base_path, 'b'))
 
@@ -170,7 +175,7 @@ class SVNWrapperTestCase(BaseTestCase):
     items = [
       ('~      ', 'a'),
     ]
-    gclient_scm.CaptureSVNStatus(base_path).AndReturn(items)
+    gclient_scm.scm.SVN.CaptureStatus(base_path).AndReturn(items)
     file_path = gclient_scm.os.path.join(base_path, 'a')
     print(file_path)
     gclient_scm.os.path.exists(file_path).AndReturn(True)
@@ -178,8 +183,9 @@ class SVNWrapperTestCase(BaseTestCase):
     gclient_scm.os.path.isdir(file_path).AndReturn(True)
     gclient_scm.gclient_utils.RemoveDirectory(file_path)
     file_list1 = []
-    gclient_scm.RunSVNAndGetFileList(options, ['update', '--revision', 'BASE'],
-                                     base_path, mox.IgnoreArg())
+    gclient_scm.scm.SVN.RunAndGetFileList(options,
+                                          ['update', '--revision', 'BASE'],
+                                          base_path, mox.IgnoreArg())
 
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -191,8 +197,9 @@ class SVNWrapperTestCase(BaseTestCase):
     options = self.Options(verbose=True)
     base_path = gclient_scm.os.path.join(self.root_dir, self.relpath)
     gclient_scm.os.path.isdir(base_path).AndReturn(True)
-    gclient_scm.RunSVNAndGetFileList(options, ['status'] + self.args,
-                                     base_path, []).AndReturn(None)
+    gclient_scm.scm.SVN.RunAndGetFileList(options,
+                                          ['status'] + self.args,
+                                          base_path, []).AndReturn(None)
 
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -216,8 +223,9 @@ class SVNWrapperTestCase(BaseTestCase):
     # Checkout.
     gclient_scm.os.path.exists(base_path).AndReturn(False)
     files_list = self.mox.CreateMockAnything()
-    gclient_scm.RunSVNAndGetFileList(options, ['checkout', self.url,
-                                     base_path], self.root_dir, files_list)
+    gclient_scm.scm.SVN.RunAndGetFileList(options,
+                                          ['checkout', self.url, base_path],
+                                          self.root_dir, files_list)
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
                             relpath=self.relpath)
@@ -238,17 +246,19 @@ class SVNWrapperTestCase(BaseTestCase):
                                ).AndReturn(False)
     # Checkout or update.
     gclient_scm.os.path.exists(base_path).AndReturn(True)
-    gclient_scm.CaptureSVNInfo(gclient_scm.os.path.join(base_path, "."), '.'
-                               ).AndReturn(file_info)
+    gclient_scm.scm.SVN.CaptureInfo(
+        gclient_scm.os.path.join(base_path, "."), '.'
+        ).AndReturn(file_info)
     # Cheat a bit here.
-    gclient_scm.CaptureSVNInfo(file_info['URL'], '.').AndReturn(file_info)
+    gclient_scm.scm.SVN.CaptureInfo(file_info['URL'], '.').AndReturn(file_info)
     additional_args = []
     if options.manually_grab_svn_rev:
       additional_args = ['--revision', str(file_info['Revision'])]
     files_list = []
-    gclient_scm.RunSVNAndGetFileList(options,
-                                     ['update', base_path] + additional_args,
-                                     self.root_dir, files_list)
+    gclient_scm.scm.SVN.RunAndGetFileList(
+        options,
+        ['update', base_path] + additional_args,
+        self.root_dir, files_list)
 
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -356,9 +366,9 @@ from :3
 
   def testDir(self):
     members = [
-      'FullUrlForRelativeUrl', 'RunCommand', 'cleanup', 'diff', 'export',
-      'relpath', 'revert', 'revinfo', 'runhooks', 'scm_name', 'status',
-      'update', 'url',
+        'COMMAND', 'Capture', 'CaptureStatus', 'FullUrlForRelativeUrl',
+        'RunCommand', 'cleanup', 'diff', 'export', 'relpath', 'revert',
+        'revinfo', 'runhooks', 'scm_name', 'status', 'update', 'url',
     ]
 
     # If you add a member, be sure to add the relevant test!

tests/gclient_test.py

@@ -22,8 +22,6 @@ import __builtin__
 import StringIO
 
 import gclient
-# Temporary due to the "from scm import *" in gclient_scm.
-import scm
 from super_mox import mox, IsOneOf, SuperMoxTestBase
 
 
@@ -50,16 +48,11 @@ class GClientBaseTestCase(BaseTestCase):
     self.mox.StubOutWithMock(gclient.gclient_utils, 'SubprocessCall')
     self.mox.StubOutWithMock(gclient.gclient_utils, 'RemoveDirectory')
     # Mock them to be sure nothing bad happens.
-    self.mox.StubOutWithMock(gclient.gclient_scm, 'CaptureSVN')
-    self.mox.StubOutWithMock(gclient.gclient_scm, 'CaptureSVNInfo')
-    self.mox.StubOutWithMock(gclient.gclient_scm, 'CaptureSVNStatus')
-    self.mox.StubOutWithMock(gclient.gclient_scm, 'RunSVN')
-    self.mox.StubOutWithMock(gclient.gclient_scm, 'RunSVNAndGetFileList')
-    self.mox.StubOutWithMock(scm, 'CaptureSVN')
-    self.mox.StubOutWithMock(scm, 'CaptureSVNInfo')
-    self.mox.StubOutWithMock(scm, 'CaptureSVNStatus')
-    self.mox.StubOutWithMock(scm, 'RunSVN')
-    self.mox.StubOutWithMock(scm, 'RunSVNAndGetFileList')
+    self.mox.StubOutWithMock(gclient.gclient_scm.scm.SVN, 'Capture')
+    self.mox.StubOutWithMock(gclient.gclient_scm.scm.SVN, 'CaptureInfo')
+    self.mox.StubOutWithMock(gclient.gclient_scm.scm.SVN, 'CaptureStatus')
+    self.mox.StubOutWithMock(gclient.gclient_scm.scm.SVN, 'Run')
+    self.mox.StubOutWithMock(gclient.gclient_scm.scm.SVN, 'RunAndGetFileList')
     self._gclient_gclient = gclient.GClient
     gclient.GClient = self.mox.CreateMockAnything()
     self._scm_wrapper = gclient.gclient_scm.CreateSCM

tests/revert_unittest.py

@@ -61,6 +61,7 @@ class RevertMainUnittest(RevertTestsBase):
 class RevertRevertUnittest(RevertTestsBase):
   def setUp(self):
     RevertTestsBase.setUp(self)
+    self.mox.StubOutWithMock(revert.gclient_scm.scm.SVN, 'CaptureStatus')
 
   def testRevert(self):
     revert.gcl.GetRepositoryRoot().AndReturn('foo')
@@ -73,7 +74,7 @@ class RevertRevertUnittest(RevertTestsBase):
     }]
     revert.CaptureSVNLog(['-r', '42', '-v']).AndReturn(entries)
     revert.GetRepoBase().AndReturn('proto://fqdn/repo/')
-    revert.gclient_scm.CaptureSVNStatus(['random_file']).AndReturn([])
+    revert.gclient_scm.scm.SVN.CaptureStatus(['random_file']).AndReturn([])
     revert.gcl.RunShell(['svn', 'up', 'random_file'])
     revert.os.path.isdir('random_file').AndReturn(False)
     status = """--- Reverse-merging r42 into '.':

tests/scm_unittest.py

@@ -5,9 +5,12 @@
 
 """Unit tests for scm.py."""
 
+import shutil
+import tempfile
+
 from gclient_test import BaseTestCase
 import scm
-from super_mox import mox
+from super_mox import mox, SuperMoxBaseTestBase
 
 
 class BaseSCMTestCase(BaseTestCase):
@@ -21,17 +24,14 @@ class RootTestCase(BaseSCMTestCase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-        'CaptureGit', 'CaptureGitStatus', 'GIT_COMMAND',
-        'CaptureSVN', 'CaptureSVNHeadRevision', 'CaptureSVNInfo',
-        'CaptureSVNStatus', 'RunSVN', 'RunSVNAndFilterOutput',
-        'RunSVNAndGetFileList', 'SVN_COMMAND',
-        'gclient_utils', 'os', 're', 'subprocess', 'sys', 'xml',
+        'GIT', 'SVN',
+        'gclient_utils', 'os', 're', 'subprocess', 'sys', 'tempfile', 'xml',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(scm, members)
 
 
-class GitWrapperTestCase(BaseSCMTestCase):
+class GitWrapperTestCase(SuperMoxBaseTestBase):
   sample_git_import = """blob
 mark :1
 data 6
@@ -80,30 +80,44 @@ from :3
 
   def CreateGitRepo(self, git_import, path):
     try:
-      subprocess.Popen(['git', 'init'], stdout=subprocess.PIPE,
-                       stderr=subprocess.STDOUT, cwd=path).communicate()
-    except WindowsError:
+      scm.subprocess.Popen(['git', 'init'],
+                           stdout=scm.subprocess.PIPE,
+                           stderr=scm.subprocess.STDOUT,
+                           cwd=path).communicate()
+    except OSError:
       # git is not available, skip this test.
       return False
-    subprocess.Popen(['git', 'fast-import'], stdin=subprocess.PIPE,
-                     stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-                     cwd=path).communicate(input=git_import)
-    subprocess.Popen(['git', 'checkout'], stdout=subprocess.PIPE,
-                     stderr=subprocess.STDOUT, cwd=path).communicate()
+    scm.subprocess.Popen(['git', 'fast-import'],
+                         stdin=scm.subprocess.PIPE,
+                         stdout=scm.subprocess.PIPE,
+                         stderr=scm.subprocess.STDOUT,
+                         cwd=path).communicate(input=git_import)
+    scm.subprocess.Popen(['git', 'checkout'],
+                         stdout=scm.subprocess.PIPE,
+                         stderr=scm.subprocess.STDOUT,
+                         cwd=path).communicate()
     return True
 
   def setUp(self):
-    BaseSCMTestCase.setUp(self)
+    SuperMoxBaseTestBase.setUp(self)
     self.args = self.Args()
     self.url = 'git://foo'
     self.root_dir = tempfile.mkdtemp()
     self.relpath = '.'
-    self.base_path = os.path.join(self.root_dir, self.relpath)
+    self.base_path = scm.os.path.join(self.root_dir, self.relpath)
     self.enabled = self.CreateGitRepo(self.sample_git_import, self.base_path)
 
   def tearDown(self):
     shutil.rmtree(self.root_dir)
-    gclient_test.BaseTestCase.tearDown(self)
+    SuperMoxBaseTestBase.tearDown(self)
+
+  def testMembersChanged(self):
+    self.mox.ReplayAll()
+    members = [
+        'COMMAND', 'Capture', 'CaptureStatus',
+    ]
+    # If this test fails, you should add the relevant test.
+    self.compareMembers(scm.GIT, members)
 
 
 class SVNTestCase(BaseSCMTestCase):
@@ -114,7 +128,17 @@ class SVNTestCase(BaseSCMTestCase):
     self.url = self.Url()
     self.relpath = 'asf'
 
-  def testGetSVNFileInfo(self):
+  def testMembersChanged(self):
+    self.mox.ReplayAll()
+    members = [
+        'COMMAND', 'Capture', 'CaptureHeadRevision', 'CaptureInfo',
+        'CaptureStatus', 'DiffItem', 'GetFileProperty', 'IsMoved', 'Run',
+        'RunAndFilterOutput', 'RunAndGetFileList',
+    ]
+    # If this test fails, you should add the relevant test.
+    self.compareMembers(scm.SVN, members)
+
+  def testGetFileInfo(self):
     xml_text = r"""<?xml version="1.0"?>
 <info>
 <entry kind="file" path="%s" revision="14628">
@@ -130,8 +154,8 @@ class SVNTestCase(BaseSCMTestCase):
 </entry>
 </info>
 """ % self.url
-    self.mox.StubOutWithMock(scm, 'CaptureSVN')
-    scm.CaptureSVN(['info', '--xml', self.url], '.', True).AndReturn(xml_text)
+    self.mox.StubOutWithMock(scm.SVN, 'Capture')
+    scm.SVN.Capture(['info', '--xml', self.url], '.', True).AndReturn(xml_text)
     expected = {
       'URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/d',
       'UUID': None,
@@ -145,10 +169,10 @@ class SVNTestCase(BaseSCMTestCase):
       'Node Kind': 'file',
     }
     self.mox.ReplayAll()
-    file_info = scm.CaptureSVNInfo(self.url, '.', True)
+    file_info = scm.SVN.CaptureInfo(self.url, '.', True)
     self.assertEquals(sorted(file_info.items()), sorted(expected.items()))
 
-  def testCaptureSvnInfo(self):
+  def testCaptureInfo(self):
     xml_text = """<?xml version="1.0"?>
 <info>
 <entry
@@ -172,10 +196,10 @@ class SVNTestCase(BaseSCMTestCase):
 </entry>
 </info>
 """ % (self.url, self.root_dir)
-    self.mox.StubOutWithMock(scm, 'CaptureSVN')
-    scm.CaptureSVN(['info', '--xml', self.url], '.', True).AndReturn(xml_text)
+    self.mox.StubOutWithMock(scm.SVN, 'Capture')
+    scm.SVN.Capture(['info', '--xml', self.url], '.', True).AndReturn(xml_text)
     self.mox.ReplayAll()
-    file_info = scm.CaptureSVNInfo(self.url, '.', True)
+    file_info = scm.SVN.CaptureInfo(self.url, '.', True)
     expected = {
       'URL': self.url,
       'UUID': '7b9385f5-0452-0410-af26-ad4892b7a1fb',
@@ -185,11 +209,11 @@ class SVNTestCase(BaseSCMTestCase):
       'Copied From URL': None,
       'Copied From Rev': None,
       'Path': '.',
-      'Node Kind': 'dir',
+      'Node Kind': 'directory',
     }
     self.assertEqual(file_info, expected)
 
-  def testCaptureSVNStatus(self):
+  def testCaptureStatus(self):
     text =r"""<?xml version="1.0"?>
 <status>
 <target path=".">
@@ -236,7 +260,7 @@ class SVNTestCase(BaseSCMTestCase):
     proc.communicate().AndReturn((text, 0))
 
     self.mox.ReplayAll()
-    info = scm.CaptureSVNStatus('.')
+    info = scm.SVN.CaptureStatus('.')
     expected = [
       ('?      ', 'unversionned_file.txt'),
       ('M      ', 'build\\internal\\essential.vsprops'),
@@ -246,14 +270,14 @@ class SVNTestCase(BaseSCMTestCase):
     ]
     self.assertEquals(sorted(info), sorted(expected))
 
-  def testRunSVN(self):
+  def testRun(self):
     param2 = 'bleh'
     scm.gclient_utils.SubprocessCall(['svn', 'foo', 'bar'],
                                      param2).AndReturn(None)
     self.mox.ReplayAll()
-    scm.RunSVN(['foo', 'bar'], param2)
+    scm.SVN.Run(['foo', 'bar'], param2)
 
-  def testCaptureSVNStatusEmpty(self):
+  def testCaptureStatusEmpty(self):
     text = r"""<?xml version="1.0"?>
     <status>
     <target
@@ -268,7 +292,7 @@ class SVNTestCase(BaseSCMTestCase):
                          stdout=scm.subprocess.PIPE).AndReturn(proc)
     proc.communicate().AndReturn((text, 0))
     self.mox.ReplayAll()
-    info = scm.CaptureSVNStatus(None)
+    info = scm.SVN.CaptureStatus(None)
     self.assertEquals(info, [])
 
 

tests/super_mox.py

@@ -71,7 +71,7 @@ class SuperMoxBaseTestBase(mox.MoxTestBase):
     if actual_members != expected_members:
       diff = ([i for i in actual_members if i not in expected_members] +
               [i for i in expected_members if i not in actual_members])
-      print diff
+      print>>sys.stderr, diff
     self.assertEqual(actual_members, expected_members)
 
   def UnMock(self, object, name):

tests/trychange_unittest.py

@@ -25,9 +25,9 @@ class TryChangeUnittest(TryChangeTestsBase):
       'GetTryServerSettings', 'GuessVCS',
       'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PathDifference',
       'RunCommand', 'SCM', 'SVN', 'TryChange', 'USAGE',
-      'datetime', 'gcl', 'gclient_scm', 'getpass', 'logging',
-      'optparse', 'os', 'presubmit_support', 'shutil', 'socket', 'subprocess',
-      'sys', 'tempfile', 'upload', 'urllib',
+      'datetime', 'gcl', 'getpass', 'logging',
+      'optparse', 'os', 'presubmit_support', 'scm', 'shutil', 'socket',
+      'subprocess', 'sys', 'tempfile', 'upload', 'urllib',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(trychange, members)

trychange.py

@@ -21,11 +21,11 @@ import tempfile
 import urllib
 
 import gcl
-import gclient_scm
+import scm
 import presubmit_support
 import upload
 
-__version__ = '1.1.1'
+__version__ = '1.1.2'
 
 
 # Constants
@@ -150,50 +150,8 @@ class SVN(SCM):
     else:
       os.chdir(root)
 
-    diff = []
-    for filename in files:
-      # Use svn info output instead of os.path.isdir because the latter fails
-      # when the file is deleted.
-      if gclient_scm.CaptureSVNInfo(filename).get("Node Kind") in (
-          "dir", "directory"):
-        continue
-      # If the user specified a custom diff command in their svn config file,
-      # then it'll be used when we do svn diff, which we don't want to happen
-      # since we want the unified diff.  Using --diff-cmd=diff doesn't always
-      # work, since they can have another diff executable in their path that
-      # gives different line endings.  So we use a bogus temp directory as the
-      # config directory, which gets around these problems.
-      if sys.platform.startswith("win"):
-        parent_dir = tempfile.gettempdir()
-      else:
-        parent_dir = sys.path[0]  # tempdir is not secure.
-      bogus_dir = os.path.join(parent_dir, "temp_svn_config")
-      if not os.path.exists(bogus_dir):
-        os.mkdir(bogus_dir)
-      # Grabs the diff data.
-      data = gcl.RunShell(["svn", "diff", "--config-dir", bogus_dir, filename])
-
-      # We know the diff will be incorrectly formatted. Fix it.
-      if gcl.IsSVNMoved(filename):
-        # The file is "new" in the patch sense. Generate a homebrew diff.
-        # We can't use ReadFile() since it's not using binary mode.
-        file_handle = open(filename, 'rb')
-        file_content = file_handle.read()
-        file_handle.close()
-        # Prepend '+' to every lines.
-        file_content = ['+' + i for i in file_content.splitlines(True)]
-        nb_lines = len(file_content)
-        # We need to use / since patch on unix will fail otherwise.
-        filename = filename.replace('\\', '/')
-        data = "Index: %s\n" % filename
-        data += ("============================================================="
-                 "======\n")
-        # Note: Should we use /dev/null instead?
-        data += "--- %s\n" % filename
-        data += "+++ %s\n" % filename
-        data += "@@ -0,0 +1,%d @@\n" % nb_lines
-        data += ''.join(file_content)
-      diff.append(data)
+    # Directories will return None so filter them out.
+    diff = filter(None, [scm.SVN.DiffItem(f) for f in files])
     os.chdir(previous_cwd)
     return "".join(diff)
 
@@ -407,6 +365,7 @@ def GuessVCS(options):
   Returns:
     A SCM instance. Exits if the SCM can't be guessed.
   """
+  __pychecker__ = 'no-returnvalues'
   # Subversion has a .svn in all working directories.
   if os.path.isdir('.svn'):
     logging.info("Guessed VCS = Subversion")