[Bug 457160] Review Request: Zorba - General purpose XQuery processor implemented in C++

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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


Michael Schwendt <bugs.michael at gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugs.michael at gmx.net




--- Comment #11 from Michael Schwendt <bugs.michael at gmx.net>  2009-02-11 06:22:46 EDT ---
I'd like to see some fixes prior to further reviewing. It's not trivial to
review the current packaging:


> BuildRequires: cmake >= 2.4 libxml2-devel >= 2.2.16 icu >= 2.6 libicu-devel

It's highly recommended to put on BuildRequires per line and either sort them
alphabetically or group them appropriately.

icu is > 2.6 for several years. Even in RHEL 5.

Please reconsider the decision to list the versions. Often packagers forget to
keep them up-to-date.


> sh: ruby: command not found

"BuildRequires: ruby" is missing.


> -- Warning: GNU Bison not available -- the parser will not be regenerated
> -- Warning: GNU Flex not available -- the lexer will not be regenerated
> -- PHP5 binding not generated because library and include file not installed.
> -- Looking for doxygen... - NOT found

Just quoted for completeness.


> -- Can't build Zorba with TIDY support because TIDY is not found.

"BuildRequires: libtidy-devel" makes it happy. If you don't want that, adding a
comment would be good.


> -- Found PythonInterp: /usr/bin/python2.5
> -- Could NOT find PythonLibs  (missing:  PYTHON_LIBRARIES PYTHON_INCLUDE_PATH)
> -- Python binding not generated because library and include file not installed.

"BuildRequires: python-devel" is missing.


> %description devel
> The %{name}-devel package contains headers for building applications
> that use %{zorba}.

%{zorba} is undefined.


> cmake

There are guidelines for proper cmake usage:
https://fedoraproject.org/wiki/Packaging:Cmake

In particular, execute "make VERBOSE=1 ..." for increased verbosity in the
build log.

Then you will find that Fedora's global optimisation flags are not used. This
needs another look after the cmake related fixes.


> %files

> %{_libdir}/*.so.%{version}

This means that any version upgrade will break ABI compatibility with any
programs linked against this library. That's an indication of an unstable API,
ongoing heavy development, or developers who haven't got the versioning scheme
right.


> %dir %{_datadir}/doc/%{name}-%{version}
> %{_datadir}/doc/%{name}-%{version}

The %dir statement here is superfluous, because the second line already
includes the %name-%version directory and all its contents recursively. You're
advised to make them more explicit with a trailing slash. Like this:

  %{_datadir}/doc/%{name}-%{version}/


> %files devel

Here several directories are not included.
In particular:

%dir %{_includedir}/%{name}
%dir %{_includedir}/%{name}/%{name}
%dir %{_includedir}/%{name}/%{name}/util
%dir %{_includedir}/%{name}/simplestore
%dir %{_includedir}/%{name}/simplestore/msdom
%dir %{_datadir}/doc/%{name}-%{version}/python
%dir %{_datadir}/doc/%{name}-%{version}/python/examples
%dir %{_datadir}/doc/%{name}-%{version}/python/html
%dir %{_datadir}/doc/%{name}-%{version}/ruby
%dir %{_datadir}/doc/%{name}-%{version}/ruby/examples
%dir %{_datadir}/doc/%{name}-%{version}/ruby/html

https://fedoraproject.org/wiki/Packaging:UnownedDirectories

You could shrink your %files list much by including entire trees recursively or
by increased usage of '*' wildcards where appropriate. Else the only option is
to add as many of the missing %dir statements as necessary.

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