Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 472791 - Review Request: fontbox - A Java library for parsing font files
Summary: Review Request: fontbox - A Java library for parsing font files
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jerry James
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2008-11-24 17:14 UTC by Mary Ellen Foster
Modified: 2009-05-21 21:03 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-05-21 21:03:19 UTC


Attachments (Terms of Use)

Description Mary Ellen Foster 2008-11-24 17:14:42 UTC
Spec URL: http://mef.fedorapeople.org/packages/java-libraries/fontbox.spec
SRPM URL: http://mef.fedorapeople.org/packages/java-libraries/fontbox-0.1.0-2.src.rpm

Description:
Fontbox is an open source Java library for parsing font files and providing
low level data structures for accessing font information.

Upstream seems pretty much dormant, but this is an indirect dependency of some other things I want to package ...

Comment 1 Mary Ellen Foster 2008-12-09 16:36:15 UTC
Updated to use the latest greatest BuildRoot and to include dist in the version:

http://mef.fedorapeople.org/packages/java-libraries/fontbox.spec
http://mef.fedorapeople.org/packages/java-libraries/fontbox-0.1.0-3.fc10.src.rpm

Comment 2 Jerry James 2008-12-17 17:16:29 UTC
I'll take this one.  Stand by for a full review.

Comment 3 Jerry James 2008-12-17 18:27:20 UTC
Here are a few things I noticed on an initial read-through of the spec file.  First, the Java packaging guidelines state that Java packages must "BuildRequires: java-devel" and "Requires: jpackage-utils" (see https://fedoraproject.org/wiki/Packaging/Java#BuildRequires_and_Requires), both of which are missing.

Second, I don't understand why you are using a half-maven half-ant build.  Shouldn't you go with one or the other?

Third, the GCJ guidelines have not been followed (see https://fedoraproject.org/wiki/Packaging/GCJGuidelines).  Is there some reason for this?

Fourth, the use of dos2unix is unnecessary.  You can accomplish the same result as follows:

sed -i -e 's/\r//g' <list of files>

Since sed is already in the default set of packages, this does not lead to any BuildRequires.  There is no point in fixing the line endings of javadoc files since you are regenerating those files in the %build section anyway.  Also, the list of files is too inclusive: dos2unix is being invoked on .pdf, .jpg, .png, .gif, and Thumbs.db files.  Those are binary formats, so dos2unix is very probably corrupting them.  Also, since the PDF files are just simple transformations of the HTML files, I doubt they add any value.  They're extremely short and not linked to one another, so I don't see the point in including them.

Fifth, changing the encodings of the XML files in docs/skin is not sufficient, since they use XML encoding declarations at the top:

<?xml version="1.0" encoding="ISO-8859-1"?>

which means that we are now messing up any XML processors because they are really getting UTF-8 encoded files.  This won't matter for the English file, because it uses only ASCII, which is a subset of both UTF-8 and ISO-8859-1.  However, the German, Spanish, and French versions all use non-ASCII characters, so it will matter for them.  If you really need to change the encoding, I recommend making a patch that both changes the encoding and changes the XML encoding declaration.

MUST items:
- Output of rpmlint:
2 packages and 1 specfile checked; 0 errors, 0 warnings.
- Package name: OK
- Spec file name: OK
X Packaging guidelines: see the list of items above
- Licensing guidelines: OK
- License field matches: OK
- Text license file in %doc: OK
- Spec file in American English: OK
- Spec file is legible: OK
- Sources match upstream: OK (checked with md5sum)
- Compiles into binary RPMs on at least one platform: OK (checked on x86_64)
- Use of ExcludeArch: OK (I did not check other arches, but this is noarch)
X All build dependencies in BuildRequires: need to add java-devel; see above
- Proper locale handling: OK
- ldconfig: OK
- Relocatable package: OK
- Own all created directories: OK
- No duplicate entries in %files: OK
- Proper file permissions: OK
- %clean section: OK
- Consistent use of macros: OK
- Code or permissible content: OK
- Large documentation: OK (total documentation size is 424K)
- Nothing in %doc affects runtime: OK
- Header files in -devel: OK
- Static libraries in -static: OK
- Pkgconfig files: OK
- .so files in -devel: OK
- -devel package requires main package: OK
- No libtool archives: OK
- Desktop file: OK
- Don't own files/directories owned by other packages: OK
- Clean buildroot in %install: OK
- Filenames are UTF-8: OK

SHOULD items:
- Query upstream for missing license file: OK
- Description and summary translations: OK
- Package builds in mock: OK (checked for F-10 x86_64 only)
- Builds on all supported architectures: did not check
- Package functions as described: did not check
- Sane scriptlets: see comments about maven & ant above
X Subpackages require the base package: NO, the javadoc package does not require the main package
- Placement of pkgconfig files: OK
- File dependencies: OK

Comment 4 Jerry James 2009-02-23 15:42:54 UTC
It's been over 2 months.  Are you planning to work on this package any more?

Comment 5 Jerry James 2009-02-26 14:51:19 UTC
Let me know when you are ready to continue this review.

Comment 6 Jerry James 2009-05-13 16:49:13 UTC
This review is stalled.  Please respond within one week.

Comment 7 Jerry James 2009-05-21 21:03:19 UTC
The submitter has not responded.  I am closing this bug.


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