Bug 193897 - Review Request: mysql-connector-java - Official JDBC driver for MySQL
Summary: Review Request: mysql-connector-java - Official JDBC driver for MySQL
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Anthony Green
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 193894
Blocks: FE-ACCEPT 193898
TreeView+ depends on / blocked
 
Reported: 2006-06-02 19:18 UTC by Igor Foox
Modified: 2010-04-15 23:47 UTC (History)
4 users (show)

Fixed In Version: mysql-connector-java-5.1.12-2.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-08 22:02:23 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)
Patch to .spec to allow building for EPEL5 as well. (2.61 KB, patch)
2010-03-07 10:36 UTC, Steve Traylen
no flags Details | Diff
Patch to .spec to allow building for EPEL5 as well. (2.61 KB, application/octet-stream)
2010-03-07 10:38 UTC, Steve Traylen
no flags Details
Allow older EPEL5 ant support (800 bytes, application/octet-stream)
2010-03-07 10:39 UTC, Steve Traylen
no flags Details

Description Igor Foox 2006-06-02 19:18:04 UTC
Spec URL: http://people.redhat.com/ifoox/extras/mysql-connector-java.spec
SRPM URL: http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_1fc.src.rpm
Description:
MySQL Connector/J is a native Java driver that converts JDBC (Java Database
Connectivity) calls into the network protocol used by the MySQL database.
It lets developers working with the Java programming language easily build
programs and applets that interact with MySQL and connect all corporate
data, even in a heterogeneous environment. MySQL Connector/J is a Type
IV JDBC driver and has a complete JDBC feature set that supports the
capabilities of MySQL.

I am submitting this package with several other packages (5 in total), and these are my first packages for Extras, so they may need a sponsor.

Comment 1 Igor Foox 2006-06-02 19:21:52 UTC
Adding dependency on the ant-contrib package review bug.

Comment 2 Anthony Green 2006-07-14 03:32:17 UTC
Some initial feedback....

rpmlint output is:
W: mysql-connector-java non-standard-group Development/Libraries/Java

We just use Development/Libraries in Fedora.  Perhaps there's an argument to be
made for Development/Libraries/Java, but let's use the standard for now.  I'll
send a note to fedora-devel 

W: mysql-connector-java wrong-file-end-of-line-encoding
/usr/share/doc/mysql-connector-java-3.1.12/docs/README.txt
W: mysql-connector-java wrong-file-end-of-line-encoding
/usr/share/doc/mysql-connector-java-3.1.12/README.txt
W: mysql-connector-java wrong-file-end-of-line-encoding
/usr/share/doc/mysql-connector-java-3.1.12/EXCEPTIONS-CONNECTOR-J

Fix these with sed in the %prep section like so: 
%{__sed} -i 's/\r//' README.txt

The spec file includes:
# remove all binary libs
find . \( -name "*.jar" -o -name "*.class" \) | xargs -t rm -f

I would rather that we strip the .jar files from the tarball prior to packaging.
 This will ensure that we don't accidentally ship binaries sans sources or
binaries with unfriendly licensing - even if they only show up in the SRPM.


Can you explain this part of the spec file?..
Provides: mm.mysql
Obsoletes: mm.mysql




Comment 3 Igor Foox 2006-07-20 19:16:14 UTC
Hi Anthony,

Thanks for the feedback, here are the new files:
http://people.redhat.com/ifoox/extras/mysql-connector-java.spec
http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_2fc.src.rpm

(In reply to comment #2)
> rpmlint output is:
> W: mysql-connector-java non-standard-group Development/Libraries/Java
> 
> We just use Development/Libraries in Fedora.  Perhaps there's an argument to be
> made for Development/Libraries/Java, but let's use the standard for now.  I'll
> send a note to fedora-devel 

I've changed it to Development/Libraries.

> 
> W: mysql-connector-java wrong-file-end-of-line-encoding
> /usr/share/doc/mysql-connector-java-3.1.12/docs/README.txt
> W: mysql-connector-java wrong-file-end-of-line-encoding
> /usr/share/doc/mysql-connector-java-3.1.12/README.txt
> W: mysql-connector-java wrong-file-end-of-line-encoding
> /usr/share/doc/mysql-connector-java-3.1.12/EXCEPTIONS-CONNECTOR-J
> 
> Fix these with sed in the %prep section like so: 
> %{__sed} -i 's/\r//' README.txt

Done. 

> The spec file includes:
> # remove all binary libs
> find . \( -name "*.jar" -o -name "*.class" \) | xargs -t rm -f
> 
> I would rather that we strip the .jar files from the tarball prior to packaging.
>  This will ensure that we don't accidentally ship binaries sans sources or
> binaries with unfriendly licensing - even if they only show up in the SRPM.

I feel kind of uncomfortable with doing this before pacakging, because it will
defeat the point of the pristine upstream source. I don't see how doing this
before the packaging process is better than doing it at %prep time, I'd say
it's less reproducable.


> Can you explain this part of the spec file?..
> Provides: mm.mysql
> Obsoletes: mm.mysql

Hmm, no I don't know why these are there. This package is taken from JPackage,
I'll try contacting the packager and ask if there is a reason for these to remain.

Comment 4 Igor Foox 2006-07-24 17:42:49 UTC
It seems that the java connector for mysql used to be called MM.MySQL, and so
old packages might depend on this name. It is now officially called Connector/J,
and so the name mysql-connector-java better reflects this, but I think we need
to keep the provides for mm.mysql in there.

Comment 5 Anthony Green 2006-07-25 18:12:49 UTC
(In reply to comment #3)
> > I would rather that we strip the .jar files from the tarball prior to packaging.
> >  This will ensure that we don't accidentally ship binaries sans sources or
> > binaries with unfriendly licensing - even if they only show up in the SRPM.
> 
> I feel kind of uncomfortable with doing this before pacakging, because it will
> defeat the point of the pristine upstream source. I don't see how doing this
> before the packaging process is better than doing it at %prep time, I'd say
> it's less reproducable.

I looked into the source tar ball and it contains .jar files distributed under
the terms of the LGPL.  Deleting the jar files in the %prep section doesn't make
the fact that we've distributed these things go away.  The  .jar files don't
come with source code and AFAICT there's no mention of where to get the source
code.  We really must delete these .jar files before they get packaged into the
SRPM.


Comment 6 Anthony Green 2006-07-25 18:57:34 UTC
Hi Igor.  I went through the review list.  Please see the lines with "X".

MUST Items:
 - rpmlint output is clean
 - package name is fine for now (although debate continue on "jpp" usage)
 - The spec file name matches the base package %{name}, in the format %{name}.spec
 - The package meets the Packaging Guidelines.
 - The package license is OK: (I think) GPL + headache inducing exception text.
X- The License field says GPL, which I think it fair since I think
   you're allowed to ignore the exception.  However, the license text
   (COPYING) has the old FSF address. Please update the address and
   file a bug upstream.
 - The license text is in %doc
 - The spec file is written in American English.
X- The spec file for the package is legible, although it would be nice
   to line up field names and values in columns, rather than have them
   ragged.
X- The guidelines say "The sources used to build the package must
   match the upstream source, as provided in the spec URL. Reviewers
   should use md5sum for this task." however we need to remove jar
   binaries before packaging to comply with LGPL.
X- Fails to build in in mock.
   Cannot find build req  jta >= 0:1.0.1-0.a.1. Exiting.
   $ rpm -q --whatprovides jta
   geronimo-specs-compat-1.0-0.M2.2jpp_7fc
   I don't know what the right solution here is.  Replace jta with
geronimo-specs-compat?
X- BuildRequires need adjusting. (see mock build failure, above)
 - Package is not relocatable.
 - Package owns all directories in creates.
 - No duplicate files in %files.
 - File permissions look good.
 - %clean section is good.
X- macro use is fine apart from %{__sed}.  Please use "sed".
 - package contains code (no "content")
X- %doc section seems to contain equivalent README/README.txt files in
   top level and docs dir.  Please clean this up.  Also, what is with the
  "@MYSQL_CJ_VERSION@" in the README files?
 - %doc contents don't affect runtime
 - package does not own files/dirs owned by other packages.


Comment 7 Paul Howarth 2006-07-26 13:36:24 UTC
(In reply to comment #6)
> X- macro use is fine apart from %{__sed}.  Please use "sed".

What's wrong with using %{__sed}?

I note that it was you (in Comment #2) that suggested the use of %{__sed} in the
first place...


Comment 8 Anthony Green 2006-07-26 14:26:33 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > X- macro use is fine apart from %{__sed}.  Please use "sed".
> 
> What's wrong with using %{__sed}?

I've been told not to use macros like %{__make}, %{__rm}, etc in reviews people
have done for me.  However, you're right, I don't see any ruling on this in the
FE package guidelines.  And, indeed, the %{__sed} magic I originally proposed
was directly from the FE packaging guidelines.  
Ok, ignore this comment.  %{__sed} is fine I guess.  
Thanks.



Comment 9 Igor Foox 2006-09-07 01:19:44 UTC
New Files:
http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_3fc.src.rpm
http://people.redhat.com/ifoox/extras/mysql-connector-java.spec

Thanks for the extensive review Anthony, and sorry for taking so long to finish
this up. 
I've removed all of the binary jars from the tarball, most were not even used
and the ones that were are present after all BRs are satisfied. I've also fixed
the README duplication.

I'm trying to resolve the jta issue right now. It seems that yum install 'jta >=
1.0' gives me the correct geronim-specs-compat package but mock is giving me
problems. I'm caching mock's rpms at the moment so I can do mock builds faster,
so I hope to resolve the issue by tomorrow at most. If I can't make it work then
making it depend on geronimo-specs-compat seems like a valid option.

Comment 10 Igor Foox 2006-09-07 01:35:40 UTC
It seems that after removing the '0:' epoch from the BRs, jta gets resolved
normally. Unfortunately mock gives me this:
[build@treason srpms]$  /usr/sbin/mock-helper yum --installroot
/var/lib/mock/fedora-development-i386-core/root resolvedep  'ant-contrib'
No Package Found for ant-contrib

But that may happen because ant-contrib only got built yesterday, so hopefully
that will get resolved as mirrors get updated. When I do yum install ant-contrib
on my machine, it works fine. Can anyone else give this a try? If this works, I
think that's the last item that needed to get resolved.

Comment 11 Igor Foox 2006-09-07 03:01:19 UTC
The package now builds for me in mock, I guess the problem with ant-contrib was
some sort of mirror inconsistency.

Comment 12 Anthony Green 2006-09-07 12:30:08 UTC
(In reply to comment #11)
> The package now builds for me in mock, I guess the problem with ant-contrib was
> some sort of mirror inconsistency.

Awesome.  This builds in mock and is almost perfect.

I think there are just two minor changes.

1. I gave you bad advice on using "Development/Libraries".  This should be
"System Environment/Libraries".  Sorry about that!

2. In the spec file you wrote:
# ************ NOTE ************
# The following files have been removed from the tarball since they are
# distributed under the LGPL and the upstream provider does not distribute
# any sources with them.
Not all of these files are LGPL.  I forget which one(s) it was.  Just say
something like...
# The following files have been removed from the tarball to avoid licensing 
# issues.

Did you find out about the
Provides: mm.mysql
Obsoletes: mm.mysql
?



Comment 13 Igor Foox 2006-09-07 15:56:53 UTC
New files:
http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_4fc.i386.rpm

(In reply to comment #12)
> (In reply to comment #11)
> > The package now builds for me in mock, I guess the problem with ant-contrib was
> > some sort of mirror inconsistency.
> 
> Awesome.  This builds in mock and is almost perfect.
> 
> I think there are just two minor changes.
> 
> 1. I gave you bad advice on using "Development/Libraries".  This should be
> "System Environment/Libraries".  Sorry about that!

Fixed

> 2. In the spec file you wrote:
> # ************ NOTE ************
> # The following files have been removed from the tarball since they are
> # distributed under the LGPL and the upstream provider does not distribute
> # any sources with them.
> Not all of these files are LGPL.  I forget which one(s) it was.  Just say
> something like...
> # The following files have been removed from the tarball to avoid licensing 
> # issues.
Fixed. Changed to:
# The following files have been removed from the tarball to avoid licensing
# issues due to the fact that they are not distributed with their source.
 
> Did you find out about the
> Provides: mm.mysql
> Obsoletes: mm.mysql
> ?

See comment #4 (https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193897#c4)
above.

I also formatted the preamble section so it doesn't look like a puzzle anymore :-).


Comment 14 Anthony Green 2006-09-07 16:30:53 UTC
(In reply to comment #13)
> New files:
>
http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_4fc.i386.rpm

Where is the SRPM?



Comment 15 Igor Foox 2006-09-07 17:48:34 UTC
reply to comment #14)
> Where is the SRPM?

Oops, wrong file uploaded.
http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_4fc.src.rpm


Comment 16 Anthony Green 2006-09-08 18:26:52 UTC
One final thing and this is APPROVED.  See the line starting in 'X'.

Also, why are you packaging version 3.1.12 when the most recent version is
5something.  We normally require that the most recent version be packaged.


* package meets and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
X dist tag is not present.  Add %{?dist} to Release tag.
* build root is correct. 
* license field matches the actual license.
* license is open source-compatible.
* License text included in package.
* source files match upstream (although they are modified to remove binary jars.
 upstream makes no source only release).
? latest version is not being packaged. Upstream is at version 5.  Why aren't we
packaging version 5?
* BuildRequires are proper.
* package builds in mock.
* rpmlint is silent.
* final provides and requires are sane
mysql-connector-java-3.1.12-1jpp_4fc.i386.rpm
=========
  mm.mysql
  mysql-connector-java-3.1.12.jar.so
  mysql-connector-java = 1:3.1.12-1jpp_4fc
  =
  java-gcj-compat >= 1.0.31
  jta >= 1.0
  log4j
* shared libraries are present, but no ldconfig required.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* %check is not present
* scriptlets OK
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app (no .desktop file required).
* not a web app.



Comment 17 Igor Foox 2006-09-08 20:30:17 UTC
http://people.redhat.com/ifoox/extras/mysql-connector-java-3.1.12-1jpp_5fc.src.rpm
http://people.redhat.com/ifoox/extras/mysql-connector-java.spec

(In reply to comment #16)
> One final thing and this is APPROVED.  See the line starting in 'X'.

Fixed

> Also, why are you packaging version 3.1.12 when the most recent version is
> 5something.  We normally require that the most recent version be packaged.

As far as I understand 5something is the -devel version, and so I'm not sure
that's the best thing to package. But I'll investigate it later and update if I
think it makes sense. I think having this package in for the time being so that
other packages can use it is good.

Comment 18 Anthony Green 2006-09-08 20:52:20 UTC
APPROVED

Thanks!


Comment 19 Igor Foox 2006-09-08 22:02:23 UTC
Built in -devel, pending FC-5 branch. Closing.

Thanks for the review!

Comment 20 Orion Poplawski 2009-12-11 22:18:42 UTC
Any chance of getting this into EPEL?  I'd be willing to maintain.

With the following patch and ant-contrib built for EPEL (I've requested one) it builds find on EL-5.

diff -u -r1.11 mysql-connector-java.spec
--- mysql-connector-java.spec   4 Dec 2009 16:44:20 -0000       1.11
+++ mysql-connector-java.spec   11 Dec 2009 22:15:26 -0000
@@ -57,7 +57,7 @@
 BuildRequires:         junit
 BuildRequires:         log4j
 BuildRequires:         java-1.6.0-openjdk-devel
-BuildRequires:         java-1.5.0-gcj-devel
+BuildRequires:         java-devel-gcj
 BuildRequires:         jakarta-commons-logging

 Requires:               jpackage-utils
@@ -93,7 +93,11 @@

 # We need both JDK1.5 (for JDBC3.0; appointed by $JAVA_HOME) and JDK1.6 (for JDBC4.0; appointed in the build.xml)
 export CLASSPATH=$(build-classpath ant-contrib jdbc-stdext jta junit log4j commons-logging.jar)
+%if 0%{?rhel} == 5
+export JAVA_HOME=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0
+%else
 export JAVA_HOME=/usr/lib/jvm/java-1.5.0-gcj
+%endif

 # We currently need to disable jboss integration because of missing jboss-common-jdbc-wrapper.jar (built from sources).
 # See BZ#480154 and BZ#471915 for details.

Comment 21 Steve Traylen 2010-03-07 10:36:32 UTC
Created attachment 398308 [details]
Patch to .spec to allow building for EPEL5 as well.

Comment 22 Steve Traylen 2010-03-07 10:38:51 UTC
Created attachment 398309 [details]
Patch to .spec to allow building for EPEL5 as well.

Attached is a new patch to the spec file allow building for EPEL5 as well.

Obviously I would also like to to see this in EPEL if necessary will request
a branch in a week or so.

Comment 23 Steve Traylen 2010-03-07 10:39:52 UTC
Created attachment 398310 [details]
Allow older EPEL5 ant support

This patch referenced in the previous spec allows the build.xml to
be used with older ants as contained in EPEL5.

Comment 24 Steve Traylen 2010-03-21 12:16:17 UTC
Package Change Request
======================
Package Name: mysql-connector-java
New Branches: EL-5
Owners: stevetraylen


Hi, the last 3 comments in this bug are all  requests for this 
to be maintained in EPEL.

Steve

Comment 25 Milos Jakubicek 2010-03-21 22:02:50 UTC
Ops, Steve, sorry! I must have missed that:( Please add me as comaintainer to the EPEL branches then, I'll try keep things in sync.

best
Milos

Comment 26 Steve Traylen 2010-03-21 22:15:50 UTC
Package Change Request
======================
Package Name: mysql-connector-java
New Branches: EL-5
Owners: stevetraylen mjakubicek

update to previous request, thanks for offering to co-maintain.

 Steve

Comment 27 Kevin Fenzi 2010-03-24 03:15:29 UTC
cvs done.

Comment 28 Fedora Update System 2010-03-24 21:46:59 UTC
mysql-connector-java-5.1.12-2.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/mysql-connector-java-5.1.12-2.el5

Comment 29 Fedora Update System 2010-04-15 23:47:27 UTC
mysql-connector-java-5.1.12-2.el5 has been pushed to the Fedora EPEL 5 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.