Bug 611033

Summary: Review Request: hail - core cloud computing services
Product: [Fedora] Fedora Reporter: Jeff Garzik <jgarzik>
Component: Package ReviewAssignee: Orcan Ogetbil <oget.fedora>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, oget.fedora, panemade, pbrobinson, peterm
Target Milestone: ---Flags: oget.fedora: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-07-15 06:45:42 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jeff Garzik 2010-07-03 07:37:29 UTC
Spec URL: http://gtf.org/garzik/hail/hail.spec
SRPM URL: http://gtf.org/garzik/hail/hail-0.7-1.fc12.src.rpm
Description: library and basic networking services for cloud computing.

IMPORTANT NOTE:  This is a package rename.  Existing packages "cld" and "chunkd" will be removed, and this combined package "hail" will be added.

Comment 1 Orcan Ogetbil 2010-07-08 05:45:36 UTC
- First of all, cool package name!

! Some brief explanations on the additional sources would be nice.

? Shouldn't the Group tag for the base package be "System Environment/Libraries"?

? cld/server.c and cld/cldbadm.c have references to the directory 
   CLD_LIBDIR "/cld/lib"
where CLD_LIBDIR is set to %{_libdir} by the configure script. But this directory is not packaged. Are we missing something?

* BRs: libevent-devel zlib-devel procps
seem redundant. 

? Why is this library linked to zlib? I don't see any usage.

* rpmlint on the RPMs says:
chunkd.x86_64: W: incoherent-subsys /etc/rc.d/init.d/chunkd $prog                                                                                                                                                                            
hail.src: W: name-repeated-in-summary C Hail                                                                                                                                                                                                 
cld.x86_64: W: no-documentation                                                                                                                                                                                                              
cld.x86_64: W: no-manual-page-for-binary cld                                                                                                                                                                                                 
cld.x86_64: W: no-manual-page-for-binary cldcli                                                                                                                                                                                              
cld.x86_64: W: no-manual-page-for-binary cldbadm                                                                                                                                                                                             
cld.x86_64: W: incoherent-subsys /etc/rc.d/init.d/cld $prog 

These can all be ignored.

However rpmlint on the installed hail package says:
hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 g_thread_functions_for_glib_use
hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 g_threads_got_initialized
hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 xmlFree
hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 g_free
hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 SSL_connect
... lots of them

Indeed when I run "ldd -r" on the DSO, I see lots of undefined symbols. Is this library linked improperly?


* The file chunkc.h contains
   #include <openssl/ssl.h>
   #include <glib.h>
Thus the devel subpackage nedds to require openssl-devel, glib2-devel

* I think these
   Requires(post):         chkconfig
   Requires(preun):        chkconfig initscripts
need to go to the cld and chunkd subpackages. The main package contains the library only. It doesn't have any init scripts.

Comment 2 Jeff Garzik 2010-07-13 21:25:59 UTC
(In reply to comment #1)
> - First of all, cool package name!

thanks :)

> ! Some brief explanations on the additional sources would be nice.

I'd be glad to, but am at a loss as to what people might be looking for?  Just something like "init.d and sysconf files for <this-daemon>"?  Or can you point to another Fedora package as a useful example?


> ? Shouldn't the Group tag for the base package be "System
> Environment/Libraries"?

fixed

> ? cld/server.c and cld/cldbadm.c have references to the directory 
>    CLD_LIBDIR "/cld/lib"
> where CLD_LIBDIR is set to %{_libdir} by the configure script. But this
> directory is not packaged. Are we missing something?

yes.  fixed.


> * BRs: libevent-devel zlib-devel procps
> seem redundant. 
> 
> ? Why is this library linked to zlib? I don't see any usage.

zlib seems superfluous.  Removed.  Not sure how the others are redundant?

> However rpmlint on the installed hail package says:
> hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1
> g_thread_functions_for_glib_use
> hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1
> g_threads_got_initialized
> hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 xmlFree
> hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1 g_free
> hail.x86_64: W: undefined-non-weak-symbol /usr/lib64/libhail.so.0.100.1
> SSL_connect
> ... lots of them
> 
> Indeed when I run "ldd -r" on the DSO, I see lots of undefined symbols. Is this
> library linked improperly?

The pkgconfig info file specifies additional linking libraries, which seems to fix this in downstream applications.

But regardless, I wonder if this is non-standard behavior for a shared lib?  Will investigate.


> * The file chunkc.h contains
>    #include <openssl/ssl.h>
>    #include <glib.h>
> Thus the devel subpackage nedds to require openssl-devel, glib2-devel

fixed


> * I think these
>    Requires(post):         chkconfig
>    Requires(preun):        chkconfig initscripts
> need to go to the cld and chunkd subpackages. The main package contains the
> library only. It doesn't have any init scripts.    

fixed

Comment 3 Jeff Garzik 2010-07-13 21:29:06 UTC
Updated with comments, and to the latest upstream git release.

Spec URL: http://gtf.org/garzik/hail/hail.spec
SRPM URL: http://gtf.org/garzik/hail/hail-0.8-0.1.gee257f18.fc12.src.rpm

NOTES / caveats:

1) not yet addressed (NYA): brief explanation for addnl sources

2) NYA (might not be needed?): "BRs: libevent-devel [...] procps seem redundant."  procps is used in the build tests, in particular.

3) NYA: rpmlint warnings on installed hail pkg, related to [un]linked libraries

Comment 4 Jeff Garzik 2010-07-14 02:24:18 UTC
> BR: libevent-devel

This is unneeded, and will be removed.

Comment 5 Jeff Garzik 2010-07-14 07:44:57 UTC
Updated again, this should satisfy all comments:

Spec URL: http://gtf.org/garzik/hail/hail.spec
SRPM URL: http://gtf.org/garzik/hail/hail-0.8-0.2.gf9c5b967.fc12.src.rpm

Changes since hail-0.8-0.1.g*
- addnl source brief explanations
- removed libevent-devel BR
- fixed rpmlint warnings for installed hail pkg.

A few "unused-direct-shlib-dependency" warnings remain, but these all come from libraries that xml's pkg-config and glib2's pkg-config told us to link against.

> hail.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhail.so.0.100.1 /lib64/libgthread-2.0.so.0
> hail.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhail.so.0.100.1 /lib64/librt.so.1
> hail.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhail.so.0.100.1 /lib64/libz.so.1
> hail.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libhail.so.0.100.1 /lib64/libm.so.6

Comment 6 Orcan Ogetbil 2010-07-14 21:41:25 UTC
(In reply to comment #3)
> brief explanation for addnl sources
> 

something like where the file comes from and why it is for. The brief comment in the last specfile seems good.

> 2) NYA (might not be needed?): "BRs: libevent-devel [...] procps seem
> redundant."  procps is used in the build tests, in particular.
> 

I can build the package, and the tests don't fail when I remove procps from BR. Am I missing something? Anyway, it small package and this is not a blocker. By the way there was something like "BuildRequires(check):" for such purposes.

> 3) NYA: rpmlint warnings on installed hail pkg, related to [un]linked 
> libraries    

these are okay. 

Thanks for the updates. I built the package on F-13 and on rawhide without problems. The package is good to go.

---------------------------------------
This package (hail) is APPROVED by oget
---------------------------------------

Comment 7 Jeff Garzik 2010-07-14 22:10:41 UTC
New Package CVS Request
=======================
Package Name: hail
Short Description: Project Hail core cloud computing services
Owners: jgarzik zaitcev
Branches: F-12 F-13 
InitialCC:

Comment 8 Kevin Fenzi 2010-07-15 05:29:11 UTC
CVS done (by process-cvs-requests.py).

Comment 9 Jeff Garzik 2010-07-15 06:45:42 UTC
Built in rawhide, closing.