From babd098f3684f1bc965a0f72f96cb701eec91e52 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Fri, 11 May 2018 13:32:37 -0400 Subject: [PATCH] gclient: Simplify GetScmName and CreateSCM. Bug: 839925 Change-Id: Ibf97acf3c74b6f406904e14545e13497c680b883 Reviewed-on: https://chromium-review.googlesource.com/1054852 Reviewed-by: Marc-Antoine Ruel Commit-Queue: Edward Lesmes --- gclient.py | 55 ++++++++++++++++--------------------------- tests/gclient_test.py | 23 +++++++++--------- 2 files changed, 32 insertions(+), 46 deletions(-) diff --git a/gclient.py b/gclient.py index b3de242f07..b95979f20e 100755 --- a/gclient.py +++ b/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.""" diff --git a/tests/gclient_test.py b/tests/gclient_test.py index bf0a621744..996346dfcd 100755 --- a/tests/gclient_test.py +++ b/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')