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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 #3 from Deepak Bhole <dbhole@xxxxxxxxxx> 2011-08-02 15:56:03 EDT ---
=== 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!

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]