Re: [gentoo-portage-dev] [PATCH] dep_zapdeps: exclude virtuals from new_slot_count (bug 645190)

[Alec Warner]

On Sat, Jan 20, 2018 at 7:10 PM, Zac Medico <zmedico@g.o> wrote:

> Fix new_slot_count to exclude virtual packages, since they are considered
> to have zero-cost. This solves an issue where the catalyst stage1 build
> would unexpectedly pull in static-dev to satisfy virtual/dev-manager,
> but eudev is the preferred choice.
>

> Bug: https://bugs.gentoo.org/645190
> Fixes: 9fdaf9bdbdf5 ("dep_check: use DNF to optimize overlapping virtual
> || deps (bug 632026)")
> Reported-by: Ben Kohler <bkohler@×××××.com>
> ---
>  pym/portage/dep/dep_check.py                       |  3 +-
>  .../resolver/test_virtual_minimize_children.py     | 61
> ++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/pym/portage/dep/dep_check.py b/pym/portage/dep/dep_check.py
> index 7cf338819..c56f545ec 100644
> --- a/pym/portage/dep/dep_check.py
> +++ b/pym/portage/dep/dep_check.py
> @@ -499,7 +499,8 @@ def dep_zapdeps(unreduced, reduced, myroot,
> use_binaries=0, trees=None):
>                                 cp_map[avail_pkg.cp] = avail_pkg
>
>                 new_slot_count = (len(slot_map) if graph_db is None else
> -                       sum(not graph_db.match_pkgs(slot_atom) for
> slot_atom in slot_map))
> +                       sum(not graph_db.match_pkgs(slot_atom) for
> slot_atom in slot_map
> +                       if not slot_atom.cp.startswith("virtual/")))
>

I see this logic all over dep_zapdeps. But AFAIK its already using
instances of Atom here.
Can't we encode the cost inside of Atom, so that we don't need to carve out
that virtual atoms are 0 cost?

Then we could write something like:

new_slot_count = len(slot_map) if graph_db is None else
  sum(slot_atom.cost() for slot_atom in slot_map if not
graph_db.match_pkgs(slot_atom))

Then virtuals would just have a cost() of 0. And others could have other
costs (or just 1.)


>
>                 this_choice = _dep_choice(atoms=atoms, slot_map=slot_map,
>                         cp_map=cp_map, all_available=all_available,
> diff --git a/pym/portage/tests/resolver/test_virtual_minimize_children.py
> b/pym/portage/tests/resolver/test_virtual_minimize_children.py
> index 6eb0409f2..287445e58 100644
> --- a/pym/portage/tests/resolver/test_virtual_minimize_children.py
> +++ b/pym/portage/tests/resolver/test_virtual_minimize_children.py
> @@ -226,3 +226,64 @@ class VirtualMinimizeChildrenTestCase(TestCase):
>                 finally:
>                         playground.debug = False
>                         playground.cleanup()
> +
> +       def testVirtualDevManager(self):
> +               ebuilds = {
> +                       'sys-fs/eudev-3.1.5': {},
> +                       'sys-fs/static-dev-0.1': {},
> +                       'sys-fs/udev-233': {},
> +                       'virtual/dev-manager-0': {
> +                               'RDEPEND': '''
> +                                       || (
> +                                               virtual/udev
> +                                               sys-fs/static-dev
> +                                       )'''
> +                       },
> +                       'virtual/udev-0': {
> +                               'RDEPEND': '''
> +                                       || (
> +                                               >=sys-fs/eudev-2.1.1
> +                                               >=sys-fs/udev-217
> +                                       )'''
> +                       },
> +               }
> +
> +               test_cases = (
> +                       # Test bug 645190, where static-dev was pulled in
> instead
> +                       # of eudev.
> +                       ResolverPlaygroundTestCase(
> +                               [
> +                                       'virtual/dev-manager',
> +                               ],
> +                               success=True,
> +                               mergelist=(
> +                                       'sys-fs/eudev-3.1.5',
> +                                       'virtual/udev-0',
> +                                       'virtual/dev-manager-0',
> +                               ),
> +                       ),
> +                       # Test static-dev preference.
> +                       ResolverPlaygroundTestCase(
> +                               [
> +                                       'sys-fs/static-dev',
> +                                       'virtual/dev-manager',
> +                               ],
> +                               all_permutations=True,
> +                               success=True,
> +                               mergelist=(
> +                                       'sys-fs/static-dev-0.1',
> +                                       'virtual/dev-manager-0',
> +                               ),
> +                       ),
> +               )
> +
> +               playground = ResolverPlayground(debug=False,
> ebuilds=ebuilds)
> +
> +               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.debug = False
> +                       playground.cleanup()
> --
> 2.13.6
>
>
>