Bug 611033 - Review Request: hail - core cloud computing services
Review Request: hail - core cloud computing services
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-03 03:37 EDT by Jeff Garzik
Modified: 2013-07-02 22:37 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-07-15 02:45:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jeff Garzik 2010-07-03 03:37:29 EDT
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 01:45:36 EDT
- 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 17:25:59 EDT
(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 17:29:06 EDT
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-13 22:24:18 EDT
> BR: libevent-devel

This is unneeded, and will be removed.
Comment 5 Jeff Garzik 2010-07-14 03:44:57 EDT
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 17:41:25 EDT
(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 18:10:41 EDT
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 01:29:11 EDT
CVS done (by process-cvs-requests.py).
Comment 9 Jeff Garzik 2010-07-15 02:45:42 EDT
Built in rawhide, closing.

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