Prefer overridden dependencies.

recipe_engine/package.py

 # Copyright 2015 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 import ast
 import collections
 import contextlib
 import copy
 import difflib
 import functools
 import itertools
 import logging
 import operator
 import os
 import subprocess
 import sys
 import tempfile
 
 from . import env
 
 from google.protobuf import text_format
 from . import package_pb2
 from . import fetch
 
 
 class InconsistentDependencyGraphError(Exception):
   def __init__(self, project_id, specs):
+    super(InconsistentDependencyGraphError, self).__init__(
+        'Package specs for %s do not match: %s vs %s' % (
+            project_id, specs[0], specs[1]))
     self.project_id = project_id
     self.specs = specs
 
-  def __str__(self):
-    return 'Package specs for %s do not match: %s vs %s' % (
-        project_id, self.specs[0], self.specs[1])
-
 
 class CyclicDependencyError(Exception):
   pass
 
 
 def cleanup_pyc(path):
   """Removes any .pyc files from |path|'s directory tree.
 
   This ensures we always use the fresh code.
   """
@@ skipping 300 matching lines @@
       return False
     return self._proto_file == other._proto_file
 
 
 class Package(object):
   """Package represents a loaded package, and contains path and dependency
   information.
 
   This is accessed by loader.py through RecipeDeps.get_package.
   """
-  def __init__(self, name, repo_spec, deps, repo_root, relative_recipes_dir):
+  def __init__(self, name, repo_spec, deps, repo_root, relative_recipes_dir,
+               is_override):
     self.name = name
     self.repo_spec = repo_spec
     self.deps = deps
     self.repo_root = repo_root
     self.relative_recipes_dir = relative_recipes_dir
+    self.is_override = is_override
 
   def __repr__(self):
-    return '<Package(name=%r,repo_spec=%r,deps=%r,recipes_dir=%r)>' % (
-        self.name, self.repo_spec, self.deps, self.recipes_dir)
+    return ('<Package(name=%r,repo_spec=%r,deps=%r,recipes_dir=%r,'
+            'override=%r)>' % (self.name, self.repo_spec, self.deps,
+                               self.recipes_dir, self.is_override))
 
   @property
   def recipes_dir(self):
     return os.path.join(self.repo_root, self.relative_recipes_dir)
 
   @property
   def recipe_dirs(self):
     return [os.path.join(self.recipes_dir, 'recipes')]
 
   @property
   def module_dirs(self):
     return [os.path.join(self.recipes_dir, 'recipe_modules')]
 
   def find_dep(self, dep_name):
     if dep_name == self.name:
       return self
 
     assert dep_name in self.deps, (
         '%s does not exist or is not declared as a dependency of %s' % (
             dep_name, self.name))
     return self.deps[dep_name]
 
   def module_path(self, module_name):
     return os.path.join(self.recipes_dir, 'recipe_modules', module_name)
 
   def __repr__(self):
-    return 'Package(%r, %r, %r, %r)' % (
-        self.name, self.repo_spec, self.deps, self.recipe_dirs)
+    return 'Package(%r, %r, %r, %r, %r)' % (
+        self.name, self.repo_spec, self.deps, self.recipe_dirs,
+        self.is_override)
 
   def __str__(self):
     return 'Package %s, with dependencies %s' % (self.name, self.deps.keys())
 
 
 class RollCandidate(object):
   """RollCandidate represents a recipe roll candidate, i.e. updates
   to pinned revisions of recipe dependencies.
 
   This is mostly used by recipes.py autoroll command.
@@ skipping 18 matching lines @@
     """Attempts to make the after-roll dependency graph consistent by rolling
     other package dependencies (changing their revisions). A consistent
     dependency graph means that all of the repos in the graph are pinned
     at the same revision.
 
     Returns True on success.
     """
     while True:
       try:
         package_deps = PackageDeps(self._context)
-        package_deps._create_from_spec(root_spec, self.get_rolled_spec())
+        package_deps._create_from_spec(root_spec, self.get_rolled_spec(), False)
         return True
       except InconsistentDependencyGraphError as e:
         # Don't update the same project twice - that'd mean we have two
         # conflicting updates anyway.
         if e.project_id in self._updates:
           return False
 
         # Get the spec that is different from the one we already have.
         # The order in which they're returned is not guaranteed.
         current_revision = self._package_spec.deps[e.project_id].revision
@@ skipping 137 matching lines @@
 
   def __ne__(self, other):
     return not self.__eq__(other)
 
 
 class PackageDeps(object):
   """An object containing all the transitive dependencies of the root package.
   """
   def __init__(self, context, overrides=None):
     self._context = context
+    self._overrides = overrides or {}
     self._packages = {}
-    self._overrides = overrides or {}
     self._root_package = None
 
   @property
   def root_package(self):
     return self._root_package
 
   @classmethod
   def create(cls, repo_root, proto_file, deps_path=None, allow_fetch=False,
              overrides=None):
     """Creates a PackageDeps object.
 
     Arguments:
       repo_root: the root of the repository containing this package.
       proto_file: a ProtoFile object corresponding to the repos recipes.cfg
       allow_fetch: whether to fetch dependencies rather than just checking for
                    them.
       overrides: if not None, a dictionary of project overrides. Dictionary keys
                  are the `project_id` field to override, and dictionary values
                  are the override path.
     """
     context = PackageContext.from_proto_file(repo_root, proto_file, allow_fetch,
                                              deps_path=deps_path)
 
     if overrides:
       overrides = {project_id: PathRepoSpec(path)
                    for project_id, path in overrides.iteritems()}
     package_deps = cls(context, overrides=overrides)
 
-    package_deps._root_package = package_deps._create_package(RootRepoSpec(proto_file))
+    # Install our overrides and their dependencies first. Each of these will be
+    # marked as overriding, meaning that they will supercede equivalent packages
+    # defined in the root package without error.
+    #
+    # Package resolution is required to be consistent within the overridden
+    # packages, and required to be consistent within the non-overridden
+    # packages, but a conflict between an overridden package and a
+    # non-overridden package will prefer the overridden one.
+    package_deps._register_overrides()
+
+    # Install our root package and its dependencies.
+    package_deps._root_package = package_deps._create_package(
+        None, RootRepoSpec(proto_file), False)
 
     return package_deps
 
-  def _create_package(self, repo_spec):
+  def _register_overrides(self):
+    for project_id, repo_spec in self._overrides.iteritems():
+      self._create_package(project_id, repo_spec, True)
+
+  def _create_package(self, package_id, repo_spec, overriding):
     repo_spec.checkout(self._context)
     package_spec = PackageSpec.load_proto(repo_spec.proto_file(self._context))
-    return self._create_from_spec(repo_spec, package_spec)
+    return self._create_from_spec(repo_spec, package_spec, overriding)
 
-  def _create_from_spec(self, repo_spec, package_spec):
+  def _create_from_spec(self, repo_spec, package_spec, overriding):
     project_id = package_spec.project_id
-    repo_spec = self._overrides.get(project_id, repo_spec)
+
     if project_id in self._packages:
+      current = self._packages[project_id]
+
       # TODO(phajdan.jr): Are exceptions the best way to report these errors?
       # The way this is used in practice, especially inconsistent dependency
       # graph condition, might be considered as using exceptions for control
       # flow.

[iannucci, 2016/09/16 07:17:21]

can remove this comment I think. inconsistent graph is an exceptional condition.
Handling an error doesn't count as 'using exceptions for control flow'.

[dnj, 2016/09/16 22:32:05]

Done.

-      if self._packages[project_id] is None:
+      if current is None:
         raise CyclicDependencyError(
             'Package %s depends on itself' % project_id)
-      if repo_spec != self._packages[project_id].repo_spec:
+
+      # Only enforce package consistency within the override boundary.
+      if current.is_override == overriding and repo_spec != current.repo_spec:
         raise InconsistentDependencyGraphError(
-            project_id, (repo_spec, self._packages[project_id].repo_spec))
+            project_id, (repo_spec, current.repo_spec))
     self._packages[project_id] = None
 
     deps = {}
     for dep, dep_repo in sorted(package_spec.deps.items()):
-      dep_repo = self._overrides.get(dep, dep_repo)
-      deps[dep] = self._create_package(dep_repo)
+      dep_package = self._packages.get(dep)
+      if not (dep_package and dep_package.is_override):
+        dep_package = self._create_package(dep, dep_repo, overriding)
+      deps[dep] = dep_package
 
     package = Package(
         project_id, repo_spec, deps,
         repo_spec.repo_root(self._context),
-        package_spec.recipes_path)
+        package_spec.recipes_path,
+        overriding)
 
     self._packages[project_id] = package
     return package
 
   # TODO(luqui): Remove this, so all accesses to packages are done
   # via other packages with properly scoped deps.
   def get_package(self, package_id):
     return self._packages[package_id]
 
   @property
@@ skipping 12 matching lines @@
   >>> d = { 'x': 1, 'y': 2 }
   >>> sorted(_updated(d, { 'y': 3, 'z': 4 }).items())
   [('x', 1), ('y', 3), ('z', 4)]
   >>> sorted(d.items())
   [('x', 1), ('y', 2)]
   """
 
   d = copy.copy(d)
   d.update(updates)
   return d