Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=890589

Eduardo Echeverria <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected]

--- Comment #1 from Eduardo Echeverria <[email protected]> ---
Hi Jiri, 
Initial comments:
- please document the need for automake in BR
- please document the need for GDB in BR
- BuildRequires and Requires entries can be listed one-by-one, is easier to
read for reviewers 
- coreutils not needed in BR. see
http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
- not think there is the need to call to: 
/sbin/ldconfig openssl in Requires(post)
and 
/sbin/ldconfig in Requires(postun)
- the code is using hardcode specific library paths when linking binaries
/usr/sbin/csprngd ['/usr/lib64']
see http://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath
use 
%configure
sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
see http://fedoraproject.org/wiki/Packaging:Guidelines#Removing_Rpath

- Don't use the macros:
%{__rm}
%{__install}
such macros are deprecated and shouldn't be used anymore, see
    http://fedoraproject.org/wiki/Packaging:Guidelines#Macros
    https://bugzilla.redhat.com/show_bug.cgi?id=669311#c14
- Please be consistent in the use of the macros $RPM_BUILD_ROOT or
%{buildroot}, choose one of two
- Specify if you want to ship your package to EPEL5 , otherwise please remove
* BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}
-n) see
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#BuildRoot_tag
* rm -rf %{buildroot} after %install
* The section %clean see
http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean
* and the %defattr(-, root, root, -) in %files see
http://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

the output of licensecheck is:

GPL (v3.1)
----------
/var/lib/mock/fedora-17-x86_64/root/builddir/build/BUILD/csprng-1.1.1/src/cpuid-43.h

GPL (v3 or later)
-----------------
/var/lib/mock/fedora-17-x86_64/root/builddir/build/BUILD/csprng-1.1.1/utils/csprngd.c

Unknown or generated
--------------------
/var/lib/mock/fedora-17-x86_64/root/builddir/build/BUILD/csprng-1.1.1/man/create_from_help_message.sh

MIT/X11 (BSD like)
------------------
/var/lib/mock/fedora-17-x86_64/root/builddir/build/BUILD/csprng-1.1.1/src/QRBG.h

ISC GPL (v3 or later)
---------------------
/var/lib/mock/fedora-17-x86_64/root/builddir/build/BUILD/csprng-1.1.1/include/cs:


In my experience, when there are many licenses involved in the upstream source
files and these licenses do not apply to your own source, is clear indication
that may contain bundled libs or bundled files

See https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

Since you are the developer of upstream, tell me if this is so

in case the header or source file is in Fedora patch the Makefile to build
against them, otherwise build the devel package containing these files.

fixed these points, I'll do the formal review

Best Regards

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=w8oIxF0IJO&a=cc_unsubscribe
_______________________________________________
package-review mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to