Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1456722
Summary: | kvm_stat adds python2 dependency | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Peter Robinson <pbrobinson> |
Component: | kernel | Assignee: | Kernel Maintainer List <kernel-maint> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | bkabrda, crobinso, cstratak, gansalmon, ichavero, ishcherb, itamar, jeremy, jforbes, jonathan, kernel-maint, madhu.chinakonda, mchehab, mcyprian, mhroncok, pviktori, rkuska, tomspur, torsava |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2018-03-23 22:15:47 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 1269538 |
Description
Peter Robinson
2017-05-30 08:32:35 UTC
This is not a kernel issue. kvm_stat was added to the kernel-tools package (moved from kvm). The dep is on /usr/bin/python which is correct, it doesn't care what python version you have installed. If python3 is *the* system python, then it should also provide /usr/bin/python. rpm -qlp --requires kernel-tools-4.12.3-1.fc26.x86_64.rpm | grep py /usr/bin/python Assigning this to python3 so a suitable solution can be determined. (In reply to Justin M. Forbes from comment #1) > This is not a kernel issue. kvm_stat was added to the kernel-tools package > (moved from kvm). The dep is on /usr/bin/python which is correct, it > doesn't care what python version you have installed. If python3 is *the* > system python, then it should also provide /usr/bin/python. > > rpm -qlp --requires kernel-tools-4.12.3-1.fc26.x86_64.rpm | grep py > /usr/bin/python > > > Assigning this to python3 so a suitable solution can be determined. That is wrong actually. Fedora follows the upstream recommendation in regards to /usr/bin/python (see PEP 394), which means that for the time being /usr/bin/python points to python2. Python 3 is the default system-python we ship (on which also anaconda, dnf, abrt and other system tools depend on), but when it comes to shebangs, they should be explicit, so either /usr/bin/python2 or /usr/bin/python3. The recommended way to address that is to replace the shebang with a 'sed' command during %prep. According to the same PEP "python should be used in the shebang line only for scripts that are source compatible with both Python 2 and 3" An application which is source compatible with both would be correct in using /usr/bin/python. We basically have a dep problem. Yes, using /usr/bin/python in the shebang will work correctly -- but in Fedora, it'll use the Python 2 stack, and it drags that in as a dependency. Changing the shebang to /usr/bin/python3 is the easiest way to avoid this. Sorry for that. (Build systems like setuptools can do that change automatically, but with a custom Makefile, you unfortunately need a Fedora-specific patch.) (In reply to Justin M. Forbes from comment #3) > According to the same PEP "python should be used in the shebang line only > for scripts that are source compatible with both Python 2 and 3" An > application which is source compatible with both would be correct in using > /usr/bin/python. Yes. And as an upstream project, using /usr/bin/python to indicate "whatever python" is fine. However that is not the case in Fedora: From https://fedoraproject.org/wiki/Packaging:Python "If the executables provide the same functionality independent of whether they are run on top of Python 2 or Python 3, then only the Python 3 version of the executable should be packaged." => In this case, we want to package the "Python 3 version of the executable". "packages in Fedora should not depend on where /usr/bin/python happens to point but instead should call the proper executable for the needed python major version directly, either /usr/bin/python2 or /usr/bin/python3 as appropriate" => The Python 3 version of the executable we need to package should have /usr/bin/python3 in shebang. Note that those are all "shoulds" guidelines, so if there is a good enough reason for kvm_stat to run on Python 2, rules can be broken. However, using Python 2 just because using Python 3 would involve changing the shebang(s) is IMHO not a good enough reason. There is a how to for changing the shebang in spec: http://python-rpm-porting.readthedocs.io/en/latest/applications.html#are-shebangs-dragging-you-down-to-python-2 kernel-4.12.4-300.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-14ad2c5d17 I wonder why adding passive aggressive comments in the specfile is necessary :( kernel-4.12.4-300.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-14ad2c5d17 (In reply to Miro Hrončok from comment #8) > I wonder why adding passive aggressive comments in the specfile is necessary > :( I don't see any passive aggressive comments in the spec... The only log I see is: * Tue Jul 25 2017 Justin M. Forbes <jforbes> - Force python3 for kvm_stat because we can't dep (rhbz 1456722) which is a technically accurate description of the fix and problem. I was actually referring to: +# Because the python 3 transition is fail +Patch124: force-python3-in-kvm_stat.patch I might have misjudged the tone of that, but comments like this make me feel that every effort we make to move things forward is just bothering others. kvm_stat didn't actually support Python 3 (I assume that's why the patch forcing it to Python 3 was dropped) so I submitted a patch that makes it compatible: https://patchwork.kernel.org/patch/9954979/ (In reply to Jeremy Cline from comment #12) > kvm_stat didn't actually support Python 3 (I assume that's why the patch > forcing it to Python 3 was dropped) so I submitted a patch that makes it > compatible: https://patchwork.kernel.org/patch/9954979/ I don't have an account there, so I'll just post a comment here: - print ' %9d' % s[k][1], + print(' %9d' % s[k][1]) This is not an equivalent change (notice the ,). The equivalent change would be: + from __future__ import print_function ... + print(' %9d' % s[k][1], end=' ') (In reply to Miro Hrončok from comment #13) > I don't have an account there, so I'll just post a comment here: > > - print ' %9d' % s[k][1], > + print(' %9d' % s[k][1]) > > This is not an equivalent change (notice the ,). The equivalent change would > be: > > + from __future__ import print_function > > ... > > + print(' %9d' % s[k][1], end=' ') Good catch, thanks for taking a look at the patch! I also discovered there's a few cases where str and bytes are getting mixed so I'm going to need to rework this patch a bit. Pretty sure this is fixed, kernel 4.15 shipped with kvm_stat python3 support and all the packaging bits are in place AFAICT |