Bug 227050

Taking up for initial review.

X suggests the subsection needs attention
+ is a positive comment
. is a specific comment about a problem

MUST:
X * package is named appropriately
   . 0:3.4.5-2jpp.1 -> 0:3.4.5-2jpp.2%{?dist} to be inline with 
     http://fedoraproject.org/wiki/PackagingDrafts/ExceptionJPackage
 - match upstream tarball or project name
   + MD5SUMs match
 - try to match previous incarnations in other distributions/packagers for
consistency
   + Consistent with JPackage

 - specfile should be %{name}.spec
   + OK
 - non-numeric characters should only be used in Release (ie. cvs or
   something)
   + OK
 - for non-numerics (pre-release, CVS snapshots, etc.), see
   http://fedoraproject.org/wiki/Packaging/NamingGuidelines#PackageRelease
   + N/A
 - if case sensitivity is requested by upstream or you feel it should be
   not just lowercase, do so; otherwise, use all lower case for the name
   + N/A

* is it legal for Fedora to distribute this?
 - OSI-approved
   + LGPL OK.

 - not a kernel module
 - not shareware
 - is it covered by patents?
 - it *probably* shouldn't be an emulator
 - no binary firmware
  + None of these apply

* license field matches the actual license.
  + OK

* license is open source-compatible.
 - use acronyms for licences where common
  + OK

* specfile name matches %{name}
  + OK

* verify source and patches (md5sum matches upstream, know what the patches do)
  + No patches, MD5 OK

 - if upstream doesn't release source drops, put *clear* instructions on
   how to generate the the source drop; ie. 
  # svn export blah/tag blah
  # tar cjf blah-version-src.tar.bz2 blah
  + N/A

* skim the summary and description for typos, etc.
   + OK

X correct buildroot
 - should be:
   %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

X if %{?dist} is used, it should be in that form (note the ? and %
locations)
  . See above about naming convention

* license text included in package and marked with %doc
  + OK

* keep old changelog entries; use judgement when removing (too old?
useless?)
  + N/A

* packages meets FHS (http://www.pathname.com/fhs/)
  + OK

X* rpmlint on <this package>.srpm and rpms gives no output
 - justify warnings if you think they shouldn't be there
W: dtdparser non-standard-group Development/Libraries/Java
The value of the Group tag in the package is not valid.  Valid groups are:
"Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
"Applications/Communications", "Applications/Databases",
"Applications/Editors", "Applications/Emulators", "Applications/Engineering",
"Applications/File", "Applications/Internet", "Applications/Multimedia",
"Applications/Productivity", "Applications/Publishing", "Applications/System",
"Applications/Text", "Development/Debug", "Development/Debuggers",
"Development/Languages", "Development/Libraries", "Development/System",
"Development/Tools", "Documentation", "System Environment/Base", "System
Environment/Daemons", "System Environment/Kernel", "System
Environment/Libraries", "System Environment/Shells", "User
Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".

 E: dtdparser tag-not-utf8 %changelog
The character encoding of the value of this tag is not UTF-8.
. use iconv to convert to UTF8

E: dtdparser non-utf8-spec-file dtdparser.spec
The character encoding of the spec file is not UTF-8.  Convert it for
example using iconv(1).
. use iconv to convert to UTF8

W: dtdparser mixed-use-of-spaces-and-tabs (spaces: line 9, tab: line 36)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.
. Replace the tabs with spaces (:set tabexpand :%retab in vim)

W: dtdparser non-standard-group Development/Libraries/Java
The value of the Group tag in the package is not valid.  Valid groups are:
"Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
"Applications/Communications", "Applications/Databases",
"Applications/Editors", "Applications/Emulators", "Applications/Engineering",
"Applications/File", "Applications/Internet", "Applications/Multimedia",
"Applications/Productivity", "Applications/Publishing", "Applications/System",
"Applications/Text", "Development/Debug", "Development/Debuggers",
"Development/Languages", "Development/Libraries", "Development/System",
"Development/Tools", "Documentation", "System Environment/Base", "System
Environment/Daemons", "System Environment/Kernel", "System
Environment/Libraries", "System Environment/Shells", "User
Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".

E: dtdparser tag-not-utf8 %changelog
The character encoding of the value of this tag is not UTF-8.
. use iconv to convert to UTF8

W: dtdparser-javadoc non-standard-group Development/Documentation
The value of the Group tag in the package is not valid.  Valid groups are:
"Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
"Applications/Communications", "Applications/Databases",
"Applications/Editors", "Applications/Emulators", "Applications/Engineering",
"Applications/File", "Applications/Internet", "Applications/Multimedia",
"Applications/Productivity", "Applications/Publishing", "Applications/System",
"Applications/Text", "Development/Debug", "Development/Debuggers",
"Development/Languages", "Development/Libraries", "Development/System",
"Development/Tools", "Documentation", "System Environment/Base", "System
Environment/Daemons", "System Environment/Kernel", "System
Environment/Libraries", "System Environment/Shells", "User
Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".
+ All group warnings can be ignored.

E: dtdparser-javadoc tag-not-utf8 %changelog
The character encoding of the value of this tag is not UTF-8.
. use iconv to convert to UTF8

E: dtdparser-javadoc zero-length
/usr/share/javadoc/dtdparser-1.21/package-list
 + I checked the build root on a local build and this seems to be created by
   the javadoc task in ant. This can probably be ignored?

* changelog should be in one of these formats:

  * Fri Jun 23 2006 Jesse Keating <jkeating> - 0.6-4
  - And fix the link syntax.

  * Fri Jun 23 2006 Jesse Keating <jkeating> 0.6-4
  - And fix the link syntax.

  * Fri Jun 23 2006 Jesse Keating <jkeating>
  - 0.6-4
  - And fix the link syntax.
  + OK

* Packager tag should not be used
  + OK
* Vendor and distribution tag should not be used
  + OK
* use License and not Copyright 
  + OK
* Summary tag should not end in a period
  + OK
* if possible, replace PreReq with Requires(pre) and/or Requires(post)
  + N/A
* specfile is legible
 - this is largely subjective; use your judgement
* package successfully compiles and builds on at least x86
* BuildRequires are proper
  + Seems OK, built on mock
 - builds in mock will flush out problems here
* summary should be a short and concise description of the package
  + OK
* description expands upon summary (don't include installation
instructions)
  + OK
X make sure lines are <= 80 characters
  . minor fixes needed

* specfile written in American English
  + OK
X make a -doc sub-package if necessary
  Standardize the javadoc package handling around
  https://zarb.org/pipermail/jpackage-discuss/2007-February/011119.html
 - see
  
http://fedoraproject.org/wiki/Packaging/Guidelines#head-9bbfa57478f0460c6160947a6bf795249488182b
* packages including libraries should exclude static libraries if possible
* don't use rpath
* config files should usually be marked with %config(noreplace)
* GUI apps should contain .desktop files
  + The above dont apply

* should the package contain a -devel sub-package?
  + N/A

* use macros appropriately and consistently
 - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS
  + RPM_BUILD_ROOT seems to be used consistently

* don't use %makeinstall
  + N/A

* locale data handling correct (find_lang)
 - if translations included, add BR: gettext and use %find_lang %{name} at the
   end of %install
 + N/A

* consider using cp -p to preserve timestamps
  + OK

* split Requires(pre,post) into two separate lines
  + None used yet

* package should probably not be relocatable
  + Non relocatable

* package contains code
 - see http://fedoraproject.org/wiki/Packaging/Guidelines#CodeVsContent
 - in general, there should be no offensive content
  + OK
X* package should own all directories and files
 + Use jpackage-utils in Requires(x), Requires since installing to
%{_javadir}/%{_javadocdir} 
* there should be no %files duplicates
  + OK
* file permissions should be okay; %defattrs should be present
  + OK
* %clean should be present
  + OK
X* %doc files should not affect runtime
  . javadoc should use %doc for its files
* if it is a web apps, it should be in /usr/share/%{name} and *not* /var/www
  + Not a webapp
X* verify the final provides and requires of the binary RPM
rpm -qp --provides ../RPMS/noarch/dtdparser-*
dtdparser = 0:1.21-3jpp
dtdparser-javadoc = 0:1.21-3jpp
rpm -qp --requires ../RPMS/noarch/dtdparser-*
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
 . Requires needs jpakage-utils as mentioned earlier
 . Should have a requires on java?

SHOULD:
* package should include license text in the package and mark it with %doc
  + OK
* package should build on i386
  + Builds on mock

* package should build in mock
  + OK

> X * package is named appropriately
>    . 0:3.4.5-2jpp.1 -> 0:3.4.5-2jpp.2%{?dist} to be inline with 
>      http://fedoraproject.org/wiki/PackagingDrafts/ExceptionJPackage

Fixed.

> X correct buildroot
>  - should be:
>    %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Fixed.


> X if %{?dist} is used, it should be in that form (note the ? and %
> locations)
>   . See above about naming convention

Fixed
 
> X* rpmlint on <this package>.srpm and rpms gives no output
>  - justify warnings if you think they shouldn't be there
> W: dtdparser non-standard-group Development/Libraries/Java
> The value of the Group tag in the package is not valid.  Valid groups are:
> "Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
> "Applications/Communications", "Applications/Databases",
> "Applications/Editors", "Applications/Emulators", "Applications/Engineering",
> "Applications/File", "Applications/Internet", "Applications/Multimedia",
> "Applications/Productivity", "Applications/Publishing", "Applications/System",
> "Applications/Text", "Development/Debug", "Development/Debuggers",
> "Development/Languages", "Development/Libraries", "Development/System",
> "Development/Tools", "Documentation", "System Environment/Base", "System
> Environment/Daemons", "System Environment/Kernel", "System
> Environment/Libraries", "System Environment/Shells", "User
> Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".

This warning can be ignored.

>  E: dtdparser tag-not-utf8 %changelog
> The character encoding of the value of this tag is not UTF-8.
> . use iconv to convert to UTF8

Fixed.

> E: dtdparser non-utf8-spec-file dtdparser.spec
> The character encoding of the spec file is not UTF-8.  Convert it for
> example using iconv(1).
> . use iconv to convert to UTF8

Fixed.

> W: dtdparser mixed-use-of-spaces-and-tabs (spaces: line 9, tab: line 36)
> The specfile mixes use of spaces and tabs for indentation, which is a
> cosmetic annoyance.  Use either spaces or tabs for indentation, not both.
> . Replace the tabs with spaces (:set tabexpand :%retab in vim)

Fixed.

> W: dtdparser non-standard-group Development/Libraries/Java
> The value of the Group tag in the package is not valid.  Valid groups are:
> "Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
> "Applications/Communications", "Applications/Databases",
> "Applications/Editors", "Applications/Emulators", "Applications/Engineering",
> "Applications/File", "Applications/Internet", "Applications/Multimedia",
> "Applications/Productivity", "Applications/Publishing", "Applications/System",
> "Applications/Text", "Development/Debug", "Development/Debuggers",
> "Development/Languages", "Development/Libraries", "Development/System",
> "Development/Tools", "Documentation", "System Environment/Base", "System
> Environment/Daemons", "System Environment/Kernel", "System
> Environment/Libraries", "System Environment/Shells", "User
> Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".

This warning can be ignored.

> E: dtdparser tag-not-utf8 %changelog
> The character encoding of the value of this tag is not UTF-8.
> . use iconv to convert to UTF8

Fixed.

> W: dtdparser-javadoc non-standard-group Development/Documentation
> The value of the Group tag in the package is not valid.  Valid groups are:
> "Amusements/Games", "Amusements/Graphics", "Applications/Archiving",
> "Applications/Communications", "Applications/Databases",
> "Applications/Editors", "Applications/Emulators", "Applications/Engineering",
> "Applications/File", "Applications/Internet", "Applications/Multimedia",
> "Applications/Productivity", "Applications/Publishing", "Applications/System",
> "Applications/Text", "Development/Debug", "Development/Debuggers",
> "Development/Languages", "Development/Libraries", "Development/System",
> "Development/Tools", "Documentation", "System Environment/Base", "System
> Environment/Daemons", "System Environment/Kernel", "System
> Environment/Libraries", "System Environment/Shells", "User
> Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support".

This warning can be ignored.

> E: dtdparser-javadoc tag-not-utf8 %changelog
> The character encoding of the value of this tag is not UTF-8.
> . use iconv to convert to UTF8

Fixed.

> E: dtdparser-javadoc zero-length
> /usr/share/javadoc/dtdparser-1.21/package-list
>  + I checked the build root on a local build and this seems to be created by
>    the javadoc task in ant. This can probably be ignored?

This warning can be ignored because the file is automated and thus, changing on
a daily basis.

> X make sure lines are <= 80 characters
>   . minor fixes needed

Fixed.


> X make a -doc sub-package if necessary
>   Standardize the javadoc package handling around
>   https://zarb.org/pipermail/jpackage-discuss/2007-February/011119.html
>  - see
>   
>http://fedoraproject.org/wiki/Packaging/Guidelines#head-9bbfa57478f0460c6160947a6bf795249488182b

Fixed.

> X* package should own all directories and files
>  + Use jpackage-utils in Requires(x), Requires since installing to
> %{_javadir}/%{_javadocdir} 

Fixed.

> X* %doc files should not affect runtime
>   . javadoc should use %doc for its files

Fixed.

> X* verify the final provides and requires of the binary RPM
> rpm -qp --provides ../RPMS/noarch/dtdparser-*
> dtdparser = 0:1.21-3jpp
> dtdparser-javadoc = 0:1.21-3jpp
> rpm -qp --requires ../RPMS/noarch/dtdparser-*
> rpmlib(CompressedFileNames) <= 3.0.4-1
> rpmlib(PayloadFilesHavePrefix) <= 4.0-1
> rpmlib(CompressedFileNames) <= 3.0.4-1
> rpmlib(PayloadFilesHavePrefix) <= 4.0-1
>  . Requires needs jpakage-utils as mentioned earlier
>  . Should have a requires on java?

Fixed. 

The src rpm can be found at this link:
http://tequila-sunrise.ath.cx/rpmreviews/F7/dtdparser/dtdparser-1.21-3jpp.1.src.rpm

Let me know if there's anything else I need to fix or forgot to fix.  
Thanks.

Hmmm you seem to have uploaded the wrong srpm... Verify the above changes have
been made. Also, please add gcj support while you are at it.

Everything seems okay - it even runs on mock with no errors!!!  

Here's the link to the source rpm:
http://tequila-sunrise.ath.cx/rpmreviews/F7/dtdparser/dtdparser-1.21-3jpp.1.fc7.src.rpm

Thanks Vivek.

rpmlint says:
W: dtdparser non-standard-group Development/Libraries/Java
W: dtdparser-javadoc non-standard-group Development/Documentation
Ignoring proup usage

E: dtdparser-javadoc zero-length /usr/share/javadoc/dtdparser/package-list
Ignoring since the package-list is generated at build time...

rpm -qp --provides
/home/vivekl/topdir-test/RPMS/i386/dtdparser-1.21-3jpp.1.fc7.i386.rpm
/home/vivekl/topdir-test/RPMS/i386/dtdparser-javadoc-1.21-3jpp.1.fc7.i386.rpm
/home/vivekl/topdir-test/RPMS/i386/dtdparser-debuginfo-1.21-3jpp.1.fc7.i386.rpm
dtdparser-1.21.jar.so  
dtdparser = 0:1.21-3jpp.1.fc7
dtdparser-javadoc = 0:1.21-3jpp.1.fc7
dtdparser-1.21.jar.so.debug  
dtdparser-debuginfo = 0:1.21-3jpp.1.fc7


rpm -qp --requires
/home/vivekl/topdir-test/RPMS/i386/dtdparser-1.21-3jpp.1.fc7.i386.rpm
/home/vivekl/topdir-test/RPMS/i386/dtdparser-javadoc-1.21-3jpp.1.fc7.i386.rpm
/home/vivekl/topdir-test/RPMS/i386/dtdparser-debuginfo-1.21-3jpp.1.fc7.i386.rpm
/bin/sh  
/bin/sh  
java  
java-gcj-compat  
java-gcj-compat  
jpackage-utils >= 0:1.6
jpackage-utils >= 0:1.6
libc.so.6  
libc.so.6(GLIBC_2.1.3)  
libdl.so.2  
libgcc_s.so.1  
libgcc_s.so.1(GCC_3.0)  
libgcj_bc.so.1  
libm.so.6  
libpthread.so.0  
librt.so.1  
libz.so.1  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rtld(GNU_HASH)  
jpackage-utils >= 0:1.6
jpackage-utils >= 0:1.6
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1

Everything looks good, APPROVED.
Assigning to nsantos to build it into rawhide. Please update when it makes it
into rawhide, on verification the bug can be closed.

New Package CVS Request
=======================
Package Name: dtdparser-1.21-3jpp
Short Description: A Java DTD Parser
Owners: nsantos
Branches: FC-7
InitialCC: rafaels,dbhole