[Bug 192889] Review Request: openais standards based cluster framework

bugzilla at redhat.com bugzilla at redhat.com
Wed May 24 12:11:36 UTC 2006


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: openais standards based cluster framework


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192889


paul at city-fan.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |paul at city-fan.org




------- Additional Comments From paul at city-fan.org  2006-05-24 08:03 EST -------
(In reply to comment #0)
> I have followed the packaging guidelines as closely as possible.  I have also
run rpmlint (with some strange complaints about the init script).

I took a look at this package with a view to fixing the rpmlint/initscript
issues. However, I found a lot more to do here as well.

(In reply to comment #9)
> Addressing comment #3:
> Do you mean by this comment that I should not list out each individual shared
> object file, but instead use wildcards?  It appears from the many examples I
> found that each file is listed separately but I could remove all of the separate
> filenames and then use the wildcard lines.  I have no preference either way,
> although it is somewhat helpful for the package maintainer (me) if the specfile
> were to complain during build because of a shared object additiion/removal.

I agree with you here, and tend to explicitly list binaries and important shared
libs explictly. It's a personal preference thing. However, I'd be inclined to
just have:

%{_includedir}/openais/

rather than enumerating each and every include file, and you might consider
something similar for the manpages if there eventually going to be hundreds of them.

(In reply to comment #10)
> Nope. man pages should always be part of the package, which implements what they
> document. I.e. in general, API docs should be part of *-devel packages, while
> user application man pages would be part of <non-devel> packages.
> 
> A separate *-doc packages would be useful for other (non-man, non-info)
> documentation, e.g. html formated books.

I agree enirely with this.

Now, on to my suggested changes to the spec.

Much of what rpmlint was complaining about was due to the initscript, which
wasn't installed using chkconfig in %post despite the PreReq: of chkconfig being
in the spec, and the initscript itself was "enabled by default", which is
generally a no-no.

Suggestions:
* Add %post and %preun calls to chkconfig to install and remove the initscript
links properly
* Use Requires(post) and Requires(presun) style scriptlet deps instead of PreReq:
* Patch the initscript to not be enabled by default

Now for the other things I came across:
* The URL points to a tarball and not the project home page
* Source0 is not a full URL (the value of the current Url: would work)
* "%define _exec_prefix /usr" should be avoided
* Directory ownership is still not right, e.g. for include files
* I'd suggest ending manpage %files entries with "*" rather than ".gz" so as to
enable the package to build on systems where manpages aren't compressed, or are
compressed with something else, like bzip
* Please bump the release for every package change during the review process so
that it's easier to tell which comments apply to which version of the package

I'll attach an updated spec and patches.

And finally for now...
* The package does not honor %{optflags}. Worse still, when CFLAGS is set to
"%{optflags}" to correct this, the package fails to build, with some
worrying-looking errors:

...
==== /nis-home/phowarth/BUILD/BUILD/openais-0.75/exec ===
make[1]: Entering directory `/nis-home/phowarth/BUILD/BUILD/openais-0.75/exec'
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o aispoll.o aispoll.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totemip.o totemip.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totemnet.o totemnet.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totemrrp.o totemrrp.c
totemrrp.c: In function 'active_instance_initialize':
totemrrp.c:873: warning: call to __builtin___memset_chk will always overflow
destination buffer
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totemsrp.o totemsrp.c
totemsrp.c:1151: warning: 'memb_set_print' defined but not used
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totemmrp.o totemmrp.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totempg.o totempg.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o tlist.o tlist.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o crypto.o crypto.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o wthread.o wthread.c
ar -rc libtotem_pg.a aispoll.o totemip.o totemnet.o totemrrp.o totemsrp.o
totemmrp.o totempg.o tlist.o crypto.o wthread.o
cc   -ldl -lpthread -L./  -rdynamic -ldl -shared -Wl,-soname,libtotem_pg.so.1
aispoll.o totemip.o totemnet.o totemrrp.o totemsrp.o totemmrp.o totempg.o
tlist.o crypto.o wthread.o -o libtotem_pg.so.1.0
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o main.o main.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o print.o print.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o mempool.o mempool.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o util.o util.c
util.c: In function 'getSaNameT':
util.c:99: warning: pointer targets in return differ in signedness
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o sync.o sync.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o service.o service.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o ipc.o ipc.c
ipc.c: In function 'libais_deliver':
ipc.c:532: warning: implicit declaration of function 'getpeereid'
ipc.c: At top level:
ipc.c:675: warning: 'deliver_fn' defined but not used
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -fPIC -c -o totemconfig.o totemconfig.c
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables   -c -o mainconfig.o mainconfig.c
cc   -ldl -lpthread -L./  -rdynamic -ldl aispoll.o totemip.o totemnet.o
totemrrp.o totemsrp.o totemmrp.o totempg.o tlist.o crypto.o wthread.o main.o
print.o mempool.o util.o sync.o service.o ipc.o totemconfig.o mainconfig.o
../lcr/lcr_ifact.o libtotem_pg.a -o aisexec
totemnet.o: In function `netif_determine':
/nis-home/phowarth/BUILD/BUILD/openais-0.75/exec/totemnet.c:696: undefined
reference to `totemip_iface_check'
ipc.o: In function `libais_deliver':
/nis-home/phowarth/BUILD/BUILD/openais-0.75/exec/ipc.c:532: undefined reference
to `getpeereid'
collect2: ld returned 1 exit status
make[1]: *** [aisexec] Error 1
make[1]: Leaving directory `/nis-home/phowarth/BUILD/BUILD/openais-0.75/exec'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.861 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.861 (%build)


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.




More information about the Fedora-package-review mailing list