Move DEPS parsing into a single function. This is a move towards having each...

Move DEPS parsing into a single function. This is a move towards having each DEPS entry being a Dependency instance.

TEST=new revinfo unit tests

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

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

gclient.py

@@ -166,6 +166,8 @@ class Dependency(GClientKeywords):
     self.deps_hooks = []
     self.dependencies = []
     self.deps_file = deps_file or self.DEPS_FILE
+    self.deps_parsed = False
+    self.direct_reference = False
 
     # Sanity checks
     if not self.name and self.parent:
@@ -179,49 +181,37 @@ class Dependency(GClientKeywords):
       raise gclient_utils.Error('deps_file name must not be a path, just a '
                                 'filename. %s' % self.deps_file)
 
-  def _ParseSolutionDeps(self, solution_name, solution_deps_content,
-                         custom_vars, parse_hooks):
-    """Parses the DEPS file for the specified solution.
-
-    Args:
-      solution_name: The name of the solution to query.
-      solution_deps_content: Content of the DEPS file for the solution
-      custom_vars: A dict of vars to override any vars defined in the DEPS file.
-
-    Returns:
-      A dict mapping module names (as relative paths) to URLs or an empty
-      dict if the solution does not have a DEPS file.
-    """
-    # Skip empty
-    if not solution_deps_content:
+  def ParseDepsFile(self, direct_reference):
+    """Parses the DEPS file for this dependency."""
+    if direct_reference:
+      # Maybe it was referenced earlier by a From() keyword but it's now
+      # directly referenced.
+      self.direct_reference = direct_reference
+    self.deps_parsed = True
+    filepath = os.path.join(self.root_dir(), self.name, self.deps_file)
+    if not os.path.isfile(filepath):
       return {}
-    # Eval the content
+    deps_content = gclient_utils.FileRead(filepath)
+
+    # Eval the content.
+    # One thing is unintuitive, vars= {} must happen before Var() use.
     local_scope = {}
-    var = self.VarImpl(custom_vars, local_scope)
+    var = self.VarImpl(self.custom_vars, local_scope)
     global_scope = {
-      "File": self.FileImpl,
-      "From": self.FromImpl,
-      "Var": var.Lookup,
-      "deps_os": {},
+      'File': self.FileImpl,
+      'From': self.FromImpl,
+      'Var': var.Lookup,
+      'deps_os': {},
     }
-    exec(solution_deps_content, global_scope, local_scope)
-    deps = local_scope.get("deps", {})
-
+    exec(deps_content, global_scope, local_scope)
+    deps = local_scope.get('deps', {})
     # load os specific dependencies if defined.  these dependencies may
     # override or extend the values defined by the 'deps' member.
-    if "deps_os" in local_scope:
-      if self._options.deps_os is not None:
-        deps_to_include = self._options.deps_os.split(",")
-        if "all" in deps_to_include:
-          deps_to_include = list(set(self.DEPS_OS_CHOICES.itervalues()))
-      else:
-        deps_to_include = [self.DEPS_OS_CHOICES.get(sys.platform, "unix")]
-
-      deps_to_include = set(deps_to_include)
-      for deps_os_key in deps_to_include:
-        os_deps = local_scope["deps_os"].get(deps_os_key, {})
-        if len(deps_to_include) > 1:
-          # Ignore any overrides when including deps for more than one
+    if 'deps_os' in local_scope:
+      for deps_os_key in self.enforced_os():
+        os_deps = local_scope['deps_os'].get(deps_os_key, {})
+        if len(self.enforced_os()) > 1:
+          # Ignore any conflict when including deps for more than one
           # platform, so we collect the broadest set of dependencies available.
           # We may end up with the wrong revision of something for our
           # platform, but this is the best we can do.
@@ -229,36 +219,35 @@ class Dependency(GClientKeywords):
         else:
           deps.update(os_deps)
 
-    if 'hooks' in local_scope and parse_hooks:
-      # TODO(maruel): Temporary Hack. Since this function is misplaced, find the
-      # right 'self' to add the hooks.
-      for d in self.dependencies:
-        if d.name == solution_name:
-          d.deps_hooks.extend(local_scope['hooks'])
-          break
+    self.deps_hooks.extend(local_scope.get('hooks', []))
+
+    # If a line is in custom_deps, but not in the solution, we want to append
+    # this line to the solution.
+    for d in self.custom_deps:
+      if d not in deps:
+        deps[d] = self.custom_deps[d]
 
     # If use_relative_paths is set in the DEPS file, regenerate
     # the dictionary using paths relative to the directory containing
     # the DEPS file.
-    if local_scope.get('use_relative_paths'):
+    use_relative_paths = local_scope.get('use_relative_paths', False)
+    if use_relative_paths:
       rel_deps = {}
       for d, url in deps.items():
         # normpath is required to allow DEPS to use .. in their
         # dependency local path.
-        rel_deps[os.path.normpath(os.path.join(solution_name, d))] = url
-      return rel_deps
-    else:
+        rel_deps[os.path.normpath(os.path.join(self.name, d))] = url
+      deps = rel_deps
+    # TODO(maruel): Add these dependencies into self.dependencies.
     return deps
 
-  def _ParseAllDeps(self, solution_urls, solution_deps_content):
+  def _ParseAllDeps(self, solution_urls):
     """Parse the complete list of dependencies for the client.
 
     Args:
       solution_urls: A dict mapping module names (as relative paths) to URLs
         corresponding to the solutions specified by the client.  This parameter
         is passed as an optimization.
-      solution_deps_content: A dict mapping module names to the content
-        of their DEPS files
 
     Returns:
       A dict mapping module names (as relative paths) to URLs corresponding
@@ -269,11 +258,7 @@ class Dependency(GClientKeywords):
     """
     deps = {}
     for solution in self.dependencies:
-      solution_deps = self._ParseSolutionDeps(
-                              solution.name,
-                              solution_deps_content[solution.name],
-                              solution.custom_vars,
-                              True)
+      solution_deps = solution.ParseDepsFile(True)
 
       # If a line is in custom_deps, but not in the solution, we want to append
       # this line to the solution.
@@ -379,6 +364,12 @@ class Dependency(GClientKeywords):
       if matching_file_list:
         self._RunHookAction(hook_dict, matching_file_list)
 
+  def root_dir(self):
+    return self.parent.root_dir()
+
+  def enforced_os(self):
+    return self.parent.enforced_os()
+
 
 class GClient(Dependency):
   """Main gclient checkout root where .gclient resides."""
@@ -428,8 +419,15 @@ solutions = [
 
   def __init__(self, root_dir, options):
     Dependency.__init__(self, None, None, None)
-    self._root_dir = root_dir
     self._options = options
+    if options.deps_os:
+      enforced_os = options.deps_os.split(',')
+    else:
+      enforced_os = [self.DEPS_OS_CHOICES.get(sys.platform, 'unix')]
+    if 'all' in enforced_os:
+      enforced_os = self.DEPS_OS_CHOICES.itervalues()
+    self._enforced_os = list(set(enforced_os))
+    self._root_dir = root_dir
     self.config_content = None
 
   def SetConfig(self, content):
@@ -567,7 +565,6 @@ solutions = [
     run_scm = not (command == 'runhooks' and self._options.force)
 
     entries = {}
-    entries_deps_content = {}
     file_list = []
     # Run on the base solutions first.
     for solution in self.dependencies:
@@ -582,18 +579,10 @@ solutions = [
         scm.RunCommand(command, self._options, args, file_list)
         file_list = [os.path.join(name, f.strip()) for f in file_list]
         self._options.revision = None
-      try:
-        deps_content = gclient_utils.FileRead(
-            os.path.join(self.root_dir(), name, solution.deps_file))
-      except IOError, e:
-        if e.errno != errno.ENOENT:
-          raise
-        deps_content = ""
-      entries_deps_content[name] = deps_content
 
     # Process the dependencies next (sort alphanumerically to ensure that
     # containing directories get populated first and for readability)
-    deps = self._ParseAllDeps(entries, entries_deps_content)
+    deps = self._ParseAllDeps(entries)
     deps_to_process = deps.keys()
     deps_to_process.sort()
 
@@ -625,16 +614,12 @@ solutions = [
     # Second pass for inherited deps (via the From keyword)
     for d in deps_to_process:
       if isinstance(deps[d], self.FromImpl):
-        filename = os.path.join(self.root_dir(),
-                                deps[d].module_name,
-                                self.DEPS_FILE)
-        content =  gclient_utils.FileRead(filename)
-        sub_deps = self._ParseSolutionDeps(deps[d].module_name, content, {},
-                                           False)
         # Getting the URL from the sub_deps file can involve having to resolve
         # a File() or having to resolve a relative URL.  To resolve relative
         # URLs, we need to pass in the orignal sub deps URL.
         sub_deps_base_url = deps[deps[d].module_name]
+        sub_deps = Dependency(self, deps[d].module_name, sub_deps_base_url
+            ).ParseDepsFile(False)
         url = deps[d].GetUrl(d, sub_deps_base_url, self.root_dir(), sub_deps)
         entries[d] = url
         if run_scm:
@@ -722,7 +707,6 @@ solutions = [
     # Dictionary of { path : SCM url } to ensure no duplicate solutions
     solution_names = {}
     entries = {}
-    entries_deps_content = {}
     # Run on the base solutions first.
     for solution in self.dependencies:
       # Dictionary of { path : SCM url } to describe the gclient checkout
@@ -732,22 +716,10 @@ solutions = [
       (url, rev) = GetURLAndRev(name, solution.url)
       entries[name] = "%s@%s" % (url, rev)
       solution_names[name] = "%s@%s" % (url, rev)
-      deps_file = solution.deps_file
-      if '/' in deps_file or '\\' in deps_file:
-        raise gclient_utils.Error('deps_file name must not be a path, just a '
-                                  'filename.')
-      try:
-        deps_content = gclient_utils.FileRead(
-            os.path.join(self.root_dir(), name, deps_file))
-      except IOError, e:
-        if e.errno != errno.ENOENT:
-          raise
-        deps_content = ""
-      entries_deps_content[name] = deps_content
 
     # Process the dependencies next (sort alphanumerically to ensure that
     # containing directories get populated first and for readability)
-    deps = self._ParseAllDeps(entries, entries_deps_content)
+    deps = self._ParseAllDeps(entries)
     deps_to_process = deps.keys()
     deps_to_process.sort()
 
@@ -764,13 +736,11 @@ solutions = [
         if deps_parent_url.find("@") < 0:
           raise gclient_utils.Error("From %s missing revisioned url" %
                                         deps[d].module_name)
-        content =  gclient_utils.FileRead(os.path.join(
-                                            self.root_dir(),
-                                            deps[d].module_name,
-                                            self.DEPS_FILE))
-        sub_deps = self._ParseSolutionDeps(deps[d].module_name, content, {},
-                                           False)
-        (url, rev) = GetURLAndRev(d, sub_deps[d])
+        sub_deps_base_url = deps[deps[d].module_name]
+        sub_deps = Dependency(self, deps[d].module_name, sub_deps_base_url
+            ).ParseDepsFile(False)
+        url = deps[d].GetUrl(d, sub_deps_base_url, self.root_dir(), sub_deps)
+        (url, rev) = GetURLAndRev(d, url)
         entries[d] = "%s@%s" % (url, rev)
 
     # Build the snapshot configuration string
@@ -799,6 +769,9 @@ solutions = [
   def root_dir(self):
     return self._root_dir
 
+  def enforced_os(self):
+    return self._enforced_os
+
 
 #### gclient commands.
 

tests/fake_repos.py

@@ -337,8 +337,8 @@ class FakeRepos(object):
     # - deps_os
     # - var
     # - hooks
-    # TODO(maruel):
     # - From
+    # TODO(maruel):
     # - File
     # - $matching_files
     # - use_relative_paths
@@ -360,8 +360,8 @@ deps_os = {
     fs = file_system(2, """
 deps = {
   'src/other': 'svn://%(host)s/svn/trunk/other',
-  'src/third_party/foo': '/trunk/third_party/foo@1',
-  #'src/third_party/foo': From('src/other', 'foo/bar'),
+  #'src/third_party/foo': '/trunk/third_party/foo@1',
+  'src/third_party/foo': From('src/other', '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.
@@ -405,8 +405,8 @@ deps = {
     # - deps_os
     # - var
     # - hooks
-    # TODO(maruel):
     # - From
+    # TODO(maruel):
     # - File
     # - $matching_files
     # - use_relative_paths
@@ -467,8 +467,8 @@ deps = {
       'DEPS': """
 deps = {
   'src/repo2': 'git://%(host)s/git/repo_2@%(hash)s',
-  'src/repo2/repo_renamed': '/repo_3',
-  #'src/repo2/repo_renamed': From('src/repo2', 'foo/bar'),
+  #'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.

tests/gclient_smoketest.py

@@ -283,6 +283,8 @@ class GClientSmokeSVN(GClientSmokeBase):
     # matching.
     results = self.gclient(['revert', '--deps', 'mac'])
     out = self.splitBlock(results[0])
+    # src, src/other is missing, src/other, src/third_party/foo is missing,
+    # src/third_party/foo, 2 svn hooks.
     self.assertEquals(7, len(out))
     self.checkString('', results[1])
     self.assertEquals(0, results[2])
@@ -375,7 +377,6 @@ class GClientSmokeSVN(GClientSmokeBase):
     self.assertEquals([], out)
 
   def testRevInfo(self):
-    # TODO(maruel): Test multiple solutions.
     self.gclient(['config', self.svn_base + 'trunk/src/'])
     self.gclient(['sync', '--deps', 'mac'])
     results = self.gclient(['revinfo', '--deps', 'mac'])
@@ -586,6 +587,7 @@ class GClientSmokeBoth(GClientSmokeBase):
         '{"name": "src-git",'
         '"url": "' + self.git_base + 'repo_1"}]'])
     results = self.gclient(['sync', '--deps', 'mac'])
+    # 3x svn checkout, 3x run hooks
     self.checkBlock(results[0],
                     ['running', 'running', 'running', 'running', 'running',
                      'running', 'running'])
@@ -633,6 +635,31 @@ class GClientSmokeBoth(GClientSmokeBase):
         ('trunk/third_party/foo@2', 'src/third_party/prout')))
     self.assertTree(tree)
 
+  def testRevInfo(self):
+    if not self.enabled:
+      return
+    self.gclient(['config', '--spec',
+        'solutions=['
+        '{"name": "src",'
+        ' "url": "' + self.svn_base + 'trunk/src/"},'
+        '{"name": "src-git",'
+        '"url": "' + self.git_base + 'repo_1"}]'])
+    self.gclient(['sync', '--deps', 'mac'])
+    results = self.gclient(['revinfo', '--deps', 'mac'])
+    out = ('src: %(svn_base)s/src/@2;\n'
+           'src-git: %(git_base)srepo_1@%(hash1)s;\n'
+           'src/other: %(svn_base)s/other@2;\n'
+           'src/repo2: %(git_base)srepo_2@%(hash2)s;\n'
+           'src/repo2/repo_renamed: %(git_base)srepo_3@%(hash3)s;\n'
+           'src/third_party/foo: %(svn_base)s/third_party/foo@1\n') % {
+               'svn_base': self.svn_base + 'trunk',
+               'git_base': self.git_base,
+               'hash1': self.githash('repo_1', 2),
+               'hash2': self.githash('repo_2', 1),
+               'hash3': self.githash('repo_3', 2),
+          }
+    self.check((out, '', 0), results)
+
 
 if __name__ == '__main__':
   if '-c' in sys.argv: