Bug 475603 - Review Request: jFormatString - Java format string compile-time checker
Review Request: jFormatString - Java format string compile-time checker
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Andrew Overholt
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 1128852 464014
  Show dependency treegraph
 
Reported: 2008-12-09 13:47 EST by Jerry James
Modified: 2015-01-01 17:47 EST (History)
4 users (show)

See Also:
Fixed In Version: 0-0.2.20081016svn.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-09 18:50:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
overholt: fedora‑review+
opensource: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jerry James 2008-12-09 13:47:51 EST
Spec URL: http://jjames.fedorapeople.org/jFormatString/jFormatString.spec
SRPM URL: http://jjames.fedorapeople.org/jFormatString/jFormatString-0-0.1.20081016svn.src.rpm
Description: This project is derived from Sun's implementation of java.util.Formatter.  It is designed to allow compile time checks as to whether or not a use of a format string will be erroneous when executed at runtime.

This package is required by findbugs.
Comment 1 Andrew Overholt 2009-03-04 08:44:29 EST
I'll take this one.  I'm still working on a full review, but here are some initial questions:

rpmlint complains about a few things with the SRPM:

$ rpmlint jFormatString-0-0.1.20081016svn.src.rpm 
jFormatString.src:104: W: libdir-macro-in-noarch-package %{_libdir}/gcj/%{name}
jFormatString.src: W: non-standard-group Development/Libraries/Java
jFormatString.src: W: non-coherent-filename jFormatString-0-0.1.20081016svn.src.rpm jFormatString-0-0.1.20081016svn.fc10.src.rpm
jFormatString.src: W: strange-permission jFormatString-0.tar.bz2 0745
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

and a few things with the resulting RPMs:

$ rpmlint ../RPMS/x86_64/jFormatString-*
jFormatString.x86_64: W: non-standard-group Development/Libraries/Java
jFormatString-javadoc.x86_64: W: non-standard-group Development/Documentation
3 packages and 0 specfiles checked; 0 errors, 2 warnings.

I suggest just picking Groups to make rpmlint quiet.

The license field is correct but I would like spot to weigh in on their statement:

"The library produced by compiling this project is used by the FindBugs project. To avoid any licensing questions due to incompatible licenses (FindBugs is licensed under the LGPL), it is broken out as a separate project. While there may be some confusion/discussion about the licenses, the FindBugs project does not interprete the FindBugs LGPL license to be any stronger than GPL v2 + the Classpath exception."

spot, what do you think?
Comment 2 Andrew Overholt 2009-03-04 08:45:54 EST
I forgot to CC spot.  spot, what do you think about my licensing question in comment #1?
Comment 3 Andrew Overholt 2009-03-04 09:08:01 EST
Okay, full review below.  Lines beginning with X need attention; those beginning with * are okay.  Just a few minor things :)

* verify the final provides and requires of the binary RPMs
X make sure lines are <= 80 characters
  - please add a line continuation to fix this on line 69
* package successfully compiles and builds
* BuildRequires are proper
* macros fine
* package is named appropriately
* it is legal for Fedora to distribute this
* license field matches the actual license.
X license is open source-compatible.
  - awaiting spot's comments
* specfile name matches %{name}
* md5sum matches upstream
  - the tarball I generated does not match but diff -uNr shows no differences so I assume svn timestamp differences
* skim the summary and description for typos, etc.
* summary and description good
* correct buildroot
* %{?dist} used correctly
X license text included in package and marked with %doc
  - this isn't the case.  Perhaps since you're doing an SVN snapshot you can include a coyp of it?
* packages meets FHS (http://www.pathname.com/fhs/)
X rpmlint on this package's srpm gives no output
  - see comment #1
* changelog format okay
* Summary tag does not end in a period
* no PreReq
* specfile is legible
* specfile written in American English
* no -doc sub-package necessary
* not native, so no rpath, static linking, etc.
* no config files
* not a GUI app
* no -devel necessary
* install section begins with rm -rf $RPM_BUILD_ROOT or %{buildroot}
* no translations so no locale handling
* Requires(pre,post) used correctly
* package not relocatable
* package contains code
* package owns all directories and files
* no %files duplicates
* file permissions fine
* %clean present
* %doc files do not affect runtime
* not a web app
X run rpmlint on the binary RPMs => no output

- see comment #1

* I verified that it installs cleanly.  Nothing erroneous is in the MANIFEST.MF.  I clicked through some of the javadocs and they look fine.

Thanks for the submission and sorry it took so long to review!
Comment 4 Jerry James 2009-03-05 11:19:07 EST
Thanks, Andrew.  Here are my responses to the flagged items.  First the rpmlint complaints from comment #1.

> jFormatString.src:104: W: libdir-macro-in-noarch-package %{_libdir}/gcj/%{name}

This is a side effect of the standard spec file template for using gcj.  I can't do anything about it (and there is actually nothing wrong).

> jFormatString.src: W: non-standard-group Development/Libraries/Java

Dozens of Fedora packages already use this group (it is derived from jpackage.org) and the Group name doesn't matter anyway.  I don't see any reason to change it.  This goes for the rpmlint complaints about the binary rpm, too.

> jFormatString.src: W: non-coherent-filename
> jFormatString-0-0.1.20081016svn.src.rpm
> jFormatString-0-0.1.20081016svn.fc10.src.rpm

That's just me being dumb in the way I copied the file to my web site.  If you build it yourself, this won't happen.

> jFormatString.src: W: strange-permission jFormatString-0.tar.bz2 0745

That is a strange permission.  It looks like the file must have passed through a Windows machine on its way to my web site.  Fixed.

> X make sure lines are <= 80 characters
>   - please add a line continuation to fix this on line 69

Fixed.

> * md5sum matches upstream
>   - the tarball I generated does not match but diff -uNr shows no differences
> so I assume svn timestamp differences

When you do an svn checkout, it goes into a freshly created directory.  Tar then faithfully preserves the timestamp on that directory.  For that reason, tarballs created from upstream SCM snapshots will never have matching checksums.  I hadn't thought about checking with diff.  That one goes into my bag of reviewer tricks.  Thanks!

> X license text included in package and marked with %doc
>   - this isn't the case.  Perhaps since you're doing an SVN snapshot you can
> include a coyp of it?

Oops, that was an oversight on my part.  But that's why we do package reviews, right?  Fixed.

So I guess we're just waiting for the license question to be resolved.  Here are the new versions:

http://jjames.fedorapeople.org/jFormatString/jFormatString.spec
http://jjames.fedorapeople.org/jFormatString/jFormatString-0-0.2.20081016svn.fc10.src.rpm
Comment 5 Tom "spot" Callaway 2009-03-05 11:28:00 EST
(In reply to comment #1)

> The license field is correct but I would like spot to weigh in on their
> statement:
> 
> "The library produced by compiling this project is used by the FindBugs
> project. To avoid any licensing questions due to incompatible licenses
> (FindBugs is licensed under the LGPL), it is broken out as a separate project.
> While there may be some confusion/discussion about the licenses, the FindBugs
> project does not interprete the FindBugs LGPL license to be any stronger than
> GPL v2 + the Classpath exception."
> 
> spot, what do you think?

I think that:

A) There is no compatibility concern between LGPL and GPLv2 with Classpath exception
B) The GPLv2 (even with the classpath exception) is a more restrictive license than LGPL.
C) This musing is reasonably irrelevant to anything. :)

There is no legal restriction on this package going forward (that I am aware of at this time).
Comment 6 Andrew Overholt 2009-03-05 11:30:14 EST
Great, thanks Jerry and spot!  This package is now approved.
Comment 7 Jerry James 2009-03-05 15:19:33 EST
Thank you very much, Andrew.

New Package CVS Request
=======================
Package Name: jFormatString
Short Description: Java format string compile-time checker
Owners: jjames
Branches: F-10
InitialCC:
Comment 8 Kevin Fenzi 2009-03-05 15:34:37 EST
cvs done.
Comment 9 Fedora Update System 2009-03-05 18:35:11 EST
jFormatString-0-0.2.20081016svn.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/jFormatString-0-0.2.20081016svn.fc10
Comment 10 Fedora Update System 2009-03-09 18:49:57 EDT
jFormatString-0-0.2.20081016svn.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 11 Richard Fearn 2015-01-01 08:26:52 EST
Package Change Request
======================
Package Name: jFormatString
New Branches: epel7
Owners: richardfearn
Comment 12 Richard Fearn 2015-01-01 08:29:38 EST
(Please disregard comment 11)

Package Change Request
======================
Package Name: jFormatString
New Branches: el6 epel7
Owners: richardfearn
Comment 13 Till Maas 2015-01-01 17:47:36 EST
Git done (by process-git-requests).

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