Prefer overridden dependencies.

recipe_engine/package.py

Index: recipe_engine/package.py
diff --git a/recipe_engine/package.py b/recipe_engine/package.py
index dd235ffedfbd55b677be3bffb0de9019a75633ab..732227066adc345fffe31c1b98a55ac30eb8559b 100644
--- a/recipe_engine/package.py
+++ b/recipe_engine/package.py
@@ -25,13 +25,12 @@ 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
@@ -352,16 +351,19 @@ class Package(object):
 
   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):
@@ -388,8 +390,9 @@ class Package(object):
     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())
@@ -428,7 +431,7 @@ class RollCandidate(object):
     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
@@ -586,8 +589,8 @@ class PackageDeps(object):
   """
   def __init__(self, context, overrides=None):
     self._context = context
-    self._packages = {}
     self._overrides = overrides or {}
+    self._packages = {}
     self._root_package = None
 
   @property
@@ -616,40 +619,63 @@ class PackageDeps(object):
                    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