[Bug 1259061] Review request: python-securepass - SecurePass Python library & tools

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1259061



--- Comment #3 from Parag AN(पराग) <panemade@xxxxxxxxx> ---
Hi, If you run fedora-review tool on this review bugzilla you will see
review.txt output from it showing

Issues:
=======
- Package contains BR: python2-devel or python3-devel

=> You need to specify for which version of python you are building package. I
see on upstream that there is no information if the module is dual compatible
that mean same code can be run under python2 or python3 environment. So, Let's
take default that upstream code is python2 code. You may want to ask upstream
if existing code supports python3 as well.

We are soon moving to python3 as default so all python module should start
providing python3 subpackages as well.

You can fix this for now as
BuildRequires: python2-devel


- Package uses either %{buildroot} or $RPM_BUILD_ROOT
  Note: Using both %{buildroot} and $RPM_BUILD_ROOT
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros

=> Use only one style and not to mix it.

- 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.
  Note: License file LICENSE is marked as %doc instead of %license
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

==> This is okay as it looks you want your package to be built for EPEL6 also.


There are other issues in the spec that need to be cleaned. See more
information for those on  https://fedoraproject.org/wiki/EPEL:Packaging

1) Spec file contains Buildroot tag which is not needed now. See
https://fedoraproject.org/wiki/EPEL:Packaging#BuildRoot_tag , remove it form
spec

2) You also don;t need Group tag. See
https://fedoraproject.org/wiki/EPEL:Packaging#Group_tag
remove it from spec file.

3) You also don't need to specify in spec file
[ "%{buildroot}" != "/" ] && rm -rf $RPM_BUILD_ROOT

remove this line. See
https://fedoraproject.org/wiki/EPEL:Packaging#Prepping_BuildRoot_For_.25install

4) prefix macro is not needed as install already using it as /usr

5) You don't need %clean section at all, its optional, see
https://fedoraproject.org/wiki/EPEL:Packaging#Cleaning_BuildRoot_in_.25clean

6) Then you created subpackage securepass-tools but written it below %build and
%install. You should have written all subpackages before %prep section

Move following lines just before %prep section
=====================================================
%package -n securepass-tools
Requires:   python-securepass
Summary:    SecurePass Tools

%description -n securepass-tools
The official tools for accessing SecurePass platform.

It uses the SecurePass public APIs.
=====================================================

7) we have macros for /usr/bin why not to use it?
change
%{_usr}/bin/*
to
%{_bindir}/*

See more on this https://fedoraproject.org/wiki/Packaging:RPMMacros

8) securepass-tools subpackage needs securepass library at runtime so add
following to %package section of securepass-tools package

Requires: python-securepass


9) You need to follow python packaging guidelines as well. 

a) For that you should use specific python version macros. e.g.
%build
%{__python} setup.py build

%install
%{__python} setup.py install --skip-build --root="%{buildroot}"

Should be written as
%build
%{__python2} setup.py build

%install
%{__python2} setup.py install --skip-build --root %{buildroot}

b) If you want to build this package for EPEL6 as well then add following lines
at top of spec file
==================================================
%if 0%{?rhel} && 0%{?rhel} <= 6
%{!?__python2: %global __python2 /usr/bin/python2}
%{!?python2_sitelib: %global python2_sitelib %(%{__python2} -c "from
distutils.sysconfig import get_python_lib; print(get_python_lib())")}
%{!?python2_sitearch: %global python2_sitearch %(%{__python2} -c "from
distutils.sysconfig import get_python_lib; print(get_python_lib(1))")}
%endif
=================================================

This is as per given on
https://fedoraproject.org/wiki/Packaging:Python_Old#Macros



I suppose there can be few more issues in this package but for now please fix
above and submit a new SPEC and SRPM by increasing release number and adding
relevant changelog.

You should also read all requires Guidelines pages like 
1) https://fedoraproject.org/wiki/EPEL:Packaging
2) https://fedoraproject.org/wiki/Packaging:Python_Old
3) https://fedoraproject.org/wiki/Packaging:Guidelines

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]