[Bug 727635] Review Request: java-1.7.0-openjdk - OpenJDK runtime environment

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=727635

--- Comment #4 from Omair Majid <omajid at redhat.com> 2011-08-02 16:25:32 EDT ---
(In reply to comment #3)
> === REQUIRED ITEMS ===
> [!]  Rpmlint output:

> SPECS/java-1.7.0-openjdk.spec:785: E: hardcoded-library-path in
> /usr/lib/jvm/java-gcj/*
> SPECS/java-1.7.0-openjdk.spec:818: E: hardcoded-library-path in
> /usr/lib/jvm/java-gcj/jre/lib/rt.jar
> 
> Fixed.
> 

Minor suggestion (and so feel free to ignore): JDK_TO_BUILD_WITH should also be
fixed.

> 
> SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source14: pulseaudio.tar.gz
> SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source12: desktop-files.tar.gz
> SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source11: systemtap-tapset.tar.gz
> SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source10: class-rewriter.tar.gz
> SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source9: generated-files.tar.gz
> 
> Instructions/comments addded for each. Some are not yet separate upstream
> because we need to know how well the RPM works first. Once we are certain, the
> projects will be split as needed and the urls added.
> 

It would be nice to have instructions on how to create these tarballs.

> 
> [!]  Buildroot definition is not present
>      Defining build root is depricated; it should not be defined.
> 

The new spec file still defines a buildroot. Please remove it.

> [!]  All independent sub-packages have license of their own
>      javadoc subpackage does not include the LICENSE file
> 
> Added to add sub-packages.

Actually, that's not quite right. It should only be added to subpackages if it
isnt being pulled in via a dependency. If the main package has the LICENSE
file, and -devel requires the main package then devel does not need the LICENSE
file. From what I can see in the spec file, only the javadoc subpackage does
not require the main package and needs the LICENSE file.

> [!]  Sources used to build the package matches the upstream source, as provided
> in the spec URL.
>      I cant find the source for generated-files.tar.gz, class-rewriter.tar.gz,
> systemtap-tapset.tar.gz and pulseaudio.tar.gz - I can guess it's from icedtea6
> or 7.
> 
> They are from 7. As mentioned above, once we know that the rpm works, we will
> find a separate home for them.
> 

Any chance you can include the instructions to create these tarball?

> [!]  Package does NOT have a %clean section which contains rm -rf %{buildroot}
> (or $RPM_BUILD_ROOT). (not needed anymore)
> 

Please remove the %clean section.

> 1. Avoid having BuildRequires on exact NVR unless necessary
>      (freetype-devel, pulseaudio-libs-devel pulseaudio,pkgconfig)
> 
> Add requires are >=, not exact. They were added after problems were found with
> lower versions.
> 

Hm.. all these packages have a higher NVR in F15. I am quite positive that the
F16 packages will be higher still. I suppose it's not an issue.

> 3. The forest at icedtea.classpath.org/hg/icedtea7-forest is more up to date
> than hg.openjdk.java.net/icedtea/jdk7
> 
> We tested with the latter, so I kept it. Going forward, we will be switching.
> 

Yeah, this was a more FYI than anything else.

> 6. License field contents should use 'and' or 'or'
> (http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios)
> 
> Different parts ahve different licences. Neither and nor or apply.
> 

IANAL. But the text at
http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios
suggests that 'and' should be used here:

"""
Example: Package bar-utils contains some files under the Python License, some
other files under the GNU Lesser General Public License v2 or later, and one
file under the BSD License (no advertising). The package spec must have:

License: Python and LGPLv2+ and BSD
"""

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.