https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Bug ID: 1427341 Summary: Review Request: python-gamera - Gamera is a framework for building document analysis applications. Product: Fedora Version: rawhide Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: vincent@casperlefantom.net QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.2-1.fc25.src.rpm
Description: Gamera is a framework for building document analysis applications. It is not a packaged document recognition system, but a toolkit for building document image recognition systems. For more information about Gamera, visit the Gamera website at: http://gamera.informatik.hsnr.de/
I'm not really new or beginner in Fedora, but it is my first package. I would contribute to Fedora community and packaging is an really important task for me. These days I learnt building RPM, I asked for help on IRC. After more experiences, I would join Python SIG to help for porting to Python3 and because it is my favorite programming language. I planned to ensure the porting to Python3 as soon as possible. Don't hesitate to contact me if you have questions or comments. Moreover, it is my first package. If anybody want to sponsor me I would be grateful.
Thanks in advance.
Best regards,
VincentS
Fedora Account System Username: vincents
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Vini vincent@casperlefantom.net changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |http://gamera.informatik.hs | |nr.de/ Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Charalampos Stratakis cstratak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |cstratak@redhat.com Assignee|nobody@fedoraproject.org |cstratak@redhat.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #1 from Charalampos Stratakis cstratak@redhat.com --- Some initial comments:
You should get rid of the macros on top of the SPEC file as these are required only for EPEL.
You should make consistent use of macros so URL should be in the form of: https://github.com/hsnr-%%7Bsrcname%7D/%%7Bsrcname%7D
and the Source0 url in a similar fashion (don't forget the %{version} macro)
For the '%{__python2} setup.py build' there is already a macro that you can use: '%py2_build'
If the c++ sources need to be removed (what is the reason?) they should be removed during prep and not after compiling them.
Also it seems that upstream there is no activity for 3 months now and the requirements for the project seems quite low, are you sure that gamera is compatible with the latest versions of its requirements (e.g. in Fedora rawhide we have now gcc 7)?
On another note please read the Fedora Python Packaging Guidelines [0] for more info on how to structure these kind of SPEC files.
[0] https://fedoraproject.org/wiki/Packaging:Python
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #2 from Charalampos Stratakis cstratak@redhat.com --- The compilation fails as well when trying to build the SRPM in mock.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mhroncok@redhat.com Blocks|177841 (FE-NEEDSPONSOR) |
--- Comment #3 from Miro Hrončok mhroncok@redhat.com --- Hi VincentS, I'm willing to sponsor you if you finish this and will review some packages as well. Lifting FE-NEEDSPONSOR.
Charalampos, please go ahead with the review, I'll be watching ;)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #4 from VincentS vincent@casperlefantom.net --- Thanks Charalampos and Miro,
I achieved modifications according to comments.
Updated srpm and spec file: Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.2-2.fc25.src.rpm
I contacted upstrem about GCC7 because it doesn't compile only with it.
Miro, I reviewed Bug 1385244 - pew: https://bugzilla.redhat.com/show_bug.cgi?id=1385244 and Bug 1333928 - python-searchlightclient: https://bugzilla.redhat.com/show_bug.cgi?id=1333928 If you have packages to review, you can ask me.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #5 from Miro Hrončok mhroncok@redhat.com --- (In reply to VincentS from comment #4)
Miro, I reviewed Bug 1385244 - pew: https://bugzilla.redhat.com/show_bug.cgi?id=1385244 and Bug 1333928 - python-searchlightclient: https://bugzilla.redhat.com/show_bug.cgi?id=1333928 If you have packages to review, you can ask me.
Please go on with those than.
Tip: In case you have a different opinion than the author on something but the thing os still valid (e.g. with the macros in Bug 1333928), please be less demanding when expressing your opinion - it's better to say: "I'd rather do it like this (show), because I think it would make the spec easier to understand, what do you think?" etc., than "I think you should do this and that".
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #6 from VincentS vincent@casperlefantom.net --- Miro, thanks for your recommendation. I'll be less harsh.
While waiting for modifications on these review requests, I continue to do some informal reviews.
https://bugzilla.redhat.com/show_bug.cgi?id=1413474 https://bugzilla.redhat.com/show_bug.cgi?id=1339227
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #7 from Charalampos Stratakis cstratak@redhat.com --- Created attachment 1260845 --> https://bugzilla.redhat.com/attachment.cgi?id=1260845&action=edit gcc 7 build log
So here is the build log from the failures with gcc 7.0.1, which can be sent to the upstream developers.
Also another useful document for them would be https://gcc.gnu.org/gcc-7/porting_to.html
Fedora 25 (which includes gcc 6.3.1) compiles just fine, however it would be for the best, that this compilation issue gets resolved so the package can be built against the latest Fedora branches as well.
The review will need to be stalled until the compilation issue is resolved.
As soon as this is resolved upstream, a patch with the fixes can be applied to the package (I can explain the steps if we reach that point) or upstream will release a new version, so just an update to the latest sources will be sufficient.
Please post in the bugzilla if there is some movement from upstream in that direction so the package review can proceed.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #8 from VincentS vincent@casperlefantom.net --- After having discussed with upstream, he told me that he couldn't solve this problem because he doesn't use GCC7. So I decided to try to find a solution.
Finally, I fixed the issue. I'm waiting the validation of the pull request. https://github.com/hsnr-gamera/gamera/pull/7
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #9 from VincentS vincent@casperlefantom.net --- Upstream updated to a new release with gcc7 supported.
Here are new spec file and srpm: Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-1.fc25.src.rpm
Moreover, I build with Koji to verify if it compile for rawhide. https://koji.fedoraproject.org/koji/taskinfo?taskID=18270494
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #10 from VincentS vincent@casperlefantom.net --- I did another informal review. https://bugzilla.redhat.com/show_bug.cgi?id=1429226
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #11 from Charalampos Stratakis cstratak@redhat.com --- As this package is quite complex I will post my findings so far and after we get these out of the way, we can proceed with the rest.
All the BuildRequires are fine.
You should remove the comment after %install about the python2 and python3 install, since the package is python2 and only uses the %py2_install macro
The desktop file installation should happen after the %py2_install.
Also what is the reason for this line? chmod 755 %{buildroot}/%{python2_sitearch}/%{srcname}/gendoc.py Same for: %doc %attr(644,-,-) README And: %defattr(644,root,root,755)
The devel subpackage should have a Requires tag for the main package. See: https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
This comment is not required: # For noarch packages: sitelib
Also while it is good to keep some level of abstraction when specifying the directories and binaries at the files section, I would recommend to be more specific about some things, since it helps while working with the package. e.g.
Remove: '%{python2_sitearch}/%{srcname}*' Add '%{python2_sitearch}/%{srcname}' and '%{python2_sitearch}/%{srcname}-%{version}-py?.?.egg-info'
Also the %{_includedir}/python2.7/%{srcname}/ should be changed to %{_includedir}/python%{python2_version}/%{srcname}/
Now as for the binaries. The package provides two of them for which it would be good practice to list them individually at the respective files sections, so basically instead of %{_bindir}/*, use %{_bindir}/gamera_gui and %{_bindir}/gamera_post_install.py
However what is the purpose of gamera_post_install.py binary? I kinda think that it might be required to be included at the devel subpackage.
Also the package ships a lot of .so libraries so you should read the respective section at the packaging guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #12 from Miro Hrončok mhroncok@redhat.com --- (In reply to Charalampos Stratakis from comment #11)
Also the package ships a lot of .so libraries so you should read the respective section at the packaging guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
In python2_sitearch or in /usr/lib64/lib...so ?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #13 from Charalampos Stratakis cstratak@redhat.com --- (In reply to Miro Hrončok from comment #12)
(In reply to Charalampos Stratakis from comment #11)
Also the package ships a lot of .so libraries so you should read the respective section at the packaging guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
In python2_sitearch or in /usr/lib64/lib...so ?
At
/usr/lib64/python2.7/site-packages/gamera/
and
/usr/lib64/python2.7/site-packages/gamera/plugins/
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #14 from Miro Hrončok mhroncok@redhat.com --- Those are fine and the https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries does not apply for those.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #15 from VincentS vincent@casperlefantom.net --- (In reply to Charalampos Stratakis from comment #11)
As this package is quite complex I will post my findings so far and after we get these out of the way, we can proceed with the rest.
All the BuildRequires are fine.
You should remove the comment after %install about the python2 and python3 install, since the package is python2 and only uses the %py2_install macro
All right.
The desktop file installation should happen after the %py2_install.
Done
Also what is the reason for this line? chmod 755 %{buildroot}/%{python2_sitearch}/%{srcname}/gendoc.py Same for: %doc %attr(644,-,-) README And: %defattr(644,root,root,755)
As guidelines say, Permissions on files must be set properly. I saw with rmplint : - python-gamera.x86_64: E: wrong-script-interpreter /usr/lib64/python2.7/site-packages/gamera/gendoc.py /usr/bin/env python - python-gamera.x86_64: E: non-executable-script /usr/lib64/python2.7/site-packages/gamera/gendoc.py 644 /usr/bin/env python - python-gamera.x86_64: W: spurious-executable-perm /usr/share/doc/python-gamera/README I thought permissions hadn't set properly, so I added chmod and %defattr to solve these problems. What do you think about ?
The devel subpackage should have a Requires tag for the main package. See: https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
Yes I forgot.
This comment is not required: # For noarch packages: sitelib
Done
Also while it is good to keep some level of abstraction when specifying the directories and binaries at the files section, I would recommend to be more specific about some things, since it helps while working with the package. e.g.
Remove: '%{python2_sitearch}/%{srcname}*' Add '%{python2_sitearch}/%{srcname}' and '%{python2_sitearch}/%{srcname}-%{version}-py?.?.egg-info'
Ok I understand.
Also the %{_includedir}/python2.7/%{srcname}/ should be changed to %{_includedir}/python%{python2_version}/%{srcname}/
All right.
Now as for the binaries. The package provides two of them for which it would be good practice to list them individually at the respective files sections, so basically instead of %{_bindir}/*, use %{_bindir}/gamera_gui and %{_bindir}/gamera_post_install.py
Great, for few binaries I need to be precise.
However what is the purpose of gamera_post_install.py binary? I kinda think that it might be required to be included at the devel subpackage.
In fact, this file is only needed for Windows installation. So I removed it.
Also the package ships a lot of .so libraries so you should read the respective section at the packaging guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
Thanks Miro for your reply and Charalampos for your comments.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #16 from Miro Hrončok mhroncok@redhat.com --- (In reply to VincentS from comment #15)
(In reply to Charalampos Stratakis from comment #11)
Also what is the reason for this line? chmod 755 %{buildroot}/%{python2_sitearch}/%{srcname}/gendoc.py Same for: %doc %attr(644,-,-) README And: %defattr(644,root,root,755)
As guidelines say, Permissions on files must be set properly. I saw with rmplint :
- python-gamera.x86_64: E: wrong-script-interpreter
/usr/lib64/python2.7/site-packages/gamera/gendoc.py /usr/bin/env python
- python-gamera.x86_64: E: non-executable-script
/usr/lib64/python2.7/site-packages/gamera/gendoc.py 644 /usr/bin/env python
- python-gamera.x86_64: W: spurious-executable-perm
/usr/share/doc/python-gamera/README I thought permissions hadn't set properly, so I added chmod and %defattr to solve these problems. What do you think about ?
First, /usr/lib64/python2.7/site-packages/gamera/gendoc.py:
This is a bit more complicated.
Based on the rpmlint output you can say that:
* the file has a shebang that says "#!/usr/bin/env python" (note that using /usr/bin/env in shebangs is forbidden in Fedora packages) * the file does not have executable permissions
So you have to ask yourself the question: "Shall this file be treated as executable or not?" The answer is usually "not" unless the file is in PATH (such as /usr/bin). So in this case instead of giving it permissions, remove the shebang. This is nice approach, as it checks if the shebang is there and if so, it removes it:
head -n 1 gamera/gendoc.py | grep '#!/usr/' && sed -i '1d' gamera/gendoc.py
If the file should have been an executable (not this case), you would need to change the shebang to "#!/usr/bin/python2" and add the executable permission.
Second: README:
I'd suggest running "chmod -x README" in %prep instead, as it explicitly says "remove the executable permission" rather than "set the permissions to exactly this and that".
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #17 from VincentS vincent@casperlefantom.net --- (In reply to Miro Hrončok from comment #16)
First, /usr/lib64/python2.7/site-packages/gamera/gendoc.py:
This is a bit more complicated.
Based on the rpmlint output you can say that:
- the file has a shebang that says "#!/usr/bin/env python" (note that using
/usr/bin/env in shebangs is forbidden in Fedora packages)
- the file does not have executable permissions
So you have to ask yourself the question: "Shall this file be treated as executable or not?" The answer is usually "not" unless the file is in PATH (such as /usr/bin). So in this case instead of giving it permissions, remove the shebang. This is nice approach, as it checks if the shebang is there and if so, it removes it:
head -n 1 gamera/gendoc.py | grep '#!/usr/' && sed -i '1d'
gamera/gendoc.py
If the file should have been an executable (not this case), you would need to change the shebang to "#!/usr/bin/python2" and add the executable permission.
Second: README:
I'd suggest running "chmod -x README" in %prep instead, as it explicitly says "remove the executable permission" rather than "set the permissions to exactly this and that".
Ok, all right.
Now I'll know for future through your really good explanations.
I fixed this according to your recommendations.
Here are new SPEC and SRPM: Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-2.fc25.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #18 from Miro Hrončok mhroncok@redhat.com --- OK, let me have a look.
1) Summary. You don't have to set the %{sum} macro, you can write a summary into the Summary field and then use %{summary} in the subpackages.
2) Description. I don't think an URL belongs there. Just use it as the URL instead of the github one.
3) Since this is Python 2 only, I think you should have python2-gamera subpackage instead of python-gamera package. Once python- will mean python3, you would have trouble this way.
4) The summary of devel subpackage should definitely not bee the same as the main package.
5) "Devel gamera package." is not verbose enough as description. "Development package for gamera. Allows you to develop your apps on top of the gamera framework." might be a little bit better.
6) I'd add some whitespace to %install so the lines that do one thing are together
7) Add comment to the shebang removal so it's obvious what's happening there and WHY
8) I don't think the devel subpackage is a header only library per se, so the -static provide here is bogus.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #19 from Miro Hrončok mhroncok@redhat.com --- s/bee/be/
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #20 from VincentS vincent@casperlefantom.net --- Here are new links according to Miro's advises.
Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-3.fc25.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #21 from Charalampos Stratakis cstratak@redhat.com --- Hello Vincent,
Please omit this sentence from the description: "For more information about Gamera, visit the Gamera website at:"
Use the summary macro as %{summary}, not %summary
Add this description for the python2 subpackage (which is basically the same as the original description):
"Gamera is a framework for building document analysis applications. It is not a packaged document recognition system, but a toolkit for building document image recognition systems."
Also a comment above 'chmod -x README' would be nice. Something like "Remove the executable bit from the README file' or something equivalent.
I believe these should be the last items required before I run the fedora-review tool for the package.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #22 from Miro Hrončok mhroncok@redhat.com --- (In reply to Charalampos Stratakis from comment #21)
Also a comment above 'chmod -x README' would be nice. Something like "Remove the executable bit from the README file' or something equivalent.
I disagree here. That would be "captain obvious comment". If a comment is desired, better explain why are we doing that, not what are we doing (that's clear from the command itself, it's not a super ugly complicated nested bash-fu). Something like: "The README is obviously not supposed to be executed, it doesn't even have a shebang". Also, maybe fix this upstream instead?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #23 from VincentS vincent@casperlefantom.net --- I followed your advises Charalampos and Miro and I will discuss with upstream about executable right on README file.
Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-4.fc25.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #24 from Charalampos Stratakis cstratak@redhat.com --- So after some discussions with Miro and running the fedora-review tool we figured out some more things.
The devel subpackage should be named python2-gamera-devel and this macro should be used to provide the python namespace for it (similar to the main package):
%{?python_provide:%python_provide python2-%{srcname}-devel}
The package bundles some software as well as it can be seen from these directories:
https://github.com/hsnr-gamera/gamera/tree/master/src https://github.com/hsnr-gamera/gamera/tree/master/include
It bundles vigra, zlib, eo, libpng and libtiff (hope I didn't miss anything). The zlib folders should be removed during prep (from the Include and src subdirs) and instead a BuildRequires for the zlib-devel package should be used.
Same for libpng-1.2.5 and libtiff folders.
As for the rest of the bundled software, they should be provided as virtual provides according to: https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Librar...
So you will need to add at the main package: Provides: bundled(vigra) = 1.6.0 Provides: bundled(eo) = 1.3.1
Also there is an executable bit which should be removed during prep from:
/include/plugins/deformations.hpp
The package has documentation as well which must be built. I checked the sources and docs are being generated by executing the file gendoc.py. Then it should be be placed at a separate subpackage, named python2-gamera-doc, with a similar naming pattern as the devel subpackage.
I still haven't figured out the best way to do that, as gamera uses itself to build the documentation, so some environment variable will have to be exported during build. I will conduct some more testing and let you know.
Now about the license.
From the fedora-review output: Licenses found: "LGPL (v2)", "*No copyright* LGPL (v2 or later)", "LGPL (v2 or later)", "GPL (v2 or later)", "CC by-sa GPL", "Unknown or generated", "MIT/X11 (BSD like)", "NTP", "MIT/X11 (BSD like) NTP", "zlib/libpng", "LGPL", "GPL (v2 or later) (with incorrect FSF address)"
The final license of the package (indicated at the License field) should be "GPLv2+ and MIT and CC BY-SA" (thanks Miro), however I will need to dig a bit deeper to verify that, so don't change the field yet.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #25 from Miro Hrončok mhroncok@redhat.com --- (In reply to Charalampos Stratakis from comment #24)
The package has documentation as well which must be built.
Should, not must.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #26 from Charalampos Stratakis cstratak@redhat.com --- Created attachment 1271137 --> https://bugzilla.redhat.com/attachment.cgi?id=1271137&action=edit licenses
After some digging it seems that the original deduction is true, so the actual license of the package is "GPLv2+ and MIT and CC BY-SA", however I would highly recommend to send an email at the fedora-legal mailing list ( legal@lists.fedoraproject.org ) with the attached text and a description of the situation, to ensure that this should be the final license of the package.
I have omitted the licenses of zlib/libtiff from the file, as these files should be removed during prep.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #27 from Charalampos Stratakis cstratak@redhat.com --- Also in regards to the documentation. I figured out how to generate it, but it will require some hacks at the SPEC file and certainly it's not a blocker for accepting the package.
If you would like to generate the documentation (before or after the review of the package) please reach out to me or Miro, so we can guide you in that respect.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Charalampos Stratakis cstratak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |vincent@casperlefantom.net Flags| |needinfo?(vincent@casperlef | |antom.net)
--- Comment #28 from Charalampos Stratakis cstratak@redhat.com --- Any news on the package?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #29 from Charalampos Stratakis cstratak@redhat.com --- The submitter hasn't answered in over a month. According to the policy for stalled package reviews [0] I'm declaring this review as stalled, and if no response is given within a week, this bugzilla will be closed.
[0] https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews?rd=Extras/...
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
VincentS vincent@casperlefantom.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(vincent@casperlef | |antom.net) |
--- Comment #30 from VincentS vincent@casperlefantom.net --- I'm sorry about no news and thank you for all your work on it, actually I'm waiting a response from legal@lists.fedoraproject.org about license.
Otherwise, I modified Spec with all your recommendations. I only hadn't add documentation.
Do I should have a response before send new version of SPEC file ?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #31 from Miro Hrončok mhroncok@redhat.com --- (In reply to VincentS from comment #30)
I'm sorry about no news and thank you for all your work on it, actually I'm waiting a response from legal@lists.fedoraproject.org about license.
I don't see your e-mail there.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #32 from VincentS vincent@casperlefantom.net --- Really ? I don't understand.
I just resent it.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #33 from VincentS vincent@casperlefantom.net --- (In reply to Miro Hrončok from comment #31)
(In reply to VincentS from comment #30)
I'm sorry about no news and thank you for all your work on it, actually I'm waiting a response from legal@lists.fedoraproject.org about license.
I don't see your e-mail there.
I received this answer :
Your mail to 'legal@lists.fedoraproject.org' with the subject
Fwd: Difficulty about package license
Is being held until the list moderator can review it for approval.
The message is being held because:
The message is not from a list member
Either the message will get posted to the list, or you will receive notification of the moderator's decision.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #34 from Matthieu Saulnier casper@casperlefantom.net --- you have to subscribe to the ML first
see "Manage subscription" button here https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #35 from VincentS vincent@casperlefantom.net --- (In reply to Matthieu Saulnier from comment #34)
you have to subscribe to the ML first
see "Manage subscription" button here https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/
Thank you for your reply Matthieu.
I signed up to the list and resent the mail.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #36 from VincentS vincent@casperlefantom.net --- I've done all modifications recommended, here are new links.
I didn't try to generate documentation, I'm going to see this later on.
Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-5.fc25.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #37 from Charalampos Stratakis cstratak@redhat.com --- Hello Vincent and thanks for continuing working on it. I currently replied to the legal mailing list as the license issue is not yet entirely clear. However I will proceed with the fedora-review tool so the review can move a bit forward.
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. Note: These BR are not needed: gcc See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
Note from reviewer: gcc is now required to be listed as a dependency. See: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Req...
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path.
Note from reviewer: The shared libraries are in /usr/lib64/python2.7/site-packages/gamera/ and upstream does not version them however they are required for runtime, thus they are not development .so files.
They are also not included in ld path.
[x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs.
Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
[?]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "LGPL (v2)", "*No copyright* LGPL (v2 or later)", "LGPL (v2 or later)", "GPL (v2 or later)", "CC by-sa GPL", "Unknown or generated", "MIT/X11 (BSD like)", "NTP", "MIT/X11 (BSD like) NTP", "LGPL", "GPL (v2 or later) (with incorrect FSF address)". 295 files have unknown license.
Note from reviewer: Still waiting on an answer from the legal mailing list. Thread here: https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/...
[x]: License file installed when any subpackage combination is installed. [?]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec.
Note from reviewer: Still waiting on an answer from the legal mailing list
[x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception.
Note from reviewer: FPC involvement is not required anymore, see https://pagure.io/fesco/issue/1483
[x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [!]: Requires correct, justified where necessary.
Note from reviewer: Package should have a runtime requirement for wxPython as the gamera_gui binary needs it to run. If it's not present a traceback is shown.
You should add:
Requires: wxPython
Right after the summary tag of the python2 subpackage.
[x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files.
Note from reviewer: Documentation is not being built. It is not a blocker for the review and it's up to the packager if he wants to do it at some point in the future.
[x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local
Python: [x]: Python eggs must not download any dependencies during the build process. [x]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Binary eggs must be removed in %prep
===== SHOULD items =====
Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments).
Also proposing to add two additional provides tags as the package can also be considered an application. e.g.
Provides: gamera = %{version}-%{release}
and
Provides: gamera%{?_isa} = %{version}-%{release}
See also: https://fedoraproject.org/wiki/Packaging:Guidelines#Libraries_and_Applicatio...
[x]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described.
Note from reviewer: Did some initial testing and it imports successfully with python2. Also after installing the dependency on wxPython (see above)the binary works. Will need to do some more testing although it seems that the package is functional. Leaving this option as "not evaluated" till I conduct some more testing.
[x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: python2-gamera-3.4.3-5.fc27.x86_64.rpm python2-gamera-devel-3.4.3-5.fc27.x86_64.rpm python-gamera-debuginfo-3.4.3-5.fc27.x86_64.rpm python-gamera-3.4.3-5.fc27.src.rpm python2-gamera.x86_64: W: hidden-file-or-dir /usr/lib/.build-id python2-gamera.x86_64: W: hidden-file-or-dir /usr/lib/.build-id python2-gamera.x86_64: W: no-manual-page-for-binary gamera_gui python2-gamera-devel.x86_64: W: no-documentation <snip>
Too many warnings for wrong address of FSF however they are coming from the bundled libraries.
Requires -------- python2-gamera (rpmlib, GLIBC filtered): /usr/bin/python2 libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_4.0.0)(64bit) libgomp.so.1()(64bit) libgomp.so.1(GOMP_1.0)(64bit) libgomp.so.1(GOMP_2.0)(64bit) libgomp.so.1(GOMP_4.0)(64bit) libgomp.so.1(OMP_1.0)(64bit) libgomp.so.1(OMP_2.0)(64bit) libm.so.6()(64bit) libpng16.so.16()(64bit) libpng16.so.16(PNG16_0)(64bit) libpthread.so.0()(64bit) libpython2.7.so.1.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.1)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libtiff.so.5()(64bit) libtiff.so.5(LIBTIFF_4.0)(64bit) python(abi) rtld(GNU_HASH)
python-gamera-debuginfo (rpmlib, GLIBC filtered):
python2-gamera-devel (rpmlib, GLIBC filtered): python(abi) python-gamera(x86-64)
Provides -------- python2-gamera: application() application(python-gamera.desktop) python-gamera python-gamera(x86-64) python2-gamera python2-gamera(x86-64) python2.7dist(gamera) python2dist(gamera)
python-gamera-debuginfo: python-gamera-debuginfo python-gamera-debuginfo(x86-64)
python2-gamera-devel: python-gamera-devel python-gamera-devel(x86-64) python2-gamera-devel python2-gamera-devel(x86-64)
Unversioned so-files -------------------- python2-gamera: /usr/lib64/python2.7/site-packages/gamera/gameracore.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/graph.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/kdtree.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/knncore.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/knnga.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_arithmetic.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_binarization.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_color.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_contour.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_convolution.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_corelation.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_deformation.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_draw.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_edgedetect.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_features.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_fourier_features.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_geometry.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_gui_support.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_image_conversion.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_image_utilities.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_listutilities.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_logical.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_misc_filters.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_misc_free_functions.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_morphology.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_pagesegmentation.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_png_support.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_projections.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_runlength.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_segmentation.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_string_io.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_structural.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_thinning.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_threshold.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_tiff_support.so python2-gamera: /usr/lib64/python2.7/site-packages/gamera/plugins/_transformation.so
Source checksums ---------------- https://github.com/hsnr-gamera/gamera/releases/download/3.4.3/gamera-3.4.3.t... : CHECKSUM(SHA256) this package : 02c194e95822e6f5499ddc4770ec5ad02ac613af274f31aa7ada158d657ac284 CHECKSUM(SHA256) upstream package : 02c194e95822e6f5499ddc4770ec5ad02ac613af274f31aa7ada158d657ac284
Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -b 1427341 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Python, Generic, Shell-api, C/C++ Disabled plugins: Java, SugarActivity, fonts, Haskell, Ocaml, Perl, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #38 from Charalampos Stratakis cstratak@redhat.com --- Conducted some additional testing and the gui works fine under wayland as well as xorg on a F25 VM.
[x]: Package functions as described.
The blockers now are adding the requires for wxPython and getting a definite answer from lega.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #39 from VincentS vincent@casperlefantom.net --- Thanks for the review. I'm going to do the correction and resend an email to legal during the next week.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Charalampos Stratakis cstratak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(mhroncok@redhat.c | |om)
--- Comment #40 from Charalampos Stratakis cstratak@redhat.com --- So since legal hasn't answered for some time now [0] I'm gonna assume that the correct license(s) should be "GPLv2 and GPLv2+ and MIT and CC-BY-SA", so the review can be continued.
Miro what would you think?
Also do not forget to add the Requires: wxPython
[0] https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/...
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mhroncok@redhat.c | |om) |
--- Comment #41 from Miro Hrončok mhroncok@redhat.com --- I would only use CC-BY-SA for the docs subpackage.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #42 from VincentS vincent@casperlefantom.net --- So, after all you said. Should I put "GPLv2 and GPLv2+ and MIT and CC-BY-SA" as package licence and only "CC-BY-SA" as docs subpackage licence ?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #43 from Miro Hrončok mhroncok@redhat.com --- (In reply to VincentS from comment #42)
So, after all you said. Should I put "GPLv2 and GPLv2+ and MIT and CC-BY-SA" as package licence and only "CC-BY-SA" as docs subpackage licence ?
I'd go with "GPLv2 and GPLv2+ and MIT" as the main package licence (note that it specifies the license of the built binary RPM, not the whole source RPM) and "CC-BY-SA" as docs subpackage licence.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #44 from VincentS vincent@casperlefantom.net --- Thanks for your help, here are new links.
Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-6.fc25.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #45 from Charalampos Stratakis cstratak@redhat.com --- Hello Vincent and thanks for your work and your patience with the package review.
Technically all seems fine now. The package builds and works as expected.
[x]: License field in the package spec file matches the actual license.
[x]: Requires correct, justified where necessary.
Just one last item on the list. Please add a comment above the license tag with a small explanation of the licenses breakdown [0]. Also please mention there that the CC-BY-SA is for the documentation subpackage that hasn't been built/shipped yet.
[!]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec.
[0] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/Li...
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #46 from VincentS vincent@casperlefantom.net --- Hello Charalampos, thanks you for your time on this package review.
I've done all your recommendations.
Spec URL: https://dl.casperlefantom.net/pub/review/python-gamera.spec SRPM URL: https://dl.casperlefantom.net/pub/review/python-gamera-3.4.3-7.fc25.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Charalampos Stratakis cstratak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ | |needinfo?(mhroncok@redhat.c | |om)
--- Comment #47 from Charalampos Stratakis cstratak@redhat.com --- Thanks.
Package accepted.
Waiting on Miro for the sponsorship approval and final comments.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #48 from Miro Hrončok mhroncok@redhat.com --- Will try to do this by the end of this week.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mhroncok@redhat.c | |om) |
--- Comment #49 from Miro Hrončok mhroncok@redhat.com --- Oh, sorry. I've just sponsored Vincent. Use your powers responsibly. Don't hesitate to ask me anything on #fedora-devel, or CC me in your reviews when in trouble.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #50 from VincentS vincent@casperlefantom.net --- Thank you very much ! Of course ! Please, may you tell me what is the next step for this package ?
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #51 from Charalampos Stratakis cstratak@redhat.com --- Hello Vincent,
The instructions are outlined here [0]. You are at the point now for using the fedrepo-req tool.
Please note though that due to some recent changes in the build infrastructure, there is the case that some things might not be clear or documented yet. If you stumble upon any of that, please notify us so we can clear it up for you.
[0] https://fedoraproject.org/wiki/Package_Review_Process
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Petr Viktorin pviktori@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pviktori@redhat.com Flags| |needinfo?(vincent@casperlef | |antom.net)
--- Comment #52 from Petr Viktorin pviktori@redhat.com --- Hello Vincent, I'm setting "needinfo" on this bug to indicate it's currently waiting on you. Please let us know if you need any info or instructions.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #53 from VincentS vincent@casperlefantom.net --- I'm sorry for the response time, at the moment, I'm really busy. I'll keep you in touch.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Linda Wang lwang@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1512975
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|1512975 |
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #54 from Charalampos Stratakis cstratak@redhat.com --- Pinging here.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
--- Comment #55 from Charalampos Stratakis cstratak@redhat.com --- The review is stalled. If there would be no response within one week I'll close the review request.
https://bugzilla.redhat.com/show_bug.cgi?id=1427341
Charalampos Stratakis cstratak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Blocks| |201449 (FE-DEADREVIEW) Resolution|--- |NOTABUG Flags|needinfo?(vincent@casperlef | |antom.net) | Last Closed| |2018-11-12 08:49:48
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=201449 [Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter response should be blocking this bug.
package-review@lists.fedoraproject.org