Red Hat Bugzilla – Bug 691541
Review Request: icedtea-web - Additional Java components for OpenJDK
Last modified: 2011-04-27 03:06:04 EDT
Spec URL: http://people.redhat.com/dbhole/scratch/icedtea-web.spec SRPM URL: http://people.redhat.com/dbhole/scratch/icedtea-web-1.0.1-1.fc15.src.rpm Description: The IcedTea-Web project provides a Java web browser plugin, an implementation of Java Web Start (originally based on the Netx project) and a settings tool to manage deployment settings for the aforementioned plugin and Web Start implementations.
Packaging Guidelines: - Naming: OK. - Version and Release: OK - MD5 sum of source tar matches with upstream: OK - Spec Legibility: OK - Architecture Support: Issues Any reason for ExclusiveArch? - File System Layout: OK Ideally icedtea-web should install into a normal prefix, not into a JDK dir.But since this is how upstream is handling it _and_ how this was handled when it was a part of java-1.6.0-openjdk-plugin, this is OK for now. - rpmlint: OK icedtea-web.spec:97: W: configure-without-libdir-spec This is the same problem as above. OK for now. icedtea-web.spec:42: W: mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 42) Please fix this? - Requires: OK - BuildRequires: OK - Summary and Description: OK - Documentation: Issues Please use %{_javadocdir} for javadoc directory path. The javadoc package needs to be declared as noarch. - Compiler Flags: Issues Compiler flags (RPM_OPT_FLAGS, etc) are not being passed. - DebugInfo Packages: OK - Devel Packages: N/A - Requiring Base Package: Issues The Javadocs dont depend on the main package for functionality; please remove the requires on main package. - Static and Shared Libraries: OK - Desktop files: OK - Macros: Issues %define used instead of %global. Please use %global - Scriptlets: OK I dont understand why only the first alternatives command in %post is if'd. Shouldnt the second alternatives invocation be if'd as well? - File and directory ownership: OK - Others: Please remove all invocations of rm -rf $RPM_BUILD_ROOT. It is unneeded on recent version of Fedora.
(In reply to comment #1) > Packaging Guidelines: > > - Naming: OK. > - Version and Release: OK > - MD5 sum of source tar matches with upstream: OK > - Spec Legibility: OK > - Architecture Support: Issues > Any reason for ExclusiveArch? > - File System Layout: OK > Ideally icedtea-web should install into a normal prefix, not into a JDK dir.But > since this is how upstream is handling it _and_ how this was handled when it > was a part of java-1.6.0-openjdk-plugin, this is OK for now. > - rpmlint: OK > icedtea-web.spec:97: W: configure-without-libdir-spec > This is the same problem as above. OK for now. > icedtea-web.spec:42: W: mixed-use-of-spaces-and-tabs (spaces: line 11, tab: > line 42) > Please fix this? Fixed. > - Requires: OK > - BuildRequires: OK > - Summary and Description: OK > - Documentation: Issues > Please use %{_javadocdir} for javadoc directory path. The javadoc package needs Fixed. > to be declared as noarch. > - Compiler Flags: Issues > Compiler flags (RPM_OPT_FLAGS, etc) are not being passed. Fixed. > - DebugInfo Packages: OK > - Devel Packages: N/A > - Requiring Base Package: Issues > The Javadocs dont depend on the main package for functionality; please remove > the requires on main package. Done. > - Static and Shared Libraries: OK > - Desktop files: OK > - Macros: Issues > %define used instead of %global. Please use %global > - Scriptlets: OK > I dont understand why only the first alternatives command in %post is if'd. > Shouldnt the second alternatives invocation be if'd as well? Actually, the if in %post shouldn't be an if at all. I had copied and pasted it from the original rpm. We want it to run each time. The if has been removed now. The one in %postun needs to stay in an if as we want it to run only during uninstall and not during upgrade. > - File and directory ownership: OK > - Others: > Please remove all invocations of rm -rf $RPM_BUILD_ROOT. It is unneeded on > recent version of Fedora. Done. New spec file and srpm uploaded.
Thanks for the fixes. Some more minor issues. - archbuild, archinstall: are these macros used anywhere? Or is this some rpm-internal magic? - Why is the main package exclusive arch? Is it because openjdk is exclusive arch? - Javadoc subpackage needs to be built as noarch [1]. Since it does not depend on the main package, it needs its own copy of COPYING [2]. - %files javadoc uses %{datadir}/javadoc but configure uses %{_javadocdir} [1] http://fedoraproject.org/wiki/Packaging/Java#Javadoc_installation [2] http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Subpackage_Licensing
Omair, you should set fedora-review flag to ? when starting review and to + when package is approved. I've changed the bug status to assigned which also must be done by the reviewer.
(In reply to comment #4) > Omair, you should set fedora-review flag to ? when starting review and to + > when package is approved. I've changed the bug status to assigned which also > must be done by the reviewer. Sorry, I wasnt aware of that. Done now.
(In reply to comment #3) > Thanks for the fixes. Some more minor issues. > > - archbuild, archinstall: are these macros used anywhere? Or is this some > rpm-internal magic? > archbuild was used in an older version of the spec file, not any more though. I've removed it. archinstall is used in the %install section of the spec file. > - Why is the main package exclusive arch? Is it because openjdk is exclusive > arch? > Yes. I've added a comment stating so now. > - Javadoc subpackage needs to be built as noarch [1]. Since it does not depend > on the main package, it needs its own copy of COPYING [2]. > Fixed. > - %files javadoc uses %{datadir}/javadoc but configure uses %{_javadocdir} > Fixed. New spec file and srpm attached.
Sorry, new spec file and srpm uploaded I meant: http://people.redhat.com/dbhole/scratch/icedtea-web.spec http://people.redhat.com/dbhole/scratch/icedtea-web-1.0.1-1.fc15.src.rpm
(In reply to comment #7) > Sorry, new spec file and srpm uploaded I meant: > > http://people.redhat.com/dbhole/scratch/icedtea-web.spec > http://people.redhat.com/dbhole/scratch/icedtea-web-1.0.1-1.fc15.src.rpm You have added a patch, but there is no bug number or note that says it's actually an upstream patch backported to fix a bug. It would be great if you could add that.
Oops, sorry. That patch was intended to go in (not yet). I was just testing something else. I've removed it and re-uploaded.
Looks good to me now. Approved.
Setting review to +
There is no SCM request to process here. Please provide one and set the fedora-cvs flag back to '?'. https://fedoraproject.org/wiki/Package_SCM_admin_requests
Sorry about that. Info: New Package SCM Request ======================= Package Name: icedtea-web Short Description: Additional Java components for OpenJDK Owners: dbhole@redhat.com Branches: f15 InitialCC: dbhole@redhat.com
Please note that SCM requests should reference FAS account names, not email addresses. I've fixed things up. Git done (by process-git-requests).
Built in koji weeks ago. http://koji.fedoraproject.org/koji/buildinfo?buildID=237494