Re: [gentoo-portage-dev] [PATCH] dep_zapdeps: fix virtual/rust handling (bug 645416)

[Zac Medico]

On 01/23/2018 03:11 PM, Alec Warner wrote:
> I'm starting to be a bit sad about this for a couple of reasons.
> 
> 1) dep_zapdeps is pretty big at this point, and every tweak for || ()
> just grows it. Do we have any idea:
>   a) When dep_zapdeps will be 'done'?

We're gradually approaching a point were it can't be improved much more,
without major refactoring.

>   b) Plans for splitting it into more manageable pieces?

The next step is to evaluate the overlapping || deps for multiple
packages at the same time, using DNF. That will be a good time for
refactoring.

> 2) In general I worry that the logic for || () will essentially never be
> complete, and we will be forced to tweak the implementation forever as
> cases arise where the algorithm fails to compute the 'right' answer.
> I'm a bit curious what 'right' even is anymore. The devmanual doesn't
> really describe how any-of dependencies are resolved and so the changes
> seem rather arbitrary.

The 'right' answer is optimal in the sense that it:

1) Installs the minimum number of packages necessary
2) Upgrades all packages to the latest version possible
3) Prefers choices toward the left when there's no overriding reason to
do otherwise (1 or 2 above)

> I am not worried about specific changes (like
> this one) but the fact that we continually apply changes to make the
> 'right' thing happen is the larger concern.

I'd worried if we didn't have unit tests which prove that it's behaving
optimally for that cases that it's designed to handle. It's gradually
approaching the point where we can't change the behavior much without
making it behave sub-optimally for some of our existing test cases.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] [PATCH] dep_zapdeps: fix virtual/rust handling (bug 645416)

[Alec Warner]

I'm starting to be a bit sad about this for a couple of reasons.

1) dep_zapdeps is pretty big at this point, and every tweak for || () just
grows it. Do we have any idea:
  a) When dep_zapdeps will be 'done'?
  b) Plans for splitting it into more manageable pieces?
2) In general I worry that the logic for || () will essentially never be
complete, and we will be forced to tweak the implementation forever as
cases arise where the algorithm fails to compute the 'right' answer.
I'm a bit curious what 'right' even is anymore. The devmanual doesn't
really describe how any-of dependencies are resolved and so the changes
seem rather arbitrary. I am not worried about specific changes (like this
one) but the fact that we continually apply changes to make the 'right'
thing happen is the larger concern.

-A

On Tue, Jan 23, 2018 at 2:17 AM, Zac Medico  wrote:

> Fix the code from bug 643974 to set the installed_downgrade flag only
> if the selected newer package is either installed or in the graph. This
> is currently needed for appropriate handling of virtual/rust-1.19.0,
> since there's an upgrade to dev-lang/rust-1.23.0 available which may
> not be desired since it would mean that dev-lang/rust-bin-1.19.0 has
> to be installed in order to satisfy virtual/rust-1.19.0:
>
>  || ( =dev-lang/rust-1.19.0* =dev-lang/rust-bin-1.19.0* )
>
> So, the rust-bin choice is desirable only if the rust-1.23.0 package is
> already installed or in the graph.
>
> Fixes: 86ba22da7a2f ("dep_zapdeps: install new package, allow upgrade (bug
> 643974)")
> Bug: https://bugs.gentoo.org/645416
> ---
>  pym/portage/dep/dep_check.py   | 10 ++--
>  .../tests/resolver/test_or_upgrade_installed.py| 59
> ++
>  2 files changed, 66 insertions(+), 3 deletions(-)
>
> diff --git a/pym/portage/dep/dep_check.py b/pym/portage/dep/dep_check.py
> index c56f545ec..7e5a3186e 100644
> --- a/pym/portage/dep/dep_check.py
> +++ b/pym/portage/dep/dep_check.py
> @@ -463,11 +463,15 @@ def dep_zapdeps(unreduced, reduced, myroot,
> use_binaries=0, trees=None):
> avail_pkg = avail_pkg_use
> avail_slot = Atom("%s:%s" %
> (atom.cp, avail_pkg.slot))
>
> -   if downgrade_probe is not None:
> +   if downgrade_probe is not None and graph is not
> None:
> highest_in_slot =
> mydbapi_match_pkgs(avail_slot)
> +   highest_in_slot = (highest_in_slot[-1]
> +   if highest_in_slot else None)
> if (avail_pkg and highest_in_slot and
> -   avail_pkg < highest_in_slot[-1] and
> -   not downgrade_probe(avail_pkg)):
> +   avail_pkg < highest_in_slot and
> +   not downgrade_probe(avail_pkg) and
> +   (highest_in_slot.installed or
> +   highest_in_slot in graph)):
> installed_downgrade = True
>
> slot_map[avail_slot] = avail_pkg
> diff --git a/pym/portage/tests/resolver/test_or_upgrade_installed.py
> b/pym/portage/tests/resolver/test_or_upgrade_installed.py
> index 6e01d321d..7018e08de 100644
> --- a/pym/portage/tests/resolver/test_or_upgrade_installed.py
> +++ b/pym/portage/tests/resolver/test_or_upgrade_installed.py
> @@ -99,3 +99,62 @@ class OrUpgradeInstalledTestCase(TestCase):
> finally:
> playground.debug = False
> playground.cleanup()
> +
> +   def testVirtualRust(self):
> +   ebuilds = {
> +   'dev-lang/rust-1.19.0': {},
> +   'dev-lang/rust-1.23.0': {},
> +   'dev-lang/rust-bin-1.19.0': {},
> +   'virtual/rust-1.19.0': {
> +   'RDEPEND': '|| ( =dev-lang/rust-1.19.0*
> =dev-lang/rust-bin-1.19.0* )'
> +   },
> +   }
> +
> +   installed = {
> +   'dev-lang/rust-1.19.0': {},
> +   'virtual/rust-1.19.0': {
> +   'RDEPEND': '|| ( =dev-lang/rust-1.19.0*
> =dev-lang/rust-bin-1.19.0* )'
> +   },
> +   }
> +
> +   world = ['virtual/rust']
> +
> +   test_cases = (
> +   # Test bug 645416, where rust-bin-1.19.0 was
> pulled in
> +   # inappropriately due to the rust-1.23.0 update
> being
> +   # available.
> +   ResolverPlaygroundTestCase(
> +   ['virtual/rust'],
> +   options={'--update': True, '--deep': True},
> +