Good:
- builds on FC4 i386
- license (GPL)
- devel package with includes, *.so, and no *.la
- %defattr
- %post and %postun ldconfig
- source matches upstream
- BuildRoot
- %clean
- Groups make sense


Look at:
- rpmlint:
E: pari shlib-with-non-pic-code /usr/lib/libpari.so.2.1.7
W: pari-devel no-documentation

- needs to Require something that owns %{_datadir}/emacs/site-lisp/, probably
emacs-common
- Only some of docs are in dvi or ps format. Recommend either removing them or
making all tex into dvi/ps
- Consider not referencing the package name itself in the devel package, and
just do  "Header files for PARI/GP" like the devel summary.
- any particular reason to use perl instead of sed?

(In reply to comment #1)
> E: pari shlib-with-non-pic-code /usr/lib/libpari.so.2.1.7
I investigated the problem: there is an assembly part that gets linked
into the library that causes it, I don't think there can done anything
to it.
> W: pari-devel no-documentation
I don't think splitting documentation makes much sense here.
> 
> - needs to Require something that owns %{_datadir}/emacs/site-lisp/, probably
> emacs-common
Ok.

> - Only some of docs are in dvi or ps format. Recommend either removing them or
> making all tex into dvi/ps
AFAIS, dvi files are present for the entire documentation (users.tex is split
into several .tex files). The dvi files are shown by invoking gp's help
function. However probably the tex files and Makefile etc... are not needed
and may be left out.

> - Consider not referencing the package name itself in the devel package, and
> just do  "Header files for PARI/GP" like the devel summary.
Ok.

> - any particular reason to use perl instead of sed?
I normally use perl, sed would work too, of course. Both of them
are implicitly present without requiring them.

(In reply to comment #2)
> (In reply to comment #1)
> > E: pari shlib-with-non-pic-code /usr/lib/libpari.so.2.1.7
> I investigated the problem: there is an assembly part that gets linked
> into the library that causes it, I don't think there can done anything
> to it.

Does this assembly part work properly on different architectures?

There are alternatives for alpha, hppa, ix86, m68k, ppc and sparc.
There is also a none alternative, which I suppose is used if none
of the architectures matches. But I think, the only way to find out
is to try to build it on x86_64 and ppc which I cannot do, unfortunately.

The non-pic code error would (most likely) go away if you simply forced the use
of the "none" alternative on all(*) archs (though you'd likely lose the
performance benefits of the hand crafted assembly).

(*) or at least 64 bit ones where pic code is required in shared libs.

http://math.ifi.unizh.ch/fedora/tmp/pari-2.1.7-3.src.rpm
http://math.ifi.unizh.ch/fedora/tmp/pari.spec

Builds now for the 'none' architecture. In this config
static libraries are built instead of shared ones.
I renamed the libpari.a.2.1 to libpari.a, since versioned
static libraries seem to be very unusual.
rpmlint is satisfied.

Would you also consider packaging the perl interface to pari, Math::Pari
(http://search.cpan.org/dist/Math-Pari/)? This module is used by several crypo
modules I'm interested in.

(In reply to comment #7)
> Would you also consider packaging the perl interface to pari, Math::Pari
> (http://search.cpan.org/dist/Math-Pari/)? This module is used by several crypto
> modules I'm interested in.

Following up on this, since the perl package wants to patch the library source,
I think it's better to do that as a completely separate package that includes
and patches the pari source itself. I've submitted this as Bug 175198.

(In reply to comment #8)
> Following up on this, since the perl package wants to patch the library source,
> I think it's better to do that as a completely separate package that includes
> and patches the pari source itself. I've submitted this as Bug 175198.
Are these patches specific to the Perl package, or can they be
included here without comprimising the package. If so, the Perl package
needn't include pari and can depend on this.

(In reply to comment #9)
> (In reply to comment #8)
> > Following up on this, since the perl package wants to patch the library source,
> > I think it's better to do that as a completely separate package that includes
> > and patches the pari source itself. I've submitted this as Bug 175198.
>
> Are these patches specific to the Perl package, or can they be
> included here without comprimising the package.

Possibly. You can browse them at
http://search.cpan.org/src/ILYAZ/Math-Pari-2.010702/patches/

The ones that get applied against pari 2.1.7 are:
* patch-pari-unnormalized-float
* diff_2.1.7_-O
* diff_2.1.7_restart

> If so, the Perl package needn't include pari and can depend on this.

There are actually other benefits to building perl-Math-Pari independently,
which include:
* building against an external library isn't supported by upstream
* PARI tests can't get run as perl tests
* the POD doesn't get created
* it's conceivable that the library and perl module could get built with
incompatible compiler options
This is discussed in more depth here:
http://search.cpan.org/src/ILYAZ/Math-Pari-2.010702/INSTALL

Doesn't work on rawhide with modular X.
http://fedoraproject.org/wiki/Xorg/Modularization

Re: comment #6.  Too bad using 'none' makes static libs instead of shared ones.
 IMO it's better to live with the "non-pic in shared libs" warning that to force
static libs on everyone...

Re: comment #11:
Probably will have to do something like:

%if "%{?fedora}" > "4"
# who knows exactly which X libs are required, but this is a start
BuildRequires: libX11-devel
%else
BuildRequires: xorg-x11-devel
%endif

I'll do a quick build here to check...

Yep, only -lX11 is used for linking, so BR: libX11-devel should be sufficient on
fc5+.

Re: comment #6.  Looks like you can get shared libs back by using
--host=linux-none instead of --host=none

comment #12: IIRC, having non-pic shared libraries will cause problems on x86_64.
comment #14: Rex, are you positive, that this will work on all architectures?

(In reply to comment #14)
> Re: comment #6.  Looks like you can get shared libs back by using
> --host=linux-none instead of --host=none
Tried it, but still only libpari.a.

Is there any hope of progress here?  Someone here actually wants this package so
it would be nice to get it into the distro.

I've been meaning to do a full/proper review for awhile, haven't found the time
(yet).

We have still to resolve the static/shared library issue, including
the pic problem. It would be helpful to have someone able to test
on x86_64 and ppc architecture.

I can test on x86_64, but no PPC here.

In FC5 mock, I get:

dvips -t landscape -t a4 refcard.dvi -o refcard.ps
make[2]: dvips: Command not found
make[2]: *** [refcard.ps] Error 127

Looks like a needed BR on tetex-dvips.

I ended up dropping the --host= line altogether and patching the Makefile to
correctly build the shared library.  SRPM here:

http://www.cora.nwra.com/~orion/fedora/pari-2.1.7-4.src.rpm

Builds on FC5 x86, x86_64, and ppc here.

Haven't fixed:

W: pari doc-file-dependency /usr/share/doc/pari-2.1.7/misc/gpflog /bin/sh
W: pari doc-file-dependency /usr/share/doc/pari-2.1.7/misc/xgp /bin/sh
W: pari-devel no-documentation

Still get:

E: pari shlib-with-non-pic-code /usr/lib/libpari.so.2.1.7

This appears to be because of the following line from objdump:

# objdump --headers --private-headers /usr/lib/libpari.so.2.1.7 | grep TEXTREL
  TEXTREL     0x0

which rpmlint looks for.  Other .so's with this:

  TEXTREL     0x0
/usr/lib/libGL.so
  TEXTREL     0x0
/usr/lib/libGLU.so
  TEXTREL     0x0
/usr/lib/libj3dcore-ogl.so
  TEXTREL     0x0
/usr/lib/libj3dutils.so
  TEXTREL     0x0
/usr/lib/libmpeg-0.3.0.so
  TEXTREL     0x0
/usr/lib/libmpeg.so
 
So it might be caused by the assembly code in the library.  No sure about that.

(In reply to comment #21)
> In FC5 mock, I get:
> 
> dvips -t landscape -t a4 refcard.dvi -o refcard.ps
> make[2]: dvips: Command not found
> make[2]: *** [refcard.ps] Error 127
> 
> Looks like a needed BR on tetex-dvips.
Ok added.

> I ended up dropping the --host= line altogether and patching the Makefile to
> correctly build the shared library.  SRPM here:
What is difference to the situation before we used --host=none?
It then already built fine, the problem was were the non-pic libraries,
and, as you say, this is still the case. (See also comment #5)

(In reply to comment #22)
> What is difference to the situation before we used --host=none?
> It then already built fine, the problem was were the non-pic libraries,
> and, as you say, this is still the case. (See also comment #5)

The patch fixes the following error /usr/bin/gcc -o gp-dyn -O3 -DGCC_INLINE
-Wall -Wno-implicit -fomit-frame-pointer  -fPIC -Xlinker -export-dynamic gp.o
gp_init.o gp_rl.o highlvl.o whatnow.o plot.o plotport.o  
-L/export/home/orion/redhat/pari-2.1.7/pari-2.1.7/Olinux-i686 -L/usr/lib
-lreadline -lncurses -L/usr/lib -lpari -L/usr/lib -lX11  -ldl -lm
/usr/bin/ld: gp-dyn: hidden symbol `__stack_chk_fail_local' in
/usr/lib/libc_nonshared.a(stack_chk_fail_local.oS) is referenced by DSO
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: ld returned 1 exit status
make[1]: *** [gp-dyn] Error 1
make[1]: Leaving directory
`/export/home/orion/redhat/pari-2.1.7/pari-2.1.7/Olinux-i686'
make: *** [gp] Error 2

That I get on my FC5 w/updates-testing i386 box.

Asking about the non-pic error on the extras list.

I updated to the latest release 2.3.0:
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/pari-2.3.0-1.src.rpm
http://math.ifi.unizh.ch/fedora/spec/pari.spec

I made the following changes:
* package split into:
  pari: contains the shared libraries only
  pari-devel: the header files, etc.
  pari-gp: the calculator, this also contains the documentation, but not in
     the standard doc directory, since they are integral part of the
     program, and can be opened from within gp
  pari-emacs: the emacs mode

This compiles and runs fine on FC5/i386. It appears that there is no
TEXTREL problem anymore, there is no TEXTREL entry on libpari.so.*
I hope this also works on x86_64 and ppc.
There is also an option to compile with ftlk or qt instead of just X11.
This can be used for the plot output. I have not tried this out, however
they probably do not add much of functionality over X11 and are mainly
used for portability to OSX/Windows.

Good:

- md5sums match
- -devel requires versioned package
- calls ldconfig
- owns all files


Minor:

- Don't need BR gzip and perl
- Why the Requires: gzip?
- I would clean up some of the commented out stuff from the spec file, and the
unneeded Patch.

- rpmlint:
W: pari-gp devel-file-in-non-devel-package /usr/share/pari/examples/extgcd.c

this should probably be in the -devel package.

- The last sentence of the %description is odd, since this package contains the
library:

PARI is also available as a C library
to allow for faster computations.

perhaps:

PARI includes a C library
to allow for faster computations for other applications.

- Do we even care about pari.cfg? 

- why not ship xgp and gpflog?

Looks like:

# we move pari.cfg to the docdir
rm -fr $RPM_BUILD_ROOT%{_libdir}/pari

needs to be:

# we move pari.cfg to the docdir
rm -fr $RPM_BUILD_ROOT%{_prefix}/lib/pari

As it is installed there even on x86_64.

(In reply to comment #25)
> - Don't need BR gzip and perl
Ok.
> - Why the Requires: gzip?
I fact, gp can use gzip to load gzipped files, it seems. I moved the
Requires to the gp subpackage.

> - I would clean up some of the commented out stuff from the spec file, and the
> unneeded Patch.
Ok.

> - rpmlint:
> W: pari-gp devel-file-in-non-devel-package /usr/share/pari/examples/extgcd.c
> this should probably be in the -devel package.
The C example is in fact an example for a dynamic module loadable by gp.
So it effectively belong to gp and not to devel.
> 
> - The last sentence of the %description is odd, since this package contains the
> library:
I replace the last sentence by:
"This package contains the shared libraries. The interactive
calculator PARI/GP is in package %{name}-gp."

> - Do we even care about pari.cfg? 
users.dvi mentions it, so I thought I leave it in the docdir.

> - why not ship xgp and gpflog?
I would not ship xgp. I just added a .desktop file to start gp
in a terminal. I leave gpflog in.


(In reply to comment #26)
> Looks like:
> 
> # we move pari.cfg to the docdir
> rm -fr $RPM_BUILD_ROOT%{_libdir}/pari
> 
> needs to be:
> 
> # we move pari.cfg to the docdir
> rm -fr $RPM_BUILD_ROOT%{_prefix}/lib/pari
Ok.

http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/pari-2.3.0-1.src.rpm

- a revision bump would have been good.

- Should "make dobench" be in %check?  What about "make test-all" or "make
test-kernel" or "make test-compat"?

- Do you want to use GNU mp (Configure --with-mp)?  Install guide says it is
faster than the pari version.

- Why -fomit-frame-pointer?  Looks like it may be set automatically now.

- export DLCFLAGS=-fPIC is unneeded.

- Looks like -DGCC_INLINE is unneeded as well.

(In reply to comment #28)
> - Should "make dobench" be in %check?  What about "make test-all" or "make
> test-kernel" or "make test-compat"?
moved "make dobench" to %check

> - Do you want to use GNU mp (Configure --with-mp)?  Install guide says it is
> faster than the pari version.
Builds fine with gmp. Note that the library is called libpari-gmp now.

> - Why -fomit-frame-pointer?  Looks like it may be set automatically now.
> - export DLCFLAGS=-fPIC is unneeded.
> - Looks like -DGCC_INLINE is unneeded as well.
Ok.

The -fPIC in CFLAGS is still necessary.

http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/pari-2.3.0-2.src.rpm

On final suggestion (feel free to ignore), but I'm a sucker for tests:

%check
make dobench
make dotest-compat
make dotest-intnum
make dotest-qfbsolve
make dotest-rfrac
make dotest-round4
make dotest-stark

Just runs the extra tests that don't require and external dataset.

In any case, the package is APPROVED.

Imported to FC-4, FC-5 and FC-6.
The last test "dostest-stark" fails with a segmentation fault.
On ppc several test fail.
Should we still go on and remove the failing tests? This is probably something
we can't easily fix and should be reported upstream.

Where does stark fail?  It works for me on FC5 i386 and FCdev x86_64.

As for ppc, for now we should ExcludeArch ppc and open a new bug against pari
that blocks FE-ExcludeArch-ppc.  I'll start doing some testing on my ppc machine
to try and see what's up.

Ok, build succeeded on FC-4, FC-5 and FC-6 excluding ppc.
We should leave this bug open for now, a new bug can be opened
only after pari is a bugzilla component.

I have opened a new bug #193727 for the failed-test problem on ppc.
Orion, it would be nice if could try a little to find out what the problem is.

Have attempted to contact the Fedora maintainer with no response:
https://bugzilla.redhat.com/show_bug.cgi?id=574077

Package Change Request
======================
Package Name: pari
New Branches: EL-5
Owners: tremble

cvs done.