Bug 735225

Summary: Review Request: axis2c - Web services engine implemented in C
Product: [Fedora] Fedora Reporter: Garrett Holmstrom <gholms>
Component: Package ReviewAssignee: Tom "spot" Callaway <tcallawa>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: notting, package-review, tcallawa
Target Milestone: ---Flags: tcallawa: fedora-review+
gwync: 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: 2012-03-05 22:42:38 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 Garrett Holmstrom 2011-09-01 20:29:57 UTC
Spec URL: http://gholms.fedorapeople.org/review/axis2c.spec
SRPM URL: http://gholms.fedorapeople.org/review/axis2c-1.6.0-1.src.rpm

Description:
Apache Axis2/C is a Web services engine implemented in the C programming
language.  It is based on the extensible and flexible Axis2 architecture.
Apache Axis2/C can be used to provide and consume WebServices.  It has
been implemented with portability and ability to embed in mind, hence
could be used as a Web services enabler in other software.

Apache Axis2/C supports SOAP 1.1 and SOAP 1.2, as well as REST style
of Webservices.  A single service could be exposed both as a SOAP style
as well as a REST style service simultaneously.  It also has built in
MTOM support, that can be used to exchange binary data.

Note to reviewer:
A lot of software that depends on Axis2/C assumes that everything is laid out in subdirectories of a single installation directory.  This doesn't comply with the FHS, so the %install section installs everything into such a single directory (%axis2c_home), then moves things into FHS-compliant locations and adds symlinks to %axis2c_home so dependent software that assumes the single-dir layout can still build.

Comment 1 Tom "spot" Callaway 2011-09-03 16:37:07 UTC
== Review ==

The usual notes apply:

* BuildRoot is unnecessary except on EL5 or older. If you're not building for EPEL, consider dropping it.
* rm -rf $RPM_BUILD_ROOT at the beginning of %install is unnecessary except on EL5 or older. If you're not building for EPEL, consider dropping it.
* %clean rm -rf $RPM_BUILD_ROOT is the default, except on EL5 or older. If you're not building for EPEL, consider dropping it.
* %defattr(-,root,root,-) is the default for %files sections, except on EL5 or older. If you're not building for EPEL, consider dropping it.

In addition, check-rpaths says:
ERROR   0001: file '/usr/libexec/axis2c/tools/md5/md5' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/libexec/axis2c/tools/tcpmon/tcpmon' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/bin/axis2_http_server' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/bin/axis2_tcp_server' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_tcp_receiver.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_http_common.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_axiom.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_parser.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_http_sender.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_engine.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/axis2c/modules/logging/libaxis2_mod_log.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/axis2c/modules/addressing/libaxis2_mod_addr.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_http_receiver.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libguththila.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libaxis2_tcp_sender.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libneethi.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/httpd/modules/libmod_axis2.so.0.6.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]

I tried --disable-rpath, but it didn't work. You must address this with one of the other methods: https://fedoraproject.org/wiki/Packaging/Guidelines#Removing_Rpath

== RPMLINT ==

axis2c.x86_64: W: shared-lib-calls-exit /usr/lib64/libaxutil.so.0.6.0 exit.5
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxutil.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libguththila.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_axiom.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/httpd/modules/libmod_axis2.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_http_common.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/axis2c/modules/logging/libaxis2_mod_log.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_engine.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_http_sender.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_http_receiver.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_tcp_receiver.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libneethi.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_parser.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_xpath.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_tcp_sender.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/axis2c/modules/addressing/libaxis2_mod_addr.so
axis2c.x86_64: W: dangling-symlink /usr/lib64/axis2c/include /usr/include
axis2c.x86_64: W: dangling-symlink /usr/lib64/axis2c/lib /usr/lib64
axis2c.x86_64: W: no-manual-page-for-binary axis2_http_server
axis2c.x86_64: W: no-manual-page-for-binary axis2_tcp_server
axis2c-devel.x86_64: W: no-documentation
axis2c-devel.x86_64: W: non-conffile-in-etc /etc/rpm/macros.axis2c
axis2c-doc.noarch: W: no-documentation

All safe to ignore.

axis2c.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/axis2c-1.6.0/INSTALL
axis2c.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/axis2c-1.6.0/NEWS
axis2c.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/axis2c-1.6.0/ChangeLog

Please fix these.

axis2c.x86_64: E: non-executable-script /usr/libexec/axis2c/tools/wsdl2c/WSDL2C.sh 0644L /bin/sh

This should be set +x.

axis2c.x86_64: W: log-files-without-logrotate /var/log/axis2c

You should enable logrotate support here. Create a logrotate config as $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/%{name}, and be sure to add Requires: logrotate. man logrotate will give you the config syntax.

axis2c-doc.noarch: E: non-executable-script /usr/share/axis2c/docs/download.cgi 0644L /bin/sh

Does this really belong in the -doc package?

axis2c-doc.noarch: W: devel-file-in-non-devel-package /usr/share/axis2c/docs/docs/hello/service/hello_svc.c
axis2c-doc.noarch: W: devel-file-in-non-devel-package /usr/share/axis2c/docs/docs/hello/client/hello.c

Safe to ignore.

axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath_streaming.c
axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath_streaming.h
axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath_internals.h
axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath_internals.c
axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath.c
axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath_internals_parser.c
axis2c-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/axis2c-src-1.6.0/axiom/src/xpath/xpath_functions.c

Please fix these (chmod -x on the source files in %prep should do the trick).

Resolve these items in a new SRPM and I'll finish the review.

Comment 2 Garrett Holmstrom 2012-01-14 13:51:14 UTC
Looks like this review slipped through the cracks.  Sorry.  :-(  Here's an update:

http://gholms.fedorapeople.org/review/axis2c-1.6.0-2.src.rpm
http://gholms.fedorapeople.org/review/axis2c-1.6.0-2.spec

Since I plan to add an el5 branch I can't get rid of the extra cruft.

Comment 4 Tom "spot" Callaway 2012-02-10 20:28:08 UTC
Okay, so I'm not sure why I originally said the rpmlint errors like this were safe to ignore:

axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxutil.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libguththila.so
axis2c.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libaxis2_axiom.so

The unversioned .so files that are simply symlinks to the versioned lib belong in -devel, which appears to be all of them. I'm not sure about the httpd/modules/libmod_axis2.so, the schema there seems to imply that versioned .so files are not being found or used by httpd, so in that specific sort of scenario, the unversioned .so file is probably fine to stay in the main package, but everything in %{_libdir} needs to be fixed.

Comment 5 Garrett Holmstrom 2012-02-17 16:32:06 UTC
Unfortunately axis2c dlopens the plain .so files, forcing the main package to contain the -devel symlinks.  Is there something I could do with the spec file's commentary to make that more clear?

Comment 6 Tom "spot" Callaway 2012-02-17 17:27:54 UTC
It dlopens ALL of them? Even the ones in /usr/lib64 ?

Ignoring how awful that is, please just confirm it.

Comment 7 Garrett Holmstrom 2012-02-17 23:59:03 UTC
I share your disgust.  It dlopens modules based on the contents of the particular application's configuration file.  Failure to include the plain .so files results in nasty errors like this one:

[error] class_loader.c(162) Loading shared library /usr/lib64/axis2c/lib/libaxis2_http_sender.so  Failed. DLERROR IS /usr/lib64/axis2c/lib/libaxis2_http_sender.so: cannot open shared object file: No such file or directory
[error] conf_builder.c(903) Transport sender is NULL for transport http, unable to continue
[error] conf_builder.c(262) Processing transport senders failed, unable to continue
[error] dep_engine.c(752) Populating Axis2 Configuration failed
[error] conf_init.c(64) Loading deployment engine failed for repository /usr/lib64/axis2c.

Comment 8 Tom "spot" Callaway 2012-02-20 14:51:02 UTC
While that sort of error is fixable (they're probably doing dlopen calls on hardcoded file names), I'm willing to let those slide. The ones I'm concerned about are the libs in %{_libdir} directly. Those shouldn't be dlopened, so it should be okay to put the unversioned so files that live there into -devel.

Comment 9 Garrett Holmstrom 2012-02-20 23:13:20 UTC
That *is* one of the libraries in %{_libdir} directly since %{_libdir}/axis2c/lib is a symlink to %{_libdir}.  It is one of several symlinks that lets the package put things in FHS-compliant locations while remaining compatible with software that assumes everything is underneath a singular $AXIS2C_HOME directory.

The problem here is that the package has to satisfy programs that load its libraries dynamically, like those that use the mod_axis2 httpd module, while simultaneously allowing programs to link directly against it like "regular" software.  Right now it serves both of these needs by putting everything, including the plain .so files, in %{_libdir}, but I am open to ideas.  AFAIK one could put everything inside of a package-specific subdirectory of %{_libdir}, but then one would have to add that path to ld.so.conf.d to satisfy the second case, effectively putting us right back where we started.

Thoughts?

Comment 10 Tom "spot" Callaway 2012-02-21 14:34:14 UTC
Besides *BARF*, no. :/

Okay. You're making the best of an ugly situation. APPROVED.

Comment 11 Garrett Holmstrom 2012-02-21 17:56:46 UTC
Thanks, spot.

New Package SCM Request
=======================
Package Name: axis2c
Short Description: Web services engine implemented in C
Owners: gholms
Branches: master f16 el6 el5
InitialCC:

Comment 12 Gwyn Ciesla 2012-02-21 18:04:53 UTC
Git done (by process-git-requests).

Added f17.