Bug 439941

Excellent, I was about to submit this but you beat me to it.  Here's the first
pass of the review.

Many of the following have rationales at
http://fedoraproject.org/wiki/PackagingDrafts/Java, and my locale package at
http://cachalot.mine.nu/8/SRPMS/javasqlite-20080315-1.fc8.src.rpm should have
fixes for most of them.

- Group: Libraries: please change to Development/Libraries or System/Libraries

- BuildRequires: sqlite-devel missing

- Javadocs not built nor packaged

- Requires JNI lib -> jar can't be installed in /usr/share/java, install both to
%{_libdir}/%{name} instead (see eg. jnipath patch in my package)

- Documentation missing, %doc ChangeLog license.terms

- Test suite not run during build

- Should probably use specific -source argument to javac (eg. -source 5.0)
(mostly for gcj)

- Dependency on jre (versioned) missing

- I think the versioned jar filenames are pretty much pointless (see recent
discussion on fedora-devel and fedora-packaging)

- GCJ stuff not needed?

- $RPM_OPT_FLAGS not honored (see my cflags patch)

Your spec looks a lot more complete; let's just replace this review with yours?

(Assuming we do that)

* Would be good to at least have a comment about the status of upstreaming the
relevant patches; ideally links to a bug tracker but it doesn't look like
upstream has one =/

That's the only issue I saw going through your spec in a first pass.

(In reply to comment #2)
> Your spec looks a lot more complete; let's just replace this review with yours?

Works for me.  Would you still like to maintain it or do we switch roles and you
become the reviewer?  Either way is fine with me, you make the call.

(In reply to comment #3)
> * Would be good to at least have a comment about the status of upstreaming the
> relevant patches; ideally links to a bug tracker but it doesn't look like
> upstream has one =/

Yes, I already have a history of submitting patches to javasqlite upstream, and
will send the cflags and libtool patches there right now (I thought I already
had, but seems I remembered wrong).  The jnipath patch is Fedora specific, no
need to submit it.

I get this error trying to build in mock:

config.status: creating Makefile
+ make -j2 'JAVAC_FLAGS=-source 5.0' LIBTOOL=/usr/bin/libtool
/usr/bin/libtool --mode=link gcc  -I/usr/include \
            -DHAVE_SQLITE2=0 -DHAVE_SQLITE3=1 \
            -o native/mkconst native/mkconst.c -lsqlite3
make: *** 
No rule to make target `SQLite/DBDump.class', needed by `sqlite.jar'
.  Stop.
make: 
*** Waiting for unfinished jobs....

For ownership, if you have contacts with upstream it'd probably be better for
you to be the primary maintainer, though I do tend to like a
collaborative/multi-maintainer style approach.

I'll take the review.  

Created attachment 299981 [details]
fix parallel make

This patch makes the Makefile.in work in parallel.

Also, I needed to change the "-source 5.0" to "-source 5"; this is with
java version "1.6.0"
IcedTea Runtime Environment (build 1.6.0-b06)
IcedTea 64-Bit Server VM (build 1.6.0-b06, mixed mode)

Thanks for those fixes.

http://scop.fedorapeople.org/packages/javasqlite.spec
http://scop.fedorapeople.org/packages/javasqlite-20080401-1.fc9.src.rpm

* Wed Apr  2 2008 Ville Skyttä <ville.skytta at iki.fi> - 20080401-1
- Update to 20080401.
- Patch to install *.so as an unversioned module.
- Patch to fix parallel make (#439941, Colin Walters).
- Build with "-source 5" instead of "-source 5.0" (#439941, Colin Walters).
- Use %%{_jvmdir} instead of %%{_prefix}/lib/jvm.

Builds fine in mock now, spec looks good according to the guidelines.  Marking
as reviewed.

I'd still like a comment above the patches stating their current upstream status
(and comment the non-upstreamable loadLibrary patch specifically), but that's
not a blocker.

Thanks.  Will add patch statuses before the first build.

Here's the request for branches I'm interested in.  Please feel free to request
co-maintainership for branches you're interested in in pkgdb once this is in (I
don't know your FAS username and the search doesn't seem to work so I can't do
it right now), or request additional ones in case you're interested in some that
are not listed here.

New Package CVS Request
=======================
Package Name: javasqlite
Short Description: SQLite Java Wrapper/JDBC Driver
Owners: scop
Branches: F-8 EL-5
InitialCC: 
Cvsextras Commits: yes

cvs done.

Rawhide build done, F-8 in progress.
http://koji.fedoraproject.org/koji/buildinfo?buildID=44865

Will look into building for EPEL-5 next week.

javasqlite-20080401-2.fc8 has been submitted as an update for Fedora 8

javasqlite-20080401-2.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.