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

Side by Side Diff: recipe_engine/package.py

Issue 2343733003: Prefer overridden dependencies. (Closed)
Patch Set: Remove old comment. 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 unified diff | Download patch
« no previous file with comments | « no previous file | recipes.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright 2015 The LUCI Authors. All rights reserved. 1 # Copyright 2015 The LUCI Authors. All rights reserved.
2 # Use of this source code is governed under the Apache License, Version 2.0 2 # Use of this source code is governed under the Apache License, Version 2.0
3 # that can be found in the LICENSE file. 3 # that can be found in the LICENSE file.
4 4
5 import ast 5 import ast
6 import collections 6 import collections
7 import contextlib 7 import contextlib
8 import copy 8 import copy
9 import difflib 9 import difflib
10 import functools 10 import functools
11 import itertools 11 import itertools
12 import logging 12 import logging
13 import operator 13 import operator
14 import os 14 import os
15 import subprocess 15 import subprocess
16 import sys 16 import sys
17 import tempfile 17 import tempfile
18 18
19 from . import env 19 from . import env
20 20
21 from google.protobuf import text_format 21 from google.protobuf import text_format
22 from . import package_pb2 22 from . import package_pb2
23 from . import fetch 23 from . import fetch
24 24
25 25
26 class InconsistentDependencyGraphError(Exception): 26 class InconsistentDependencyGraphError(Exception):
27 def __init__(self, project_id, specs): 27 def __init__(self, project_id, specs):
28 super(InconsistentDependencyGraphError, self).__init__(
29 'Package specs for %s do not match: %s vs %s' % (
30 project_id, specs[0], specs[1]))
28 self.project_id = project_id 31 self.project_id = project_id
29 self.specs = specs 32 self.specs = specs
30 33
31 def __str__(self):
32 return 'Package specs for %s do not match: %s vs %s' % (
33 project_id, self.specs[0], self.specs[1])
34
35 34
36 class CyclicDependencyError(Exception): 35 class CyclicDependencyError(Exception):
37 pass 36 pass
38 37
39 38
40 def cleanup_pyc(path): 39 def cleanup_pyc(path):
41 """Removes any .pyc files from |path|'s directory tree. 40 """Removes any .pyc files from |path|'s directory tree.
42 41
43 This ensures we always use the fresh code. 42 This ensures we always use the fresh code.
44 """ 43 """
(...skipping 300 matching lines...) Expand 10 before | Expand all | Expand 10 after
345 return False 344 return False
346 return self._proto_file == other._proto_file 345 return self._proto_file == other._proto_file
347 346
348 347
349 class Package(object): 348 class Package(object):
350 """Package represents a loaded package, and contains path and dependency 349 """Package represents a loaded package, and contains path and dependency
351 information. 350 information.
352 351
353 This is accessed by loader.py through RecipeDeps.get_package. 352 This is accessed by loader.py through RecipeDeps.get_package.
354 """ 353 """
355 def __init__(self, name, repo_spec, deps, repo_root, relative_recipes_dir): 354 def __init__(self, name, repo_spec, deps, repo_root, relative_recipes_dir,
355 is_override):
356 self.name = name 356 self.name = name
357 self.repo_spec = repo_spec 357 self.repo_spec = repo_spec
358 self.deps = deps 358 self.deps = deps
359 self.repo_root = repo_root 359 self.repo_root = repo_root
360 self.relative_recipes_dir = relative_recipes_dir 360 self.relative_recipes_dir = relative_recipes_dir
361 self.is_override = is_override
361 362
362 def __repr__(self): 363 def __repr__(self):
363 return '<Package(name=%r,repo_spec=%r,deps=%r,recipes_dir=%r)>' % ( 364 return ('<Package(name=%r,repo_spec=%r,deps=%r,recipes_dir=%r,'
364 self.name, self.repo_spec, self.deps, self.recipes_dir) 365 'override=%r)>' % (self.name, self.repo_spec, self.deps,
366 self.recipes_dir, self.is_override))
365 367
366 @property 368 @property
367 def recipes_dir(self): 369 def recipes_dir(self):
368 return os.path.join(self.repo_root, self.relative_recipes_dir) 370 return os.path.join(self.repo_root, self.relative_recipes_dir)
369 371
370 @property 372 @property
371 def recipe_dirs(self): 373 def recipe_dirs(self):
372 return [os.path.join(self.recipes_dir, 'recipes')] 374 return [os.path.join(self.recipes_dir, 'recipes')]
373 375
374 @property 376 @property
375 def module_dirs(self): 377 def module_dirs(self):
376 return [os.path.join(self.recipes_dir, 'recipe_modules')] 378 return [os.path.join(self.recipes_dir, 'recipe_modules')]
377 379
378 def find_dep(self, dep_name): 380 def find_dep(self, dep_name):
379 if dep_name == self.name: 381 if dep_name == self.name:
380 return self 382 return self
381 383
382 assert dep_name in self.deps, ( 384 assert dep_name in self.deps, (
383 '%s does not exist or is not declared as a dependency of %s' % ( 385 '%s does not exist or is not declared as a dependency of %s' % (
384 dep_name, self.name)) 386 dep_name, self.name))
385 return self.deps[dep_name] 387 return self.deps[dep_name]
386 388
387 def module_path(self, module_name): 389 def module_path(self, module_name):
388 return os.path.join(self.recipes_dir, 'recipe_modules', module_name) 390 return os.path.join(self.recipes_dir, 'recipe_modules', module_name)
389 391
390 def __repr__(self): 392 def __repr__(self):
391 return 'Package(%r, %r, %r, %r)' % ( 393 return 'Package(%r, %r, %r, %r, %r)' % (
392 self.name, self.repo_spec, self.deps, self.recipe_dirs) 394 self.name, self.repo_spec, self.deps, self.recipe_dirs,
395 self.is_override)
393 396
394 def __str__(self): 397 def __str__(self):
395 return 'Package %s, with dependencies %s' % (self.name, self.deps.keys()) 398 return 'Package %s, with dependencies %s' % (self.name, self.deps.keys())
396 399
397 400
398 class RollCandidate(object): 401 class RollCandidate(object):
399 """RollCandidate represents a recipe roll candidate, i.e. updates 402 """RollCandidate represents a recipe roll candidate, i.e. updates
400 to pinned revisions of recipe dependencies. 403 to pinned revisions of recipe dependencies.
401 404
402 This is mostly used by recipes.py autoroll command. 405 This is mostly used by recipes.py autoroll command.
(...skipping 18 matching lines...) Expand all
421 """Attempts to make the after-roll dependency graph consistent by rolling 424 """Attempts to make the after-roll dependency graph consistent by rolling
422 other package dependencies (changing their revisions). A consistent 425 other package dependencies (changing their revisions). A consistent
423 dependency graph means that all of the repos in the graph are pinned 426 dependency graph means that all of the repos in the graph are pinned
424 at the same revision. 427 at the same revision.
425 428
426 Returns True on success. 429 Returns True on success.
427 """ 430 """
428 while True: 431 while True:
429 try: 432 try:
430 package_deps = PackageDeps(self._context) 433 package_deps = PackageDeps(self._context)
431 package_deps._create_from_spec(root_spec, self.get_rolled_spec()) 434 package_deps._create_from_spec(root_spec, self.get_rolled_spec(), False)
432 return True 435 return True
433 except InconsistentDependencyGraphError as e: 436 except InconsistentDependencyGraphError as e:
434 # Don't update the same project twice - that'd mean we have two 437 # Don't update the same project twice - that'd mean we have two
435 # conflicting updates anyway. 438 # conflicting updates anyway.
436 if e.project_id in self._updates: 439 if e.project_id in self._updates:
437 return False 440 return False
438 441
439 # Get the spec that is different from the one we already have. 442 # Get the spec that is different from the one we already have.
440 # The order in which they're returned is not guaranteed. 443 # The order in which they're returned is not guaranteed.
441 current_revision = self._package_spec.deps[e.project_id].revision 444 current_revision = self._package_spec.deps[e.project_id].revision
(...skipping 137 matching lines...) Expand 10 before | Expand all | Expand 10 after
579 582
580 def __ne__(self, other): 583 def __ne__(self, other):
581 return not self.__eq__(other) 584 return not self.__eq__(other)
582 585
583 586
584 class PackageDeps(object): 587 class PackageDeps(object):
585 """An object containing all the transitive dependencies of the root package. 588 """An object containing all the transitive dependencies of the root package.
586 """ 589 """
587 def __init__(self, context, overrides=None): 590 def __init__(self, context, overrides=None):
588 self._context = context 591 self._context = context
592 self._overrides = overrides or {}
589 self._packages = {} 593 self._packages = {}
590 self._overrides = overrides or {}
591 self._root_package = None 594 self._root_package = None
592 595
593 @property 596 @property
594 def root_package(self): 597 def root_package(self):
595 return self._root_package 598 return self._root_package
596 599
597 @classmethod 600 @classmethod
598 def create(cls, repo_root, proto_file, deps_path=None, allow_fetch=False, 601 def create(cls, repo_root, proto_file, deps_path=None, allow_fetch=False,
599 overrides=None): 602 overrides=None):
600 """Creates a PackageDeps object. 603 """Creates a PackageDeps object.
601 604
602 Arguments: 605 Arguments:
603 repo_root: the root of the repository containing this package. 606 repo_root: the root of the repository containing this package.
604 proto_file: a ProtoFile object corresponding to the repos recipes.cfg 607 proto_file: a ProtoFile object corresponding to the repos recipes.cfg
605 allow_fetch: whether to fetch dependencies rather than just checking for 608 allow_fetch: whether to fetch dependencies rather than just checking for
606 them. 609 them.
607 overrides: if not None, a dictionary of project overrides. Dictionary keys 610 overrides: if not None, a dictionary of project overrides. Dictionary keys
608 are the `project_id` field to override, and dictionary values 611 are the `project_id` field to override, and dictionary values
609 are the override path. 612 are the override path.
610 """ 613 """
611 context = PackageContext.from_proto_file(repo_root, proto_file, allow_fetch, 614 context = PackageContext.from_proto_file(repo_root, proto_file, allow_fetch,
612 deps_path=deps_path) 615 deps_path=deps_path)
613 616
614 if overrides: 617 if overrides:
615 overrides = {project_id: PathRepoSpec(path) 618 overrides = {project_id: PathRepoSpec(path)
616 for project_id, path in overrides.iteritems()} 619 for project_id, path in overrides.iteritems()}
617 package_deps = cls(context, overrides=overrides) 620 package_deps = cls(context, overrides=overrides)
618 621
619 package_deps._root_package = package_deps._create_package(RootRepoSpec(proto _file)) 622 # Install our overrides and their dependencies first. Each of these will be
623 # marked as overriding, meaning that they will supercede equivalent packages
624 # defined in the root package without error.
625 #
626 # Package resolution is required to be consistent within the overridden
627 # packages, and required to be consistent within the non-overridden
628 # packages, but a conflict between an overridden package and a
629 # non-overridden package will prefer the overridden one.
630 package_deps._register_overrides()
631
632 # Install our root package and its dependencies.
633 package_deps._root_package = package_deps._create_package(
634 None, RootRepoSpec(proto_file), False)
620 635
621 return package_deps 636 return package_deps
622 637
623 def _create_package(self, repo_spec): 638 def _register_overrides(self):
639 for project_id, repo_spec in self._overrides.iteritems():
640 self._create_package(project_id, repo_spec, True)
641
642 def _create_package(self, package_id, repo_spec, overriding):
martiniss 2016/09/19 19:07:42 nit: unused argument package_id
dnj 2016/09/19 20:43:50 Done.
624 repo_spec.checkout(self._context) 643 repo_spec.checkout(self._context)
625 package_spec = PackageSpec.load_proto(repo_spec.proto_file(self._context)) 644 package_spec = PackageSpec.load_proto(repo_spec.proto_file(self._context))
626 return self._create_from_spec(repo_spec, package_spec) 645 return self._create_from_spec(repo_spec, package_spec, overriding)
627 646
628 def _create_from_spec(self, repo_spec, package_spec): 647 def _create_from_spec(self, repo_spec, package_spec, overriding):
629 project_id = package_spec.project_id 648 project_id = package_spec.project_id
630 repo_spec = self._overrides.get(project_id, repo_spec) 649
631 if project_id in self._packages: 650 if project_id in self._packages:
632 # TODO(phajdan.jr): Are exceptions the best way to report these errors? 651 current = self._packages[project_id]
633 # The way this is used in practice, especially inconsistent dependency 652 if current is None:
634 # graph condition, might be considered as using exceptions for control
635 # flow.
636 if self._packages[project_id] is None:
637 raise CyclicDependencyError( 653 raise CyclicDependencyError(
638 'Package %s depends on itself' % project_id) 654 'Package %s depends on itself' % project_id)
639 if repo_spec != self._packages[project_id].repo_spec: 655
656 # Only enforce package consistency within the override boundary.
657 if current.is_override == overriding and repo_spec != current.repo_spec:
640 raise InconsistentDependencyGraphError( 658 raise InconsistentDependencyGraphError(
641 project_id, (repo_spec, self._packages[project_id].repo_spec)) 659 project_id, (repo_spec, current.repo_spec))
642 self._packages[project_id] = None 660 self._packages[project_id] = None
643 661
644 deps = {} 662 deps = {}
645 for dep, dep_repo in sorted(package_spec.deps.items()): 663 for dep, dep_repo in sorted(package_spec.deps.items()):
646 dep_repo = self._overrides.get(dep, dep_repo) 664 dep_package = self._packages.get(dep)
647 deps[dep] = self._create_package(dep_repo) 665 if not (dep_package and dep_package.is_override):
666 dep_package = self._create_package(dep, dep_repo, overriding)
667 deps[dep] = dep_package
648 668
649 package = Package( 669 package = Package(
650 project_id, repo_spec, deps, 670 project_id, repo_spec, deps,
651 repo_spec.repo_root(self._context), 671 repo_spec.repo_root(self._context),
652 package_spec.recipes_path) 672 package_spec.recipes_path,
673 overriding)
653 674
654 self._packages[project_id] = package 675 self._packages[project_id] = package
655 return package 676 return package
656 677
657 # TODO(luqui): Remove this, so all accesses to packages are done 678 # TODO(luqui): Remove this, so all accesses to packages are done
658 # via other packages with properly scoped deps. 679 # via other packages with properly scoped deps.
659 def get_package(self, package_id): 680 def get_package(self, package_id):
660 return self._packages[package_id] 681 return self._packages[package_id]
661 682
662 @property 683 @property
(...skipping 12 matching lines...) Expand all
675 >>> d = { 'x': 1, 'y': 2 } 696 >>> d = { 'x': 1, 'y': 2 }
676 >>> sorted(_updated(d, { 'y': 3, 'z': 4 }).items()) 697 >>> sorted(_updated(d, { 'y': 3, 'z': 4 }).items())
677 [('x', 1), ('y', 3), ('z', 4)] 698 [('x', 1), ('y', 3), ('z', 4)]
678 >>> sorted(d.items()) 699 >>> sorted(d.items())
679 [('x', 1), ('y', 2)] 700 [('x', 1), ('y', 2)]
680 """ 701 """
681 702
682 d = copy.copy(d) 703 d = copy.copy(d)
683 d.update(updates) 704 d.update(updates)
684 return d 705 return d
OLDNEW
« 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