Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 645914 - virtual deps no longer correctly satisfied when using ||() deps
Summary: virtual deps no longer correctly satisfied when using ||() deps
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core - Dependencies (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Portage team
Depends on:
Blocks: 155723
  Show dependency tree
Reported: 2018-01-27 20:24 UTC by SpanKY
Modified: 2019-09-08 22:18 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---

unit test for virtual/target-os (,1.86 KB, text/x-python)
2018-01-27 21:32 UTC, Zac Medico
unit test for virtual/target-os with targetroot=True (,1.98 KB, text/x-python)
2018-01-27 21:43 UTC, Zac Medico
modified version of pym/portage/tests/emerge/ (,4.38 KB, text/x-python)
2018-01-28 02:38 UTC, Zac Medico
unit test for virtual/target-os (,2.66 KB, text/x-python)
2018-01-28 03:13 UTC, Zac Medico

Note You need to log in before you can comment on or make changes to this bug.
Description SpanKY gentoo-dev 2018-01-27 20:24:57 UTC
the change that went in for bug 526160 broke virtual dep checking in ||() deps.
_dep_check_composite_db: fix bug #526160

here's a test case:

$ cat virtual/target-os/target-os-1.ebuild 

$ cat virtual/implicit-system/implicit-system-1.ebuild 

we first build up binpkgs (i don't think it's a requirement, but does make it easier to see).
# emerge -b virtual/target-os

then we look at the deptree:
$ ROOT=/var/empty emerge -eKp --tree virtual/target-os
[binary  N     ] virtual/target-os-1::xxx to /var/empty/
[binary  N     ]  virtual/implicit-system-1::xxx to /var/empty/
[binary  N     ]   sys-apps/mawk-1.3.4_p20171017-r1::gentoo to /var/empty/
[binary  N     ]    app-eselect/eselect-awk-0.2::gentoo to /var/empty/
[binary  N     ]  virtual/awk-1::gentoo to /var/empty/
[binary  N     ]   sys-apps/gawk-4.1.4::gentoo to /var/empty/
[binary  N     ]    sys-libs/readline-7.0_p3:0/7::gentoo to /var/empty/
[binary  N     ]     sys-libs/ncurses-6.0-r2:0/6::gentoo to /var/empty/

before this commit, gawk wouldn't be pulled in ... the explicitly selected mawk in the depgraph would have satisfied the ||() dep in virtual/awk.
Comment 1 Zac Medico gentoo-dev 2018-01-27 21:32:01 UTC
Created attachment 516924 [details]
unit test for virtual/target-os

This unit test is passing with current master branch (5c888fcd2e87270523f55fb180ced5c3c8689f76) and portage-2.3.20.

If it also works for you in practice, we can go ahead and include this in our test suite in order to guard against regressions.
Comment 2 Zac Medico gentoo-dev 2018-01-27 21:43:13 UTC
Created attachment 516926 [details]
unit test for virtual/target-os with targetroot=True

This the the same as the other unit test, but sets targetroot=True, and it still succeeds.
Comment 3 SpanKY gentoo-dev 2018-01-27 23:46:45 UTC
i'm testing with portage-2.3.20 already ... the emerge output i posted here was from that.  i wasn't showing behavior from the older CrOS versions (although that's where i discovered it initially).

are my test ebuilds not showing the same problem for you w/2.3.20 ?
Comment 4 Zac Medico gentoo-dev 2018-01-28 02:38:58 UTC
Created attachment 516934 [details]
modified version of pym/portage/tests/emerge/

(In reply to SpanKY from comment #3)
> are my test ebuilds not showing the same problem for you w/2.3.20 ?

Right, I can't reproduce it. Attached is a modified version of pym/portage/tests/emerge/ which calls emerge with some generated binary package stubs, and the result for your `ROOT=/var/empty emerge -eKp --tree virtual/target-os` command looks like this:

> $ pym/portage/tests/ pym/portage/tests/emerge/
> testSimple (portage.tests.emerge.test_emerge_virtual_target_os.SimpleEmergeTestCase) ... 
> These are the packages that would be merged, in reverse order:
> Calculating dependencies... done!
> [binary  N     ] virtual/target-os-1 to /tmp/tmpybcrw1nr/cross_root/tmp/tmpybcrw1nr/
> [binary  N     ]  virtual/implicit-system-1 to /tmp/tmpybcrw1nr/cross_root/tmp/tmpybcrw1nr/
> [binary  N     ]   sys-apps/mawk-1.3.4_p20171017-r1 to /tmp/tmpybcrw1nr/cross_root/tmp/tmpybcrw1nr/
> [binary  N     ]    app-eselect/eselect-awk-0.2 to /tmp/tmpybcrw1nr/cross_root/tmp/tmpybcrw1nr/
Comment 5 Zac Medico gentoo-dev 2018-01-28 03:13:10 UTC
Created attachment 516936 [details]
unit test for virtual/target-os

I just noticed that my test cases use || ( virtual/implicit-system virtual/awk ) for the target-os-1 RDEPEND that's not what you have. I can reproduce the problem with RDEPEND="virtual/implicit-system virtual/awk".
Comment 6 Zac Medico gentoo-dev 2018-01-28 03:51:38 UTC
It seems like maybe it chose mawk for you before out of luck, since the outcome depends on the order of evaluation of the virtual/implicit-system and virtual/awk dependencies. Reversing the order to atoms in the virtual/target-os-1 dependencies is enough to change the outcome:


The fact that virtual/implicit-system is a virtual is working against you here, since portage delays evaluation of virtuals by queuing them in the depgraph's _dep_disjunctive_stack. If virtual/implicit-system was not a virtual, then it's dependencies would be evaluated earlier, and the preference for mawk would propagate to virtual/awk when it is evaluated later.

Ultimately, the plan is to evaluate all overlapping || deps and virtuals in a combined DNF expression (like the solution for bug 632026), which will cause the preference for mawk to be appropriately recognized during evaluation of the DNF expression.
Comment 7 Zac Medico gentoo-dev 2018-01-28 04:40:12 UTC
I have an unfinished patch solves the problem by making it evaluate the virtual dependencies of virtual/target-os-1 all together:
Comment 8 SpanKY gentoo-dev 2018-01-29 06:55:11 UTC
we didn't actually need the virtual/awk dep, so i just dropped it to avoid the issue.  hopefully it doesn't show up elsewhere ;).

do we still need to delay virtual evaluation anymore now that they're "normal" packages ?  the test case i posted was reduced down ... in reality we have virtuals on top of virtuals, so doing immediate evaluation on the top node wouldn't have helped.
Comment 9 Zac Medico gentoo-dev 2018-01-29 07:38:04 UTC
Yes, the delayed virtual evaluation solves many problems. The idea is that we evaluate virtuals late so what we have the benefit of all of the earlier package selections to guide the selection of packages to satisfy the virtual. This means that the selection of packages to satisfy the virtual is less likely to choose the wrong package, since more information is available.

This test case is a reasonable demonstration:

# Test swapping of providers for a new-style virtual package,
# which relies on delayed evaluation of disjunctive (virtual
# and ||) deps as required to solve bug #264434. Note that
# this behavior is not supported for old-style PROVIDE virtuals,
# as reported in bug #339164.
	mergelist = [

Since media-video/libav is added to the graph earlier, it makes it possible to prefer the media-video/libav choice when satisfying virtual/ffmpeg, and then uninstall media-video/ffmpeg since it is no longer needed.

This test case would work the same way if media-video/libav was a dependency of some random package in @world, since that dependency would be evaluated early in comparison to the delayed evaluation of virtual/ffmpeg.