Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 264434 - Bad handling of || operator (it may create blockers)
Summary: Bad handling of || operator (it may create blockers)
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core - Dependencies (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: InVCS
: 269055 469588 (view as bug list)
Depends on: 1343
Blocks: 155723 300071 384107 210077 288499 339164
  Show dependency tree
 
Reported: 2009-03-31 20:16 UTC by David Carlos Manuelda
Modified: 2017-11-04 22:42 UTC (History)
5 users (show)

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


Attachments
Make emerge expand all non-|| dependencies first. (portage-emerge-delay-or-operator-expansion.patch,4.60 KB, patch)
2009-06-18 20:36 UTC, Sebastian Luther (few)
Details | Diff
my incomplete patch, roughly based on few's (delayed_disjunctive.patch,3.07 KB, patch)
2009-06-19 07:26 UTC, Zac Medico
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Carlos Manuelda 2009-03-31 20:16:30 UTC
I had KDE 4.2 and I formatted. WHen I tried to reinstall from scratch kde 4.2 it showed some blocks.
Despite it seems KDE related I located the bug, and it comes from:

|| ( ~x11-libs/qt-phonon-${PV}:${SLOT}[debug=] media-sound/phonon ) inside qt-webkit ebuild (4.5.0).

The problem is that kde-base/startkde requires kde-phonon to be installed, and other programs requires also qt-webkit, and then, this is triggered.

It can be solved by installing media-sound/phonon by hand first, but may confuse users.

I suggest CCing portage herd to have better || handling: for example, having || (1 2)
If a block is caused by 1, and neither of those two are installed, rebuild dep tree with 2 instead, and see if blocks dessappear.

How about this issue?

Reproducible: Always
Comment 1 Zac Medico gentoo-dev 2009-04-01 07:44:26 UTC
The best way to fix this is to separate dependency choices into a separate stack and evaluate them as late as possible. That way, as much of the dependency graph as possible is known when a choice is made, allowing for a more optimal choice.
Comment 2 David Carlos Manuelda 2009-04-01 11:03:19 UTC
(In reply to comment #1)
> The best way to fix this is to separate dependency choices into a separate
> stack and evaluate them as late as possible. That way, as much of the
> dependency graph as possible is known when a choice is made, allowing for a
> more optimal choice.
> 

Then, could you CC the ebuild creator team for fix/improve this?
Comment 3 Zac Medico gentoo-dev 2009-04-01 17:23:18 UTC
@qt team: Do you want to consider making phonon the preferred choice in these qt-webkit deps?

  || ( ~x11-libs/qt-phonon-4.5.0:4[debug=] media-sound/phonon )

I guess the assumption for the qt-phonon preference is that a significant proportion of users don't have or don't want kde-4 installed (maybe they use kde-3.5, gnome, or some other desktop). Considering that many people still don't consider kde-4 to be "ready", and many people may be using some other desktop, I suspect that the current preference is pretty reasonable.
Comment 4 Zac Medico gentoo-dev 2009-04-01 17:25:41 UTC
(In reply to comment #0)
> It can be solved by installing media-sound/phonon by hand first, but may
> confuse users.

An easier workaround is to uninstall qt-phonon and mask it:

  emerge -C qt-phonon
  mkdir -p /etc/portage
  echo x11-libs/qt-phonon >> /etc/portage/package.mask
Comment 5 Markos Chandras (RETIRED) gentoo-dev 2009-04-01 17:43:01 UTC
(In reply to comment #3)
> @qt team: Do you want to consider making phonon the preferred choice in these
> qt-webkit deps?
> 
>   || ( ~x11-libs/qt-phonon-4.5.0:4[debug=] media-sound/phonon )
> 
> I guess the assumption for the qt-phonon preference is that a significant
> proportion of users don't have or don't want kde-4 installed (maybe they use
> kde-3.5, gnome, or some other desktop). Considering that many people still
> don't consider kde-4 to be "ready", and many people may be using some other
> desktop, I suspect that the current preference is pretty reasonable.
> 

Exactly. This is our thought as well. So I wouldnt change the dependencies in order to help kde4 installation cause not everybody is using kde4. Qt and kde4 are independent projects, so the dependencies should stay just like they are :)

This bug will close as WONTFIX
Comment 6 David Carlos Manuelda 2009-04-01 17:47:14 UTC
I think it is better to keep this open, because it has also a portage side bug, and it is not fixed yet (dep handling).
Maybe it is good to not touch the ebuild to help kde4 users, but I really think it can be improved (at least portage side)
Comment 7 David Carlos Manuelda 2009-04-01 23:23:09 UTC
By the way, zac, could you include your advise to mask that package in order to aboid the block in the kde 4.x guide from gentoo.org website? It could help others
Comment 8 Zac Medico gentoo-dev 2009-04-02 00:01:46 UTC
(In reply to comment #4)
> (In reply to comment #0)
> > It can be solved by installing media-sound/phonon by hand first, but may
> > confuse users.
> 
> An easier workaround is to uninstall qt-phonon and mask it:
> 
>   emerge -C qt-phonon
>   mkdir -p /etc/portage
>   echo x11-libs/qt-phonon >> /etc/portage/package.mask
> 

@kde team: Does somebody want to add a note about this to kde4 guide?
Comment 9 Zac Medico gentoo-dev 2009-05-08 21:12:37 UTC
*** Bug 269055 has been marked as a duplicate of this bug. ***
Comment 10 Sebastian Luther (few) 2009-06-18 20:36:35 UTC
Created attachment 195115 [details, diff]
Make emerge expand all non-|| dependencies first.
Comment 11 Zac Medico gentoo-dev 2009-06-19 04:31:29 UTC
(In reply to comment #10)
> Created an attachment (id=195115) [edit]
> Make emerge expand all non-|| dependencies first.

It's a really good start. Is see a couple issues though:

1) It looks like depgraph._process_deps() doesn't separate out the parts that aren't || deps and process those immediately. It's only the || parts that need to be delayed.

2) It seems like the patch breaks ROOT logic inside depgraph._add_pkg_deps().

Those seem like minor issues though. I see if I can quickly fix them up now...
Comment 12 Zac Medico gentoo-dev 2009-06-19 04:36:57 UTC
3) Any virtual atoms (both old style and new-style meta-ebuilds) should be separated out just like || deps are.
Comment 13 Sebastian Luther (few) 2009-06-19 06:24:02 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Created an attachment (id=195115) [edit]
> > Make emerge expand all non-|| dependencies first.
> 
> It's a really good start. Is see a couple issues though:
> 
> 1) It looks like depgraph._process_deps() doesn't separate out the parts that
> aren't || deps and process those immediately. It's only the || parts that need
> to be delayed.

That's right, but the separation has already taken place at the end of _agg_pkg_deps.

dep_string = portage.dep.dep_restructure(dep_string)
+                       
+                       for dep in dep_string:
+                               self._deps.append((pkg, bdeps_root, dep,
+                                       self._priority(buildtime=(not bdeps_optional),
+                                       optional=bdeps_optional)))


> 2) It seems like the patch breaks ROOT logic inside depgraph._add_pkg_deps().
> 
> Those seem like minor issues though. I see if I can quickly fix them up now...

I missed that (R,P)DEPEND were handled differently at the point which is now the end of _add_pkg_deps.

(In reply to comment #12)
> 3) Any virtual atoms (both old style and new-style meta-ebuilds) should be
> separated out just like || deps are.

That's right. How do I detect old-style virtuals?

Comment 14 Zac Medico gentoo-dev 2009-06-19 07:03:03 UTC
(In reply to comment #13)
> That's right, but the separation has already taken place at the end of
> _agg_pkg_deps.

True. However, the separation isn't recursive, so it's not as thorough as it could be. Also use_reduce() can be called inside _add_pkg_deps to evaluate USE conditionals (atm it doesn't seem necessary to preserve them past that point).

> > 3) Any virtual atoms (both old style and new-style meta-ebuilds) should be
> > separated out just like || deps are.
> 
> That's right. How do I detect old-style virtuals?

The simplest solution for now is to check atom.cp.startswith("virtual/"). It's a little hackish, but it should serve to separate out both new and old-style virtuals. To verify that a virtual really exists (not strictly necessary here), you can use config.getvirtuals() for old-style, and depgraph._have_new_virt() for new-style.

In the long run, we want to support virtuals from other categories, via PROPERTIES. For example, this approach can be used to identify virtuals from the java-virtuals category (or elsewhere) if the ebuilds define PROPERTIES=virtual.
Comment 15 Zac Medico gentoo-dev 2009-06-19 07:26:14 UTC
Created attachment 195143 [details, diff]
my incomplete patch, roughly based on few's

This problem is so much fun that I started on my own patch. It looks like I won't finish it tonight though, so I'm attaching it here for reference. It's pretty close to the other patch, but organized a little differently.
Comment 16 Zac Medico gentoo-dev 2009-06-19 20:21:00 UTC
This is in svn r13655.
Comment 17 Zac Medico gentoo-dev 2009-08-03 23:01:25 UTC
This is fixed in 2.2_rc34.
Comment 18 Zac Medico gentoo-dev 2009-10-11 00:46:21 UTC
This is fixed in 2.1.7.
Comment 19 Zac Medico gentoo-dev 2011-05-31 02:19:22 UTC
There's a fix for additional case in git now:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=4f66159887fc4e3ec8bd87ae0f08ba249f98631b
Comment 20 Zac Medico gentoo-dev 2013-05-12 20:03:51 UTC
*** Bug 469588 has been marked as a duplicate of this bug. ***