Move mutation in run() at the end.

gclient.py

 #!/usr/bin/env python
 # Copyright (c) 2011 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Meta checkout manager supporting both Subversion and GIT.
 
 Files
   .gclient      : Current client configuration, written by 'config' command.
                   Format is a Python script defining 'solutions', a list whose
@@ skipping 328 matching lines @@
           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
       # Make sure the referenced dependency DEPS file is loaded and file the
       # inner referenced dependency.
+      # TODO(maruel): Shouldn't do that.
       ref.ParseDepsFile()
       found_dep = None
       for d in ref.dependencies:
         if d.name == sub_target:
           found_dep = d
           break
       if not found_dep:
         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,
@@ skipping 24 matching lines @@
       logging.info('LateOverride(%s, %s) -> %s' % (self.name, url, parsed_url))
       return parsed_url
 
     if isinstance(url, self.FileImpl):
       logging.info('LateOverride(%s, %s) -> %s (File)' % (self.name, url, url))
       return url
 
     if url is None:
       logging.info('LateOverride(%s, %s) -> %s' % (self.name, url, url))
       return url
-    else:
-      raise gclient_utils.Error('Unknown url type')
+
+    raise gclient_utils.Error('Unknown url type')
 
   def ParseDepsFile(self):
     """Parses the DEPS file for this dependency."""
-    assert self.processed == True
     if self.deps_parsed:
       logging.debug('%s was already parsed' % self.name)
       return
-    # One thing is unintuitive, vars= {} must happen before Var() use.
+    assert not self.dependencies
+    # One thing is unintuitive, vars = {} must happen before Var() use.
     local_scope = {}
     var = self.VarImpl(self.custom_vars, local_scope)
     global_scope = {
       'File': self.FileImpl,
       'From': self.FromImpl,
       'Var': var.Lookup,
       'deps_os': {},
     }
     filepath = os.path.join(self.root.root_dir, self.name, self.deps_file)
     if not os.path.isfile(filepath):
@@ skipping 52 matching lines @@
     self.add_dependencies_and_close(deps_to_add, local_scope.get('hooks', []))
     logging.info('ParseDepsFile(%s) done' % self.name)
 
   def add_dependencies_and_close(self, deps_to_add, hooks):
     """Adds the dependencies, hooks and mark the parsing as done."""
     for dep in deps_to_add:
       if dep.setup_requirements():
         self.add_dependency(dep)
     self._mark_as_parsed(hooks)
 
+  @staticmethod
+  def maybeGetParentRevision(
+      command, options, parsed_url, parent_name, revision_overrides):
+    """If we are performing an update and --transitive is set, set the
+    revision to the parent's revision. If we have an explicit revision
+    do nothing."""
+    if command == 'update' and options.transitive and not options.revision:
+      _, revision = gclient_utils.SplitUrlRevision(parsed_url)
+      if not revision:
+        options.revision = revision_overrides.get(parent_name)
+        if options.verbose and options.revision:
+          print("Using parent's revision date: %s" % options.revision)
+        # If the parent has a revision override, then it must have been
+        # converted to date format.
+        assert (not options.revision or
+                gclient_utils.IsDateRevision(options.revision))
+
+  @staticmethod
+  def maybeConvertToDateRevision(
+      command, options, name, scm, revision_overrides):
+    """If we are performing an update and --transitive is set, convert the
+    revision to a date-revision (if necessary). Instead of having
+    -r 101 replace the revision with the time stamp of 101 (e.g.
+    "{2011-18-04}").
+    This way dependencies are upgraded to the revision they had at the
+    check-in of revision 101."""
+    if (command == 'update' and
+        options.transitive and
+        options.revision and
+        not gclient_utils.IsDateRevision(options.revision)):
+      revision_date = scm.GetRevisionDate(options.revision)
+      revision = gclient_utils.MakeDateRevision(revision_date)
+      if options.verbose:
+        print("Updating revision override from %s to %s." %
+              (options.revision, revision))
+      revision_overrides[name] = revision
+
   # Arguments number differs from overridden method
   # pylint: disable=W0221
   def run(self, revision_overrides, command, args, work_queue, options):
-    """Runs 'command' before parsing the DEPS in case it's a initial checkout
-    or a revert."""
-
-    def maybeGetParentRevision(options):
-      """If we are performing an update and --transitive is set, set the
-      revision to the parent's revision. If we have an explicit revision
-      do nothing."""
-      if command == 'update' and options.transitive and not options.revision:
-        _, revision = gclient_utils.SplitUrlRevision(self.parsed_url)
-        if not revision:
-          options.revision = revision_overrides.get(self.parent.name)
-          if options.verbose and options.revision:
-            print("Using parent's revision date: %s" % options.revision)
-          # If the parent has a revision override, then it must have been
-          # converted to date format.
-          assert (not options.revision or
-                  gclient_utils.IsDateRevision(options.revision))
-
-    def maybeConvertToDateRevision(options):
-      """If we are performing an update and --transitive is set, convert the
-      revision to a date-revision (if necessary). Instead of having
-      -r 101 replace the revision with the time stamp of 101 (e.g.
-      "{2011-18-04}").
-      This way dependencies are upgraded to the revision they had at the
-      check-in of revision 101."""
-      if (command == 'update' and
-          options.transitive and
-          options.revision and
-          not gclient_utils.IsDateRevision(options.revision)):
-        revision_date = scm.GetRevisionDate(options.revision)
-        revision = gclient_utils.MakeDateRevision(revision_date)
-        if options.verbose:
-          print("Updating revision override from %s to %s." %
-                (options.revision, revision))
-        revision_overrides[self.name] = revision
-
+    """Runs |command| then parse the DEPS file."""
     assert self._file_list == []
     if not self.should_process:
       return
     # When running runhooks, there's no need to consult the SCM.
     # All known hooks are expected to run unconditionally regardless of working
     # copy state, so skip the SCM status check.
     run_scm = command not in ('runhooks', None)
-    self._parsed_url = self.LateOverride(self.url)
-    if run_scm and self.parsed_url:
-      if isinstance(self.parsed_url, self.FileImpl):
+    parsed_url = self.LateOverride(self.url)
+    file_list = []
+    if run_scm and parsed_url:
+      if isinstance(parsed_url, self.FileImpl):
         # Special support for single-file checkout.
         if not command in (None, 'cleanup', 'diff', 'pack', 'status'):
-          options.revision = self.parsed_url.GetRevision()
-          scm = gclient_scm.SVNWrapper(self.parsed_url.GetPath(),
+          # Sadly, pylint doesn't realize that parsed_url is of FileImpl.
+          # pylint: disable=E1103
+          options.revision = parsed_url.GetRevision()
+          scm = gclient_scm.SVNWrapper(parsed_url.GetPath(),
                                        self.root.root_dir,
                                        self.name)
           scm.RunCommand('updatesingle', options,
-                         args + [self.parsed_url.GetFilename()],
-                         self._file_list)
+                         args + [parsed_url.GetFilename()],
+                         file_list)
       else:
         # Create a shallow copy to mutate revision.
         options = copy.copy(options)
         options.revision = revision_overrides.get(self.name)
-        maybeGetParentRevision(options)
-        scm = gclient_scm.CreateSCM(
-            self.parsed_url, self.root.root_dir, self.name)
-        scm.RunCommand(command, options, args, self._file_list)
-        maybeConvertToDateRevision(options)
-        self._file_list = [os.path.join(self.name, f.strip())
-                           for f in self._file_list]
+        self.maybeGetParentRevision(
+            command, options, parsed_url, self.parent.name, revision_overrides)
+        scm = gclient_scm.CreateSCM(parsed_url, self.root.root_dir, self.name)
+        scm.RunCommand(command, options, args, file_list)
+        self.maybeConvertToDateRevision(
+            command, options, self.name, scm, revision_overrides)
+        file_list = [os.path.join(self.name, f.strip()) for f in file_list]
 
       # TODO(phajdan.jr): We should know exactly when the paths are absolute.
       # Convert all absolute paths to relative.
-      for i in range(len(self._file_list)):
+      for i in range(len(file_list)):
         # It depends on the command being executed (like runhooks vs sync).
-        if not os.path.isabs(self._file_list[i]):
+        if not os.path.isabs(file_list[i]):
           continue
         prefix = os.path.commonprefix(
-            [self.root.root_dir.lower(), self._file_list[i].lower()])
-        self._file_list[i] = self._file_list[i][len(prefix):]
+            [self.root.root_dir.lower(), file_list[i].lower()])
+        file_list[i] = file_list[i][len(prefix):]
         # Strip any leading path separators.
-        while (self._file_list[i].startswith('\\') or
-                self._file_list[i].startswith('/')):
-          self._file_list[i] = self._file_list[i][1:]
-    self._processed = True
+        while file_list[i].startswith(('\\', '/')):
+          file_list[i] = file_list[i][1:]
+
     if self.recursion_limit:
       # Then we can parse the DEPS file.
       self.ParseDepsFile()
 
+    self._run_is_done(file_list, parsed_url)
+
+    if self.recursion_limit:
       # Parse the dependencies of this dependency.
       for s in self.dependencies:
         work_queue.enqueue(s)
 
+  @gclient_utils.lockedmethod
+  def _run_is_done(self, file_list, parsed_url):
+    # Both these are kept for hooks that are run as a separate tree traversal.
+    self._file_list = file_list
+    self._parsed_url = parsed_url
+    self._processed = True
+
   def RunHooksRecursively(self, options):
     """Evaluates all hooks, running actions as needed. run()
     must have been called before to load the DEPS."""
     assert self.hooks_ran == False
     if not self.should_process or not self.recursion_limit:
       # Don't run the hook when it is above recursion_limit.
       return
     # If "--force" was specified, run all hooks regardless of what files have
     # changed.
     if self.deps_hooks:
       # TODO(maruel): If the user is using git or git-svn, 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
           isinstance(self.parsed_url, self.FileImpl) or
           gclient_scm.GetScmName(self.parsed_url) in ('git', None) or
           os.path.isdir(os.path.join(self.root.root_dir, self.name, '.git'))):
         for hook_dict in self.deps_hooks:
           self._RunHookAction(hook_dict, [])
       else:
         # Run hooks on the basis of whether the files from the gclient operation
         # match each hook's pattern.
         for hook_dict in self.deps_hooks:
           pattern = re.compile(hook_dict['pattern'])
-          matching_file_list = [f for f in self.file_list if pattern.search(f)]
+          matching_file_list = [
+              f for f in self.file_list_and_children if pattern.search(f)
+          ]
           if matching_file_list:
             self._RunHookAction(hook_dict, matching_file_list)
     for s in self.dependencies:
       s.RunHooksRecursively(options)
 
   def _RunHookAction(self, hook_dict, matching_file_list):
     """Runs the action from a single hook."""
     # A single DEPS file can specify multiple hooks so this function can be
     # called multiple times on a single Dependency.
     #assert self.hooks_ran == False
@@ skipping 47 matching lines @@
   def _mark_as_parsed(self, new_hooks):
     self._deps_hooks.extend(new_hooks)
     self._deps_parsed = True
 
   @property
   @gclient_utils.lockedmethod
   def dependencies(self):
     return tuple(self._dependencies)
 
   @property
+  @gclient_utils.lockedmethod
   def deps_hooks(self):
     return tuple(self._deps_hooks)
 
   @property
+  @gclient_utils.lockedmethod
   def parsed_url(self):
     return self._parsed_url
 
   @property
+  @gclient_utils.lockedmethod
   def deps_parsed(self):
     return self._deps_parsed
 
   @property
+  @gclient_utils.lockedmethod
   def processed(self):
     return self._processed
 
   @property
+  @gclient_utils.lockedmethod
   def hooks_ran(self):
     return self._hooks_ran
 
   @property
+  @gclient_utils.lockedmethod
   def file_list(self):
-    result = self._file_list[:]
+    return tuple(self._file_list)
+
+  @property
+  def file_list_and_children(self):
+    result = list(self.file_list)
     for d in self.dependencies:
-      result.extend(d.file_list)
+      result.extend(d.file_list_and_children)
     return tuple(result)
 
   def __str__(self):
     out = []
     for i in ('name', 'url', 'parsed_url', 'safesync_url', 'custom_deps',
               'custom_vars', 'deps_hooks', 'file_list', 'should_process',
               'processed', 'hooks_ran', 'deps_parsed', 'requirements'):
       # First try the native property if it exists.
       if hasattr(self, '_' + i):
         value = getattr(self, '_' + i, False)
@@ skipping 770 matching lines @@
   except (gclient_utils.Error, subprocess2.CalledProcessError), e:
     print >> sys.stderr, 'Error: %s' % str(e)
     return 1
 
 
 if '__main__' == __name__:
   fix_encoding.fix_encoding()
   sys.exit(Main(sys.argv[1:]))
 
 # vim: ts=2:sw=2:tw=80:et: