Bug 457160

zorba.review.txt:
Hi, you don't seem to be sponsored, so I can't perform an official review. Instead, I can get the review process started with a pre-review. Have you requested a fedoraproject account ?
===
[OK] OK
[NA] not applicable
[??] unsure, more info needed.
[ x] must fix

[OK] naming meets guidelines [same as upstream source], doesn't conflict
[OK] .spec is named as the package %{name}
[OK] generally meets packaging guidelines
[OK] upstream source md5sum matches.
dc4ffe43b191700b93c4802b8baeec38  zorba-0.9.21.tar.gz [within source rpm]
dc4ffe43b191700b93c4802b8baeec38  ../zorba-0.9.21.tar.gz [upstream]
[OK] buildroot is the second most preferred option
[OK] doesn't use %{locale} to handle locales.
[NA] doesn't use Prefix: tag.
[OK] header files are in the -devel sub package.
[OK] contains code not content
[OK] no static libs, no .pc files, no .la files
[OK] -devel package requires the base package
[OK] file names are UTF-8
[NA] .desktop: not a gui app.

[??] -devel include shows %name/%name/* . Is this what was intended, why ?

[??] are the .TAGFILEs needed by an end user of devel-docs ? Perhaps they are a side result of the compile process ?

[??] license is Apache license v2 from web site. extracted upstream source mentions "the Apache License" more than 700 times. The short name "APL 2.0" is the correct fedora reference.

[??] NOTICE.txt also provides some license information / history. I haven't analysed whether the license would be considered free for Fedora purposes. Have each of the authors mentioned 

[??] devel-doc is created as a separate package. It isn't overly large, and could potentially be part of the -devel package ? What reasoning caused you to split the devel-docs out ?

[??] spec legible: 
- could be improved by sticking to a certain coding style within the spec with relation to eg 2x blank lines between sections, rather than 0 or 1.
- the files section has one layout for some subpackages, and different spacing for the last ones.

[ ?] might as well fix the spelling of grammer and headerss !

[ ?] while individually specifying each %files to include can be done, would it be simpler to glob the folders instead (or have you already factored this in) ?

[??] python guidelines suggest placing python_sitelib determination at the top of the spec. Any reason to do it elsewhere ?

[??] places files directly in the %{python_sitelib}, rather than a module named subfolder. Not sure if that is allowed ?

[??] -python doesn't require the base package. Is there a reason why ?

[??] why put the *.py*, *.gif, *.rb examples in the python/ruby sub packages. Would these be more appropriate for the devel-doc package ?

[??] %build turns on debug output. I don't know whether that is allowed in the final package ?

[ x] rpmlint problems: rpmlint zorba-0.9.21-2.fc9.src.rpm 
zorba.src:116: E: files-attr-not-set
zorba.src:117: E: files-attr-not-set
zorba.src:120: E: files-attr-not-set
zorba.src:121: E: files-attr-not-set
zorba.src:122: E: files-attr-not-set
zorba.src:123: E: files-attr-not-set
zorba.src:124: E: files-attr-not-set
zorba.src:125: E: files-attr-not-set
zorba.src:126: E: files-attr-not-set
zorba.src:127: E: files-attr-not-set
zorba.src:128: E: files-attr-not-set
zorba.src:129: E: files-attr-not-set
zorba.src:130: E: files-attr-not-set
zorba.src:131: E: files-attr-not-set
zorba.src:132: E: files-attr-not-set
zorba.src:133: E: files-attr-not-set
zorba.src:134: E: files-attr-not-set
zorba.src:135: E: files-attr-not-set
zorba.src:136: E: files-attr-not-set
zorba.src:137: E: files-attr-not-set
zorba.src:138: E: files-attr-not-set
zorba.src:139: E: files-attr-not-set
zorba.src:140: E: files-attr-not-set
zorba.src:141: E: files-attr-not-set
zorba.src:142: E: files-attr-not-set
zorba.src:143: E: files-attr-not-set
zorba.src:144: E: files-attr-not-set
zorba.src:145: E: files-attr-not-set
zorba.src:146: E: files-attr-not-set
zorba.src:147: E: files-attr-not-set
zorba.src:148: E: files-attr-not-set
zorba.src:149: E: files-attr-not-set
zorba.src:150: E: files-attr-not-set
zorba.src:151: E: files-attr-not-set
zorba.src:152: E: files-attr-not-set
zorba.src:153: E: files-attr-not-set
zorba.src:154: E: files-attr-not-set
zorba.src:155: E: files-attr-not-set
zorba.src:160: E: files-attr-not-set
zorba.src:161: E: files-attr-not-set
zorba.src:162: E: files-attr-not-set
zorba.src:163: E: files-attr-not-set
zorba.src:164: E: files-attr-not-set
zorba.src:165: E: files-attr-not-set
zorba.src:169: E: files-attr-not-set
zorba.src:170: E: files-attr-not-set
zorba.src:171: E: files-attr-not-set
1 packages and 0 specfiles checked; 47 errors, 0 warnings.

use --info more abit more info about the errors.

[ x] doesn't build on i386. Is a build require missing ? Perhaps need to try one of the methods to help determine build requires at:
http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequires
---------------
-- look for component program_options
-- found ? Boost_PROGRAM_OPTIONS_LIBRARY_RELEASE-NOTFOUND
-- Boost_INCLUDE_DIRS: Boost_INCLUDE_DIR-NOTFOUND
-- Boost_LIBRARIES: 
-- Boost Version required: . Found: ..
CMake Error: Error in cmake code at
/home/davidt/rpmbuild/BUILD/zorba-0.9.21/cmake_modules/FindBoost.cmake:575:
MESSAGE Couldn't find the Boost libraries and/or include directory, or the version found is too old. Please install the Boost libraries AND development packages. You can set BOOST_ROOT, BOOST_INCLUDEDIR and BOOST_LIBRARYDIR to help find Boost.
Current CMake stack: 
[2]	/home/davidt/rpmbuild/BUILD/zorba-0.9.21/cmake_modules/FindBoost.cmake
[1]	/home/davidt/rpmbuild/BUILD/zorba-0.9.21/CMakeLists.txt
-- Configuring done
error: Bad exit status from /var/tmp/rpm-tmp.46222 (%build)


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

[ x] doesn't own all the dirs it creates:
%dir %{_datadir}/doc/%{name}-%{version} is the dir only
the subfolders c, cxx, zorba, python ruby aren't owned {I could be wrong here, since it won't build}.

[ x] unversioned .so must be in the -devel package

[ x] no excludearch yet does not build on i386

[ x] package provides .sos in normal lib dir, but doesn't use the guideline must %post/un -p /sbin/ldconfig

[ x] clean rm -rf is commented out. Why has this been done ? I don't think it could make it into Fedora like this.

[ x] not all %files sections include the %defattr() 

[ x] main package doc files are not marked as %doc. I assume they aren't required for the executable to run.

[ x] LICENSE.txt is included in source, and hence must be included in package, but is not marked %doc

[ x] -python summary line is repeated under description

[ x] -ruby package must indicate the required Ruby ABI version

[ x] -ruby library must indicate what it provides with a Provides: ruby(LIBRARY) = VERSION

[ x] must bump release with each adjustment of the package. This provides tracability, and ensures an update path.

You don't appear to have begun the fedoraproject account creation process. Note the email you use there should be the one used in the changlelog as well. I also notice that you are an upstream contributor. What applications are using zorba so far ?

Forgot to include:
[ x] Source0 for a sourceforge file needs to be writtent in a specific way, so that the main sf download server is used. It can't be a file://

Thank you very much for the review.   It wa very helpful.   I fixed most of the problems but ran out of time today to get to them all, so I have nothing new to show.   I do already have a Fedora project account but changed my primary
e-mail address since getting it.  Will continue working on it tomorrow.

(In reply to comment #1)
> zorba.review.txt:
> Hi, you don't seem to be sponsored, so I can't perform an official review.
> Instead, I can get the review process started with a pre-review. Have you
> requested a fedoraproject account ?

I have an account.   It is pfkeb.   I recently changed my e-mail contact from
paul_kunz.edu to paulfkunz.


> [??] -devel include shows %name/%name/* . Is this what was intended, why ?
This is the way the upstream installs itself.   I agree it is a little strange.

> 
> [??] are the .TAGFILEs needed by an end user of devel-docs ? Perhaps they are a
> side result of the compile process ?

They come from Doxygen. If the end user wants to link his Doxygen generated documentation to Zorba's locally then he needs them.

> 
> [??] license is Apache license v2 from web site. extracted upstream source
> mentions "the Apache License" more than 700 times. The short name "APL 2.0" is
> the correct fedora reference.
> 
I put the correct Fedora reference in the spec file.


> [??] NOTICE.txt also provides some license information / history. I haven't
> analysed whether the license would be considered free for Fedora purposes. Have
> each of the authors mentioned 
> 
   I'm not sure what you are suggesting here.

> [??] devel-doc is created as a separate package. It isn't overly large, and
> could potentially be part of the -devel package ? What reasoning caused you to
> split the devel-docs out ?
> 
I've removed devel-docs subpackage.

> [??] spec legible: 
> - could be improved by sticking to a certain coding style within the spec with
> relation to eg 2x blank lines between sections, rather than 0 or 1.
> - the files section has one layout for some subpackages, and different spacing
> for the last ones.
> 
   I've taken you suggestions.   Thanks, it does look better.

> [ ?] might as well fix the spelling of grammer and headerss !
> 
Ran the spell checker on the spec file and fixed the errors.

> [ ?] while individually specifying each %files to include can be done, would it
> be simpler to glob the folders instead (or have you already factored this in) ?
> 
   Good suggestion and done.

> [??] python guidelines suggest placing python_sitelib determination at the top
> of the spec. Any reason to do it elsewhere ?
> 
   its been moved.

> [??] places files directly in the %{python_sitelib}, rather than a module named
> subfolder. Not sure if that is allowed ?
> 
I've seen it done both ways.   Generally, when the package contains multiiple files and subdirectory is used.   However, zorba has only two files.

> [??] -python doesn't require the base package. Is there a reason why ?
> 
Was oversigth.  Now fixed.

> [??] why put the *.py*, *.gif, *.rb examples in the python/ruby sub packages.
> Would these be more appropriate for the devel-doc package ?
> 
Fixed.
> [??] %build turns on debug output. I don't know whether that is allowed in the
> final package ?
> 
Yes this is allowed.   The debugging symbols are stripped and put in zorba-debuginfo rpm.

> [ x] rpmlint problems: rpmlint zorba-0.9.21-2.fc9.src.rpm 

I added the attr tag.


> [ x] doesn't build on i386. Is a build require missing ? Perhaps need to try
> one of the methods to help determine build requires at:
> http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequire

I tried that procedure but it didn't work for me.   Nevertheless I found additional requires that needed to be added.
> 
> [ x] doesn't own all the dirs it creates:
> %dir %{_datadir}/doc/%{name}-%{version} is the dir only
> the subfolders c, cxx, zorba, python ruby aren't owned {I could be wrong here,
> since it won't build}.

Fixed.
> 
> [ x] unversioned .so must be in the -devel package
> 
moved it.

> 
> [ x] package provides .sos in normal lib dir, but doesn't use the guideline
> must %post/un -p /sbin/ldconfig
> 
Added it.

> [ x] clean rm -rf is commented out. Why has this been done ? I don't think it
> could make it into Fedora like this.
> 
Was commented out for debugging, now put back in.

> [ x] not all %files sections include the %defattr() 
> 
Fixed.
> [ x] main package doc files are not marked as %doc. I assume they aren't
> required for the executable to run.
> 
Correct.

> [ x] LICENSE.txt is included in source, and hence must be included in package,
> but is not marked %doc
> 
Fixed.

> [ x] -python summary line is repeated under description
> 
fixed.
> [ x] -ruby package must indicate the required Ruby ABI version
> 
Done.
> [ x] -ruby library must indicate what it provides with a Provides:
> ruby(LIBRARY) = VERSION
> 
Isn't this done automatically because of the .so file

> [ x] must bump release with each adjustment of the package. This provides
> tracability, and ensures an update path.
> 
Done.

> You don't appear to have begun the fedoraproject account creation process. 

My account if pfkeb

>Note
> the email you use there should be the one used in the changlelog as well. I
> also notice that you are an upstream contributor. What applications are using
> zorba so far ?

Fixed for recent change log entries.

Did you post updated spec and srpm somewhere ? Please provide those links each time your spec is upgraded {make it easy for your reviewers to confirm suggested changes have occurred}.

It's been two months since David's comment with no response; I will close this ticket soon if there is no further activity.

Jason, I checked the original .spec URL and there has not been changes to it. However, the project seems active and is now at zorba-0.9.4.tar.gz. Paul has also built packages for 0.9.4 {f8,f9} on the ftp site.

Trying his old email just in case, {Paul feel free to remove}...

I honestly thought that I had reponded to David's comment, but I don't see it.

The Zorba project is indeed active, however the 0.9.4 release is not yet final, there're waiting for one small patch for the Windows build.   On the ftp site, I have the latest release candidate.

SRPM: ftp://zorba-xquery.com/zorba-0.9.4-2.fc8.src.rpm
SPEC: ftp://zorba-xquery.com/zorba.spec

I would appreciate a review of this, so I can quickly move when the final release is out.

Correction, the SRPM should be

SRPM: ftp://zorba-xquery.com/zorba-0.9.4-2.fc9.src.rpm
SPEC: ftp://zorba-xquery.com/zorba.spec


that is, fc9 instead of fc8.

upstream sources finalized their release, so I've updated the files and bumped the release number by 1...

SRPM: ftp://zorba-xquery.com/zorba-0.9.4-3.fc9.src.rpm
SPEC: ftp://zorba-xquery.com/zorba.spec

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.

(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

Now missing "BuildRequires: libicu-devel" instead of "libicu".


> http://koji.fedoraproject.org/koji/taskinfo?taskID=1144346

Failed to build because of missing build requirements:

BuildRequires: tex(latex)
BuildRequires: tex(dvips)
# redundant through latex
# BuildRequires: tex(tex)


After adding those, it fails on i386 (dist-f11-gcc44) with a strange CMake Error occuring multiple times:

CMake Error at cmake_modules/AddSrcSubfolder.cmake:42 (FILE):
  file RelativePath must be passed a full path to the file:
  ÅE	ÐÅE	ÐÅE	<°C	̱C	

http://koji.fedoraproject.org/koji/taskinfo?taskID=1144362
http://koji.fedoraproject.org/koji/getfile?taskID=1144362&name=build.log


Pending F10 scratch-build attempt is here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1144376

Fedora 10 x86_64:

RPM build errors:
    File not found by glob: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib64/libzorba_simplestore.*

These are installed into /usr/lib instead of /usr/lib64 (%_libdir). Same applies to other files.

> -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so.0.9.5
> -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so
> -- Up-to-date: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so.0.9.5
> -- Up-to-date: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so
> -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/python2.5/site-packages/zorba_api.py
> -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/python2.5/site-packages/_zorba_api.so

(In reply to comment #13)
> Now missing "BuildRequires: libicu-devel" instead of "libicu".
> 
fixed, stupid mistake.

> 
> > http://koji.fedoraproject.org/koji/taskinfo?taskID=1144346
> 
> Failed to build because of missing build requirements:
> 
> BuildRequires: tex(latex)
> BuildRequires: tex(dvips)
> # redundant through latex
> # BuildRequires: tex(tex)
> 
tex is not used except by dosygen, but make docs doesn't generate tex.
I added the above anyway.

> 
> After adding those, it fails on i386 (dist-f11-gcc44) with a strange CMake
> Error occuring multiple times:
> 
> CMake Error at cmake_modules/AddSrcSubfolder.cmake:42 (FILE):
>   file RelativePath must be passed a full path to the file:
>   ÅE ÐÅE ÐÅE <°C ̱C 
> 
   I don't know what to do about that error as I don't see it on Fedora 10 i386
nor Fedora 9 x64

> http://koji.fedoraproject.org/koji/taskinfo?taskID=1144362
> http://koji.fedoraproject.org/koji/getfile?taskID=1144362&name=build.log
> 
> 
> Pending F10 scratch-build attempt is here:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1144376

Fedora.org was down for maintainence most of the weekend, so I couldn't see these logs. However, see next comment.

(In reply to comment #14)
> Fedora 10 x86_64:
> 
> RPM build errors:
>     File not found by glob:
> /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib64/libzorba_simplestore.*
> 
> These are installed into /usr/lib instead of /usr/lib64 (%_libdir). Same
> applies to other files.
> 
> > -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so.0.9.5
> > -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so
> > -- Up-to-date: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so.0.9.5
> > -- Up-to-date: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/libzorba_simplestore.so
> > -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/python2.5/site-packages/zorba_api.py
> > -- Installing: /builddir/build/BUILDROOT/zorba-0.9.5-3.fc10.x86_64/usr/lib/python2.5/site-packages/_zorba_api.so

I had to patch the upstream sources, but this is fixed and tested on x86_64 machine

New upload..

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

The LaTeX/TeX/dvips BuildRequires in comment 13 are because of this build failure in koji:

-- latex command LATEX_COMPILER not found but usually required. You will probably get warnings and user inetraction on doxy run.
-- makeindex command MAKEINDEX_COMPILER not found but usually required.
-- dvips command DVIPS_CONVERTER not found but usually required.
[...]
-- Configuring incomplete, errors occurred!

/usr/bin/makeindex is provided by package "texlive", so if LaTeX is truely optional, you would need the "tex(tex)" BuildRequires I marked redundant.

[...]

* src.rpm size has increased by factor 3. The spec %changelog doesn't mention that you've replaced the source tarball with one that differs from the previous package by a 25M diff. In case it is a snapshot retrieved from a SCM system, you need to follow:

  https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control
  https://fedoraproject.org/wiki/Packaging/NamingGuidelines#SnapshotPackages

* rpmdev-diff also reveals an added space character in the %cmake invocation that is not commented on. Two of the three -D options now put a space between -D and the variable name. No reason to believe it doesn't work, it's just strange. With other commands, silently added whitespace may lead to problems.

(In reply to comment #17)
> The LaTeX/TeX/dvips BuildRequires in comment 13 are because of this build
> failure in koji:
> 
> -- latex command LATEX_COMPILER not found but usually required. You will
> probably get warnings and user inetraction on doxy run.
> -- makeindex command MAKEINDEX_COMPILER not found but usually required.
> -- dvips command DVIPS_CONVERTER not found but usually required.
> [...]
> -- Configuring incomplete, errors occurred!
> 
> /usr/bin/makeindex is provided by package "texlive", so if LaTeX is truely
> optional, you would need the "tex(tex)" BuildRequires I marked redundant.
> 
Ok, I put that in.
> [...]
> 
> * src.rpm size has increased by factor 3. The spec %changelog doesn't mention
> that you've replaced the source tarball with one that differs from the previous
> package by a 25M diff. 

I realized in the middle of the night that I packaged the wrong version of the sources.   The large size difference is due to additional files releated to the testsuite used by the upstream developers.   This is fixed now.   Sorry about the error.



> * rpmdev-diff also reveals an added space character in the %cmake invocation
> that is not commented on. Two of the three -D options now put a space between
> -D and the variable name. No reason to believe it doesn't work, it's just
> strange. With other commands, silently added whitespace may lead to problems.

I've made the white space consistent.

New upload
SRPM: ftp://zorba-xquery.com/zorba-0.9.5-4.fc10.src.rpm
SPEC: ftp://zorba-xquery.com/zorba.spec

This failed to build for me in current rawhide, but I don't understand the error.  During the cmake invocation I get this:

-- processing store dir /builddir/build/BUILD/zorba-0.9.5/src/store/naive
CMake Error at cmake_modules/AddSrcSubfolder.cmake:42 (FILE):
  file RelativePath must be passed a full path to the file: �

(with the non-ascii character).  What follows is a call stack, then the cmake run continues but aborts at the end with

-- Configuring incomplete, errors occurred!

I don't really know anything about cmake, so I can't suggest much.

Jason, see comment 13. Asking the Fedora cmake owners hasn't lead to anything. This problem hasn't been encountered before by them.

If the package doesn't build, is best to remove it from the review queue.

In reference to comments #13 and #19.
The line in question is

	FILE(RELATIVE_PATH REL_PATH ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_LIST_FILE})

This line has been in the upstream sources since Oct 2007.
The file in question must be ${CMAKE_CURRENT_LIST_FILE} which is supposed to be set automatically by CMake.   I can not reproduce this problem on a Fedora 10
system with all updates applied.

I don't know what to do next.   I've asked the upstream author for help.

> I don't know what to do next.

Simply assume it had been approved some time ago for Fedora 10 or older, where it builds. Then the development of Fedora 11 causes breakage. I suggest filing a bug report about Fedora cmake and possibly also upstream cmake.

I suspect this may be the same bug which triggers the random crashes during kdepimlibs builds (which vanish when we try to debug them). See bug 475876 for that one.

Hmmm, this is probably different from the other bug. The problem here is that ${CMAKE_CURRENT_LIST_FILE} is invalid for some reason. We need to figure out why. The CMake call stack could help, but what would be even more useful would be running Valgrind over cmake (with cmake-debuginfo installed) to see where the memory corruption or the invalid pointer happens.

Actually I think I already know what's wrong. This is how CMAKE_CURRENT_LIST_FILE is implemented (unless it changed since):
http://www.cmake.org/Bug/file_download.php?file_id=600&type=bug

The problem here is that it saves the value into a std::string, then restores it from that string's c_str. That pointer is no longer valid once the std::string goes out of scope, which is soon afterwards. The value probably needs to be saved into a const char * instead.

Actually, the definitions are stored in a map using std::string, so this can't be it (AddDefinition copies the passed const char * into a new std::string, so when the pointer goes out of scope, it's supposed to be copied already).

I think we really need a Valgrind log.

Any chance we can get a log from running cmake in Valgrind, if possible with cmake-debuginfo installed?

I see that cmake-2.6.4-1 is now in updates for Fedora 10.  This version fixes
the bug discussed in the previous comments.   So I fixed the zorba.spec file to require this version and things build fine for me on Fedora 10.

I had gcc 4.4 installed in /usr/local on my development machine and the sources
required a few patches to compile with that version.   So I've included them
with this update.   These patches do not break the build with older versions of gcc.

New upload
SRPM: ftp://zorba-xquery.com/zorba-0.9.5-6.fc10.src.rpm
SPEC: ftp://zorba-xquery.com/zorba.spec

Created attachment 345244 [details]
rpmbuild failure on F11- (rawhide)

(In reply to comment #29)
> New upload
> SRPM: ftp://zorba-xquery.com/zorba-0.9.5-6.fc10.src.rpm
> SPEC: ftp://zorba-xquery.com/zorba.spec  
> I see that cmake-2.6.4-1 is now in updates for Fedora 10.  This version fixes
> the bug discussed in the previous comments.   So I fixed the zorba.spec file
> to require this version and things build fine for me on Fedora 10.
No go on F11- see attached end of compile.

> I had gcc 4.4 installed in /usr/local on my development machine and the
> sources required a few patches to compile with that version.   So I've
> included them with this update. These patches do not break the build with
> older versions of gcc.

I've triggered a test build on koji: (f10-updates)
http://koji.fedoraproject.org/koji/taskinfo?taskID=1374263

ps. package versions. The upstream seems to have gone:
ls -1
zorba.spec-0.9.21-2
zorba.spec-0.9.21-3
zorba.spec-0.9.4-2
zorba.spec-0.9.5-2
zorba.spec-0.9.5-6

$ rpmdev-vercmp 0 0.9.21 2 0 0.9.4 2
0:0.9.21-2 is newer

You would have to be careful with those, I think to ensure that 0.9.4 was packaged as 0.9.21-4.4 or something (to avoid epochs). Alternately it might have been better knowing what we know now to package 
0.9.21-1 as 0.9.2-3.21 and
0.9.21-2 as 0.9.2-4.21 and
0.9.21-3 as 0.9.2-5.21. 
or
0.9.21-1 as 0.9.2.1-3.21 and so forth.
Otherwise rpm upgrade paths wont work. More on https://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease

The F11 build failure is a different issue (a GCC 4.4 issue in the Zorba source code, nothing to do with CMake).

(In reply to comment #31)
> The F11 build failure is a different issue (a GCC 4.4 issue in the Zorba source
> code, nothing to do with CMake).  

I don't understand it.   The compiler is says FunctionSig has no type, but it is declared as a class in the same file (xquery_parser.y) on line 63.   Also, the code compiles with gcc 4.4 installed in /usr/local on my Fedora 10 machine.

Any suggestions on what to do next?

Maybe a different version of Bison or of some library? The line before declares "expr", maybe that's defined as a macro somewhere?

If you change your:
make VERBOSE=1 %{?_smp_mflags}
in your specfile to:
make VERBOSE=1 %{?_smp_mflags} || ( cd src && g++ -E -Dzorba_simplestore_EXPORTS -Dzorba_EXPORTS -DFLEX_FILES_REGENERATED -D_FORTIFY_SOURCE=2 -I. -I../external -I/usr/include/libxml2 -I../include compiler/parser/query_loc.cpp && exit 1 )
you should get the preprocessed source code in your build.log, that might tell us some more about the error.

SPEC: http://mgieseki.fedorapeople.org/zorba/zorba.spec
SRPM: http://mgieseki.fedorapeople.org/zorba/zorba-0.9.5-7.fc11.src.rpm

I'd also like to see zorba packaged for Fedora. Unfortunately, the review process seems to have stalled. To keep it alive, I digged into the source and fixed the build problems. There are still some spec file issues to be resolved but that should be doable. 
Paul, if you can afford the time, please continue working on this package. :)


$ rpmlint ../RPMS/i586/zorba-*
zorba.i586: W: shared-lib-calls-exit /usr/lib/libzorba_simplestore.so.0.9.5 exit
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/options_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/path_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/zorbastring_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/c/html/options_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/properties__base_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/simplestorec_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/simplestore_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/identtypes_8h__incl.map
zorba.i586: W: devel-file-in-non-devel-package /usr/lib/libzorba_simplestore.so
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/smart__ptr_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/debugger__exception_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/store__consts_8h__incl.map
zorba.i586: E: zero-length /usr/share/doc/zorba-0.9.5/cxx/html/version_8h__incl.map
zorba-python.i586: W: summary-not-capitalized zorba Python module for zorba
zorba-python.i586: W: unstripped-binary-or-object /usr/lib/python2.6/site-packages/_zorba_api.so
zorba-python.i586: W: no-documentation
zorba-ruby.i586: W: summary-not-capitalized zorba Ruby module
zorba-ruby.i586: W: unstripped-binary-or-object /usr/lib/ruby/site_ruby/1.8/i386-linux/zorba_api.so
zorba-ruby.i586: W: no-documentation
5 packages and 0 specfiles checked; 12 errors, 8 warnings.

This is still marked as not being ready, and the last comment from the submitter was the better part of a year ago.  Setting needinfo; I'll close this out soon if there's no further progress.  If someone else wishes to submit this package, please go ahead and open a new ticket and close this one as a duplicate.

*** This bug has been marked as a duplicate of bug 658420 ***