Bug 198835 - (Atlas-C++) Review Request: atlascpp - WorldForge message protocol library
Review Request: atlascpp - WorldForge message protocol library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT eris
  Show dependency treegraph
 
Reported: 2006-07-13 18:57 EDT by Wart
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-17 19:08:08 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Wart 2006-07-13 18:57:10 EDT
Spec URL: http://www.kobold.org/~wart/fedora/Atlas-C++.spec
SRPM URL: http://www.kobold.org/~wart/fedora/Atlas-C++-0.6.0-1.src.rpm
Description:
Atlas-C++ is the perhaps the most important library in the entire WorldForge
project, since nearly every other module requires it. Atlas-C++ provides a
native implementation of the entire Atlas specification including negotiation,
message encode and decode and the overlying Objects layer.
Comment 1 Christopher Stone 2006-07-13 22:19:39 EDT
-rpmlint output:
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZNK5Atlas9Exception4whatEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZTIN5Atlas9ExceptionE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZTVN5Atlas9ExceptionE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase11streamBeginEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase13streamMessageEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase9streamEndEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase10mapMapItemERKSs
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase11mapListItemERKSs
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase10mapIntItemERKSsl
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase12mapFloatItemERKSsd
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase13mapStringItemERKSsS3_
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase6mapEndEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase11listMapItemEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase12listListItemEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase11listIntItemEl
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase13listFloatItemEd
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase14listStringItemERKSs
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBase7listEndEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZTIN5Atlas7Message11DecoderBaseE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBaseC2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message7ElementC1ERKS1_
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message11DecoderBaseD2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message7Element5clearENS1_4TypeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message7Encoder14mapElementItemERKSsRKNS0_7ElementE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message7EncoderD1Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message7ElementaSERKS1_
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas6Codecs3XMLC1ERSdRNS_6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas7Message7EncoderC1ERNS_6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
_ZN5Atlas9ExceptionD2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasMessage-0.6.so.1.0.0
_ZTIN5Atlas9ExceptionE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasMessage-0.6.so.1.0.0
_ZNK5Atlas9Exception4whatEv
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasMessage-0.6.so.1.0.0
_ZTIN5Atlas6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasMessage-0.6.so.1.0.0
_ZTVN5Atlas9ExceptionE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasMessage-0.6.so.1.0.0
_ZN5Atlas6BridgeD2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasMessage-0.6.so.1.0.0
_ZN5Atlas9ExceptionD2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZTIN5Atlas9NegotiateE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZTIN5Atlas6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZN5Atlas9NegotiateD2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZN5Atlas6Codecs6PackedC1ERSdRNS_6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZN5Atlas6Codecs4BachC1ERSdRNS_6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZN5Atlas6Codecs3XMLC1ERSdRNS_6BridgeE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasNet-0.6.so.1.0.0
_ZN5Atlas6BridgeD2Ev
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasCodecs-0.6.so.1.0.0
_ZTIN5Atlas5CodecE
W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasCodecs-0.6.so.1.0.0
_ZN5Atlas5CodecD2Ev


It looks like linking is not done proplerly, there are many undefined non-weak
symbols in the .so files.  These libraries need to be linked against libAtlas.so.

- package named according to package naming guidelines
  - package could be named atlascpp as upstream doesnt seem to mind

- spec file matches package %{name}
- package meets packaging guidelines
- package licensed with open source compatible license
O license field does not match actual license
- license contained in %doc
- spec file written in American english
- spec file is legible
O Unable to download source pacakge:
wget http://dl.sourceforge.net/worldforge/Atlas-C++-0.6.0.tar.gz
--19:18:24--  http://dl.sourceforge.net/worldforge/Atlas-C++-0.6.0.tar.gz
           => `Atlas-C++-0.6.0.tar.gz'
Resolving localhost... 127.0.0.1
Connecting to localhost|127.0.0.1|:3128... connected.
Proxy request sent, awaiting response... 404 Not Found
19:18:24 ERROR 404: Not Found.
- package successfully compiles and builds on x86_64 FC-5
- All build dependencies are in BuildRequires, but optional build dependencies
are not listed (zlib and/or libbz2)
- package does not contain locales
- package properly calls ldconfig in %post/%postun
- package is not relocatable
- package owns all directories it creates
- package does not contain duplicate files
- package sets proper permissions on files
- package contains proper %clean section
- macro usage is consistant
- package contains permissible content
- package does not contain large documentation
- files in %doc do not affect runtime
- header files are contained in devel package
- pkgconfig files in devel package
- library files w/o suffix are in devel
- devel package requires base package
- package does not contain .la files
- package is not a GUI needing a .desktop file
- package does not own files or directories owned by other packages

=== MUST ===
- Add Requires: pkgconfig to devel package
- Comments say test fails on FC6, but infact it is failing on all x86_64 arches
because of an x86_64 warning.  Patch the code to not use -Werror so that checks
can be run
- Fix linking of all the .so files, they should be linked with -lAtlas and
libAtlas needs to be built first.
- Explain why you do not build with optional zlib or libbz2
- README indicates that this package requires socket streams such as skstream,
explain why this is not in the Requires
- Why are man pages in doc/man not installed?
- Should tutoral/ be installed?
- Fix license to match actual license
- Fix Source0 URL so that I can actually verify the upstream source is 0.6.0
actually out yet?
- Why name Atlas-C++ instead of atlascpp?
Comment 2 Jason Tibbitts 2006-07-14 12:46:26 EDT
I'm concerned about the potential for conflict with the existing atlas package
in Extras:

The ATLAS (Automatically Tuned Linear Algebra Software) project is an
ongoing research effort focusing on applying empirical techniques in
order to provide portable performance. At present, it provides C and
Fortran77 interfaces to a portably efficient BLAS implementation, as
well as a few routines from LAPACK.

The current naming makes this package look like it supplies C++ bindings for atlas.
Comment 3 Wart 2006-07-14 13:51:35 EDT
(In reply to comment #1)
> -rpmlint output:
> W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0
> _ZNK5Atlas9Exception4whatEv
...

rpmlint on FC4 x86_64 and FC5 i386 both missed this one.


> === MUST ===
> - Add Requires: pkgconfig to devel package

Done.

> - Comments say test fails on FC6, but infact it is failing on all x86_64 arches
> because of an x86_64 warning.  Patch the code to not use -Werror so that checks
> can be run

I think it would be better just to fix the test code and keep the -Werror.  This
might get sent upstream.

> - Fix linking of all the .so files, they should be linked with -lAtlas and
> libAtlas needs to be built first.

I wonder if this is a smp_mflags build 
> - Explain why you do not build with optional zlib or libbz2

Because I didn't notice the configure output?  :)

> - README indicates that this package requires socket streams such as skstream,
> explain why this is not in the Requires

It probably should be.  But since the package that Requires: this one also
Requires: skstream directly, I didn't catch this.

> - Why are man pages in doc/man not installed?

Oversight.  I've added most of them.  Others are not included as they contain
the build root path in the man page name (probably some doxygen artifact)

> - Should tutoral/ be installed?

No.  These are doxygen sources.  The tutorial is included in %doc doc/html

> - Fix license to match actual license

Fixed.

> - Fix Source0 URL so that I can actually verify the upstream source is 0.6.0
> actually out yet?

Works for me?

> - Why name Atlas-C++ instead of atlascpp?

Because even though upstream uses both Atlas-C++ and atlas-cpp, the former best
matches the actual tarball name.

I'll post a new srpm once the tests have been fixed.
Comment 4 Wart 2006-07-14 14:26:53 EDT
(In reply to comment #1)
> -rpmlint output:
> W: Atlas-C++ undefined-non-weak-symbol /usr/lib64/libAtlasObjects-0.6.so.1.0.0

Which dist and version of rpmlint (rpm -q rpmlint) are you running that produced
this warning?  I can't reproduce it on FC4-x86_64 or FC5-i386.
Comment 5 Wart 2006-07-14 14:30:26 EDT
(In reply to comment #2)
> I'm concerned about the potential for conflict with the existing atlas package
> in Extras:
...
> 
> The current naming makes this package look like it supplies C++ bindings for
atlas.

Upstream's name is unfortunate in this sense, but I think it's better to
preserve upstream's naming than to change it to something different.
Comment 6 Christopher Stone 2006-07-14 15:11:10 EDT
In order to get the rpmlint warnings, you must install the rpm first, then run
"rpmlint Atlas-C++"

As far as the name is concerned, all the configure files that check for this
package use the name "atlascpp", as well the web page uses this term as well. 
Since this also conforms more with Fedora naming standards I think the package
should be renamed to this.  Whatever you decide to name it, you should have a
provides with the other name to cover all bases.
Comment 7 Wart 2006-07-15 02:22:13 EDT
Ok, I'll concede to use atlascpp for the package name.  Here's an updated
package that I believe fixes all of the MUSTFIX issues:

http://www.kobold.org/~wart/fedora/atlascpp-0.6.0-2.src.rpm
http://www.kobold.org/~wart/fedora/atlascpp.spec
Comment 8 Wart 2006-07-17 19:08:08 EDT
Imported and built for rawhide.

Thanks!

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