Spec URL: http://www.annexia.org/tmp/ocaml/nekovm.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/nekovm-1.7.1-3.fc10.src.rpm

Better method of stopping rpm from stripping the binaries.

- build in Rawhide/x86_64 fails (http://koji.fedoraproject.org/koji/taskinfo?taskID=799312) - maybe BR: sqlite-devel is missing?
- Fedora's CFLAGS are not used, could be solved with using "make CFLAGS="$RPM_OPT_FLAGS -I vm -D_GNU_SOURCE"" for building
- what about 64-bit compatibility - I see "CFLAGS += -D_64BITS" in the Makefile?

Yes, it was rather broken on 64 bit.  This seems to resolve the
issues, and I've also built it in Koji so the dependency / 64-bit
problems should be fixed.

Spec URL: http://www.annexia.org/tmp/ocaml/nekovm.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/nekovm-1.7.1-6.fc10.src.rpm

* Tue Sep  2 2008 Richard W.M. Jones <rjones> - 1.7.1-6
- BR sqlite-devel
- Remove DOS line-endings and executable bits from the source.
- Add RPM CFLAGS.
- *.ndll files are always installed in /usr/lib/neko (even on 64 bit).
- Search /usr/lib64 directory for libraries when building.
- When building, link against libmysqlclient.so (not .a).
- Avoid a compiler stack overflow when building on 64 bit.
- Stop prelink from stripping the binaries.

http://koji.fedoraproject.org/koji/taskinfo?taskID=799546

formal review is here, see notes at the end

OK	source files match upstream:
	    cf2d1ebcb483fe88abb263fe276015f710a751bb  neko-1.7.1.tar.gz
OK	package meets naming and versioning guidelines.
OK	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
OK	build root is correct.
OK	license field matches the actual license.
OK	license is open source-compatible. License text included in package.
OK	latest version is being packaged.
OK	BuildRequires are proper.
OK	compiler flags are appropriate.
OK	%clean is present.
OK	package builds in mock (Rawhide/x86_64).
OK	debuginfo package looks complete.
??	rpmlint is silent.
OK	final provides and requires look sane.
N/A	%check is present and all tests pass.
BAD	no shared libraries are added to the regular linker search paths.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
BAD	no scriptlets present.
OK	code, not content.
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	headers in -devel.
OK	no pkgconfig files.
OK	no libtool .la droppings.
OK	not a GUI app.

- use --keepdate when running dos2unix to preserve the timestamps for the files
- shared library is added, but ldconfig scriptlets are missing
- rpmlint complains
nekovm.src:102: E: hardcoded-library-path in %{_prefix}/lib/neko
nekovm.src:109: E: hardcoded-library-path in %{_prefix}/lib
nekovm.src:110: E: hardcoded-library-path in %{_prefix}/lib/*.so
nekovm.src:127: E: hardcoded-library-path in %{_prefix}/lib/neko/
 - I think these will create a multilib issue together with shared libneko.so in the main package
 - what is the purpose of the *.ndll? runtime libs for nekovm based apps?

nekovm.x86_64: W: no-soname /usr/lib64/libneko.so
 - result of not-so-right linking command

I think you need to split the package to
main - %{_bindir} + /usr/lib/neko
libs - %{_libdir}
devel - %{_includedir}

- I think that more appropriate Group for the main package is Development/Languages, the -libs should go into System Environment/Libraries and -devel should be in Development/Libraries

ping?

pong?  Sorry, I need to look at this.  Set to NEEDINFO of me.

(In reply to comment #4)
> BAD no shared libraries are added to the regular linker search paths.
> BAD no scriptlets present.

This is fixed now so that the shared library has
an SONAME, and we run ldconfig in the %post/%postun
scriptlets.

> - use --keepdate when running dos2unix to preserve the timestamps for the files

Done.

> - shared library is added, but ldconfig scriptlets are missing

Done.

> - rpmlint complains

rpmlint now says:

nekovm.src:114: E: hardcoded-library-path in %{_prefix}/lib
nekovm.src:121: E: hardcoded-library-path in %{_prefix}/lib
nekovm.src:122: E: hardcoded-library-path in %{_prefix}/lib/*
nekovm.src:123: E: hardcoded-library-path in %{_prefix}/lib
3 packages and 0 specfiles checked; 4 errors, 0 warnings.

But in this case it's just complaining about my
install script which is moving the files FROM
%{_prefix}/lib to %{_libdir}.  I think this is
rpmlint getting it wrong.

>  - I think these will create a multilib issue together with shared libneko.so
> in the main package

Fixed so now it uses %{_libdir} always.

>  - what is the purpose of the *.ndll? runtime libs for nekovm based apps?

These are some type of runtime shared library
containing neko VM code.

> nekovm.x86_64: W: no-soname /usr/lib64/libneko.so
>  - result of not-so-right linking command

This is now fixed.

> I think you need to split the package to
> main - %{_bindir} + /usr/lib/neko
> libs - %{_libdir}
> devel - %{_includedir}

I don't think this split will work.  The reason is
that the main package will always require the -libs
package, because it always needs (at least) std.ndll.

This is the updated package:

Spec URL: http://www.annexia.org/tmp/ocaml/nekovm.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/nekovm-1.8.0-2.fc10.src.rpm

* Tue Dec 16 2008 Richard W.M. Jones <rjones> - 1.8.0-2
- New upstream release 1.8.0.
- Use dos2unix --keepdate.
- Use scriptlets to run ldconfig.
- Use _libdir instead of _prefix/lib, and modify so it searches
  /usr/lib64 as well as /usr/lib.
- Set the soname correctly and include libneko.so.1.

So all issues should be fixed, but there will be multilib/multiarch problems due the existence of the devel subpackage. I tend to agree with you that there is no simple solution, so you should ask rel-eng to add nekovm to the list of multilib incompatible packages. And/or try to discuss the issue on the fedora-devel mailing list.

But anyway, this package is APPROVED.

Thanks for approving this.

Are you sure there are still multilib issues remaining?
The new package (1.8.0-2) should only install packages
in %{_libdir}.  Can this still cause a conflict?

New Package CVS Request
=======================
Package Name: nekovm
Short Description: Neko embedded scripting language and virtual machine
Owners: rjones
Branches: F-10
InitialCC:

(In reply to comment #9)
> Thanks for approving this.
> 
> Are you sure there are still multilib issues remaining?
> The new package (1.8.0-2) should only install packages
> in %{_libdir}.  Can this still cause a conflict?

The binaries from %{_bindir} will differ and thus will block parallel installation of the nekovm packages.

I still think it can be split to be multilib compliant
main - %{_bindir} + %{_libdir}/neko/
-libs - %{_libdir}/libneko.so.*
-devel - %{_includedir} + %{_libdir}/libneko.so

both main and -devel will Require -libs

Created attachment 327224 [details]
update set-soname patch

- make libneko.so.1 real file and libneko.so symlink
- use "cp -d" when copying the libs otherwise (plain "cp") the symlink was dereferenced to the real file

Created attachment 327225 [details]
create -libs subpackage to be multilib compliant

cvs done.

nekovm-1.8.0-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/nekovm-1.8.0-2.fc10

I've created a new bug for multilib support: bug 476962

nekovm-1.8.0-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update nekovm'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11575

nekovm-1.8.0-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.