Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 790764 - dev-python/QtPy: USE flags are broken, also they're killing CI
Summary: dev-python/QtPy: USE flags are broken, also they're killing CI
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal
Assignee: Python Gentoo Team
URL:
Whiteboard:
Keywords: PullRequest
Depends on:
Blocks:
 
Reported: 2021-05-18 07:04 UTC by Michał Górny
Modified: 2021-05-20 09:42 UTC (History)
4 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-05-18 07:04:31 UTC
The ebuild is using a lot of conditions such as:

  || (
    dev-python/PyQt5[${PYTHON_USEDEP},designer?,gui?...]
    dev-python/pyside2[${PYTHON_USEDEP},designer?,gui?,...]
  )

Aside from the fact that the long dep list is killing CI, it's incorrect.

The ebuild will be satisfied by either PyQt5 or pyside2 having the necessary USE flags enabled.  However, the package will use PyQt5 if it's installed *even if it's pyside2 satisfying the dep*.

Furthermore, the existing dep split makes it already possible for Portage to satisfy the dep via a combination of PyQt5 and pyside2, e.g. USE="gui qml" could be satisfied by a combination of PyQt5[gui] and pyside2[qml] even though neither of these packages fully satisfy the dep.

I'm sorry but I'm going to revert this to unbreak CI, and I don't really see any way to fix this ebuild that's going to work.
Comment 1 Larry the Git Cow gentoo-dev 2021-05-18 07:21:27 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=7a9268be4bcba43f3206b51da598312a105dd0dd

commit 7a9268be4bcba43f3206b51da598312a105dd0dd
Author:     Michał Górny <mgorny@gentoo.org>
AuthorDate: 2021-05-18 07:05:53 +0000
Commit:     Michał Górny <mgorny@gentoo.org>
CommitDate: 2021-05-18 07:21:21 +0000

    dev-python/QtPy: Revert "allow dependency to be satisfied by pyside2"
    
    The lot of USE dependencies break pkgcheck, plus the any-of logic is
    wrong and does not match what the package does.  If PyQt5 is installed
    at all, the package will default to using it even if the dependencies
    are satisfied by pyside2.
    
    Reverts: 8bdd53a2d42010f0ec8f83273938a62195bfbcd5
    Bug: https://bugs.gentoo.org/790764
    Signed-off-by: Michał Górny <mgorny@gentoo.org>

 dev-python/QtPy/QtPy-1.9.0-r3.ebuild | 107 -----------------------------------
 dev-python/QtPy/metadata.xml         |  17 ------
 2 files changed, 124 deletions(-)
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-05-18 07:31:26 UTC
I"m trying to backport test support into -r2 now.
Comment 3 Nowa Ammerlaan gentoo-dev 2021-05-18 08:01:34 UTC
>  However, the package will use PyQt5 if it's installed *even if it's pyside2 satisfying the dep*.

This is where app-eselect/eselect-QtPy comes in, it controls which one is used at runtime.

> I"m trying to backport test support into -r2 now.

The problem with -r2 is that it explicitly makes QtPy not work with pyside2 at all, which IMO defeats the whole purpose of the package.

> 	sed -i -e "s/from PyQt4.Qt import/raise ImportError #/" qtpy/__init__.py || die
>	sed -i -e "s/from PySide import/raise ImportError #/" qtpy/__init__.py || die
>	sed -i -e "s/from PySide2 import/raise ImportError #/" qtpy/__init__.py || die

At the very least we should have:
|| (
   dev-python/pyside2
   dev-python/PyQt5
)

To make this package do what it was designed to do, i.e. provide an abstraction layer for pyside/pyqt
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-05-18 08:06:16 UTC
(In reply to Andrew Ammerlaan from comment #3)
> >  However, the package will use PyQt5 if it's installed *even if it's pyside2 satisfying the dep*.
> 
> This is where app-eselect/eselect-QtPy comes in, it controls which one is
> used at runtime.

This is not how we do things in Gentoo these days.  Correct eselect choice must not be a precondition to things working.

> > I"m trying to backport test support into -r2 now.
> 
> The problem with -r2 is that it explicitly makes QtPy not work with pyside2
> at all, which IMO defeats the whole purpose of the package.

The purpose of this package in Gentoo is not to try hard to make useless choice available but to satisfy reverse dependencies.
Comment 5 Nowa Ammerlaan gentoo-dev 2021-05-18 08:24:14 UTC
> This is not how we do things in Gentoo these days.  Correct eselect choice must not be a precondition to things working.

It will work regardless of what eselect is set to, it will even work if it is not set at all. The eselect module is just a useful tool in case you happen to want to force the use of the one or the other. If PyQt5 is installed it will use that, if PySide2 is installed it will use that, if both are installed it uses PyQt5. Either way it will work, so I don't see how the eselect choice is a precondition to things working.

> The purpose of this package in Gentoo is not to try hard to make useless choice available but to satisfy reverse dependencies.

It is far from a useless choice. Say an user has installed some package that pulls in pyside2, and now wants to install some other package that pulls in QtPy. With the -r2 ebuild PyQt5 will be pulled in as well, which is a complete waste of resources since whatever package is pulling in QtPy will work just fine with pyside2 as well. And it is exactly this that -r3 aims to accomplish, it avoids the situation where a user has to compile and install both PyQt5 and PySide2, which is completely unnecessary since the two are more or less identical. So, it is not a "useless choice", it effectively reduces the build-time of this package.

I strongly disagree with the removal of -r3, we can discuss reducing the number of flags (which will 'fix' the CI issue (see linked PR), but to emphasize it is a CI issue, and not an issue with the ebuild) but completely removing -r3 and putting this package back in a semi-broken state is the wrong course of action in my opinion.
Comment 6 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-05-18 08:47:38 UTC
Except it won't work because whatever backend is selected may not actually have the USE flags needed by the package.  And this is the real problem here.
Comment 7 Nowa Ammerlaan gentoo-dev 2021-05-18 10:21:31 UTC
> Except it won't work because whatever backend is selected may not actually have the USE flags needed by the package.  And this is the real problem here.

So what can we do to fix this? I recognize that my solution in -r3 is far from perfect, as juippis and I already discussed in the linked PR. However, I still think that my solution in -r3 (allowing both pyside and pyqt5, at the cost of sometimes requiring some small additional configuration which is as simple as calling the application with 'QT_API=".." application') is better than the -r2 solution (forcing PyQt5, at the cost of wasting quite a significant amount of build-time by often having to install both PyQt5 and PySide2 on the same system)

Both -r2 and -r3 have their problems, but in -r2 we have users who have to waste ~20 minutes of build-time to install both PySide2 and PyQt5, while in -r3 we sometimes have the situation where an application should be called with QT_API="..." set, where eselect-QtPy already goes a long way to alleviate this problem. The latter is in my opinion less problematic. 

I would love to hear if there are ways of improving -r3, as I said it is not perfect. However, in my opinion the situation in -r2 is worse.
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-05-18 10:41:31 UTC
You still either don't understand the problem or ignore it.

USE dependencies like this will not work inside ||.  By the very design of package, if PyQt5 is installed, all needed USE flags must be enabled on PyQt5.  If pyside2 is to be used, it must guarantee that QtPy will never use PyQt5 which it simply can't.

eselect does not solve any problems, it only introduces more.  Now you tell user that he must switch backends that are silently magically controlled by || dep.  You only introduce a cheap workaround for the ebuild being broken, and pretend that it solved the problems.


One possible option is to control PyQt5/pyside2 via an explicit USE flag, and then sed the package to respect that.  It's far from perfect but I suppose better than what we have now.

Another option is to remove USE flags from the package entirely, and always depend on all features of both backends inside the ||.  This way, whether the package ends up choosing PyQt5 or pyside2 backend, it will always have what it needs.

I suppose you could also try some blocker hackery but that's just going to be horrible.  You'd effectively have to uninstall PyQt5 if it doesn't satisfy the USE dependency.  I don't think there's a legitimate way of enforcing that.
Comment 9 Nowa Ammerlaan gentoo-dev 2021-05-18 11:25:11 UTC
> One possible option is to control PyQt5/pyside2 via an explicit USE flag, and then sed the package to respect that.  It's far from perfect but I suppose better than what we have now.

This would indeed be better. Though it has the disadvantage of not being able to switch at runtime, and instead only allowing to switch by re-emerging the package. Though the compile time is not that long so this might not be that problematic. As an added bonus, packages that use QtPy but have bugs with either PyQt5/pyside2 can specifically depend on the implementation that they do work bug-free with (e.g. spyder, which currently only works with PyQt5 even though it does use QtPy). A disadvantage is that we would have to use REQUIRED_USE for this to work.

> Another option is to remove USE flags from the package entirely, and always depend on all features of both backends inside the ||.  This way, whether the package ends up choosing PyQt5 or pyside2 backend, it will always have what it needs.

This would add some compile time because it would enable flags which would not be strictly required, though it would still be less compile time then compiling both PyQt5 and PySide2 so it would still be a huge improvement over -r2. Additionally, this has the advantage of still being able to switch at runtime and allows for per-application control if required.

> I suppose you could also try some blocker hackery but that's just going to be horrible.  You'd effectively have to uninstall PyQt5 if it doesn't satisfy the USE dependency.  I don't think there's a legitimate way of enforcing that.

This sounds like it would create huge mess, so let's not do this.

I'm not yet sure whether option 1 or option 2 is the better one, I will think about this some more. Would also love to hear other people's opinions on this.

> You still either don't understand the problem or ignore it.

I understand and recognize the problem, I simply disagree that the -r2 solution is better then the -r3 solution. In any case the solutions 1 and 2 that you mentioned are better then both -r2 and -r3 so let's just focus on making an improved -r4.
Comment 10 Nowa Ammerlaan gentoo-dev 2021-05-18 17:48:47 UTC
Option 1 still triggers the UncheckableDep problem. I prefer it to option 2 because option 2 means basically pulling in almost all of Qt unconditionally.

There is also a third option which popped into my head just now. We forget about USE flags in QtPy entirely, and just have || ( pyside2 pyqt5 ) in QtPy, and have each ebuild that depends on additional pyqt5/pyside2 flags specify these in the ebuild itself e.g. 
DEPEND="
     dev-python/QtPy
     || (
         dev-python/pyside2[printsupport]
         dev-python/PyQt5[printsupport]
     )"
It is far from perfect, but it fixes the CI issue, does not pull in half of qt, and does not have the whole REQUIRED_USE mess that solution 1 has. Thoughts?
Comment 11 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-05-18 17:54:30 UTC
This doesn't solve the problem, just moves it.  The CI problem is separate, I'll leave a comment on the PR.
Comment 12 Larry the Git Cow gentoo-dev 2021-05-20 09:42:30 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=5cf79d0718816ddef25b7ddc3353d4f4975dbb6e

commit 5cf79d0718816ddef25b7ddc3353d4f4975dbb6e
Author:     Andrew Ammerlaan <andrewammerlaan@gentoo.org>
AuthorDate: 2021-05-19 13:15:23 +0000
Commit:     Andrew Ammerlaan <andrewammerlaan@gentoo.org>
CommitDate: 2021-05-20 09:42:27 +0000

    dev-python/QtPy: allow use of pyside2, but better this time
    
    Closes: https://bugs.gentoo.org/790764
    Package-Manager: Portage-3.0.18, Repoman-3.0.3
    Closes: https://github.com/gentoo/gentoo/pull/20871
    Signed-off-by: Andrew Ammerlaan <andrewammerlaan@gentoo.org>

 dev-python/QtPy/QtPy-1.9.0-r4.ebuild | 137 +++++++++++++++++++++++++++++++++++
 dev-python/QtPy/metadata.xml         |  17 +++++
 2 files changed, 154 insertions(+)