Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Summary: Review Request: SSM - coordinate superposition library Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: fenn@stanford.edu QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://www.stanford.edu/~fenn/packs/ssm.spec SRPM URL: http://www.stanford.edu/~fenn/packs/ssm-0.1-2.f8.src.rpm Description: SSM is a macromolecular coordinate superposition library, written by Eugene Krissinel of the EBI.
The library implements the SSM algorithm of protein structure comparison in three dimensions, which includes an original procedure of matching graphs built on the protein's secondary-structure elements, followed by an iterative three-dimensional alignment of protein backbone Calpha atoms.
Also see: http://www.ebi.ac.uk/msd-srv/ssm/ http://www.bioxray.dk/~mok/ssm.php
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From fenn@stanford.edu 2008-05-26 19:44 EST ------- Source RPM and spec file updated to bring RPM inline with current Fedora guidelines.
Spec URL: http://www.stanford.edu/~fenn/packs/ssm.spec SRPM URL: http://www.stanford.edu/~fenn/packs/ssm-0.1-3.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From rc040203@freenet.de 2008-05-26 21:04 EST ------- some comments based on coarse visual inspection of the rpm.spec:
MUSTFIX: - Don't ship *.la's Remove them in %install
SHOULD: - %makeinstall is considered obsolete. Better use "make DESTDIR=${RPM_BUILD_ROOT} install" if feasible.
CONSIDER: - Feel strongly encouraged not to ship static libraries (consider adding --disable-static to %configure)
[Analogous comments also apply to further packages related to this package you currently have pending for review]
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From fenn@stanford.edu 2008-05-27 05:55 EST ------- MUSTFIX: - Don't ship *.la's Remove them in %install
done
SHOULD: - %makeinstall is considered obsolete. Better use "make DESTDIR=${RPM_BUILD_ROOT} install" if feasible.
done
CONSIDER: - Feel strongly encouraged not to ship static libraries (consider adding --disable-static to %configure)
Unfortunately, many scientific folk still use statically linked libs when building execs, and like to have them available.
[Analogous comments also apply to further packages related to this package you currently have pending for review]
done
new spec: http://www.stanford.edu/~fenn/packs/ssm.spec new url: http://www.stanford.edu/~fenn/packs/ssm-0.1-4.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From rc040203@freenet.de 2008-05-27 09:07 EST ------- (In reply to comment #3)
CONSIDER:
- Feel strongly encouraged not to ship static libraries (consider adding
--disable-static to %configure)
Unfortunately, many scientific folk still use statically linked libs when building execs, and like to have them available.
These people should reconsider their habits. They should learn to use LD_LIBRARY_PATH and to bundle the shared libs their applications are using.
To me, presence of static libs is sufficient reason for not wanting to continue this review.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From fenn@stanford.edu 2008-05-27 17:18 EST ------- (In reply to comment #4)
(In reply to comment #3)
CONSIDER:
- Feel strongly encouraged not to ship static libraries (consider adding
--disable-static to %configure)
Unfortunately, many scientific folk still use statically linked libs when building execs, and like to have them available.
These people should reconsider their habits. They should learn to use LD_LIBRARY_PATH and to bundle the shared libs their applications are using.
To me, presence of static libs is sufficient reason for not wanting to continue this review.
Removed.
new spec: http://www.stanford.edu/~fenn/packs/ssm.spec new srpm: http://www.stanford.edu/~fenn/packs/ssm-0.1-5.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From rc040203@freenet.de 2008-05-28 01:03 EST ------- Thanks for not shipping static libs :)
Several major issues: * Building in mocks fails: ... checking for MMDB... configure: error: The pkg-config script could not be found or is too old. Make sure it is in your PATH or set the PKG_CONFIG environment variable to the full path to pkg-config. Alternatively, you may set the environment variables MMDB_CFLAGS and MMDB_LIBS to avoid the need to call pkg-config. See the pkg-config man page for more details. ...
=> Missing "BuildRequires: mmdb-devel"
* *-devel ships a *.pc => "Requires: pkgconfig" needs to be added to *-devel
* Your BuildRoot: doesn't comply to the FPG. Please use one of the options described in https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
* /sbin/ldconfig scriptlets missing from "main-package" cf. https://fedoraproject.org/wiki/PackagingDrafts/ScriptletSnippets#Shared_libr...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
rc040203@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- BugsThisDependsOn| |435016
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From fenn@stanford.edu 2008-05-28 21:37 EST ------- (In reply to comment #6)
Thanks for not shipping static libs :)
:) Thanks for the help in finding my silly errors!
Several major issues:
=> Missing "BuildRequires: mmdb-devel"
Done.
- *-devel ships a *.pc
=> "Requires: pkgconfig" needs to be added to *-devel
Done.
- Your BuildRoot: doesn't comply to the FPG.
Please use one of the options described in https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
Fixed.
- /sbin/ldconfig scriptlets missing from "main-package"
cf. https://fedoraproject.org/wiki/PackagingDrafts/ScriptletSnippets#Shared_libr...
Done.
new spec: http://www.stanford.edu/~fenn/packs/ssm.spec new srpm: http://www.stanford.edu/~fenn/packs/ssm-0.1-6.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From rc040203@freenet.de 2008-05-29 03:49 EST ------- Created an attachment (id=307021) --> (https://bugzilla.redhat.com/attachment.cgi?id=307021&action=view) Proposed spec changes
Attaching patch to fix remaining issues.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: SSM - coordinate superposition library
https://bugzilla.redhat.com/show_bug.cgi?id=435017
------- Additional Comments From fenn@stanford.edu 2008-05-29 23:42 EST ------- (In reply to comment #8)
Created an attachment (id=307021)
--> (https://bugzilla.redhat.com/attachment.cgi?id=307021&action=view) [edit]
Proposed spec changes
Attaching patch to fix remaining issues.
applied.
new spec: http://www.stanford.edu/~fenn/packs/ssm.spec new srpm: http://www.stanford.edu/~fenn/packs/ssm-0.1-7.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
--- Comment #10 from Tim Fenn fenn@stanford.edu 2008-10-25 20:28:42 EDT --- Changed naming from libssm to ssm (see https://bugzilla.redhat.com/show_bug.cgi?id=435015 and discussion therein).
new spec: http://www.stanford.edu/~fenn/packs/ssm.spec new srpm: http://www.stanford.edu/~fenn/packs/ssm-0.1-8.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Bug 435017 depends on bug 435016, which changed state.
Bug 435016 Summary: Review Request: mmdb - MMDB coordinate library https://bugzilla.redhat.com/show_bug.cgi?id=435016
What |Old Value |New Value ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |mtasaka@ioa.s.u-tokyo.ac.jp Flag| |fedora-review?
--- Comment #11 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2008-11-08 02:35:23 EDT --- For -8:
* License - The license tag should be LGPLv2+.
* BuildRequires - This package won't build without "BuildRequires: mmdb-devel".
* configure option ------------------------------------------------------------ 55 ++ pkg-config --variable=prefix mmdb 56 + ./configure --build=i386-redhat-linux-gnu --host=i386-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 --with-mmdb=/usr --disable-static 57 configure: WARNING: unrecognized options: --with-mmdb ------------------------------------------------------------ - Is the option "--with-mmdb=foo" useful?
* Timestamps ------------------------------------------------------------ make install DESTDIR=$RPM_BUILD_ROOT install='install -p' ------------------------------------------------------------ - The last argument should be "INSTALL='install -p'".
* Document - There is no need that -devel subpackage should own the same document files in main package.
* rpmlint ** undefined-non-weak-symbol - "$ rpmlint ssm" (please try rpmlint for 'installed' ssm) shows that libssm.so.0 contains many undefined non-weak symbols. ! You can check this also by $ ldd -r %_libdir/libssm.so.0 For packages providing -devel subpackage this cannot be allowed because this causes linkage error.
I guess making libssm.so linked against libmmdb.so will fix this issue (because LD_PRELOAD=%_libdir/libmmdb.so.0.0.0 ldd -r %_libdir/libssm.so.0.0.0 shows no undefined non-weak symbols)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
--- Comment #12 from Tim Fenn fenn@stanford.edu 2008-11-10 13:26:45 EDT --- (In reply to comment #11)
For -8:
- License
- The license tag should be LGPLv2+.
Fixed.
- BuildRequires
- This package won't build without "BuildRequires: mmdb-devel".
Fixed.
- configure option
55 ++ pkg-config --variable=prefix mmdb 56 + ./configure --build=i386-redhat-linux-gnu--host=i386-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 --with-mmdb=/usr --disable-static 57 configure: WARNING: unrecognized options: --with-mmdb
- Is the option "--with-mmdb=foo" useful?
I forgot to include a patch to one of the makefiles to go along with the configure patch - fixed.
- Timestamps
make install DESTDIR=$RPM_BUILD_ROOT install='install -p'
- The last argument should be "INSTALL='install -p'".
Fixed.
- Document
- There is no need that -devel subpackage should own the same document files in main package.
Removed.
- rpmlint
** undefined-non-weak-symbol - "$ rpmlint ssm" (please try rpmlint for 'installed' ssm) shows that libssm.so.0 contains many undefined non-weak symbols. ! You can check this also by $ ldd -r %_libdir/libssm.so.0 For packages providing -devel subpackage this cannot be allowed because this causes linkage error.
I guess making libssm.so linked against libmmdb.so will fix this issue (because LD_PRELOAD=%_libdir/libmmdb.so.0.0.0 ldd -r %_libdir/libssm.so.0.0.0 shows no undefined non-weak symbols)
Yeah, this all gets cleaned up with proper linking to libmmdb.
new spec: http://www.stanford.edu/~fenn/packs/ssm.spec new srpm: http://www.stanford.edu/~fenn/packs/ssm-0.1-9.f8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: SSM - |Review Request: ssm - |coordinate superposition |coordinate superposition |library |library Flag|fedora-review? |fedora-review+
--- Comment #13 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2008-11-11 01:52:55 EDT --- Okay.
--------------------------------------------------- This package (ssm) is APPROVED by mtasaka ---------------------------------------------------
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
--- Comment #14 from Tim Fenn fenn@stanford.edu 2008-11-11 15:32:00 EDT --- New Package CVS Request ======================= Package Name: ssm Short Description: coordinate superposition library Owners: timfenn Branches: F-9 F-10 EL-5 InitialCC: timfenn
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Tim Fenn fenn@stanford.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #15 from Kevin Fenzi kevin@tummy.com 2008-11-12 13:55:06 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
--- Comment #16 from Tim Fenn fenn@stanford.edu 2008-11-12 19:27:27 EDT --- cvs builds done, update request submitted to bodhi for F-9 and F-10.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=435017
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
--- Comment #17 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2008-11-18 23:00:21 EDT --- Closing, thanks.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla