Bug 472791

Summary: Review Request: fontbox - A Java library for parsing font files
Product: [Fedora] Fedora Reporter: Mary Ellen Foster <mefoster>
Component: Package ReviewAssignee: Jerry James <loganjerry>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, loganjerry, notting
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-05-21 21:03:19 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:
Bug Depends On:    
Bug Blocks: 201449    

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.