From b4f9d904734930d83bfbadcfc5d165becd6e3181 Mon Sep 17 00:00:00 2001 From: Andrii Shyshkalov Date: Tue, 23 May 2017 08:49:41 +0000 Subject: [PATCH] Revert "gclient: remove support for From()" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit bf72b593a2fac54e0ac7961be2706581ea47f7f9. Reason for revert: maybe caused outage. Original change's description: > gclient: remove support for From() > > This feature appears unused, and removing it will simplify the codebase. > > Bug: 661382 > Change-Id: I545befb2c592eea53c54552018ce2d3dda7670f5 > Reviewed-on: https://chromium-review.googlesource.com/509693 > Commit-Queue: Paweł Hajdan Jr. > Reviewed-by: Dirk Pranke > TBR=phajdan.jr@chromium.org,dpranke@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Bug: 661382 Change-Id: I4db9554a0a3a64a3a69908560b6da2a9963518f2 Reviewed-on: https://chromium-review.googlesource.com/512343 Reviewed-by: Andrii Shyshkalov Commit-Queue: Andrii Shyshkalov --- gclient.py | 57 ++++++++++++++++++++++++++++++++--- gclient_eval.py | 2 ++ gclient_utils.py | 3 +- testing_support/fake_repos.py | 9 ++++-- tests/gclient_test.py | 24 ++++++++++++++- 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/gclient.py b/gclient.py index 6873406fa..600c83ddb 100755 --- a/gclient.py +++ b/gclient.py @@ -158,6 +158,22 @@ def ast2str(node, indent=0): class GClientKeywords(object): + class FromImpl(object): + """Used to implement the From() syntax.""" + + def __init__(self, module_name, sub_target_name=None): + """module_name is the dep module we want to include from. It can also be + the name of a subdirectory to include from. + + sub_target_name is an optional parameter if the module name in the other + DEPS file is different. E.g., you might want to map src/net to net.""" + self.module_name = module_name + self.sub_target_name = sub_target_name + + def __str__(self): + return 'From(%s, %s)' % (repr(self.module_name), + repr(self.sub_target_name)) + class VarImpl(object): def __init__(self, custom_vars, local_scope): self._custom_vars = custom_vars @@ -210,10 +226,10 @@ class DependencySettings(GClientKeywords): # urls are sometime incorrectly written as proto://host/path/@rev. Replace # it to proto://host/path@rev. self._url = self._url.replace('/@', '@') - elif not isinstance(self._url, (None.__class__)): + elif not isinstance(self._url, (self.FromImpl, None.__class__)): raise gclient_utils.Error( - ('dependency url must be either string or None, ' - 'instead of %s') % self._url.__class__.__name__) + ('dependency url must be either a string, None, ' + 'or From() instead of %s') % self._url.__class__.__name__) # Make any deps_file path platform-appropriate. for sep in ['/', '\\']: self._deps_file = self._deps_file.replace(sep, os.sep) @@ -361,6 +377,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): if self.parent and self.parent.parent and not self.parent.parent.parent: requirements |= set(i.name for i in self.root.dependencies if i.name) + if isinstance(self.url, self.FromImpl): + requirements.add(self.url.module_name) + if self.name: requirements |= set( obj.name for obj in self.root.subtree(False) @@ -430,7 +449,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): return True def LateOverride(self, url): - """Resolves the parsed url from url.""" + """Resolves the parsed url from url. + + Manages From() keyword accordingly. Do not touch self.parsed_url nor + self.url because it may called with other urls due to From().""" assert self.parsed_url == None or not self.should_process, self.parsed_url parsed_url = self.get_custom_deps(self.name, url) if parsed_url != url: @@ -439,6 +461,32 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): (self.name, url, parsed_url)) return parsed_url + if isinstance(url, self.FromImpl): + # Requires tree traversal. + ref = [ + dep for dep in self.root.subtree(True) if url.module_name == dep.name + ] + if not ref: + raise gclient_utils.Error('Failed to find one reference to %s. %s' % ( + url.module_name, ref)) + # It may happen that len(ref) > 1 but it's no big deal. + ref = ref[0] + sub_target = url.sub_target_name or self.name + found_deps = [d for d in ref.dependencies if d.name == sub_target] + if len(found_deps) != 1: + raise gclient_utils.Error( + 'Couldn\'t find %s in %s, referenced by %s (parent: %s)\n%s' % ( + sub_target, ref.name, self.name, self.parent.name, + str(self.root))) + + # Call LateOverride() again. + found_dep = found_deps[0] + parsed_url = found_dep.LateOverride(found_dep.url) + logging.info( + 'Dependency(%s).LateOverride(%s) -> %s (From)' % + (self.name, url, parsed_url)) + return parsed_url + if isinstance(url, basestring): parsed_url = urlparse.urlparse(url) if (not parsed_url[0] and @@ -550,6 +598,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): } else: global_scope = { + 'From': self.FromImpl, 'Var': var.Lookup, 'deps_os': {}, } diff --git a/gclient_eval.py b/gclient_eval.py index 8a0d7fde1..ff13b97d8 100644 --- a/gclient_eval.py +++ b/gclient_eval.py @@ -33,6 +33,8 @@ _GCLIENT_SCHEMA = schema.Schema({ # # The following functions are allowed: # + # File(): specifies to expect to checkout a file instead of a directory + # From(): used to fetch a dependency definition from another DEPS file # Var(): allows variable substitution (either from 'vars' dict below, # or command-line override) schema.Optional('deps'): {schema.Optional(basestring): basestring}, diff --git a/gclient_utils.py b/gclient_utils.py index 8fe0c5ed8..d46dbd150 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -813,7 +813,8 @@ class ExecutionQueue(object): """Runs a set of WorkItem that have interdependencies and were WorkItem are added as they are processed. - This class manages that all the required dependencies are run + In gclient's case, Dependencies sometime needs to be run out of order due to + From() keyword. This class manages that all the required dependencies are run before running each one. Methods of this class are thread safe. diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index 6bd3dc569..1c7ae54c5 100755 --- a/testing_support/fake_repos.py +++ b/testing_support/fake_repos.py @@ -134,7 +134,7 @@ def wait_for_port_to_free(host, port): class FakeReposBase(object): """Generate git repositories to test gclient functionality. - Many DEPS functionalities need to be tested: Var, deps_os, hooks, + Many DEPS functionalities need to be tested: Var, File, From, deps_os, hooks, use_relative_paths. And types of dependencies: Relative urls, Full urls, git. @@ -309,7 +309,11 @@ class FakeRepos(FakeReposBase): # - deps_os # - var # - hooks + # - From # TODO(maruel): + # - File: File is hard to test here because it's SVN-only. It's + # implementation should probably be replaced to use urllib instead. + # - $matching_files # - use_relative_paths self._commit_git('repo_3', { 'origin': 'git/repo_3@1\n', @@ -368,7 +372,8 @@ deps = { 'DEPS': """ deps = { 'src/repo2': '%(git_base)srepo_2@%(hash)s', - 'src/repo2/repo_renamed': '/repo_3', + #'src/repo2/repo_renamed': '/repo_3', + 'src/repo2/repo_renamed': From('src/repo2', 'foo/bar'), } # I think this is wrong to have the hooks run from the base of the gclient # checkout. It's maybe a bit too late to change that behavior. diff --git a/tests/gclient_test.py b/tests/gclient_test.py index cde2c9826..0b0e6c0da 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -89,6 +89,8 @@ class GclientTest(trial_dir.TestCase): e.g. if there is a dependency 'src' and another 'src/third_party/bar', that bar isn't fetched until 'src' is done. + Also test that a From() dependency should not be processed when it is listed + as a requirement. Args: |jobs| is the number of parallel jobs simulated. @@ -109,6 +111,8 @@ class GclientTest(trial_dir.TestCase): # This one will depend on dir1/dir2 in bar. ' "foo/dir1/dir2/dir3": "/dir1/dir2/dir3",\n' ' "foo/dir1/dir2/dir3/dir4": "/dir1/dir2/dir3/dir4",\n' + ' "foo/dir1/dir2/dir5/dir6":\n' + ' From("foo/dir1/dir2/dir3/dir4", "foo/dir1/dir2"),\n' '}') write( os.path.join('bar', 'DEPS'), @@ -120,6 +124,15 @@ class GclientTest(trial_dir.TestCase): os.path.join('bar/empty', 'DEPS'), 'deps = {\n' '}') + # Test From() + write( + os.path.join('foo/dir1/dir2/dir3/dir4', 'DEPS'), + 'deps = {\n' + # This one should not be fetched or set as a requirement. + ' "foo/dir1/dir2/dir5": "svn://example.com/x",\n' + # This foo/dir1/dir2 points to a different url than the one in bar. + ' "foo/dir1/dir2": "/dir1/another",\n' + '}') obj = gclient.GClient.LoadCurrentConfig(options) self._check_requirements(obj.dependencies[0], {}) @@ -147,6 +160,8 @@ class GclientTest(trial_dir.TestCase): ('foo/dir1/dir2/dir3', 'svn://example.com/foo/dir1/dir2/dir3'), ('foo/dir1/dir2/dir3/dir4', 'svn://example.com/foo/dir1/dir2/dir3/dir4'), + ('foo/dir1/dir2/dir5/dir6', + 'svn://example.com/foo/dir1/dir2/dir3/dir4/dir1/another'), ], actual[3:]) @@ -163,6 +178,9 @@ class GclientTest(trial_dir.TestCase): 'foo/dir1/dir2/dir3/dir4': [ 'bar', 'bar/empty', 'foo', 'foo/dir1', 'foo/dir1/dir2', 'foo/dir1/dir2/dir3'], + 'foo/dir1/dir2/dir5/dir6': + [ 'bar', 'bar/empty', 'foo', 'foo/dir1', 'foo/dir1/dir2', + 'foo/dir1/dir2/dir3/dir4'], }) self._check_requirements( obj.dependencies[1], @@ -222,13 +240,17 @@ class GclientTest(trial_dir.TestCase): gclient.Dependency( obj.dependencies[0], 'foo/dir1', 'url', None, None, None, None, 'DEPS', True, False), + gclient.Dependency( + obj.dependencies[0], 'foo/dir2', + gclient.GClientKeywords.FromImpl('bar'), None, None, None, + None, 'DEPS', True, False), ], []) # Make sure __str__() works fine. # pylint: disable=protected-access obj.dependencies[0]._file_list.append('foo') str_obj = str(obj) - self.assertEquals(263, len(str_obj), '%d\n%s' % (len(str_obj), str_obj)) + self.assertEquals(370, len(str_obj), '%d\n%s' % (len(str_obj), str_obj)) def testHooks(self): topdir = self.root_dir