Add unit tests for overrides in multi-repo workflow

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 107 matching lines @@
   def checkout(self, context):
     """Fetches the specified package and returns the path of the package root
     (the directory that contains recipes and recipe_modules).
     """
     raise NotImplementedError()
 
   def repo_root(self, context):
     """Returns the root of this repository."""
     raise NotImplementedError()
 
-  def is_consistent_with(self, other):
-    """Returns True iff |other| can be used in place of |self| while keeping
-    the dependency graph consistent. Most often it means being pinned at the
-    same revision, but some specs (like path-based) are always compatible."""
-    raise NotImplementedError()
-
   def __eq__(self, other):
     raise NotImplementedError()
 
   def __ne__(self, other):
     return not (self == other)
 
   def proto_file(self, context):
     """Returns the ProtoFile of the recipes config file in this repository. 
     Requires a good checkout."""
     return ProtoFile(InfraRepoConfig().to_recipes_cfg(self.repo_root(context)))
@@ skipping 24 matching lines @@
 
   def checkout(self, context):
     checkout_dir = self._dep_dir(context)
     self.backend.checkout(
         self.repo, self.revision, checkout_dir, context.allow_fetch)
     cleanup_pyc(checkout_dir)
 
   def repo_root(self, context):
     return os.path.join(self._dep_dir(context), self.path)
 
-  def is_consistent_with(self, other):
-    return (self == other)
-
   def dump(self):
     buf = package_pb2.DepSpec(
         project_id=self.project_id,
         url=self.repo,
         branch=self.branch,
         revision=self.revision)
     if self.path:
       buf.path_override = self.path
 
     # Only dump repo_type if it's different from default. This preserves
@@ skipping 102 matching lines @@
   def proto_file(self, context):
     """Returns the ProtoFile of the recipes config file in this repository. 
     Requires a good checkout."""
     return ProtoFile(InfraRepoConfig().to_recipes_cfg(self.path))
 
   def __eq__(self, other):
     if not isinstance(other, type(self)):
       return False
     return self.path == other.path
 
-  def is_consistent_with(self, other):
-    # PathRepoSpec is always compatible, unless it's PathRepoSpec with a
-    # different path.
-    if isinstance(other, type(self)):
-      return self.path == other.path
-    return True
-
 
 class RootRepoSpec(RepoSpec):
   def __init__(self, proto_file):
     self._proto_file = proto_file
 
   def checkout(self, context):
     # We assume this is already checked out.
     pass
 
   def repo_root(self, context):
     return context.repo_root
 
-  def is_consistent_with(self, other):
-    return (self == other)
-
   def proto_file(self, context):
     return self._proto_file
 
   def __eq__(self, other):
     if not isinstance(other, type(self)):
       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,
-               is_override):
+  def __init__(self, name, repo_spec, deps, repo_root, relative_recipes_dir):
     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,'
-            'override=%r)>' % (self.name, self.repo_spec, self.deps,
-                               self.recipes_dir, self.is_override))
+    return '<Package(name=%r,repo_spec=%r,deps=%r,recipes_dir=%r)>' % (
+        self.name, self.repo_spec, self.deps, self.recipes_dir)
 
   @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, %r)' % (
-        self.name, self.repo_spec, self.deps, self.recipe_dirs,
-        self.is_override)
+    return 'Package(%r, %r, %r, %r)' % (
+        self.name, self.repo_spec, self.deps, self.recipe_dirs)
 
   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(), False)
+        package_deps._create_from_spec(root_spec, self.get_rolled_spec())
         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._packages = {}
     self._overrides = overrides or {}
-    self._packages = {}
     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)
 
-    # 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(
-        RootRepoSpec(proto_file), False)
+    package_deps._root_package = package_deps._create_package(RootRepoSpec(proto_file))
 
     return package_deps
 
-  def _register_overrides(self):
-    for project_id, repo_spec in self._overrides.iteritems():
-      self._create_package(repo_spec, True)
-
-  def _create_package(self, repo_spec, overriding):
+  def _create_package(self, repo_spec):
     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, overriding)
+    return self._create_from_spec(repo_spec, package_spec)
 
-  def _create_from_spec(self, repo_spec, package_spec, overriding):
+  def _create_from_spec(self, repo_spec, package_spec):
     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]
-      if current is None:
+      # 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.
+      if self._packages[project_id] is None:
         raise CyclicDependencyError(
             'Package %s depends on itself' % project_id)
-
-      # Only enforce package consistency within the override boundary.
-      # It's fine if either spec claims it's consistent with the other.
-      if (current.is_override == overriding and
-          not repo_spec.is_consistent_with(current.repo_spec) and
-          not current.repo_spec.is_consistent_with(repo_spec)):
+      if repo_spec != self._packages[project_id].repo_spec:
         raise InconsistentDependencyGraphError(
-            project_id, (repo_spec, current.repo_spec))
+            project_id, (repo_spec, self._packages[project_id].repo_spec))
     self._packages[project_id] = None
 
     deps = {}
     for dep, dep_repo in sorted(package_spec.deps.items()):
-      dep_package = self._packages.get(dep)
-      if not (dep_package and dep_package.is_override):
-        dep_package = self._create_package(dep_repo, overriding)
-      deps[dep] = dep_package
+      dep_repo = self._overrides.get(dep, dep_repo)
+      deps[dep] = self._create_package(dep_repo)
 
     package = Package(
         project_id, repo_spec, deps,
         repo_spec.repo_root(self._context),
-        package_spec.recipes_path,
-        overriding)
+        package_spec.recipes_path)
 
     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