Bug 435016 (mmdb)

Source RPM and spec file updated to bring RPM inline with current Fedora guidelines.

Spec URL: http://www.stanford.edu/~fenn/packs/mmdb.spec
SRPM URL: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-3.f8.src.rpm

More updates as per

https://bugzilla.redhat.com/show_bug.cgi?id=435017#c2

new spec: http://www.stanford.edu/~fenn/packs/mmdb.spec
new srpm: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-4.f8.src.rpm

Removed static libs.

new spec: http://www.stanford.edu/~fenn/packs/mmdb.spec
new srpm: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-5.f8.src.rpm

Similar issues as with ssm:

* 
# rpmlint mdb-1.09.1-5.fc9.x86_64.rpm 
mmdb.x86_64: E: library-without-ldconfig-postin /usr/lib64/libmmdb.so.0.0.0
mmdb.x86_64: E: library-without-ldconfig-postun /usr/lib64/libmmdb.so.0.0.0
Your main package needs ldconfig scriptlets

mmdb.x86_64: W: no-version-in-last-changelog
Please appropriate version numbers to your changelog entries in mmdb.spec

* BuildRoot doesn't comply to FPG

* mmdb-devel.x86_64: W: no-dependency-on mmdb
The *-devel package must depend upon the main-package
=> Add
Requires: mmdb = %{version}-%{release}
to *-devel

* mmdb-devel contains *.pc
=> Add
Requires: pkgconfig
to *-devel

* mmdb-devel contains binaries and temporary files (*.o, .libs) in
/usr/share/doc/mmdb-devel-1.09.1/Examples
This is doesn't work, is multiply broken and therefore is not allowed.
The easiest work-around to this is not to ship these files.

If you want to ship the sources, you'd have to remove all binaries and temporary
files from ../doc/../Examples in %install

If you want to ship the binaries (may-be accompanied by the sources), you'd have
to install the binaries elsewhere (e.g. %{libdir}/mmdb/Examples) with all
temporary files removed.

(In reply to comment #4)
> Similar issues as with ssm:
> 

All comments to ssm propogated here.

> * 
> # rpmlint mdb-1.09.1-5.fc9.x86_64.rpm 
> mmdb.x86_64: E: library-without-ldconfig-postin /usr/lib64/libmmdb.so.0.0.0
> mmdb.x86_64: E: library-without-ldconfig-postun /usr/lib64/libmmdb.so.0.0.0
> Your main package needs ldconfig scriptlets
> 

Done.

> mmdb.x86_64: W: no-version-in-last-changelog
> Please appropriate version numbers to your changelog entries in mmdb.spec
> 

Done.

> * BuildRoot doesn't comply to FPG
> 

Fixed.

> * mmdb-devel.x86_64: W: no-dependency-on mmdb
> The *-devel package must depend upon the main-package
> => Add
> Requires: mmdb = %{version}-%{release}
> to *-devel
> 

Fixed.

> * mmdb-devel contains *.pc
> => Add
> Requires: pkgconfig
> to *-devel
> 

Fixed.

> * mmdb-devel contains binaries and temporary files (*.o, .libs) in
> /usr/share/doc/mmdb-devel-1.09.1/Examples
> This is doesn't work, is multiply broken and therefore is not allowed.
> The easiest work-around to this is not to ship these files.
> 
> If you want to ship the sources, you'd have to remove all binaries and temporary
> files from ../doc/../Examples in %install
> 

I added the following to %install:

pushd Examples ; make clean ; rm -rf .deps _deps ; popd

is that OK?

new spec: http://www.stanford.edu/~fenn/packs/mmdb.spec
new srpm: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-6.f8.src.rpm

(In reply to comment #5)
Much better now.

However, one major issue remains:
# rpmlint libmmdb-1.09.1-6.fc8.x86_64.rpm
libmmdb.x86_64: E: library-without-ldconfig-postin /usr/lib64/libmmdb.so.0.0.0
libmmdb.x86_64: E: library-without-ldconfig-postun /usr/lib64/libmmdb.so.0.0.0

You renamed "main-package" into libmmdb 
=> Add "-n libmmdb" to %post and %postun

--- mmdb.spec.orig       2008-05-29 04:29:27.000000000 +0200
+++ mmdb.spec    2008-05-29 04:35:08.000000000 +0200
@@ -104,9 +104,9 @@
 
 pushd Examples ; make clean ; rm -rf .deps _deps ; popd
 
-%post -p /sbin/ldconfig
+%post -n libmmdb -p /sbin/ldconfig
 
-%postun -p /sbin/ldconfig
+%postun -n libmmdb -p /sbin/ldconfig
 
 %clean
 rm -rf %{buildroot}


> > * mmdb-devel contains binaries and temporary files (*.o, .libs) in
> > /usr/share/doc/mmdb-devel-1.09.1/Examples

> I added the following to %install:
> 
> pushd Examples ; make clean ; rm -rf .deps _deps ; popd
> 
> is that OK?
Well, it helps for now (not a review blocker), except that it breaks 
"rpmbuild -bi --short-circuit", because it removes files being generated
in %build, which will kill subsequent "-bi --short-circuit".

# rpmbuild -bb mmdb.spec
# rpmbuild -bi --short-circuit mmdb.spec
...
Making install in Examples
...
Makefile:336: .deps/coorfile.Po: No such file or directory
Makefile:337: .deps/extr_exmp1.Po: No such file or directory
Makefile:338: .deps/sel_exmp1.Po: No such file or directory
Makefile:339: .deps/sel_exmp2.Po: No such file or directory
Makefile:340: .deps/sel_exmp3.Po: No such file or directory
Makefile:341: .deps/sel_exmp4.Po: No such file or directory
Makefile:342: .deps/sel_exmp5.Po: No such file or directory
Makefile:343: .deps/sel_exmp6.Po: No such file or directory
Makefile:344: .deps/spseatoms.Po: No such file or directory
make[1]: *** No rule to make target `.deps/spseatoms.Po'.  Stop.
...
make: *** [install-recursive] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.6865 (%install)

Avoiding this issue is a bit tricky. 

One way is to copy Examples/ to a different directory inside of the build-tree,
(manually) remove the temporary files there and let %doc pickup this directory
instead of the original Examples/

Another way is to modify Examples/Makefile.am, such that "make install" installs
the files you want to be installed.


I'll approve the package, once the ldconfig-fix is applied.

BTW: Are you familiar with rpmlint? Though it's far from being perfect, it helps
you avoiding many of the "obvious" packaging mistakes.

(In reply to comment #6)
> (In reply to comment #5)
> Much better now.
> 
> However, one major issue remains:
> # rpmlint libmmdb-1.09.1-6.fc8.x86_64.rpm
> libmmdb.x86_64: E: library-without-ldconfig-postin /usr/lib64/libmmdb.so.0.0.0
> libmmdb.x86_64: E: library-without-ldconfig-postun /usr/lib64/libmmdb.so.0.0.0
> 
> You renamed "main-package" into libmmdb 
> => Add "-n libmmdb" to %post and %postun
> 

done.

> 
> > > * mmdb-devel contains binaries and temporary files (*.o, .libs) in
> > > /usr/share/doc/mmdb-devel-1.09.1/Examples
> 
> > I added the following to %install:
> > 
> > pushd Examples ; make clean ; rm -rf .deps _deps ; popd
> > 
> > is that OK?
> Well, it helps for now (not a review blocker), except that it breaks 
> "rpmbuild -bi --short-circuit", because it removes files being generated
> in %build, which will kill subsequent "-bi --short-circuit".
> 
> # rpmbuild -bb mmdb.spec
> # rpmbuild -bi --short-circuit mmdb.spec
> ...
> Making install in Examples
> ...
> Makefile:336: .deps/coorfile.Po: No such file or directory
> Makefile:337: .deps/extr_exmp1.Po: No such file or directory
> Makefile:338: .deps/sel_exmp1.Po: No such file or directory
> Makefile:339: .deps/sel_exmp2.Po: No such file or directory
> Makefile:340: .deps/sel_exmp3.Po: No such file or directory
> Makefile:341: .deps/sel_exmp4.Po: No such file or directory
> Makefile:342: .deps/sel_exmp5.Po: No such file or directory
> Makefile:343: .deps/sel_exmp6.Po: No such file or directory
> Makefile:344: .deps/spseatoms.Po: No such file or directory
> make[1]: *** No rule to make target `.deps/spseatoms.Po'.  Stop.
> ...
> make: *** [install-recursive] Error 1
> error: Bad exit status from /var/tmp/rpm-tmp.6865 (%install)
> 
> Avoiding this issue is a bit tricky. 
> 
> One way is to copy Examples/ to a different directory inside of the build-tree,
> (manually) remove the temporary files there and let %doc pickup this directory
> instead of the original Examples/
> 

went with this approach.

> 
> I'll approve the package, once the ldconfig-fix is applied.
> 
> BTW: Are you familiar with rpmlint? Though it's far from being perfect, it helps
> you avoiding many of the "obvious" packaging mistakes.
> 

I just installed it and have started using it (noticed it mentioned on
fedora-list, but never actually checked it out).  Thanks for the pointer!

new spec: http://www.stanford.edu/~fenn/packs/mmdb.spec
new srpm: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-7.f8.src.rpm

One final, last minute issue:

Where is the upstream of this package?

AFAIU, the original tarballs are those in
http://www.ebi.ac.uk/~keb/cldoc/downloads/
while
ftp://ftp.bioxray.au.dk/pub/mok/src/ 
hosts repackaged tarballs w/ autotool-support having been added. 

This wouldn't be much of a problem, if the original upstream was dead, but it
apparently isn't. It released an update a couple of days ago.

The normal approach in Fedora would be to use the original upstream and to
provide patches (which may originate from different origins) against the
original upstream. Otherwise things easily tend to diverge.

What do you propose to do about this? Is bioxray actively working on this
package's sources?

(In reply to comment #8)
> One final, last minute issue:
> 
> Where is the upstream of this package?
> 
> AFAIU, the original tarballs are those in
> http://www.ebi.ac.uk/~keb/cldoc/downloads/
> while
> ftp://ftp.bioxray.au.dk/pub/mok/src/ 
> hosts repackaged tarballs w/ autotool-support having been added. 
> 
> This wouldn't be much of a problem, if the original upstream was dead, but it
> apparently isn't. It released an update a couple of days ago.
> 

See:

http://www.bioxray.dk/~mok/mmdb.php

quote:
This distribution is based on the last free version of mmdb, namely 1.09.
Eugene's latest version of mmdb (1.10) is licensed under the non-free CCP4 license.

The licensing, however, seems to have switched back to the LGPL:

http://www.ccp4.ac.uk/ccp4license.php

I can bring this issue up with those involved and see what the current status
is.  My hope is that the current fork will only be temporary.

Technically, you can consider this package approved, however ... this package
needs a legal review - Spot this pumpkin is yours.

In my understanding, this package, using the tarball Tim is using (an apparent
fork from it's original upstream), is licenced under a "LGPLv2+ with exceptions".

Blocking FE-Legal until I get a chance to do a proper review of this.

Well, looking at this, it looks like both the 1.09.1 fork and the 1.10 version
are under Free licenses (LGPLv2 and LGPLv3 respectively). Lifting FE-Legal.

Hmm, something went off the tracks here.  Ralf indicated that he approves this, but never assigned it to himself, set any flags or even CC'd himself.

The package in comment #7 still builds on rawhide and at a cursory glance it seems OK; if you're still interested in getting this in then I'll go ahead and give it a full looking over and get you through the process of being sponsored and importing, building and releasing the package.

(In reply to comment #13)
> Hmm, something went off the tracks here.  Ralf indicated that he approves this,
> but never assigned it to himself, set any flags or even CC'd himself.
> 
> The package in comment #7 still builds on rawhide and at a cursory glance it
> seems OK; if you're still interested in getting this in then I'll go ahead and
> give it a full looking over and get you through the process of being sponsored
> and importing, building and releasing the package.

Yes, thanks - I'd also like to finish off:

https://bugzilla.redhat.com/show_bug.cgi?id=435015
https://bugzilla.redhat.com/show_bug.cgi?id=435017
https://bugzilla.redhat.com/show_bug.cgi?id=435018

(this package blocks the latter 2)

Well, one ticket at a time.  And once this one is done, any packager can review any of those tickets.  However, there are a few issues here:

The tarball in the source tree differs from the tarball downloaded from the Source0 URL.  In fact, the diff is almost 37000 lines long.  Any idea what's going on?

I don't understand why you explicitly %define version and release.  If you just use:
  Version: 1.09.1
  Release: 7
then %version and %release are defined for you in the same way that %name is.

I would argue that according to the COPYING file, the License: should be "LGPLv2 with exceptions" but spot above says LGPLv2.  I'll double check with spot tomorrow.

I don't think its a blocker, but the first paragraph of %description is loaded with acronyms which are completely meaningless to most people.  A little elaboration there would be welcome.

Any reason why parallel make is not being used?  You should use it if possible, but if it breaks you should include a comment to that effect.

The AUTHORS, README and COPYING files are duplicated between the packages.  There's no need to do that.

X source files do not match upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summaries are OK.
* descriptions are OK (could perhaps use some elaboration of acronyms).
* 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 is silent.
* final provides and requires are sane:
  libmmdb-1.09.1-7.fc10.x86_64.rpm
   libmmdb.so.0()(64bit)
   libmmdb = 1.09.1-7.fc10
   libmmdb(x86-64) = 1.09.1-7.fc10
  =
   /sbin/ldconfig
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libmmdb.so.0()(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)

  libmmdb-devel-1.09.1-7.fc10.x86_64.rpm
   libmmdb-devel = 1.09.1-7.fc10
   libmmdb-devel(x86-64) = 1.09.1-7.fc10
  =
   libmmdb = 1.09.1-7.fc10
   libmmdb.so.0()(64bit)
   pkgconfig

* %check is not present; no test suite upstream (that I could see).
* shared libraries present:
   unversioned .so links are in the -devel package
   ldconfig called properly
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* scriptlets OK (ldconfig).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headersa re in the -devel package.
* pkgconfig files in -devel package; pkgconfig dependency is present.
* no static libraries.
* no libtool .la files.

(In reply to comment #15)

> I would argue that according to the COPYING file, the License: should be
> "LGPLv2 with exceptions" but spot above says LGPLv2.  I'll double check with
> spot tomorrow.

Yeah, this is right, it should be "LGPLv2 with exceptions". Not sure why I thought differently before.

(In reply to comment #15)
> 
> The tarball in the source tree differs from the tarball downloaded from the
> Source0 URL.  In fact, the diff is almost 37000 lines long.  Any idea what's
> going on?
> 

Oops - I had used a .tar.gz I created from a "make build" of the original package.  Fixed.

> I don't understand why you explicitly %define version and release.  If you just
> use:
>   Version: 1.09.1
>   Release: 7
> then %version and %release are defined for you in the same way that %name is.
> 

Removed.

> I would argue that according to the COPYING file, the License: should be
> "LGPLv2 with exceptions" but spot above says LGPLv2.  I'll double check with
> spot tomorrow.
> 

Fixed.

> I don't think its a blocker, but the first paragraph of %description is loaded
> with acronyms which are completely meaningless to most people.  A little
> elaboration there would be welcome.
> 

Sorry, alot of the stuff in the spec is cruft from the original author.  Rewritten.

> Any reason why parallel make is not being used?  You should use it if possible,
> but if it breaks you should include a comment to that effect.
> 

It simply wasn't present before.  It seems to work OK, included now.

> The AUTHORS, README and COPYING files are duplicated between the packages. 
> There's no need to do that.
> 

Fixed.

new spec: http://www.stanford.edu/~fenn/packs/mmdb.spec
new srpm: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-8.f8.src.rpm

(Removing NEEDSPONSOR: bug 462250)

Changed naming from libmmdb to mmdb (see https://bugzilla.redhat.com/show_bug.cgi?id=435015 and discussion therein).

new spec: http://www.stanford.edu/~fenn/packs/mmdb.spec
new srpm: http://www.stanford.edu/~fenn/packs/mmdb-1.09.1-9.f8.src.rpm

OK, the tarball matches what I get from the upstream web site now.

rpmlint has grown a new complaint:
  mmdb.x86_64: W: shared-lib-calls-exit /usr/lib64/libmmdb.so.0.0.0 
   exit.5
I didn't see this last time, which implies that either you had fixed the two places where exit() is called in the your modified tarball or that rpmlint has grown this complaint recently.  There was an rpmlint update three days ago so that might be it.

In any case, this is a rather odd thing for a shared library to do; I'd expect abnormal exits to call abort() instead, assuming they actually need to stop program execution.  Obviously you wouldn't expect any normal exits from library code, but upstream software authors often do rather dumb things.  I don't really see this as a blocker, however; it's more something that should be reported as a bug upstream.

In any case, the issues I raised earlier are fixed, and the above isn't a blocker, I'd say we're done.

APPROVED

New Package CVS Request
=======================
Package Name: mmdb
Short Description: macromolecular coordinate library
Owners: timfenn
Branches: F-9 F-10 EL-5
InitialCC: timfenn

cvs done.

tagged/built for all branches (still awaiting response from upstream re. library-calls-exit issue)

For now closing.