diff --git a/gclient.py b/gclient.py index 4867d261e..b35d98ee7 100755 --- a/gclient.py +++ b/gclient.py @@ -455,8 +455,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): ]) return s - - @property def requirements(self): """Calculate the list of requirements.""" @@ -676,7 +674,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): def _deps_to_objects(self, deps, use_relative_paths): """Convert a deps dict to a dict of Dependency objects.""" deps_to_add = [] - cipd_root = None for name, dep_value in deps.iteritems(): should_process = self.recursion_limit and self.should_process deps_file = self.deps_file @@ -709,14 +706,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): should_process = should_process and condition_value if dep_type == 'cipd': - if not cipd_root: - cipd_root = gclient_scm.CipdRoot( - os.path.join(self.root.root_dir, self.name), - # TODO(jbudorick): Support other service URLs as necessary. - # Service URLs should be constant over the scope of a cipd - # root, so a var per DEPS file specifying the service URL - # should suffice. - 'https://chrome-infra-packages.appspot.com') + cipd_root = self.GetCipdRoot() for package in dep_value.get('packages', []): if 'version' in package: # Matches version to vars value. @@ -1171,6 +1161,13 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): for hook in self.pre_deps_hooks: hook.run(self.root.root_dir) + def GetCipdRoot(self): + if self.root is self: + # Let's not infinitely recurse. If this is root and isn't an + # instance of GClient, do nothing. + return None + return self.root.GetCipdRoot() + def subtree(self, include_all): """Breadth first recursion excluding root node.""" dependencies = self.dependencies @@ -1399,6 +1396,7 @@ solutions = %(solution_list)s self._enforced_os = tuple(set(enforced_os)) self._enforced_cpu = detect_host_arch.HostArch(), self._root_dir = root_dir + self._cipd_root = None self.config_content = None def _CheckConfig(self): @@ -1637,6 +1635,9 @@ it or fix the checkout. print('Please fix your script, having invalid --revision flags will soon ' 'considered an error.', file=sys.stderr) + if self._cipd_root: + self._cipd_root.run(command) + # Once all the dependencies have been processed, it's now safe to write # out any gn_args_files and run the hooks. if command == 'update': @@ -1844,6 +1845,17 @@ it or fix the checkout. print('Loaded .gclient config in %s:\n%s' % ( self.root_dir, self.config_content)) + def GetCipdRoot(self): + if not self._cipd_root: + self._cipd_root = gclient_scm.CipdRoot( + self.root_dir, + # TODO(jbudorick): Support other service URLs as necessary. + # Service URLs should be constant over the scope of a cipd + # root, so a var per DEPS file specifying the service URL + # should suffice. + 'https://chrome-infra-packages.appspot.com') + return self._cipd_root + @property def root_dir(self): """Root directory of gclient checkout.""" @@ -1906,12 +1918,30 @@ class CipdDependency(Dependency): # TODO(jbudorick): Implement relative if necessary. raise gclient_utils.Error( 'Relative CIPD dependencies are not currently supported.') + self._cipd_package = None self._cipd_root = cipd_root - self._cipd_subdir = os.path.relpath( os.path.join(self.root.root_dir, name), cipd_root.root_dir) - self._cipd_package = self._cipd_root.add_package( - self._cipd_subdir, package, version) + self._package_name = package + self._package_version = version + + #override + def run(self, revision_overrides, command, args, work_queue, options): + """Runs |command| then parse the DEPS file.""" + logging.info('CipdDependency(%s).run()' % self.name) + if not self.should_process: + return + self._CreatePackageIfNecessary() + super(CipdDependency, self).run(revision_overrides, command, args, + work_queue, options) + + def _CreatePackageIfNecessary(self): + # We lazily create the CIPD package to make sure that only packages + # that we want (as opposed to all packages defined in all DEPS files + # we parse) get added to the root and subsequently ensured. + if not self._cipd_package: + self._cipd_package = self._cipd_root.add_package( + self._cipd_subdir, self._package_name, self._package_version) def ParseDepsFile(self): """CIPD dependencies are not currently allowed to have nested deps.""" @@ -1933,6 +1963,7 @@ class CipdDependency(Dependency): def CreateSCM(self, url, root_dir=None, relpath=None, out_fh=None, 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, @@ -1941,12 +1972,13 @@ class CipdDependency(Dependency): def ToLines(self): """Return a list of lines representing this in a DEPS file.""" s = [] + self._CreatePackageIfNecessary() if self._cipd_package.authority_for_subdir: condition_part = ([' "condition": %r,' % self.condition] if self.condition else []) s.extend([ ' # %s' % self.hierarchy(include_url=False), - ' "%s": {' % (self.name,), + ' "%s": {' % (self.name.split(':')[0],), ' "packages": [', ]) for p in self._cipd_root.packages(self._cipd_subdir): @@ -1956,6 +1988,7 @@ class CipdDependency(Dependency): ' "version": "%s",' % p.version, ' },', ]) + s.extend([ ' ],', ' "dep_type": "cipd",', diff --git a/gclient_scm.py b/gclient_scm.py index 12ea7b625..ce0ffd72f 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -1215,25 +1215,11 @@ class GitWrapper(SCMWrapper): class CipdPackage(object): """A representation of a single CIPD package.""" - def __init__(self, name, version, authority_for_root, authority_for_subdir): - self._authority_for_root = authority_for_root + def __init__(self, name, version, authority_for_subdir): self._authority_for_subdir = authority_for_subdir self._name = name self._version = version - @property - def authority_for_root(self): - """Whether this package has authority to act on behalf of its root. - - Some operations should only be performed once per cipd root. A package - that has authority for its cipd root is the only package that should - perform such operations. - - Returns: - bool; whether this package has root authority. - """ - return self._authority_for_root - @property def authority_for_subdir(self): """Whether this package has authority to act on behalf of its subdir. @@ -1284,7 +1270,6 @@ class CipdRoot(object): with self._mutator_lock: cipd_package = CipdPackage( package, version, - not self._packages_by_subdir, not self._packages_by_subdir[subdir]) self._all_packages.add(cipd_package) self._packages_by_subdir[subdir].append(cipd_package) @@ -1301,7 +1286,7 @@ class CipdRoot(object): packages. """ with self._mutator_lock: - cipd_cache_dir = os.path.join(self._cipd_root, '.cipd') + cipd_cache_dir = os.path.join(self.root_dir, '.cipd') try: gclient_utils.rmtree(os.path.join(cipd_cache_dir)) except OSError: @@ -1336,6 +1321,13 @@ class CipdRoot(object): ] gclient_utils.CheckCallAndFilterAndHeader(cmd) + def run(self, command): + if command == 'update': + self.ensure() + elif command == 'revert': + self.clobber() + self.ensure() + def created_package(self, package): """Checks whether this root created the given package. @@ -1385,10 +1377,12 @@ class CipdWrapper(SCMWrapper): return True def revert(self, options, args, file_list): - """Deletes .cipd and reruns ensure.""" - if self._package.authority_for_root: - self._root.clobber() - self._root.ensure() + """Does nothing. + + CIPD packages should be reverted at the root by running + `CipdRoot.run('revert')`. + """ + pass def diff(self, options, args, file_list): """CIPD has no notion of diffing.""" @@ -1422,6 +1416,9 @@ class CipdWrapper(SCMWrapper): pass def update(self, options, args, file_list): - """Runs ensure.""" - if self._package.authority_for_root: - self._root.ensure() + """Does nothing. + + CIPD packages should be updated at the root by running + `CipdRoot.run('update')`. + """ + pass diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 7dea9d938..dd9fc6790 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -929,27 +929,10 @@ class CipdWrapperTestCase(BaseTestCase): self.fail('Unable to find a satisfactory package.') - def testSingleRootAuthority(self): - """Checks that exactly one package has root authority.""" - self.assertEquals(1, len([p for p in self._cipd_packages - if p.authority_for_root])) - def testRevert(self): - """Checks that revert w/ root authority clobbers and reruns ensure.""" - scm = self.createScmWithPackageThatSatisfies( - lambda p: p.authority_for_root) - - self._cipd_root.clobber() - self._cipd_root.ensure() - - self.mox.ReplayAll() - - scm.revert(None, (), []) - - def testRevertWithoutAuthority(self): - """Checks that revert w/o root authority does nothing.""" + """Checks that revert does nothing.""" scm = self.createScmWithPackageThatSatisfies( - lambda p: not p.authority_for_root) + lambda _: True) self.mox.ReplayAll() @@ -991,20 +974,9 @@ class CipdWrapperTestCase(BaseTestCase): self.assertEquals(revinfo, expected_revinfo) def testUpdate(self): - """Checks that update w/ root authority runs ensure.""" - scm = self.createScmWithPackageThatSatisfies( - lambda p: p.authority_for_root) - - self._cipd_root.ensure() - - self.mox.ReplayAll() - - scm.update(None, (), []) - - def testUpdateWithoutAuthority(self): - """Checks that update w/o root authority does nothing.""" + """Checks that update does nothing.""" scm = self.createScmWithPackageThatSatisfies( - lambda p: not p.authority_for_root) + lambda _: True) self.mox.ReplayAll() diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index 270a7329c..2b8dcf4e5 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -1265,7 +1265,7 @@ class GClientSmokeGIT(GClientSmokeBase): ' },', '', ' # src -> src/cipd_dep:package0', - ' "src/cipd_dep:package0": {', + ' "src/cipd_dep": {', ' "packages": [', ' {', ' "package": "package0",',