[Bug 837004] Review Request: libpari23 - Number Theory-oriented Computer Algebra Library

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

 



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

--- Comment #5 from Paul Howarth <paul@xxxxxxxxxxxx> ---
(In reply to comment #1)
> Building on an installed system (to check for possible conflicts), not a
> buildroot I got this:
> 
> RPM build errors:
>     Installed (but unpackaged) file(s) found:
>    /usr/share/emacs/site-lisp/pari/pari-completion.el
>    /usr/share/emacs/site-lisp/pari/pari-completion.elc
>    /usr/share/emacs/site-lisp/pari/pari-conf.el
>    /usr/share/emacs/site-lisp/pari/pari-conf.elc
>    /usr/share/emacs/site-lisp/pari/pari-fontification.el
>    /usr/share/emacs/site-lisp/pari/pari-fontification.elc
>    /usr/share/emacs/site-lisp/pari/pari-help.el
>    /usr/share/emacs/site-lisp/pari/pari-messages.el
>    /usr/share/emacs/site-lisp/pari/pari-messages.elc
>    /usr/share/emacs/site-lisp/pari/pari.el
>    /usr/share/emacs/site-lisp/pari/pari.elc
>    /usr/share/emacs/site-lisp/pari/pariemacs.txt
>    /usr/share/emacs/site-lisp/pari/sli-tools.el
>    /usr/share/emacs/site-lisp/pari/sli-tools.elc
> 
> I suggest something like:
> $ diff -u SPECS/libpari23.spec 837004/libpari23.spec
> --- SPECS/libpari23.spec        2012-07-02 09:09:50.000000000 -0400
> +++ 837004/libpari23.spec       2012-07-02 11:31:53.768642232 -0400
> @@ -118,6 +118,9 @@
>  # Placate rpmlint regarding binary and library permissions
>  %{_fixperms} %{buildroot}{%{_bindir},%{_libdir}}
>  
> +# Cleanup emacs in alternate package
> rm -fr %{buildroot}%{_datadir}/emacs
> +

I've added something like this in -2.

> After the above, you will need to update perl-Math-Pari.spec, to avoid
> $ rpmbuild -ba SPECS/perl-Math-Pari.spec 
> error: Failed build dependencies:
>         pari-devel = 2.3.5 is needed by
> perl-Math-Pari-2.010806-13.fc18.x86_64
> 
> somewhat like:
> $ diff -u SPECS/perl-Math-Pari.spec{~,}
> --- SPECS/perl-Math-Pari.spec~  2012-07-02 11:35:37.129650783 -0400
> +++ SPECS/perl-Math-Pari.spec   2012-07-02 11:36:57.368653861 -0400
> @@ -30,7 +30,7 @@
>  Requires:      perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo
> $version))
>  
>  # Enforce dependency against same version of pari that we're built for
> -BuildRequires: pari-devel = %{pari_version}
> +BuildRequires: libpari23-devel
>  Requires:      pari%{?_isa} = %{pari_version}
>  
>  # Don't "provide" private Perl libs or the redundant unversioned
> perl(Math::Pari)
> 
> after the above, I only did need:
> $ diff -u SPECS/perl-Math-Pari.spec{.orig,}
> --- SPECS/perl-Math-Pari.spec.orig      2012-07-02 11:36:57.368653861 -0400
> +++ SPECS/perl-Math-Pari.spec   2012-07-02 11:40:12.704661293 -0400
> @@ -62,8 +62,8 @@
>  %build
>  perl Makefile.PL \
>         INSTALLDIRS=vendor \
> -       OPTIMIZE="-I%{_includedir}/pari %{optflags}" \
> -       parilib="-lpari"
> +       OPTIMIZE="-I%{_includedir}/libpari23/pari %{optflags}" \
> +       parilib="-lpari23"
>  make %{?_smp_mflags}

I've got a re-worked perl-Math-Pari package here, with quite a few changes for
moving to libpari23:
http://www.city-fan.org/~paul/extras/pari/perl-Math-Pari-2.010806-14.fc18.src.rpm

(In reply to comment #2)
> [!]: MUST Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
>      Note: Clean is needed only if supporting EPEL

Fixed in -2

> [x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at
> the
>      beginning of %install.

Fixed in -2

> [!]: MUST Package requires pkgconfig, if .pc files are present. (EPEL5)
>      Note: Only applicable for EL-5

Didn't remove the explicit requirement here - I think it's harmless anyway.

> [!]: MUST Rpmlint output is silent.
> 
> rpmlint libpari23-2.3.5-1.fc18.i686.rpm
> 
> libpari23.i686: W: spelling-error %description -l en_US factorizations ->
> factorization, factorization s, categorizations

False positive.

> libpari23.i686: E: shlib-with-non-pic-code /usr/lib/libpari-gmp.so.2.3.5

Fixed in -2

> libpari23.i686: W: shared-lib-calls-exit /usr/lib/libpari-gmp.so.2.3.5
> exit@GLIBC_2.0

That's one for upstream

> libpari23.i686: E: incorrect-fsf-address
> /usr/share/doc/libpari23-2.3.5/COPYING

Not allowed to change that for legal reasons. Already fixed in upstream VCS.

> rpmlint libpari23-debuginfo-2.3.5-1.fc18.i686.rpm
> 
> libpari23-debuginfo.i686: E: incorrect-fsf-address
> /usr/src/debug/pari-2.3.5/src/graph/plotport.c

These are all fixed in -2

> rpmlint libpari23-2.3.5-1.fc18.src.rpm
> 
> libpari23.src: W: spelling-error %description -l en_US factorizations ->
> factorization, factorization s, categorizations

False positive again

> libpari23.src:93: E: hardcoded-library-path in %{_prefix}/lib/pari

The place where this happens is where we remove files that upstream has put
there.

> rpmlint libpari23-devel-2.3.5-1.fc18.i686.rpm
> 
> libpari23-devel.i686: W: no-documentation

There but it's not marked as %doc as it's needed by other packages so we don't
want it omitted when installed using --nodocs

> libpari23-devel.i686: E: incorrect-fsf-address
> /usr/share/libpari23/src/headers/pariport.h

These are all fixed in -2

> libpari23-devel.i686: E: zero-length
> /usr/share/libpari23/src/test/32/qfbsolve
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/src/test/32/rfrac

I think these are intentionally zero length

> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/libpari.toc
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/refcard.idx
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/tutorial.idx
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/develop.aux
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/users.toc
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/develop.toc
> libpari23-devel.i686: E: zero-length /usr/share/libpari23/doc/refcard.aux

This is all fall-out from the tex documentation build; not sure if omitting
them would cause problems for downstream users (Math::Pari builds its own POD
documentation from the tex files).

> libpari23-devel.i686: E: wrong-script-interpreter
> /usr/share/libpari23/doc/gphelp.in @perl@

This is intended to be substituted by downstream users.

> [-]: SHOULD Latest version is packaged.

This is intentionally not the latest version, for compatibility reasons.

> [!]: SHOULD SourceX / PatchY prefixed with %{name}.
>      Note: Source0:
>      http://pari.math.u-bordeaux.fr/pub/pari/unix/OLD/pari-%{version}.tar.gz
>      (pari-%{version}.tar.gz) Patch1: libpari23-optflags.patch
>      (libpari23-optflags.patch) Patch2: Math-Pari-2.01080604-extra-stack-for-
>      test.patch (Math-Pari-2.01080604-extra-stack-for-test.patch)

Patch2 has origins in the perl-Math-Pari package, hence its prefix.

> Issues:
> [!]: MUST Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
>      Note: Clean is needed only if supporting EPEL
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
> [!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at
> the
>      beginning of %install.
>      Note: rm -rf is only needed if supporting EPEL5
> See: None
> [!]: MUST Package requires pkgconfig, if .pc files are present. (EPEL5)
>      Note: Only applicable for EL-5
> See: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#EL5
> [!]: MUST Rpmlint output is silent.

I think these are addressed in -2 as described above.

Spec URL:
http://subversion.city-fan.org/repos/cfo-repo/libpari23/branches/fedora/libpari23.spec

SRPM URL:
http://www.city-fan.org/~paul/extras/pari/libpari23-2.3.5-2.fc18.src.rpm

(In reply to comment #4)
> Ops, doing my homework, updating flags and assigning to myself.
> Waiting for some information (actually, only I am only concerned about
> %clean) to change from review-? to review-+
> 
> But I think there may be issues with the libpari23.pc file, e.g.
> 
> includes are under %{_includedir}/libpari23/pari and I think most code using
> pari does not include "pari/name.h" but "name.h"

This applies for perl-Math-Pari for instance. However, upstream's API
documentation for the current release says to use <pari/pari.h>, which is why I
went the way I did. Looking back at older documentation, it seems this has
changed since 2.3.0 where the recommendation was just <pari.h> so I'm open to
changing it but I don't think it's a big deal.

> BTW, yet another issue, also in pari.spec, is that pari.cfg should not be
> marked %doc. But not a big issue, as I believe no code linking to pari cares
> about it.

That's what I thought too, which is why I just included in %{_docdir} rather
than under %{_sysconfdir}.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
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]