Bug 225809 - Merge Review: gmp
Summary: Merge Review: gmp
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
Depends On: 211762 218041 223692
Blocks: 225778 248363
TreeView+ depends on / blocked
Reported: 2007-01-31 18:50 UTC by Nobody's working on this, feel free to take it
Modified: 2010-01-11 07:59 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-01-07 02:17:52 UTC
Type: ---
j: fedora-review+
wtogami: fedora-cvs+

Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 18:50:09 UTC
Fedora Merge Review: gmp

Initial Owner: twoerner@redhat.com

Comment 1 Karsten Hopp 2007-02-23 15:10:18 UTC
most common review issues fixed in gmp-4.1.4-12

> - remove trailing dot from summary
> - fix buildroot
> - fix post/postun/... requirements
> - use make install DESTDIR=...
> - replace tabs with spaces
> - convert changelog to utf-8

Comment 2 manuel wolfshant 2007-02-24 10:50:42 UTC
[ "$RPM_BUILD_ROOT" != "/" ] in %install and %clean should be removed

According to upstream (http://www.swox.com/gmp/), the most current version is
4.2.1, while the packaged version is 4.1.4. At least a note explaining the
reason for not upgrading should be included in the spec.

Comment 3 Laurent Rineau 2007-03-05 15:04:04 UTC
(In reply to comment #2)
> According to upstream (http://www.swox.com/gmp/), the most current version 
> 4.2.1, while the packaged version is 4.1.4. At least a note explaining the
> reason for not upgrading should be included in the spec.

That is bug #211762. gmp is a build-dependency of gcc. That is why the upgrade 
is not that safe.

Another bug that concerns this review is bug #218041: the gmp-4.1.x package 
ships a static version of libmpfr. Static libraries should be removed from 
Fedora, according to guidelines.

A bug about unsafe use of something in scriptlets, with a patch from Ville 
Skyttä: bug #223692.

Comment 4 Jochen Schmitt 2007-05-23 20:34:07 UTC
As soon as I can see, we need to create a seperate mpfr package.

Such a package doesn't exist in Fedora nowaday, becouse it was a integrated part
of the gmp package.

Comment 5 Jochen Schmitt 2007-05-23 21:00:54 UTC
I have create a mpfr-package, which you may find at:


It will be nice, if you can take the maintainership for this package, becouse
then you can best organiz the spit of gmp into the two seperate packages gmp and

Comment 6 Jochen Schmitt 2007-06-20 18:52:51 UTC
An Additional Note:

In the gcc/doc/install.texi of gcc-4.2.0 I have found a parapraph which says,
that the mpfr library bundled with gmp is buggy and some FORTRAN issue are
unfixable, if you use this buggy version

Comment 7 Jochen Schmitt 2007-07-16 16:07:56 UTC
On BZ #248363 a review for a separate mpfr package was started.

Comment 8 Jochen Schmitt 2007-07-22 19:39:04 UTC
I have create a suggested package for the most recent gmp without the mpfr library.

I have uploaded it to


Comment 9 Ivana Varekova 2007-07-26 14:20:44 UTC
Package Change Request
Package Name: gmp: 
Updated Fedora Owners: varekova@redhat.com

Comment 10 Harald Hoyer 2007-09-19 13:50:23 UTC
- change %{dist} to %{?dist} 
- consider "make check" after "make all"

Comment 11 Harald Hoyer 2007-09-19 13:52:34 UTC

Comment 12 Laurent Rineau 2007-09-20 11:38:07 UTC

Why is his bug closed?
Has GMP been reviewed (then, where is the review?)?
Is there another GMP review somewhere? Then, this bug should have been closed 
as duplicate. What is the new bug number anyway?

Comment 13 Jason Tibbitts 2008-02-06 19:50:33 UTC
I don't believe this should have been closed.  Nobody has approved it yet, and I
believe that the current package has outstanding issues.

Comment 14 Ivana Varekova 2008-02-07 09:17:14 UTC
You are right, it should be open. I will go through it now. Thanks.

Comment 15 Ivana Varekova 2008-02-07 12:18:53 UTC
Could anybody grab this review and write the list of "should be fixed" things here?

Comment 16 Laurent Rineau 2008-02-07 14:42:41 UTC
rpmlint says:
gmp.i386: W: invalid-license LGPL
gmp.i386: W: 
unused-direct-shlib-dependency /usr/lib/libgmpxx.so.4.0.2 /lib/libm.so.6

IMO the License tag should be "LGPL2+".

Comment 17 Ralf Corsepius 2008-02-07 15:08:57 UTC
Split out the static libs from *-devel into a *-static package or (if appropriate)
abandon shipping them.

Comment 18 Ivana Varekova 2008-02-08 11:09:20 UTC
Thanks for both comments the problems are fixed in gmp-4.2.2-5.fc9 - (the
license tag is LGPLv3+).

Comment 19 Robert Scheck 2008-12-20 14:58:59 UTC
For gmp-4.2.4-3, rpmlint doesn't seem to be silent (or is my rpmlint just

gmp-devel.i386: E: tag-not-utf8 %changelog
gmp-static.i386: E: tag-not-utf8 %changelog

Ivana, can you explain why a separate %configure is defined, rather using
the default configure from redhat-rpm-config? So if there still is a valid 
reason for this own define, please add an explaining comment to the spec
file, otherwise use %configure (the config.* is no longer copied since F-10
or so, if that was the reason for the own %configure).

Please use "if [ $1 = 0 ]; then" in %preun, currently you're comparing something quoted.

Please only use "rm -rf $RPM_BUILD_ROOT" in %clean rather the current.

Comment 20 Robert Scheck 2008-12-20 15:00:06 UTC

+ ./configure --build=i686-redhat-linux-gnu --host=i686-redhat-linux-gnu --target=i386-redhat-linux-gnu --program-prefix= --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/usr/com --mandir=/usr/share/man --infodir=/usr/share/info --enable-mpbsd --enable-cxx
configure: error: --target is not appropriate for GMP
Use --build=CPU-VENDOR-OS if you need to specify your CPU and/or system
explicitly.  Use --host if cross-compiling (see "Installing GMP" in the
manual for more on this).

...looks like a broken configure? Ivana, can you a bit investigate, whether there is really no fix for this and/or explain the %define in the spec file
how it should be? Thanks.

Comment 21 Ivana Varekova 2008-12-23 09:46:42 UTC
* my rpmlint was silent - so I don't fix the first point,
* %preun script changed 
* %clean part changed
* configure definition removed (can't find any reason to have it there)

Comment 22 Jason Tibbitts 2009-01-15 04:03:21 UTC
I took a look at this.  I had some weird problem getting it to build in mock, but it builds fine in koji so that must be on my end.

The only rpmlint complaint is:
  gmp.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libgmpxx.so.4.0.4 
which really isn't that big of a deal.  I tried the usual libtool tweak to pass -Wl,--as-needed but it didn't work, and I don't think it's worth trying to find a fix that works.

I'll run through a full review tomorrow.

Comment 23 Michael Schwendt 2009-01-15 12:57:25 UTC
The reasons is a [probably years old] configure check, which explicitly adds -lm to the linker options, GMP_CHECK_LIBM_FOR_BUILD in configure.in. Since autoreconf is run anyway, this check could be removed. Alternatively, and more "dirty", it's possible to drop -lm from config.status and rerun configure afterwards.

Comment 24 Susi Lehtola 2009-05-21 10:45:41 UTC
tibbs: ping?

Comment 25 Jason Tibbitts 2009-05-27 22:16:10 UTC
I note that 4.3.1 has recently been released, but that doesn't really have any bearing on this review.

There is no reason for:
  Requires(post): /sbin/install-info
  Requires(preun): /sbin/install-info
because you're using the "-p" forms of the scriptlets:
  %post -p /sbin/ldconfig
  %postun -p /sbin/ldconfig

The license of the demos is GPL, not LGPL, but those aren't packaged so the license tag is correct.

The README file is duplicated between the three packages; it should not be.

Normally I would complain about such a generic name as /usr/include/mp.h, but this package has been around approximately forever.

So I see only two minor issues.  If you like, I can just go ahead and fix them up and call this done.

* source files match upstream.  sha256sum:
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   gmp = 4.2.4-6.fc11
   gmp(x86-64) = 4.2.4-6.fc11

   gmp-devel = 4.2.4-6.fc11
   gmp-devel(x86-64) = 4.2.4-6.fc11
   gmp = 4.2.4-6.fc11

   gmp-static = 4.2.4-6.fc11
   gmp-static(x86-64) = 4.2.4-6.fc11
   gmp-devel = 4.2.4-6.fc11

* %check is present and all tests pass:
   All 9 tests passed
   All 10 tests passed
   All 56 tests passed
   All 11 tests passed
   All 26 tests passed
   All 7 tests passed
   All 3 tests passed
   All 14 tests passed
   All 3 tests passed

* shared libraries are installed:
  ldconfig is called properly.
  unversioned .so links are in the -devel package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
X README file is duplicated in %files.
* file permissions are appropriate.
* scriptlets are OK (ldconfig, info installation)
* code, not content.
* headers are in the -devel package.
* no pkgconfig files.
* static libraries are in a separate -static package.
* no libtool .la files.

Comment 26 Ivana Varekova 2009-11-27 12:47:19 UTC
Thanks - fixed in gmp-4.3.1-6.fc13

Comment 27 Laurent Rineau 2010-01-05 09:48:20 UTC
(Same as comment #12)

Why has this bug been closed? As far as I know, the review process is stalled and not complete.

Comment 28 Jason Tibbitts 2010-01-06 20:48:33 UTC
Definitely should not be closed.  I will do my best to have another look at this soon.

Comment 29 Jason Tibbitts 2010-01-07 02:17:52 UTC
I checked current CVS.  Things look good; the only thing I can see currently is that I don't really understand the need for the conditional in the devel scriptlets:

if [ -f %{_infodir}/gmp.info.gz ]; then
    /sbin/install-info %{_infodir}/gmp.info.gz %{_infodir}/dir || :

I guess the idea is to protect against an --excludedocs install, but that's not necessary as the recommended scriptlet (just the second line) handles that case just fine. In any case, it's only four pointless lines and this has gone on long enough, so I guess we're done.

Comment 30 Ivana Varekova 2010-01-11 07:59:56 UTC
The closing was prematured - so sorry for that. And thanks for the review. I will look for the scriplets and fix the problem.

Note You need to log in before you can comment on or make changes to this bug.