gclient: Simplify GetScmName and CreateSCM.

Bug: 839925
Change-Id: Ibf97acf3c74b6f406904e14545e13497c680b883
Reviewed-on: https://chromium-review.googlesource.com/1054852
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

gclient.py

@@ -485,7 +485,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     if self.url is None:
       return
     url = raw_url = None
-    scm = self.CreateSCM(self.url, self.root.root_dir, self.name, self.outbuf)
+    scm = self.CreateSCM()
     if os.path.isdir(scm.checkout_path):
       revision = scm.revinfo(None, None, None)
       url = '%s@%s' % (gclient_utils.SplitUrlRevision(self.url)[0], revision)
@@ -1000,9 +1000,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       options = copy.copy(options)
       options.revision = revision_override
       self._used_revision = options.revision
-      self._used_scm = self.CreateSCM(
-          self.url, self.root.root_dir, self.name, self.outbuf,
-          out_cb=work_queue.out_cb)
+      self._used_scm = self.CreateSCM(out_cb=work_queue.out_cb)
       self._got_revision = self._used_scm.RunCommand(command, options, args,
                                                      file_list)
 
@@ -1043,7 +1041,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     if command == 'recurse':
       # Skip file only checkout.
-      scm = self.GetScmName(self.url)
+      scm = self.GetScmName()
       if not options.scm or scm in options.scm:
         cwd = os.path.normpath(os.path.join(self.root.root_dir, self.name))
         # Pass in the SCM type as an env variable.  Make sure we don't put
@@ -1097,11 +1095,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
         else:
           print('Skipped missing %s' % cwd, file=sys.stderr)
 
-  def GetScmName(self, url):
+  def GetScmName(self):
     raise NotImplementedError()
 
-  def CreateSCM(self, url, root_dir=None, relpath=None, out_fh=None,
-                out_cb=None):
+  def CreateSCM(self, out_cb=None):
     raise NotImplementedError()
 
   def HasGNArgsFile(self):
@@ -1139,14 +1136,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       # TODO(maruel): If the user is using git, then we don't know
       # what files have changed so we always run all hooks. It'd be nice to fix
       # that.
-      if (options.force or
-          self.GetScmName(self.url) in ('git', None) or
-          os.path.isdir(os.path.join(self.root.root_dir, self.name, '.git'))):
-        result.extend(self.deps_hooks)
-      else:
-        for hook in self.deps_hooks:
-          if hook.matches(self.file_list_and_children):
-            result.append(hook)
+      result.extend(self.deps_hooks)
     for s in self.dependencies:
       result.extend(s.GetHooks(options))
     return result
@@ -1371,17 +1361,16 @@ class GitDependency(Dependency):
   """A Dependency object that represents a single git checkout."""
 
   #override
-  def GetScmName(self, url):
+  def GetScmName(self):
     """Always 'git'."""
-    del url
     return 'git'
 
   #override
-  def CreateSCM(self, url, root_dir=None, relpath=None, out_fh=None,
-                out_cb=None):
+  def CreateSCM(self, out_cb=None):
     """Create a Wrapper instance suitable for handling this git dependency."""
-    return gclient_scm.GitWrapper(url, root_dir, relpath, out_fh, out_cb,
-                                  print_outbuf=self.print_outbuf)
+    return gclient_scm.GitWrapper(
+        self.url, self.root.root_dir, self.name, self.outbuf, out_cb,
+        print_outbuf=self.print_outbuf)
 
 
 class GClient(GitDependency):
@@ -1446,8 +1435,7 @@ solutions = %(solution_list)s
     solutions."""
     for dep in self.dependencies:
       if dep.managed and dep.url:
-        scm = dep.CreateSCM(
-            dep.url, self.root_dir, dep.name, self.outbuf)
+        scm = dep.CreateSCM()
         actual_url = scm.GetActualRemoteURL(self._options)
         if actual_url and not scm.DoesRemoteURLMatch(self._options):
           mirror = scm.GetCacheMirror()
@@ -1471,10 +1459,10 @@ You should ensure that the URL listed in .gclient is correct and either change
 it or fix the checkout.
 '''  % {'checkout_path': os.path.join(self.root_dir, dep.name),
         'expected_url': dep.url,
-        'expected_scm': self.GetScmName(dep.url),
+        'expected_scm': dep.GetScmName(),
         'mirror_string' : mirror_string,
         'actual_url': actual_url,
-        'actual_scm': self.GetScmName(actual_url)})
+        'actual_scm': dep.GetScmName()})
 
   def SetConfig(self, content):
     assert not self.dependencies
@@ -1742,8 +1730,8 @@ it or fix the checkout.
             (not any(path.startswith(entry + '/') for path in entries)) and
             os.path.exists(e_dir)):
           # The entry has been removed from DEPS.
-          scm = GitDependency.CreateSCM(
-              self, prev_url, self.root_dir, entry_fixed, self.outbuf)
+          scm = gclient_scm.GitWrapper(
+              prev_url, self.root_dir, entry_fixed, self.outbuf)
 
           # Check to see if this directory is now part of a higher-up checkout.
           scm_root = None
@@ -1991,20 +1979,17 @@ class CipdDependency(Dependency):
     return True
 
   #override
-  def GetScmName(self, url):
+  def GetScmName(self):
     """Always 'cipd'."""
-    del url
     return 'cipd'
 
   #override
-  def CreateSCM(self, url, root_dir=None, relpath=None, out_fh=None,
-                out_cb=None):
+  def CreateSCM(self, out_cb=None):
     """Create a Wrapper instance suitable for handling this CIPD dependency."""
     self._CreatePackageIfNecessary()
     return gclient_scm.CipdWrapper(
-        url, root_dir, relpath, out_fh, out_cb,
-        root=self._cipd_root,
-        package=self._cipd_package)
+        self.url, self.root.root_dir, self.name, self.outbuf, out_cb,
+        root=self._cipd_root, package=self._cipd_package)
 
   def ToLines(self):
     """Return a list of lines representing this in a DEPS file."""

tests/gclient_test.py

@@ -34,10 +34,15 @@ def write(filename, content):
 
 
 class SCMMock(object):
-  def __init__(self, unit_test, name, url):
-    self.unit_test = unit_test
+  unit_test = None
+  def __init__(self, parsed_url, root_dir, name, out_fh=None, out_cb=None,
+               print_outbuf=False):
+    self.unit_test.assertTrue(
+        parsed_url.startswith('svn://example.com/'), parsed_url)
+    self.unit_test.assertTrue(
+        root_dir.startswith(self.unit_test.root_dir), root_dir)
     self.name = name
-    self.url = url
+    self.url = parsed_url
 
   def RunCommand(self, command, options, args, file_list):
     self.unit_test.assertEquals('None', command)
@@ -58,24 +63,20 @@ class GclientTest(trial_dir.TestCase):
     self.previous_dir = os.getcwd()
     os.chdir(self.root_dir)
     # Manual mocks.
-    self._old_createscm = gclient.GitDependency.CreateSCM
-    gclient.GitDependency.CreateSCM = self._createscm
+    self._old_createscm = gclient.gclient_scm.GitWrapper
+    gclient.gclient_scm.GitWrapper = SCMMock
+    SCMMock.unit_test = self
     self._old_sys_stdout = sys.stdout
     sys.stdout = gclient.gclient_utils.MakeFileAutoFlush(sys.stdout)
     sys.stdout = gclient.gclient_utils.MakeFileAnnotated(sys.stdout)
 
   def tearDown(self):
     self.assertEquals([], self._get_processed())
-    gclient.GitDependency.CreateSCM = self._old_createscm
+    gclient.gclient_scm.GitWrapper = self._old_createscm
     sys.stdout = self._old_sys_stdout
     os.chdir(self.previous_dir)
     super(GclientTest, self).tearDown()
 
-  def _createscm(self, parsed_url, root_dir, name, out_fh=None, out_cb=None):
-    self.assertTrue(parsed_url.startswith('svn://example.com/'), parsed_url)
-    self.assertTrue(root_dir.startswith(self.root_dir), root_dir)
-    return SCMMock(self, name, parsed_url)
-
   def testDependencies(self):
     self._dependencies('1')