Bug 738079 - Review Request: quartz - Enterprise Job Scheduler for Java
Summary: Review Request: quartz - Enterprise Job Scheduler for Java
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Miroslav Suchý
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-09-13 20:31 UTC by Andy Grimm
Modified: 2016-11-08 03:45 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-12-16 21:19:05 UTC
Type: ---
Embargoed:
msuchy: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Andy Grimm 2011-09-13 20:31:15 UTC
Name        : quartz
Version     : 1.6.6
Group       : Development/Libraries
License     : ASL 2.0
URL         : http://www.quartz-scheduler.org/
Summary     : Enterprise Job Scheduler for Java
Description :
Quartz is a job scheduling system that can be integrated with, or used
along side virtually any J2EE or J2SE application. Quartz can be used
to create simple or complex schedules for executing tens, hundreds, or
even tens-of-thousands of jobs; jobs whose tasks are defined as standard
Java components or EJBs.


SPEC:
https://downloads.eucalyptus.com/software/devel/fedora-16/SPECS/quartz.spec

SRPM:
https://downloads.eucalyptus.com/software/devel/fedora-16/SRPMS/quartz-1.6.6-2.fc15.src.rpm

Comment 1 Miroslav Suchý 2011-10-04 08:47:53 UTC
It build successfully in koji in Fedora 17:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3401445
But on my local machine (F16) it will fail:
~/rpmbuild/BUILD/quartz-1.6.6/lib ~/rpmbuild/BUILD/quartz-1.6.6
++ build-classpath commons-beanutils
+ ln -sf /usr/share/java/commons-beanutils.jar .
++ build-classpath commons-collections
+ ln -sf /usr/share/java/commons-collections.jar .
++ build-classpath commons-dbcp
+ ln -sf /usr/share/java/commons-dbcp.jar .
++ build-classpath commons-digester
+ ln -sf /usr/share/java/commons-digester.jar .
++ build-classpath commons-logging
+ ln -sf /usr/share/java/commons-logging.jar .
++ build-classpath commons-modeler
+ ln -sf /usr/share/java/commons-modeler.jar .
++ build-classpath commons-pool
+ ln -sf /usr/share/java/commons-pool.jar .
++ build-classpath geronimo-ejb
/usr/bin/build-classpath: error: Could not find geronimo-ejb Java extension for this JVM
/usr/bin/build-classpath: error: Some specified jars were not found
+ ln -sf .
ln: `.' and `./.' are the same file
error: Bad exit status from /var/tmp/rpm-tmp.okuvY8 (%prep)


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


Should be geronimo-ejb specified as BuildRequires?

Comment 2 Miroslav Suchý 2011-10-04 08:52:45 UTC
rpmlint output:
quartz.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/quartz-1.6.6/dbTables/tables_derby_previous.sql
quartz-demo.noarch: W: no-documentation
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/bin/buildcp.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example7/example7.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example12/server.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example6/example6.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example8/example8.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example10/example10.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example14/example14.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example1/example1.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example13/instance2.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example3/example3.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example11/example11.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example2/example2.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example12/client.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example9/example9.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example5/example5.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example13/instance1.sh 0644L /bin/sh
quartz-demo.noarch: E: non-executable-script /usr/share/quartz/example4/example4.sh 0644L /bin/sh
quartz-manual.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/quartz-1.6.6/dbTables/tables_derby_previous.sql

Can you fix the end of line encodings?

Can you set executable bits for those examples?

Comment 3 Miroslav Suchý 2011-10-04 09:02:47 UTC
I really does not understood why you define macro oname and why not use directly %{name} which has same value?

Why you specify Epoch? It is not needed when it is zero.

Just asking about these two. I do not require change for these two.

Quartz 2.1.0 has been release on 23 September 2011. You may consider to rebase.

Beside those mentioned issue above it looks good.

Please fix #1 and #2, consider the rebase and it will be good.

Comment 4 Andy Grimm 2011-10-04 21:49:53 UTC
The oname macro and epoch of 0 were left over from the jpackage spec, where the package name was quartz16.  An undefined epoch is not the same as an epoch of zero last I checked, but you are right that it shouldn't be needed.

As for the upgrade to 2.1.0, that's going to require some investigation.  From a distribution standpoint, I understand that we want to package the latest stable versions of software.  I'll need to figure out how much of a challenge it will be to migrate Eucalyptus to use 2.1.0, though, as that's my primary motivation for doing this packaging work.  Since the Quartz website no longer even mentions 1.6.x, though, it's clear that we need to move to some newer version (1.8, 2.0, or 2.1).  I need to put this package on hold for a bit.  Thanks for pointing this out.

Comment 5 Miroslav Suchý 2011-10-05 07:44:44 UTC
Re epoch - true that 0 != ''. If it was present there in jpackage already then keep it there. Removing that would break upgrades for people who already use quartz from jpackage.

Re upgrade - there was big change in 1.8 - change in XML schema. If this does not affect/bother you, everything else should go smoothly.
I use quartz in Spacewalk (my motivation for review) and we use there 1.8 (from jpackage as well).

Comment 6 Andy Grimm 2011-10-05 12:51:11 UTC
I've grep'ed through our code, and it appears that we don't use quartz directly; it's is required by ha-jdbc and mule.  ha-jdbc is a dormant project at this point, and the latest version of mule still has 1.6.6 as the quartzVersion in its pom file; I can attempt to build these against newer versions and see what happens.

Comment 7 Miroslav Suchý 2011-11-22 20:01:02 UTC
ping? any progress here?

Comment 8 Andy Grimm 2011-11-30 02:22:11 UTC
Sorry, this has not been very high on my priority list.  I will try to get back to it next week.

Comment 9 Andy Grimm 2011-12-03 01:35:30 UTC
I have confirmed now that switching to quartz-2.1.1 does not break our build process.  I have not done extensive runtime testing, but I'm going to assume that we can adapt here.  I'll post a new package soon.

Comment 11 Andy Grimm 2011-12-15 01:05:08 UTC
Another note, I've changed this review from Fedora 16 to rawhide, since that's where most of my testing is now happening.

Comment 12 Miroslav Suchý 2011-12-15 16:37:40 UTC
It builds successfully:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3587138

I briefly check it, and it all seems ok, but one thing. You have in spec:
# svn export http://svn.terracotta.org/svn/quartz/tags/quartz-2.1.2
# tar caf quartz-2.1.2.tar.xz quartz-2.1.2
Source0:        %{name}-%{version}.tar.xz

But I find this page:
  http://terracotta.org/downloads/open-source/destination?name=quartz-2.1.1.tar.gz&bucket=tcdistributions&file=quartz-2.1.1.tar.gz
with this download link:
  http://d2zwv9pap9ylyd.cloudfront.net/quartz-2.1.1.tar.gz
If you will use this as Source0, we can even check if md5sum is the same as upstream.

http://fedoraproject.org/wiki/Packaging:SourceURL

Comment 13 Andy Grimm 2011-12-15 16:47:12 UTC
I can diff them, but notice that the latest tag in SVN is 2.1.2, while the latest tarball is 2.1.1.  This is why I used SVN instead.

Comment 14 Andy Grimm 2011-12-15 16:49:16 UTC
They are not the same:

$ diff -urN quartz-2.1.1 quartz-2.1.2 | diffstat
 examples/pom.xml                                                      |    2 
 pom.xml                                                               |    2 
 quartz-all/pom.xml                                                    |    2 
 quartz-backward-compat/pom.xml                                        |    2 
 quartz-commonj/pom.xml                                                |    2 
 quartz-jboss/pom.xml                                                  |    2 
 quartz-oracle/pom.xml                                                 |    2 
 quartz-weblogic/pom.xml                                               |    2 
 quartz/pom.xml                                                        |    2 
 quartz/src/main/java/org/quartz/CalendarIntervalScheduleBuilder.java  |    6 -
 quartz/src/main/java/org/quartz/CronScheduleBuilder.java              |   40 ++++++-
 quartz/src/main/java/org/quartz/DailyTimeIntervalScheduleBuilder.java |   39 ++++++
 quartz/src/main/java/org/quartz/JobBuilder.java                       |    7 +
 quartz/src/main/java/org/quartz/SimpleScheduleBuilder.java            |    2 
 quartz/src/main/java/org/quartz/TimeOfDay.java                        |   57 +++++++++-
 quartz/src/main/java/org/quartz/TriggerBuilder.java                   |    7 +
 quartz/src/main/java/org/quartz/core/QuartzSchedulerThread.java       |    3 
 quartz/src/main/java/org/quartz/impl/StdSchedulerFactory.java         |    2 
 quartz/src/main/java/org/quartz/package.html                          |   28 ++++
 quartz/src/test/java/org/quartz/CronScheduleBuilderTest.java          |   47 ++++++++
 20 files changed, 230 insertions(+), 26 deletions(-)


and thus I would prefer to use the latest version tagged in SVN.

Comment 15 Miroslav Suchý 2011-12-15 16:53:25 UTC
Aha, ok then.
I will finish this review tomorrow.

Comment 16 Miroslav Suchý 2011-12-16 13:36:48 UTC
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [x] Package meets the Packaging Guidelines  including the Java specific items
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     tested in: f16/koji
     http://koji.fedoraproject.org/koji/taskinfo?taskID=3587138
 [x] Rpmlint output:
quartz.src: W: invalid-url Source0: quartz-2.1.2.tar.xz
3 packages and 0 specfiles checked; 0 errors, 1 warnings.
Waiving per comment #13
 [x] Package is not relocatable.
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: ASL 2.0
 [x] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided
in the spec URL.
I get tar.gz according the comment in spec and compared it to tar.gz inside of src.rpm and did
diff -Naur src.rpm upstream
and there was no difference
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -fR $RPM_BUILD_ROOT.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [x] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [-] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 2.1.2
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: koji scratch build
 [x] Package should compile and build into binary rpms on all supported
architectures.
     Tested on:koji scratch build
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [-] %check is present and the tests pass


================
*** APPROVED ***
================

Comment 17 Andy Grimm 2011-12-16 16:40:01 UTC
New Package SCM Request
=======================
Package Name:      quartz
Short Description: Enterprise Job Scheduler for Java
Owners:            arg

Comment 18 Gwyn Ciesla 2011-12-16 17:04:56 UTC
Git done (by process-git-requests).

Comment 19 Miroslav Suchý 2011-12-19 08:40:34 UTC
Can you please build package for F15,16 and if possible for EPEL too? I would be interested in them.


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