FS#59564 - [python-keyring] fakeroot prevents packages using python-keyring from being created

Attached to Project: Community Packages
Opened by Frederik “Freso” S. Olesen (Freso) - Wednesday, 08 August 2018, 08:43 GMT
Last edited by Eli Schwartz (eschwartz) - Tuesday, 12 February 2019, 03:32 GMT
Task Type Bug Report
Category General
Status Closed
Assigned To No-one
Architecture All
Severity Low
Priority Normal
Reported Version 5.1.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Summary and Info:
When trying to build a package that `import`s python-keyring, `makepkg` will error out when Python tries to `import keyring`.

See comments at https://aur.archlinux.org/packages/nagstamon/ as well as https://github.com/JonnyJD/musicbrainz-isrcsubmit/issues/119

I'm not sure the issue is with makepkg/fakeroot itself but may be upstream in python-keyring/secretstorage/jeepny, but I'm not sure how to investigate further myself, and for now at least two AUR packages are affected.

Steps to Reproduce:
* pacman -S python-keyring
* git clone https://aur.archlinux.org/isrcsubmit-git.git
* cd isrcsubmit-git
* makepkg
This task depends upon

Closed by  Eli Schwartz (eschwartz)
Tuesday, 12 February 2019, 03:32 GMT
Reason for closing:  Won't fix
Additional comments about closing:  dbus apparently does not like to be run under fakeroot, there is nothing we can do about this. Well-behaved setup.py files will never even have a chance of encountering the issue in the first place.
Comment by Allan McRae (Allan) - Wednesday, 08 August 2018, 09:00 GMT
Why do these packages need to import keyring during install. During build I understand, but install should be just moving files around.

I don't consider this a makepkg issue.
Comment by Frederik “Freso” S. Olesen (Freso) - Wednesday, 08 August 2018, 09:50 GMT
For isrcsubmit at least, it's because `setup.py` has `from isrcsubmit import __version__` (which I think is a relatively common Python thing?) and isrcsubmit.py has `import keyring`. Why it breaks in package() and not in build() or check() though I don't know, but it's been built many times before without issues, so something "recent" has changed that's caused this to not work anymore, which is why I reported it here as a first measure.

If it turns out that this was always how it was supposed to work and the packages are now broken due to bugs having been fixed, then I'll look into fixing the package and/or upstream, but could it possible be a "newly" introduced bug to how makepkg works?
Comment by Allan McRae (Allan) - Wednesday, 08 August 2018, 10:01 GMT
I doubt this is a makepkg change. This part of makepkg has not been changed in a long time.
Comment by Xiretza (xiretza) - Wednesday, 08 August 2018, 10:25 GMT
Not makepkg's fault. Easy to reproduce with fakeroot alone: `fakeroot python -c "import keyring"`. The error is because of dbus I think, so it might be slightly different from installation to installation.
Comment by Eli Schwartz (eschwartz) - Wednesday, 08 August 2018, 15:25 GMT
> isrcsubmit at least, it's because `setup.py` has `from isrcsubmit import __version__` (which I think is a relatively common Python thing?)

I think it's much more common for the combination of packages which either hardcode the version in setup.py, or get the version from a minimal toplevel import in a modularized codebase. This set off alarm bells:

```
# we load isrcsubmit on setup
args["setup_requires"] = args["install_requires"],
```

...

And how exactly does this work when you don't know until you successfully run setup.py, what is required to run setup.py ???

$ pip install --user git+https://github.com/JonnyJD/musicbrainz-isrcsubmit
Collecting git+https://github.com/JonnyJD/musicbrainz-isrcsubmit
Cloning https://github.com/JonnyJD/musicbrainz-isrcsubmit to /tmp/pip-req-build-gb15km0t
Complete output from command python setup.py egg_info:
Traceback (most recent call last):
File "/tmp/pip-req-build-gb15km0t/isrcsubmit.py", line 47, in <module>
import discid
ModuleNotFoundError: No module named 'discid'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/tmp/pip-req-build-gb15km0t/isrcsubmit.py", line 51, in <module>
from libdiscid.compat import discid
ModuleNotFoundError: No module named 'libdiscid'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/tmp/pip-req-build-gb15km0t/setup.py", line 13, in <module>
from isrcsubmit import __version__
File "/tmp/pip-req-build-gb15km0t/isrcsubmit.py", line 55, in <module>
import discid
ModuleNotFoundError: No module named 'discid'

----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-req-build-gb15km0t/
Comment by Johannes Dewender (JonnyJD) - Tuesday, 05 February 2019, 07:57 GMT
Sorry, I didn't have time to look into this fully. python-keyring was always supposed to be an optdepends, but was always making problems on update.
The solution is always to handle any errors that keyring should not have on import anyways.
In some versions it is plain broken.

@Eli:
Not sure what this rambling is about,
but "no module named 'discid'" is just telling you that "isrcsubmit.py" needs "python-discid" (or "python-libdiscid", as an alternative).
This doesn't have anything to do with the actual problem here.

In the linked tickets:
https://github.com/JonnyJD/musicbrainz-isrcsubmit/issues/119
has one valid stacktrace indicating an actual problem with python-keyring
and another stacktrace just because actual hard dependencies of iscrcsubmit are not installed (python-discid).
Comment by Eli Schwartz (eschwartz) - Tuesday, 05 February 2019, 15:36 GMT
No, the error message is telling you that even to execute setup.py you need to have preinstalled discid, but pip cannot know that discid is required until after it executes setup.py -- which is a logical circularity and a sign of broken programming. Don't import the version from ircsubmit unless ircsubmit is actually suitable to be imported in setup.py, and while you are at it, don't import the version from ircsubmit if merely importing the freaking version causes you to import keyring too.

Your code does not work according to the Python Packaging Authority. Forget about makepkg.

Your code will continue to be invalid according to the Python Packaging Authority, until https://www.python.org/dev/peps/pep-0518/ achieves broad enough acceptance that you can use pyproject.toml to specify that discid needs to be installed by the "pip" tool or other python package management tools, *before* executing setup.py, and even with pyproject.toml I would argue that the code is still bad even if it is valid, because you have unnecessary frivolous "import keyring" in your setup.py

I'm sorry if you think my attempt to explain to you both the root of your problem and the logical solution, is "rambling". I'll remember next time I have to talk with you, not to bother attempting to provide education.
Comment by Johannes Dewender (JonnyJD) - Tuesday, 05 February 2019, 18:11 GMT
Well, for most people installing with pip won't work either way because python-discid (and python-libdiscid) depend on a C library (which is usually not installed via pip):

$ pip install discid
Collecting discid
Downloading https://files.pythonhosted.org/packages/a2/76/463785b1715b461c2fc0dad73b115e6ca061dbd768154b88242974d04a9f/discid-1.1.1.tar.gz
Complete output from command python setup.py egg_info:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/tmp/pip-install-_tmy4lgs/discid/setup.py", line 6, in <module>
from discid import __version__
File "/tmp/pip-install-_tmy4lgs/discid/discid/__init__.py", line 30, in <module>
from discid.disc import read, put, Disc, DiscError, TOCError
File "/tmp/pip-install-_tmy4lgs/discid/discid/disc.py", line 24, in <module>
from discid.libdiscid import _LIB, FEATURES
File "/tmp/pip-install-_tmy4lgs/discid/discid/libdiscid.py", line 108, in <module>
_LIB = _open_library(_LIB_NAME)
File "/tmp/pip-install-_tmy4lgs/discid/discid/libdiscid.py", line 99, in _open_library
return ctypes.cdll.LoadLibrary(lib_name)
File "/usr/lib/python3.7/ctypes/__init__.py", line 434, in LoadLibrary
return self._dlltype(name)
File "/usr/lib/python3.7/ctypes/__init__.py", line 356, in __init__
self._handle = _dlopen(self._name, mode)
OSError: libdiscid.so.0: cannot open shared object file: No such file or directory

It might actually be possible to force pip to download libdiscid and to run cmake, but that is quite a bit out of scope for something that started as a small script file and only got a setup.py and PKGBUILD on request.
Having a complete pip install working (even just when libdiscid is already installed) is a good goal, but not what this problem is about.

The reason why the current behavior usually works (except for automatic dependency install with pip, which is not what we try to do here, see above)
is because importing a module does effectively nothing and only breaks on syntax problems (or missing deps, see above).
The reason why it fails is because python-keyring is doing more than just defining functions. It is running "init_backend()" on import.
That is not what I would do (I would let users run init_backend() at an appropriate time), but it is probably allowed per standards.

So since keyring is supposed to be an optdepends (quite a big package for a tiny script), I always tried all kinds of things so import keyring doesn't fail hard.
If "import keyring" runs completely fine in all circumstances when running iscrsubmit.py, (I don't know if this is the case) then this is an actual problem of setup.py in terms of the PKGBUILD that needs to be fixed.
I can't test this right now, as I don't have a machine to do this ATM (this needs a CD drive, not just any VM will do).
When "import keyring" fails for good (not having a running dbus should not break isrcsubmit), then I have to fix that problem anyways.

So yes, I have a different understanding what the "root" is.

You might be right that doing setup.py not the "traditional" way (that is how I learned it, sorry) is an improvement. It is probably at least a workaround for the problem with keyring on install.
However, your style of commenting does not actually educate people, but makes them ragequit instead of making changes after a hard day of work.
Regardless if you see it that way: Your comments did not push this issue higher in the list of my things to do in my free time.
Not last summer and not now. The person putting this back on my Agenda was Freso (thanks for that).

I will troubleshoot and test this further later.
When I have time, a usable machine available and I am in a mood that can't be shattered (2 of which are usually not the case after work currently).
Comment by Johannes Dewender (JonnyJD) - Sunday, 10 February 2019, 18:31 GMT
I can't reproduce this (the keyring issue) for isrcsubmit-git anymore.
So I guess this was fixed upstream.

For the need to be able to use pip to install this (at least to some extent, see comment above), I created a ticket on isrcsubmit:
https://github.com/JonnyJD/musicbrainz-isrcsubmit/issues/121
Comment by Eli Schwartz (eschwartz) - Tuesday, 12 February 2019, 03:31 GMT
> However, your style of commenting does not actually educate people, but makes them ragequit instead of making changes after a hard day of work.

My style of commenting is what happens when I lose my cool after you accuse me of "rambling" on about the technical source of the problem and claim my analysis "doesn't have anything to do with the actual problem".

My commenting style is what happens *after* I have long ago decided you're totally not worth my time, and the only thing I actually care about anymore is expressing to *other users* why it is the module itself at fault.

Loading...