Bug 439941 - Review Request: javasqlite
Review Request: javasqlite
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Colin Walters
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-31 21:53 EDT by Colin Walters
Modified: 2008-04-09 01:22 EDT (History)
3 users (show)

See Also:
Fixed In Version: 20080401-2.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-04-09 01:22:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
walters: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
fix parallel make (417 bytes, patch)
2008-04-01 18:28 EDT, Colin Walters
no flags Details | Diff

  None (edit)
Description Colin Walters 2008-03-31 21:53:17 EDT
Spec URL: http://cdn.verbum.org/javasqlite.spec
SRPM URL: http://cdn.verbum.org/javasqlite-20080315-1.fc9.src.rpm

%description
A Java wrapper including a basic JDBC driver for the 
SQLite 2/3 database engine. It is designed using JNI 
to interface to the SQLite API. That API is wrapped by 
methods in the SQLite.Database class. Since June, 2002, 
it includes a small JDBC driver, which allows the most 
basic things to be carried out using the java.sql package.
Since September, 2004, SQLite3 (minimum 3.0.7) is supported
which is rather experimental.
Comment 1 Ville Skyttä 2008-04-01 11:19:45 EDT
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)
Comment 2 Colin Walters 2008-04-01 11:36:08 EDT
Your spec looks a lot more complete; let's just replace this review with yours?
Comment 3 Colin Walters 2008-04-01 11:44:08 EDT
(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.
Comment 4 Ville Skyttä 2008-04-01 11:54:09 EDT
(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.
Comment 5 Colin Walters 2008-04-01 17:22:34 EDT
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....
Comment 6 Colin Walters 2008-04-01 17:24:12 EDT
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.  
Comment 7 Colin Walters 2008-04-01 18:28:25 EDT
Created attachment 299981 [details]
fix parallel make

This patch makes the Makefile.in work in parallel.
Comment 8 Colin Walters 2008-04-01 18:29:18 EDT
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)
Comment 9 Ville Skyttä 2008-04-02 13:55:43 EDT
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.
Comment 10 Colin Walters 2008-04-02 15:20:39 EDT
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.

Comment 11 Ville Skyttä 2008-04-02 16:12:21 EDT
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
Comment 12 Kevin Fenzi 2008-04-03 16:25:16 EDT
cvs done.
Comment 13 Ville Skyttä 2008-04-03 17:16:58 EDT
Rawhide build done, F-8 in progress.
http://koji.fedoraproject.org/koji/buildinfo?buildID=44865

Will look into building for EPEL-5 next week.
Comment 14 Fedora Update System 2008-04-04 17:29:37 EDT
javasqlite-20080401-2.fc8 has been submitted as an update for Fedora 8
Comment 15 Fedora Update System 2008-04-09 01:22:00 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.