[Bug 877275] Review Request: lhapdf - Les Houches Accord PDF Interface

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

 



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

--- Comment #6 from Mattias Ellert <mattias.ellert@xxxxxxxxxxxx> ---
Thank you for your comments. Many of the issues you bring up are valid for
packages intended for Fedora only. This package is intended to be built also
for EPEL 5 and later. Some of the changes you suggest requires an rpm version
newer than the one in RHEL 5 or relies on macros that are not defined in older
rpm versions.

(In reply to Björn Esser from comment #3)

> unused-direct-shlib-dependency on libquadmath.so.0 --> adding some sed-magic
> as seen on
> https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-
> dependency in spec-file might help.

Added.

> -BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Buildroot is needed for EPEL 5.

>  %install
> -rm -rf %{buildroot}

Necessary for EPEL5

> -make install DESTDIR=%{buildroot}
> +%make_install

Using the %make_install macro is not mandatory according to guidelines. The
guidelines say: "Fedora packages should use: %make_install, make
DESTDIR=%{buildroot} install or make DESTDIR=$RPM_BUILD_ROOT install. Those all
do the same thing." So any of the three is allowed. I prefer not using the
%make_install macro for clarity. In addition the %make_install macro is
undefined on RHEL 5, so if the package should build for EPEL 5 any of the other
two ways must be used.

> +%check
> +make check

This was already there

> [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
>      Note: rm -rf %{buildroot} present but not required

This is needed for EPEL 5

> [!]: Buildroot is not present

This is needed for EPEL 5

> [!]: Final provides and requires are sane (see attachments).

Added %{?_isa} to dependencies where appropriate
Added dependencies on main package to -pdfsets-minimal and -doc 

(In reply to Björn Esser from comment #5)

> --> Requires: %{name}%{?_isa} = %{version}-%{release}

Added %{?_isa} to dependencies where appropriate (i.e. not for noarch
sub-packages)

> --> sed 's!/usr/bin/env python!/usr/bin/python!' -i bin/lhapdf-*
> examples/*.py

Fixed. There was also an "/usr/bin/env bash" that needed fixing.

Updated package:

Spec URL: http://grid.tsl.uu.se/review/lhapdf.spec
SRPM URL: http://grid.tsl.uu.se/review/lhapdf-5.8.9-2.fc18.src.rpm

-- 
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=PxGp6XzeN1&a=cc_unsubscribe
_______________________________________________
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]