* Re: [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)
@ 2013-11-28 14:47 99% ` Brian Dolbec
0 siblings, 0 replies; 1+ results
From: Brian Dolbec @ 2013-11-28 14:47 UTC (permalink / raw
To: gentoo-portage-dev; +Cc: Sebastian Luther
[-- Attachment #1: Type: text/plain, Size: 6425 bytes --]
I like this one much better, thank you.
Could you rebase the 2 patches together, tis one shows removing lines
from the first. Also see inline comments below :)
On Thu, 2013-11-28 at 11:34 +0100, SebastianLuther@gmx.de wrote:
> From: Sebastian Luther <SebastianLuther@gmx.de>
>
> This time rebuilds are scheduled properly, but we
> might still forget to install the package that caused
> the rebuild.
>
> URL: https://bugs.gentoo.org/486580
> ---
> pym/_emerge/depgraph.py | 48 +++++++++++------
> .../tests/resolver/test_slot_conflict_rebuild.py | 63 ++++++++++++++++++++++
> 2 files changed, 95 insertions(+), 16 deletions(-)
>
> diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
> index da2e604..0a998b5 100644
> --- a/pym/_emerge/depgraph.py
> +++ b/pym/_emerge/depgraph.py
> @@ -2268,6 +2268,36 @@ class depgraph(object):
> finally:
> self._dynamic_config._autounmask = _autounmask_backup
>
> + def _ignore_dependency(self, atom, pkg, child, dep, mypriority, recurse_satisfied):
> + """
> + In some cases, dep_check will return deps that shouldn't
> + be proccessed any further, so they are identified and
> + discarded here. Try to discard as few as possible since
> + discarded dependencies reduce the amount of information
> + available for optimization of merge order.
> + Don't ignore dependencies if pkg has a slot operator dependency on the child
> + and the child has changed slot/sub_slot.
> + """
> + slot_operator_rebuild = False
> + if atom.slot_operator == '=' and \
> + (pkg.root, pkg.slot_atom) in self._dynamic_config._slot_operator_replace_installed and \
> + mypriority.satisfied and \
> + mypriority.satisfied is not child and \
> + mypriority.satisfied.installed and \
> + not child.installed and \
> + (child.slot != mypriority.satisfied.slot or child.sub_slot != mypriority.satisfied.sub_slot):
> + slot_operator_rebuild = True
> +
> + return not atom.blocker and \
> + not recurse_satisfied and \
> + mypriority.satisfied and \
> + mypriority.satisfied.visible and \
> + dep.child is not None and \
> + not dep.child.installed and \
> + self._dynamic_config._slot_pkg_map[dep.child.root].get(
> + dep.child.slot_atom) is None and \
> + not slot_operator_rebuild
> +
Since slot_operator_rebuild is already set either true or false, would
it be better if "not slot_operator_rebuild" is first in the return
statement. If it resolves False, it will be an early out for the
remaining checks. Personally I don't know which is the more common
case. But if it resolve False more often, it will be a slight speed
improvement. That goes for the rest of them too, they should be ordered
in the most often to resolve False order. Same for the if as well.
/end tiny nitpick
> def _wrapped_add_pkg_dep_string(self, pkg, dep_root, dep_priority,
> dep_string, allow_unsatisfied):
> depth = pkg.depth + 1
> @@ -2357,14 +2387,7 @@ class depgraph(object):
> # discarded dependencies reduce the amount of information
> # available for optimization of merge order.
> ignored = False
> - if not atom.blocker and \
> - not recurse_satisfied and \
> - mypriority.satisfied and \
> - mypriority.satisfied.visible and \
> - dep.child is not None and \
> - not dep.child.installed and \
> - self._dynamic_config._slot_pkg_map[dep.child.root].get(
> - dep.child.slot_atom) is None:
> + if self._ignore_dependency(atom, pkg, child, dep, mypriority, recurse_satisfied):
> myarg = None
> try:
> myarg = next(self._iter_atoms_for_pkg(dep.child), None)
> @@ -2467,14 +2490,7 @@ class depgraph(object):
> collapsed_parent=pkg, collapsed_priority=dep_priority)
>
> ignored = False
> - if not atom.blocker and \
> - not recurse_satisfied and \
> - mypriority.satisfied and \
> - mypriority.satisfied.visible and \
> - dep.child is not None and \
> - not dep.child.installed and \
> - self._dynamic_config._slot_pkg_map[dep.child.root].get(
> - dep.child.slot_atom) is None:
> + if self._ignore_dependency(atom, pkg, child, dep, mypriority, recurse_satisfied):
> myarg = None
> try:
> myarg = next(self._iter_atoms_for_pkg(dep.child), None)
> diff --git a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
> index 74f5cc1..e3c517d 100644
> --- a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
> +++ b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
> @@ -181,6 +181,69 @@ class SlotConflictRebuildTestCase(TestCase):
> finally:
> playground.cleanup()
>
> + def testSlotConflictForgottenChild(self):
> + """
> + Similar to testSlotConflictMassRebuild above, but this time the rebuilds are scheduled,
> + but the package causing the rebuild (the child) is not installed.
> + """
> + ebuilds = {
> +
> + "app-misc/A-2" : {
> + "EAPI": "5",
> + "DEPEND": "app-misc/B:= app-misc/C",
> + "RDEPEND": "app-misc/B:= app-misc/C",
> + },
> +
> + "app-misc/B-2" : {
> + "EAPI": "5",
> + "SLOT": "2"
> + },
> +
> + "app-misc/C-1": {
> + "EAPI": "5",
> + "DEPEND": "app-misc/B:=",
> + "RDEPEND": "app-misc/B:="
> + },
> + }
> +
> + installed = {
> + "app-misc/A-1" : {
> + "EAPI": "5",
> + "DEPEND": "app-misc/B:1/1= app-misc/C",
> + "RDEPEND": "app-misc/B:1/1= app-misc/C",
> + },
> +
> + "app-misc/B-1" : {
> + "EAPI": "5",
> + "SLOT": "1"
> + },
> +
> + "app-misc/C-1": {
> + "EAPI": "5",
> + "DEPEND": "app-misc/B:1/1=",
> + "RDEPEND": "app-misc/B:1/1="
> + },
> + }
> +
> + test_cases = (
> + ResolverPlaygroundTestCase(
> + ["app-misc/A"],
> + success = True,
> + mergelist = ['app-misc/B-2', 'app-misc/C-1', 'app-misc/A-2']),
> + )
> +
> + world = []
> +
> + playground = ResolverPlayground(ebuilds=ebuilds,
> + installed=installed, world=world, debug=False)
> + try:
> + for test_case in test_cases:
> + playground.run_TestCase(test_case)
> + self.assertEqual(test_case.test_success, True, test_case.fail_msg)
> + finally:
> + playground.cleanup()
> +
> +
> def testSlotConflictDepChange(self):
> """
> Bug 490362
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [relevance 99%]
Results 1-1 of 1 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2013-11-28 10:34 [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2) SebastianLuther
2013-11-28 14:47 99% ` Brian Dolbec
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox