Bug 434827

Summary: Review Request: jcommon - java jfreereport base utility library
Product: [Fedora] Fedora Reporter: Caolan McNamara <caolanm>
Component: Package ReviewAssignee: Jerry James <loganjerry>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: aph, dbhole, fedora-package-review, kevin, lkundrak, loganjerry, notting, overholt, viveklak
Target Milestone: ---Flags: loganjerry: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-05-03 10:14:33 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Patch to comply with Java guidelines none

Description Caolan McNamara 2008-02-25 18:11:08 UTC
Spec URL: http://people.redhat.com/caolanm/jfreereport/jcommon.spec
SRPM URL: http://people.redhat.com/caolanm/jfreereport/jcommon-1.0.12-1.fc9.src.rpm
Description: jcommon is a prerequisite of jfreereport which is itself a prerequisite of the useful OpenOffice.org reportdesigner

Comment 1 Caolan McNamara 2008-03-03 11:35:04 UTC
I see two suspicious @redhat.com CCs added to this. caolanm->dbhole/overholt: Is
there anything about this package that's of interest elsewhere in the java stack
as well as for the OOo stack ?

Comment 2 Andrew Overholt 2008-03-03 13:44:32 UTC
I just CC'd us because we were working on the Java Packaging Guidelines and this
was a new package that could be used to test the guidelines.

I'm not aware of where else jcommon is used.

Comment 3 Jerry James 2008-04-28 16:55:51 UTC
Does this supercede bug 252114?


Comment 4 Caolan McNamara 2008-04-28 19:31:55 UTC
caolanm->jerry: I have a interest in getting jfreereport and its dependencies
into fedora by F-10, but I'm not super interested in maintaining it :-) Though I
will if I have to.

I wasn't aware of bug 252114, but it doesn't look like its moving pretty fast
either. Feel free to mark one as a duplicate of the other. The above spec is pre
the java guideline finalization so I haven't checked if it needs to be resynced
to pass the new final guidelines.

Comment 5 Jerry James 2008-04-30 03:35:18 UTC
*** Bug 252114 has been marked as a duplicate of this bug. ***

Comment 6 Jerry James 2008-04-30 04:15:24 UTC
Created attachment 304171 [details]
Patch to comply with Java guidelines

I was hoping to get to a full review tonight, but ran out of time.  I'll try
again tomorrow night.  In the meantime, though, I believe you need this patch
to be compliant with the new guidelines.  The patch also contains a purported
debuginfo fix.	Without that, I get a debuginfo package with no sources.  With
it, I get a debuginfo package with sources under some weird paths.  Do you know
how to make it work correctly?

I'm happy to maintain or co-maintain this package, by the way.

Comment 7 Andrew Haley 2008-04-30 14:58:15 UTC
Jerry, what is that patch supposed to do?  As far as I can see there is
nothing wrong with the debuginfo.  All the source files are there.



zorro:SPECS $ rpm2cpio ../RPMS/x86_64/jcommon-debuginfo-1.0.12-1.fc9.x86_64.rpm
| cpio -it | head -30
./usr/lib/debug
./usr/lib/debug/.build-id
./usr/lib/debug/.build-id/7d
./usr/lib/debug/.build-id/7d/826fb964c9e141bcc5a776740154c2ec4b5fd0
./usr/lib/debug/.build-id/7d/826fb964c9e141bcc5a776740154c2ec4b5fd0.debug
./usr/lib/debug/usr
./usr/lib/debug/usr/lib64
./usr/lib/debug/usr/lib64/gcj
./usr/lib/debug/usr/lib64/gcj/jcommon
./usr/lib/debug/usr/lib64/gcj/jcommon/jcommon-1.0.12.jar.so.debug
./usr/src/debug/jcommon-1.0.12
./usr/src/debug/jcommon-1.0.12/source
./usr/src/debug/jcommon-1.0.12/source/com
./usr/src/debug/jcommon-1.0.12/source/com/keypoint
./usr/src/debug/jcommon-1.0.12/source/com/keypoint/PngEncoder.java
./usr/src/debug/jcommon-1.0.12/source/org
./usr/src/debug/jcommon-1.0.12/source/org/jfree
./usr/src/debug/jcommon-1.0.12/source/org/jfree/JCommon.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/JCommonInfo.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/AbstractBoot.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/BaseBoot.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/BasicProjectInfo.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/BootableProjectInfo.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/ClassPathDebugger.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/Library.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/config
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/config/HierarchicalConfiguration.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/config/ModifiableConfiguration.java
./usr/src/debug/jcommon-1.0.12/source/org/jfree/base/config/PropertyFileConfiguration.java
...

Comment 8 Jerry James 2008-04-30 16:17:39 UTC
I built on an F-8 i386 machine, and it looks like you built on an F-9 x86_64
machine.  I'll bet that F-8 vs. F-9 is the deciding factor here.  Just to check
my sanity, could you try building on an F-8 machine and see if you get a debug
package with no sources?

The rest of the patch makes the spec file conform to the current Java packaging
guidelines, at least by my reading.  I still plan to do a full review tonight.

Comment 9 Caolan McNamara 2008-04-30 16:21:32 UTC
Updated to guidelines:

http://people.redhat.com/caolanm/jfreereport/jcommon-1.0.12-2.fc9.src.rpm
http://people.redhat.com/caolanm/jfreereport/jcommon.spec

debuginfo works for me on F-9 (rawhide)

Comment 10 Andrew Haley 2008-04-30 16:33:51 UTC
The thing that matters is the version of java-1.5.0-gcj-devel.

I'm using java-1.5.0-gcj-devel-1.5.0.0-21.fc9.x86_64

I can't remember when the fix went in, but it was in a F8 update.

Comment 11 Jerry James 2008-04-30 17:30:08 UTC
Then the F8 update never got pushed.  I've got
java-1.5.0-gcj-devel-1.5.0.0-17.fc8 (the original F8 version) on my machines.  I
see no updates to this package on download.fedora.redhat.com for F8.  I run "yum
upgrade" every day.

Comment 12 Jerry James 2008-05-01 03:18:43 UTC
Here's my review.  First, I'd like to know why "ant compile-xml" wasn't run.  Do
we know that none of the intended users of jcommon need the XML part?

MUST:

- rpmlint output:
$ rpmlint jcommon
$ rpmlint jcommon-javadoc
jcommon-javadoc.i386: W: non-standard-group Development/Documentation
$ rpmlint jcommon-1.0.12-2.fc9.src.rpm
jcommon.src: W: mixed-use-of-spaces-and-tabs (spaces: line 16, tab: line 4)

- package naming guidelines: OK
- spec file name matches package name: OK
- packaging guidelines: see below
- licensing guidelines: OK
- license file in %doc: OK
- spec file in American English: OK
- spec file is legible: OK
- sources match upstream: OK
- binary RPM build on at least one arch: OK
- ExcludeArch used appropriately: OK
- All build dependencies in BuildRequires: OK for what is built.  If we also
build the XML jar, then a BuildRequires of "jaxp" is also necessary.  The
package containing jcommon-xml.jar (which I think should be a subpackage) also
needs to Require jaxp in that case.
- Handles locales properly: OK
- Calls ldconfig if necessary: OK
- Relocatable: OK
- Owns all directories it creates: OK
- No duplicate %files entries: OK
- Permissions on files: OK
- Clean section in spec file: OK
- Consistent use of macros: OK
- Code or permissible content: OK
- Large documentation: OK
- Documentation not needed to run: OK
- Header files in -devel: OK
- Static libraries in -static: OK
- Proper use of pkgconfig: OK
- .so files in -devel: OK
- -devel requires main package: OK
- No .la archives: OK
- Desktop file: OK
- Don't own directories owned by others: OK
- Delete buildroot before install: OK
- All filenames in UTF-8: OK

SHOULD:
- License file: OK
- Summary and description translations: OK
- Builds in mock: OK
- Compiles on all architectures: cannot check
- Package functions as described: OK
- Sane scriptlets: OK
- Subpackages require main package: NO, the javadoc subpackage does not do this
- Placement of pkgconfig files: OK
- File dependencies: OK

Packaging guidelines:
The Java and GCJ guidelines are followed, except that they want the javadoc
subpackage to Require both jpackage-utils and the main package.  This is not
listed as a MUST.

Summary: as long as the jcommon-xml.jar file is produced or a rationale for why
it should not be is offered, the only failing items are SHOULD items.  The
failing SHOULD items are:
- Mixed use of tabs and spaces in the spec file
- Javadoc subpackage does not Require jpackage-utils and the main package

Comment 13 Caolan McNamara 2008-05-01 10:38:45 UTC
No particular reason for not having the -xml stuff built, doesn't seem to hurt
to have it. So...

http://people.redhat.com/caolanm/jfreereport/jcommon.spec
http://people.redhat.com/caolanm/jfreereport/jcommon-1.0.12-3.fc9.src.rpm

Comment 14 Jerry James 2008-05-01 19:33:04 UTC
Great, thanks Caolan.  The new XML parts doesn't break any of the previously
reviewed items, so this is good.

APPROVED


Comment 15 Jerry James 2008-05-01 19:50:07 UTC
Oops, forgot to assign the bug to me.

Comment 16 Caolan McNamara 2008-05-02 08:15:21 UTC
Thanks for rescuing this from review ghetto :-)

Comment 17 Kevin Fenzi 2008-05-02 15:47:04 UTC
Caolan: Can you add a cvs template here with what branches you want, etc?

See: http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

Thanks!

Comment 18 Caolan McNamara 2008-05-02 16:12:30 UTC
ug, forgot final step

New Package CVS Request
=======================
Package Name: jcommon
Short Description: JFree Java utility classes
Owners: caolanm
Branches: devel
InitialCC: 
Cvsextras Commits: yes

Comment 19 Kevin Fenzi 2008-05-02 16:18:19 UTC
cvs done.

Comment 20 Caolan McNamara 2008-05-03 10:14:33 UTC
imported, can't actually build it yet. But I assume it'll all come together
after devel auto moves to F-10

Comment 21 Lubomir Rintel 2008-07-24 00:06:04 UTC
Package change Request
=======================
Package Name: jcommon
New branches: EL-5 F-9
New branch owners: lkundrak

Contacted Fedora maintainer two days ago, and he did't respond (though he seems
to read other mails). One of my packages is stuck on dependency on this. I'll
still be glad and thankful if caolan maintained this in the requested branches
if he wants to (if he responds, and agrees to, just ignore the relevant line of
the above request).

Comment 22 Kevin Fenzi 2008-07-25 16:40:42 UTC
cvs done.