Bug 727635 - Review Request: java-1.7.0-openjdk - OpenJDK runtime environment
Summary: Review Request: java-1.7.0-openjdk - OpenJDK runtime environment
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Omair Majid
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-08-02 16:48 UTC by Deepak Bhole
Modified: 2011-11-17 14:32 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-05 20:36:26 UTC
Type: ---
Embargoed:
dbhole: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Deepak Bhole 2011-08-02 16:48:00 UTC
Spec URL: http://dbhole.fedorapeople.org/java/7/review/java-1.7.0-openjdk.spec
SRPM URL: http://dbhole.fedorapeople.org/java/7/review/java-1.7.0-openjdk-1.7.0.0-0.1.20110729.fc15.src.rpm
Description: The OpenJDK runtime and development environment

Comment 1 Omair Majid 2011-08-02 18:42:34 UTC
=== REQUIRED ITEMS ===
[!]  Rpmlint output:

SPECS/java-1.7.0-openjdk.spec:91: W: macro-in-comment %{jit_arches} 
SPECS/java-1.7.0-openjdk.spec:121: E: hardcoded-library-path in %{_prefix}/lib
SPECS/java-1.7.0-openjdk.spec:163: W: macro-in-comment %define
SPECS/java-1.7.0-openjdk.spec:163: W: macro-in-comment %{sdkdir}
SPECS/java-1.7.0-openjdk.spec:189: W: macro-in-comment %{icedtea_jdk7_snapshot}
SPECS/java-1.7.0-openjdk.spec:190: W: macro-in-comment %{corba_snapshot}
SPECS/java-1.7.0-openjdk.spec:191: W: macro-in-comment %{hotspot_snapshot}
SPECS/java-1.7.0-openjdk.spec:192: W: macro-in-comment %{jaxp_snapshot}
SPECS/java-1.7.0-openjdk.spec:193: W: macro-in-comment %{jaxws_snapshot}
SPECS/java-1.7.0-openjdk.spec:194: W: macro-in-comment %{jdk_snapshot}
SPECS/java-1.7.0-openjdk.spec:195: W: macro-in-comment %{langtools_snapshot}
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
SPECS/java-1.7.0-openjdk.spec:908: W: configure-without-libdir-spec
SPECS/java-1.7.0-openjdk.spec:932: W: configure-without-libdir-spec
SPECS/java-1.7.0-openjdk.spec:1317: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1320: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1321: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1322: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1323: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1324: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1351: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1351: W: macro-in-comment %{buildoutputdir}
SPECS/java-1.7.0-openjdk.spec:1354: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1468: W: macro-in-%changelog %{_jvmdir}
SPECS/java-1.7.0-openjdk.spec:1468: W: macro-in-%changelog %{jredir}
SPECS/java-1.7.0-openjdk.spec:1470: W: macro-in-%changelog %{_jvmdir}
SPECS/java-1.7.0-openjdk.spec:1470: W: macro-in-%changelog %{sdkdir}
SPECS/java-1.7.0-openjdk.spec:163: W: mixed-use-of-spaces-and-tabs (spaces: line 32, tab: line 163)
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch0: java-1.7.0-openjdk-optflags.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch1: java-1.7.0-openjdk-java-access-bridge-tck.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch2: java-1.7.0-openjdk-java-access-bridge-idlj.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch3: java-1.7.0-openjdk-java-access-bridge-security.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch4: java-1.7.0-openjdk-accessible-toolkit.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch5: java-1.7.0-openjdk-debugdocs.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch6: %{name}-debuginfo.patch
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
SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source6: https://java.net/downloads/jax-ws/JDK7/jdk7-jaf-2010_08_19.zip HTTP Error 404: Not Found
SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source3: mauve-2008-10-22.tar.gz
SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source0: icedtea-jdk7.tar.gz
java-1.7.0-openjdk.src: W: spelling-error %description -l en_US runtime -> run time, run-time, rudiment
java-1.7.0-openjdk.src: W: invalid-license ASL 1.1, ASL 2.0, GPL+, GPLv2, GPLv2 with exceptions, LGPL+, LGPLv2, MPLv1.0, MPLv1.1, Public Domain, W3C
java-1.7.0-openjdk.src: W: strange-permission javac-wrapper 0775L
java-1.7.0-openjdk.src:91: W: macro-in-comment %{jit_arches}
java-1.7.0-openjdk.src:121: E: hardcoded-library-path in %{_prefix}/lib
java-1.7.0-openjdk.src:163: W: macro-in-comment %define
java-1.7.0-openjdk.src:163: W: macro-in-comment %{sdkdir}
java-1.7.0-openjdk.src:189: W: macro-in-comment %{icedtea_jdk7_snapshot}
java-1.7.0-openjdk.src:190: W: macro-in-comment %{corba_snapshot}
java-1.7.0-openjdk.src:191: W: macro-in-comment %{hotspot_snapshot}
java-1.7.0-openjdk.src:192: W: macro-in-comment %{jaxp_snapshot}
java-1.7.0-openjdk.src:193: W: macro-in-comment %{jaxws_snapshot}
java-1.7.0-openjdk.src:194: W: macro-in-comment %{jdk_snapshot}
java-1.7.0-openjdk.src:195: W: macro-in-comment %{langtools_snapshot}
java-1.7.0-openjdk.src:785: E: hardcoded-library-path in /usr/lib/jvm/java-gcj/*
java-1.7.0-openjdk.src:818: E: hardcoded-library-path in /usr/lib/jvm/java-gcj/jre/lib/rt.jar
java-1.7.0-openjdk.src:908: W: configure-without-libdir-spec
java-1.7.0-openjdk.src:932: W: configure-without-libdir-spec
java-1.7.0-openjdk.src:1317: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1320: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1321: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1322: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1323: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1324: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1351: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1351: W: macro-in-comment %{buildoutputdir}
java-1.7.0-openjdk.src:1354: W: macro-in-comment %doc
java-1.7.0-openjdk.src:1468: W: macro-in-%changelog %{_jvmdir}
java-1.7.0-openjdk.src:1468: W: macro-in-%changelog %{jredir}
java-1.7.0-openjdk.src:1470: W: macro-in-%changelog %{_jvmdir}
java-1.7.0-openjdk.src:1470: W: macro-in-%changelog %{sdkdir}
java-1.7.0-openjdk.src:163: W: mixed-use-of-spaces-and-tabs (spaces: line 32, tab: line 163)
java-1.7.0-openjdk.src: W: patch-not-applied Patch0: java-1.7.0-openjdk-optflags.patch
java-1.7.0-openjdk.src: W: patch-not-applied Patch1: java-1.7.0-openjdk-java-access-bridge-tck.patch
java-1.7.0-openjdk.src: W: patch-not-applied Patch2: java-1.7.0-openjdk-java-access-bridge-idlj.patch
java-1.7.0-openjdk.src: W: patch-not-applied Patch3: java-1.7.0-openjdk-java-access-bridge-security.patch
java-1.7.0-openjdk.src: W: patch-not-applied Patch4: java-1.7.0-openjdk-accessible-toolkit.patch
java-1.7.0-openjdk.src: W: patch-not-applied Patch5: java-1.7.0-openjdk-debugdocs.patch
java-1.7.0-openjdk.src: W: patch-not-applied Patch6: %{name}-debuginfo.patch
java-1.7.0-openjdk.src: W: invalid-url Source14: pulseaudio.tar.gz
java-1.7.0-openjdk.src: W: invalid-url Source12: desktop-files.tar.gz
java-1.7.0-openjdk.src: W: invalid-url Source11: systemtap-tapset.tar.gz
java-1.7.0-openjdk.src: W: invalid-url Source10: class-rewriter.tar.gz
java-1.7.0-openjdk.src: W: invalid-url Source9: generated-files.tar.gz
java-1.7.0-openjdk.src: W: invalid-url Source6: https://java.net/downloads/jax-ws/JDK7/jdk7-jaf-2010_08_19.zip HTTP Error 404: Not Found
java-1.7.0-openjdk.src: W: invalid-url Source3: mauve-2008-10-22.tar.gz
java-1.7.0-openjdk.src: W: invalid-url Source0: icedtea-jdk7.tar.gz
1 packages and 1 specfiles checked; 6 errors, 85 warnings.

There are many false positives, but I am quite concerned about a few of these including invalid-url and patch not applied look rather serious.

[!]  Buildroot definition is not present
     Defining build root is depricated; it should not be defined.
     
[!]  All independent sub-packages have license of their own
     javadoc subpackage does not include 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.

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

[!]  Javadoc subpackages have Require: jpackage-utils

[!]  Package uses %global not %define
     Please see http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

=== Other suggestions ===
1. Avoid having BuildRequires on exact NVR unless necessary
     (freetype-devel, pulseaudio-libs-devel pulseaudio,pkgconfig)
2. Package has BuildArch: noarch (if possible)
     The javadoc package should be noarch
3. The forest at icedtea.classpath.org/hg/icedtea7-forest is more up to date than hg.openjdk.java.net/icedtea/jdk7
4. execstack can be removed (fixed upstream: http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/dddc5753c53a)
5. Priority should be 17000 (instead of 16000)
6. License field contents should use 'and' or 'or' (http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios)
7. genurl macro defined by not used.
8. A comment says "update for jnlp handling", but this package provides no jnlp support
9. Changelogs are for icedtea6. Are they even needed?

Comment 2 Omair Majid 2011-08-02 19:00:29 UTC
On a slightly unrelated note, are there any plans to drop gnome-java-bridge (deprecated) and switch to http://git.gnome.org/browse/java-atk-wrapper/ ?

Comment 3 Deepak Bhole 2011-08-02 19:56:03 UTC
=== REQUIRED ITEMS ===
[!]  Rpmlint output:

SPECS/java-1.7.0-openjdk.spec:91: W: macro-in-comment %{jit_arches} 

Removed.

SPECS/java-1.7.0-openjdk.spec:121: E: hardcoded-library-path in %{_prefix}/lib

On purpose.

SPECS/java-1.7.0-openjdk.spec:163: W: macro-in-comment %define
SPECS/java-1.7.0-openjdk.spec:163: W: macro-in-comment %{sdkdir}

Fixed.

SPECS/java-1.7.0-openjdk.spec:189: W: macro-in-comment %{icedtea_jdk7_snapshot}
SPECS/java-1.7.0-openjdk.spec:190: W: macro-in-comment %{corba_snapshot}
SPECS/java-1.7.0-openjdk.spec:191: W: macro-in-comment %{hotspot_snapshot}
SPECS/java-1.7.0-openjdk.spec:192: W: macro-in-comment %{jaxp_snapshot}
SPECS/java-1.7.0-openjdk.spec:193: W: macro-in-comment %{jaxws_snapshot}
SPECS/java-1.7.0-openjdk.spec:194: W: macro-in-comment %{jdk_snapshot}
SPECS/java-1.7.0-openjdk.spec:195: W: macro-in-comment %{langtools_snapshot}

Needed for build instructions.

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.

SPECS/java-1.7.0-openjdk.spec:908: W: configure-without-libdir-spec
SPECS/java-1.7.0-openjdk.spec:932: W: configure-without-libdir-spec

Expected. Neither are installed via make install.

SPECS/java-1.7.0-openjdk.spec:1317: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1320: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1321: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1322: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1323: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1324: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1351: W: macro-in-comment %doc
SPECS/java-1.7.0-openjdk.spec:1351: W: macro-in-comment %{buildoutputdir}
SPECS/java-1.7.0-openjdk.spec:1354: W: macro-in-comment %doc

Fixed.

SPECS/java-1.7.0-openjdk.spec:1468: W: macro-in-%changelog %{_jvmdir}
SPECS/java-1.7.0-openjdk.spec:1468: W: macro-in-%changelog %{jredir}
SPECS/java-1.7.0-openjdk.spec:1470: W: macro-in-%changelog %{_jvmdir}
SPECS/java-1.7.0-openjdk.spec:1470: W: macro-in-%changelog %{sdkdir}

Comments..

SPECS/java-1.7.0-openjdk.spec:163: W: mixed-use-of-spaces-and-tabs (spaces:
line 32, tab: line 163)

Fixed.

SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch0:
java-1.7.0-openjdk-optflags.patch

Removed.

SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch1:
java-1.7.0-openjdk-java-access-bridge-tck.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch2:
java-1.7.0-openjdk-java-access-bridge-idlj.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch3:
java-1.7.0-openjdk-java-access-bridge-security.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch4:
java-1.7.0-openjdk-accessible-toolkit.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch5:
java-1.7.0-openjdk-debugdocs.patch
SPECS/java-1.7.0-openjdk.spec: W: patch-not-applied Patch6:
%{name}-debuginfo.patch

Applied via patch command manually.

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.

SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source6:
https://java.net/downloads/jax-ws/JDK7/jdk7-jaf-2010_08_19.zip HTTP Error 404:
Not Found

Link works for me.

SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source3: mauve-2008-10-22.tar.gz

This was imported from java-1.6.0-openjdk. Not sure what the right URL is, but
I will look into it. It is not used right now, so please ignore it for the time
being.

SPECS/java-1.7.0-openjdk.spec: W: invalid-url Source0: icedtea-jdk7.tar.gz

No direct link as it comes from a forest. Instructions to re-create are
specified.

java-1.7.0-openjdk.src: W: spelling-error %description -l en_US runtime -> run
time, run-time, rudiment

OK.

java-1.7.0-openjdk.src: W: invalid-license ASL 1.1, ASL 2.0, GPL+, GPLv2, GPLv2
with exceptions, LGPL+, LGPLv2, MPLv1.0, MPLv1.1, Public Domain, W3C

Mixed license.

java-1.7.0-openjdk.src: W: strange-permission javac-wrapper 0775L

Only used during bootstrap build. Not distributed.

Rest of the issues are same as above so I've skipped them.


There are many false positives, but I am quite concerned about a few of these
including invalid-url and patch not applied look rather serious.

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

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

Added to add sub-packages.
/
[!]  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.

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

[!]  Javadoc subpackages have Require: jpackage-utils

Added.

[!]  Package uses %global not %define
     Please see
http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Fixed.

=== Other suggestions ===
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.

2. Package has BuildArch: noarch (if possible)
     The javadoc package should be noarch

Fixed.

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.

4. execstack can be removed (fixed upstream:
http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/dddc5753c53a)

Removed.

5. Priority should be 17000 (instead of 16000)

Fixed.

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.

7. genurl macro defined by not used.

Removed.

8. A comment says "update for jnlp handling", but this package provides no jnlp
support

Comment removed. The call is still needed for policytool.

9. Changelogs are for icedtea6. Are they even needed?

Nope. Removed.

As for plans to replace access bridge -- none yet.


New files uploaded to sample place.

I've also uploaded a diff for easy comparison:
http://dbhole.fedorapeople.org/java/7/review/spec-diff.patch

Thanks for the review!

Comment 4 Omair Majid 2011-08-02 20:25:32 UTC
(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
"""

Comment 5 Deepak Bhole 2011-08-02 20:37:11 UTC
(In reply to comment #4)
> (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.

Sure, I will keep it in mind for the next iteration. Skipping now due to time constraints.

> 
> > 
> > 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.
> 

I've added them for generated-files. The rest don't really have instructions. They will when moved upstream.

> > 
> > [!]  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.

Removed.

> 
> > [!]  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.
> 

Ah okay. Fixed.

> > [!]  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?
> 

They are there for generated files and pulseaudio.

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

Done.

> > 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:
> 

Ah okay, changed.

> """
> 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
> """

new spec file uploaded.

Comment 6 Omair Majid 2011-08-02 20:46:17 UTC
Looks good to me.

APPROVED.

Comment 7 Deepak Bhole 2011-08-02 22:50:29 UTC
Setting review to + as Omair couldn't.

Comment 8 Deepak Bhole 2011-08-02 22:53:57 UTC
New Package SCM Request
=======================
Package Name: java-1.7.0-openjdk
Short Description: OpenJDK runtime environment
Owners: dbhole jvanek omajid
Branches: f16
InitialCC: dbhole jvanek omajid

Comment 9 Gwyn Ciesla 2011-08-03 11:58:12 UTC
Git done (by process-git-requests).

Comment 10 Deepak Bhole 2011-08-05 20:36:26 UTC
Thanks for the review Omair!

New builds for F16 and Rawhide are ready:
http://koji.fedoraproject.org/koji/buildinfo?buildID=257376
http://koji.fedoraproject.org/koji/buildinfo?buildID=257378

Comment 11 Xavier Bachelot 2011-11-17 13:46:19 UTC
Out of interest, is there a plan for EPEL branches ? It would be nice to have openjdk 1.7 for EL6 at least. I've rebuilt the latest rawhide srpm locally in mock for EL6 x86_64. The same openjdk build against EL6 i386 failed, but I've not looked why. The only missing BR was lcms2, which is branched but not imported for EL5/6 (owned by rhughes). Latest rawhide srpm of lcms2 rebuild fine on EL6.

Comment 12 Deepak Bhole 2011-11-17 14:32:02 UTC
The latest F16 update has the lcms dependency as well.

We do not plan to add this to EPEL. As for EL.. one of the main reasons why it is not in EL yet is the lack of a TCK. Until we can access and run the TCK, it cannot go into EL.


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