Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(77)

Unified Diff: recipe_engine/package.py

Issue 2343733003: Prefer overridden dependencies. (Closed)
Patch Set: Removed unused argument. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | recipes.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: recipe_engine/package.py
diff --git a/recipe_engine/package.py b/recipe_engine/package.py
index dd235ffedfbd55b677be3bffb0de9019a75633ab..4ada02ac0bd9c7ab3fce47e97ddb0c4d6ae958ef 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,58 @@ 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(
+ 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(repo_spec, True)
+
+ def _create_package(self, 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:
- # 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:
+ current = self._packages[project_id]
+ 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_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
« no previous file with comments | « no previous file | recipes.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698