[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





--- Comment #12 from Paul F. Kunz <paulfkunz at gmail.com>  2009-02-12 18:08:58 EDT ---
(In reply to comment #11)
> I'd like to see some fixes prior to further reviewing. It's not trivial to
> review the current packaging:
> 
Thanks for the detailed review as it was.
> 
> > 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.
> 
Good idea, I've cleaned it up and it does indeed look a lot better.

> icu is > 2.6 for several years. Even in RHEL 5.
> 
Ok, version number removed for icu

> Please reconsider the decision to list the versions. Often packagers forget to
> keep them up-to-date.
> 
   I took versions required from upstream build documentation.

> > sh: ruby: command not found
> 
> "BuildRequires: ruby" is missing.
> 
   Indeed, fixed.
> > -- Warning: GNU Bison not available -- the parser will not be regenerated
> > -- Warning: GNU Flex not available -- the lexer will not be regenerated

I thought these were on every system, guess not.

> > -- PHP5 binding not generated because library and include file not installed.
Had to add php-cli as well as php-devel for cmake to find it.

> > -- Looking for doxygen... - NOT found
> 
Added doxygen, grapviz for doxygen was already there.


> 
> 
> > -- 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
Added it.
> 
> 
> > -- 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.
>
   Correct, so I addedit.
> 
> > %description devel
> > The %{name}-devel package contains headers for building applications
> > that use %{zorba}.
> 
> %{zorba} is undefined.
> 
Fixed it should have been %{name}
> 
> > cmake
> 
> There are guidelines for proper cmake usage:
> https://fedoraproject.org/wiki/Packaging:Cmake
> 
Thanks for the pointer.

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

> 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.
> 
Fixed.
> 
> > %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}/
> 
> 
Fixed.

> > %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.

Added the missing %dir

SRPM: ftp://zorba-xquery.com/zorba-0.9.5-2.fc10.src.rpm
SPEC: ftp://zorba-xquery.com/zorba.spec

-- 
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.