Spec URL: http://potty.fedorapeople.org/Oplop/1.6-1/Oplop.spec SRPM URL: http://potty.fedorapeople.org/Oplop/1.6-1/Oplop-1.6-1.fc16.src.rpm Description: Using a single master password and various nicknames, one can create an infinite number of unique account passwords. These unique account passwords are commonly called password hashes, domain-specific passwords, or per-site passwords.
Hi, a few notes : - if you use python3, you should requires python3-devel, not python-devel https://fedoraproject.org/wiki/Packaging:Python#BuildRequires The same goes for python2 ( and the software seems to be in python2, so maybe the requires is wrong ) - %doc PKG-INFO README build/* oplop/* bin/* why is everything in %doc ? - rm -rf %{buildroot} is no longer needed, so does %defattr and %clean, and the BuildRoot tag, see : https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions - %{__python} setup.py install -O1 --root=%{buildroot} --record=INSTALLED_FILES since INSTALLED_FILES is not used, I think you should remove it https://fedoraproject.org/wiki/Packaging:Python#Byte_compiling - Requires: python3 xclip this is IMHO better to have 1 requires per line. This produce better diff , so ease review of patches. - I think the requires on python will be added automatically by rpm, so no need to add it directly. - unless you plan to use the rpm on EPEL 5, the definition of the macro is not needed at the start of the spec, this is default since fedora 14 and can be removed. - the license should in %doc https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text and if not, you should contact upstream to add a license file. - there seems to be test in the zip, they should be run in %check.
Thank you. I'm on it.
Any news ?
I have updated the package with the suggestions you did. Here is the spec and src.rpm files link: http://potty.fedorapeople.org/Oplop/1.6-1/Oplop-1.6-1.fc17.1.src.rpm http://potty.fedorapeople.org/Oplop/1.6-1/Oplop.spec
To pull python2 as a runtime requirement is superfluous here, rpm picks it up automatically since python2-devel is a build dependency.
It is updated. Please check :)
Release: 1%{?dist}.2 This is wrong, it is only applicable in some special cases [1]. Each time you change something in your spec, you have to bump the release tag. But this doesn't mean, you have to add a number after, rather you have to bump the first number: Release: 2%{?dist} See [2] for more information. [1] http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Minor_release_bumps_for_old_branches [2] http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag BTW, your srpm download link from comment #4 doesn't point to the newest version. It is http://potty.fedorapeople.org/Oplop/1.6-1/Oplop-1.6-1.fc17.2.src.rpm. Please check the links once you have build a new srpm.
Ping...?
I have updated the packages with the suggestions Mario did. Here are the links: http://potty.fedorapeople.org/Oplop/1.6-2/Oplop.spec http://potty.fedorapeople.org/Oplop/1.6-2/Oplop-1.6-2.fc18.src.rpm I'm sorry for the delay. Regards.
Abdel, I suggest you rename this package to python-oplop instead of Oplop, in the future this package will eventually have support for python3 ( as can be seen in setup.py) and you will have to provide support to upstream, and do a sub-package python3-oplop. One way would be as follows, is better do right now and not worry about having headaches after %global pkgname Oplop Name: python-oplop Source0: https://pypi.python.org/packages/source/O/%{pkgname}/%{pkgname}-%{version}.zip %prep %setup -qn %{pkgname}-%{version} if you have plan to ship this package to el5 must be define python_sitelib and python_sitearch macros i.e. %if 0%{?rhel} && 0%{?rhel} <= 5 %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())")} %{!?python_sitearch: %global python_sitearch %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1))")} %endif See https://fedoraproject.org/wiki/Packaging:Python#Macros Please add the LICENSE file.
I would like to change the package name to python-oplop instead of Oplop. Here are the updated links: SRPM: http://potty.fedorapeople.org/Oplop/1.6-3/python-oplop-1.6-3.fc18.src.rpm SPEC: http://potty.fedorapeople.org/Oplop/1.6-3/python-oplop.spec Regards.
Abdel, I'll do the review, tomorrow. I've been busy in this week
Hi Abdel == TODO == - Don't need do the duplicate declaration of %doc, easily you can include the files in one only declaration, please fix - Please add the boiler plate of the license in %Source1, upstream explain explicitely in the LICENSE file that the package is ASL2.0 and invite to obtain a copy, the copy should be obtained from http://www.apache.org/licenses/LICENSE-2.0.txt - tests should be run at some point, when the package is approved ,please seek work soon with upstream for provide tests that work well in python3 and python2, also provide the opportunity to build the package correctly on both stacks, Remember that we are not allowed to build python3 packages if upstream does not give full support (and in this case, upstream not given this support) - From what I see, you plan to ship this package to epel5 therefore need to follow appropriate guidelines, see http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#EL5, i.e. buildroot present, clean of the buildroot,and %defattr, but there are a problem, the build fails on epel5.If you ask me, I am among those who believe that we should gradually be left support to el5, and moving on to el6 directly. Why? make things easier for packager and el6 also shares the same Fedora packaging guidelines - I copy and paste the el5 build error here: + /usr/bin/unzip -qq /builddir/build/SOURCES/Oplop-1.6.zip + STATUS=0 + '[' 0 -ne 0 ']' + cd Oplop-1.6 ++ /usr/bin/id -u + '[' 1001 = 0 ']' ++ /usr/bin/id -u + '[' 1001 = 0 ']' + /bin/chmod -Rf a+rX,u+w,g-w,o-w . + exit 0 Ejecutando(%build): /bin/sh -e /var/tmp/rpm-tmp.45353 + umask 022 + cd /builddir/build/BUILD + cd Oplop-1.6 + LANG=C + export LANG + unset DISPLAY + /usr/bin/python setup.py build File "setup.py", line 15 with open('README', 'r') as file: ^ SyntaxError: invalid syntax ==================================================================== afaik, the python binary on el5 is Python 2.4, "with statement" was introduced in python 2.5, (understand because I say it is easier to move on to epel 6?) == EPEL6 == the package build fails too, due to small error: * File not found: /builddir/build/BUILDROOT/python-oplop-1.6-3.el6.x86_64/usr/lib/python2.6/site-packages/Oplop-1.6-py2.7.egg-info in the spec you have this line: %{python_sitelib}/%{zip_name}-%{version}-py2.7.egg-info this line should be %{python_sitelib}/%{zip_name}-%{version}-*.egg-info Regards
I will upload the corrections. Thanks for your help. Regards.
Ping, any update here?
anew, any update here?
Hi Eduardo! Here are my updates for this packages: SRPM: http://potty.fedorapeople.org/Oplop/1.6.1-4/python-oplop-1.6.1-4.fc19.src.rpm SPEC: http://potty.fedorapeople.org/Oplop/1.6.1-4/python-oplop.spec Hope this time is OK. Regards, Abdel
It's nonsense to add support for EL5 as el5 only has py2.4 http://pkgs.org/search/?keyword=python-devel&search_on=name&distro=2 And looking into https://github.com/brettcannon/oplop/blob/master/Python/setup.py We can see it needs at least 2.6. So please drop el5 support and remove all unneeded fields. Another note is that https://github.com/brettcannon/oplop should be the URL of this package.
I agree with @cicku As I said above the package don't have support for python =< 2.4; so do you must remove the epel5 stuff - %clean is not needed - BuildRoot is not needed and remove these lines: %if 0%{?rhel} && 0%{?rhel} <= 5 %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())")} %{!?python_sitearch: %global python_sitearch %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1))")} %endif btw, I don't know why always the pipy tarballs never contains the boilerplate of the licenses (thing that annoying me), but in the package's github we can see the license , please add of local way or build all the package directly from the github's source => https://github.com/brettcannon/oplop/blob/master/LICENSE if you decide build from github's sources please handle the url following the guidelines https://fedoraproject.org/wiki/Packaging:SourceURL#Github
Thanks for the information, @cicku and @echevemaster. I would work on the package and upload the corrections. Regards!
Hi Eduardo and Christopher. I follow your previous advices. Please review this package: SPEC -> http://potty.fedorapeople.org/Oplop/1.6.1-5/python-oplop.spec SRPM -> http://potty.fedorapeople.org/Oplop/1.6.1-5/python-oplop-1.6.1-5.fc20.src.rpm Thanks in advance. Merry Xmas!
Thank you. New thoughts only stand by me: 1. It's packager's choice to choose which kind of sources should they use for packaging, however I always choose the smallest one. In this case, I would choose tar.gz sources instead of zipball as RPM Source0 tag. But, since only differences is 7 kb, I think it's trivial to let you change. 2. I can see that this package support python3, as many reviewer have said, you should enable python3 subpackage, and Fedora 22(maybe higher) will set python3 as default stack so you'd better do that. Currently the way of supporting python3 in your spec is invalid, checkout a template or do it by yourself: http://cicku.me/python-pygit2.spec 3. In pace with python3's moving on, you should set old python2 macro from unversioned to versioned: %{__python} --> %{__python2} %{python_sitelib} --> %{python2_sitelib} 4. Upstream has clearly told you that this package supports testing: " Run python3 test_main.py. Do note that Python 3 is required to run the test suite." So you should add check section for it. 5. I don't know which name is better, Oplop or oplop? Because Oplop is the tarball name, but I'm not sure about naming the package now for python packages, maybe Eduardo can answer that. ;) ------------ I will let Eduardo finish this review and quit now.
with respect to naming, guidelines can help to clarify . https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming " When naming a package you can take some cues from the name of the upstream tarball, project name from which this software came, and what has been used for this package by other distributions/packagers in the past. Do not just blindly follow those examples, however, as package names should strive to be consistent within Fedora more than consistent between distros. You should generally use lowercase and turn underscores into dashes unless there's a compelling reason to follow a different upstream convention." Therefore, guidelines advise the use of lowercase, and in other distros (like Arch linux), oplop is named in lowercase => https://aur.archlinux.org/packages/oplop/?setlang=es Please if you will to support py3 and py2, the correct BR are python2-devel and python3-devel and as said @cicku, use the appropiate macros. Best Regards and happy holidays
This project is now (not so) active on github: https://github.com/brettcannon/oplop Is there still a desire to package that?
It seems not.
The needinfo request[s] on this closed bug have been removed as they have been unresolved for 1000 days