Spec URL: <spec info here> SRPM URL: <srpm info here> Description: wxPython-Phoenix is a is a new implementation of wxPython focused on improving speed, maintainability and extensibility. Just like "Classic" wxPython it wraps the wxWidgets C++ toolkit and provides access to the user interface portions of the wx API, enabling Python applications to have a GUI on Windows, Macs or Unix systems with a native look and feel and requiring very little (if any) platform specific code. Fedora Account System Username: swt2c At the moment, this is just a placeholder review request to let people know that I'm working on packaging Phoenix for Fedora.
Spec URL: https://www.techie.net/~talbert/python-wxpython4.spec SRPM URL: https://www.techie.net/~talbert/python-wxpython4-4.0.1-1.fc28.src.rpm Description: wxPython4 is a is a new implementation of wxPython focused on improving speed, maintainability and extensibility. Just like "Classic" wxPython it wraps the wxWidgets C++ toolkit and provides access to the user interface portions of the wx API, enabling Python applications to have a GUI on Windows, Macs or Unix systems with a native look and feel and requiring very little (if any) platform specific code. Fedora Account System Username: swt2c
Python 2 install uses easy_install, Python 3 uses build.py, is there a reason for the difference? What's up with tests? They seem to be both disabled and || true. Do they fail totally, or only some fail? Maybe use bcond for disabling tests, so it's easy to enable them locally with command line witch. The docs package doesn't seem to provide/obsolete the copr python-wx-phoenix-docs. Maybe you can get rid fo groups declarations? Maybe you can just set Summary and use %{summary} everywhere instead of %{sum}? (Not important, just a suggestion.) Building now for more stuff...
What Fedoras/EPELs are you targeting here? Use python2-numpy instead of numpy if possible.
License file LICENSE_BSD_Simple.txt is not marked as %license. /usr/lib64/python2.7/site-packages/wxPython-4.0.1-py2.7-linux-x86_64.egg/wx/lib/pubsub/LICENSE_BSD_Simple.txt /usr/lib64/python3.6/site-packages/wx/lib/pubsub/LICENSE_BSD_Simple.txt Found multiple licenses, will attach licenses.txt. License files installed when any subpackage combination is installed: No license in docs. However media and webview does not need licenses, as they require the main packages. There are wxWidgets in ext/wxWidgets, are those used or not? If not, can the folder be deleted, just to make it clear? Rpmlint screams on me: bad shebangs, bad end of lines, bad utf8, bad executable bits. Will attach rpmlint.txt with some filtering done by hand. Python 2 installs to /usr/lib64/python2.7/site-packages/wxPython-4.0.1-py2.7-linux-x86_64.egg/wx/... ehile Python 3 installs to /usr/lib64/python3.6/site-packages/wx/... - I suppose that's related to the easy_install invocation. IS this not to conflict with wxpython 3? That however installs to /usr/lib64/python2.7/site-packages/wx-3.0-gtk3/wx/...
Created attachment 1394590 [details] rpmlint filtered a bit
Created attachment 1394591 [details] license breakdown
$ mock -r fedora-rawhide-x86_64 --shell <mock-chroot> sh-4.4# python3 Python 3.6.4 (default, Feb 1 2018, 11:03:59) [GCC 8.0.1 20180127 (Red Hat 8.0.1-0.6)] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import wx 14:51:46: Warning: Mismatch between the program and library build versions detected. The library used 3.0 (wchar_t,compiler with C++ ABI 1011,wx containers,compatible with 2.8), and wxPython used 3.0 (wchar_t,compiler with C++ ABI 1012,wx containers,compatible with 2.8).
Thanks a lot for the review! I'll get working on your comments. (In reply to Miro Hrončok from comment #2) > Python 2 install uses easy_install, Python 3 uses build.py, is there a > reason for the difference? Yes, it's so the Python 2 version will get installed in a location that doesn't conflict with the wxPython 3.0 package. I believe I roughly followed this guidance here for having multiple versions of the same module: https://fedoraproject.org/wiki/Packaging:Python_Eggs#Multiple_Versions > What's up with tests? They seem to be both disabled and || true. Do they > fail totally, or only some fail? Hah. It's been so long since I tried to run them in mock or koji that I can't exactly remember. There are definitely some tests that fail, but I'm not sure if I ever got the tests to run at all successfully under mock/koji (due to needing a display since most of the tests are GUI-based). I'll try them again and see what happens now. > Maybe use bcond for disabling tests, so it's easy to enable them locally > with command line witch. > > The docs package doesn't seem to provide/obsolete the copr > python-wx-phoenix-docs. > > > Maybe you can get rid fo groups declarations? > > > Maybe you can just set Summary and use %{summary} everywhere instead of > %{sum}? (Not important, just a suggestion.) > > > Building now for more stuff... All good catches, thanks!
(In reply to Miro Hrončok from comment #4) > There are wxWidgets in ext/wxWidgets, are those used or not? If not, can the > folder be deleted, just to make it clear? The wxWidgets source isn't used EXCEPT for the interface headers (ext/wxWidgets/interface/...) and the doxygen stuff. So, we can't delete ext/wxWidgets completely, but we could delete some subset of it. What do you think about that? > Python 2 installs to > /usr/lib64/python2.7/site-packages/wxPython-4.0.1-py2.7-linux-x86_64.egg/wx/. > .. ehile Python 3 installs to /usr/lib64/python3.6/site-packages/wx/... - I > suppose that's related to the easy_install invocation. IS this not to > conflict with wxpython 3? That however installs to > /usr/lib64/python2.7/site-packages/wx-3.0-gtk3/wx/... Yes, that is all part of keeping wxPython 3 as 'primary' for Python 2. wxPython 3 DOES install to /usr/lib64/python2.7/site-packages/wx-3.0-gtk3/wx/..., but it has a wx.pth file (/usr/lib64/python2.7/site-packages/wx.pth) that points to wx-3.0-gtk3.
(In reply to Miro Hrončok from comment #7) > $ mock -r fedora-rawhide-x86_64 --shell > <mock-chroot> sh-4.4# python3 > Python 3.6.4 (default, Feb 1 2018, 11:03:59) > [GCC 8.0.1 20180127 (Red Hat 8.0.1-0.6)] on linux > Type "help", "copyright", "credits" or "license" for more information. > >>> import wx > 14:51:46: Warning: Mismatch between the program and library build versions > detected. > The library used 3.0 (wchar_t,compiler with C++ ABI 1011,wx > containers,compatible with 2.8), > and wxPython used 3.0 (wchar_t,compiler with C++ ABI 1012,wx > containers,compatible with 2.8). Sigh. I thought we had gotten rid of that, but I guess we did not. That's coming from wxGTK3 not wxPython4. I think we're really only seeing it because we're in the middle of the mass rebuild right now. Once the mass rebuild completes for wxGTK3, that should go away.
(In reply to Scott Talbert from comment #9) > (In reply to Miro Hrončok from comment #4) > > There are wxWidgets in ext/wxWidgets, are those used or not? If not, can the > > folder be deleted, just to make it clear? > > The wxWidgets source isn't used EXCEPT for the interface headers > (ext/wxWidgets/interface/...) and the doxygen stuff. So, we can't delete > ext/wxWidgets completely, but we could delete some subset of it. What do > you think about that? Can we somehow use the wxGTK3-devel header files instead? If not, just maybe add a note to the spec?
(In reply to Miro Hrončok from comment #11) > (In reply to Scott Talbert from comment #9) > > (In reply to Miro Hrončok from comment #4) > > > There are wxWidgets in ext/wxWidgets, are those used or not? If not, can the > > > folder be deleted, just to make it clear? > > > > The wxWidgets source isn't used EXCEPT for the interface headers > > (ext/wxWidgets/interface/...) and the doxygen stuff. So, we can't delete > > ext/wxWidgets completely, but we could delete some subset of it. What do > > you think about that? > > Can we somehow use the wxGTK3-devel header files instead? If not, just maybe > add a note to the spec? The wxGTK3-devel headers do not include the interface headers. At one point, however, I had created a wxGTK3-xmldocs package which had the information needed for the wxPython4 build. However, the interface headers were/are full of bugs, so I was frequently having to patch them in wxGTK3 package. Eventually, I abandoned that approach to use the bundled copies of the headers which have received lots of bug fixes.
Okay, I think this should address most of the rpmlint issues, re-enables the tests (but there are still some that fail, and some that fail only in koji, so I'm going to leave them || true for now), and addresses your other comments: Spec URL: https://www.techie.net/~talbert/python-wxpython4.spec SRPM URL: https://www.techie.net/~talbert/python-wxpython4-4.0.1-2.fc28.src.rpm Thanks again Miro for your review. :-)
Please add a comment about the license. "wxWidgets and BSD" doesn't provide enough information. I've just noticed the docs subpackage. It's common to name it doc, not docs. Guidelines says it is recommended, so feel free to keep it as it is. https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation Please also change sip and sip-devel to python2-sip and python2-sip-devel. Sorry for not noticing this earlier. Still no license file in docs package. python-wxpython4-docs.noarch: W: file-not-utf8 /usr/share/doc/python-wxpython4-docs/demo/TestTable.txt python-wxpython4-docs.noarch: W: file-not-utf8 /usr/share/doc/python-wxpython4-docs/docs/sphinx/_downloads/i18nwxapp/i18nwxapp.zip python-wxpython4-docs.noarch: W: file-not-utf8 /usr/share/doc/python-wxpython4-docs/docs/sphinx/_downloads/i18nwxapp/locale/I18Nwxapp.pot python-wxpython4-docs.noarch: W: file-not-utf8 /usr/share/doc/python-wxpython4-docs/docs/sphinx/class_summary.pkl python-wxpython4-docs.noarch: W: file-not-utf8 /usr/share/doc/python-wxpython4-docs/docs/sphinx/wx.1moduleindex.pkl python-wxpython4-docs.noarch: W: spurious-executable-perm /usr/share/doc/python-wxpython4-docs/samples/mainloop/mainloop.py
Fixed all of the above. Spec URL: https://www.techie.net/~talbert/python-wxpython4.spec SRPM URL: https://www.techie.net/~talbert/python-wxpython4-4.0.1-3.fc28.src.rpm
spec diff shows no changes.
(In reply to Miro Hrončok from comment #16) > spec diff shows no changes. My bad, I failed to copy into public_html on my server. Fixed now.
Thanks. Ship it (also known as: the package is APPROVED).
(fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-wxpython4. You may commit to the branch "f27" in about 10 minutes.