https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Bug ID: 1594313 Summary: Review Request: java-11-openjdk - next LTS OpenJDK for fedora - techpreview Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: jvanek@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk.spec SRPM URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk-11.0.ea.19-1... Description: https://src.fedoraproject.org/rpms/java-openjdk/blob/java-11-openjdk/f/READM... Fedora Account System Username: jvanek
Work repo - https://src.fedoraproject.org/rpms/java-openjdk/tree/java-11-openjdk (as java-openjdk will flow to JDK11 once it it is officially released)
See also: https://bugzilla.redhat.com/show_bug.cgi?id=1557371 and https://bugzilla.redhat.com/show_bug.cgi?id=1592196 and https://fedoraproject.org/wiki/Changes/java-11-openjdk-TechPreview
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
jiri vanek jvanek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1592196
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1592196 [Bug 1592196] java-11-openjdk - next LTS OpenJDK release and future main JDK in Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
jiri vanek jvanek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |java-11-openjdk - next LTS |java-11-openjdk - next LTS |OpenJDK for fedora - |OpenJDK for Fedora |techpreview |
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Severin Gehwolf sgehwolf@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |sgehwolf@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #1 from Severin Gehwolf sgehwolf@redhat.com --- This:
%global _privatelibs libjsoundalsa[.]so.*|libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so.*|libjli[.]so.*|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so.*|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so.*|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so.*|libjaas_unix[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so.*|libjimage[.]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so.*|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so.*|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so.*|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[.]so.*|lib[.]so\(SUNWprivate_.*
Should be this:
%global _privatelibs libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so.*|libjli[.]so.*|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so.*|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so.*|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so.*|libjaas[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so.*|libjimage[.]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so.*|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so.*|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so.*|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[.]so.*
lib.so provides isn't generated for JDK 10+, libjaas_unix.so is no longer in JDK 11, etc.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #2 from Severin Gehwolf sgehwolf@redhat.com --- Patch6: systemLcmsAndJpgFixFor-f0aeede1b855.patch
We should start sticking to a better naming scheme for patches. This one is an upstream bug and, thus, we need to create an upstream bug if there doesn't exist one yet. In this case it's: https://bugs.openjdk.java.net/browse/JDK-8205616
Suggested naming conventions for upstream bugs:
JDK-<id>-some-name-with-hyphens.patch
Similarly for downstream-only patches, we need to have a RHBZ bug for each patch. Suggested naming convention:
RHBZ-<id>-some-name-with-hyphens.patch
Sticking to some agreed upon naming convention makes archaeology for patches easier. What's more, it gets easier to track the upstream status for them (if any).
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #3 from Severin Gehwolf sgehwolf@redhat.com --- Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz Source8: systemtap-tapset-3.6.0pre02.tar.xz
Each of these sources should have a comment preceding them how *exactly* the tarball was generated. I've been asked before by other fedora contributors how our sources are generated. When being asked I mostly don't remember myself and need to go digging. If every source was preceded by a comment where it came from those issues go away. Example:
# Generated by: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk bash generate_source_tarball.sh Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz [...]
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #4 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #2)
Patch6: systemLcmsAndJpgFixFor-f0aeede1b855.patch
We should start sticking to a better naming scheme for patches. This one is an upstream bug and, thus, we need to create an upstream bug if there doesn't exist one yet. In this case it's: https://bugs.openjdk.java.net/browse/JDK-8205616
Suggested naming conventions for upstream bugs:
JDK-<id>-some-name-with-hyphens.patch
Similarly for downstream-only patches, we need to have a RHBZ bug for each patch. Suggested naming convention:
RHBZ-<id>-some-name-with-hyphens.patch
Sticking to some agreed upon naming convention makes archaeology for patches easier. What's more, it gets easier to track the upstream status for them (if any).
Agree - I have failed to found the upstream bug, This the stupid name. One more detail - many bugs are duplicated, and it prove itself to have the name convention in
JDKID-PRID-RHBZID-name.patch
as all those thre ids aree usually necessary.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #5 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #3)
Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz Source8: systemtap-tapset-3.6.0pre02.tar.xz
Each of these sources should have a comment preceding them how *exactly* the tarball was generated. I've been asked before by other fedora contributors how our sources are generated. When being asked I mostly don't remember myself and need to go digging. If every source was preceded by a comment where it came from those issues go away. Example:
# Generated by: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk bash
This kind of comment should not be necessary. Those valueas are exactly for this purpose hardcoded in update_package.sh
generate_source_tarball.sh Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz [...]
Still I agree on improvement here. TBD!
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #6 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #5)
(In reply to Severin Gehwolf from comment #3)
Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz Source8: systemtap-tapset-3.6.0pre02.tar.xz
Each of these sources should have a comment preceding them how *exactly* the tarball was generated. I've been asked before by other fedora contributors how our sources are generated. When being asked I mostly don't remember myself and need to go digging. If every source was preceded by a comment where it came from those issues go away. Example:
# Generated by: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk bash
This kind of comment should not be necessary.
It's absolutely necessary.
Those valueas are exactly for this purpose hardcoded in update_package.sh
Yet, nothing mentions "update_package.sh" in Source{0,1,8} comments. So for somebody new to the package, why would they look at update_package.sh? They wouldn't. There is "generate_source_tarball.sh", "generate_tarballs.sh" and "update_package.sh" as auxiliary scripts. Knowing nothing about a specific work-flow one is lost which one to use for which tarball source. Then by the time they've looked at the third script they are giving up trying to figure out the exact parameters one is supposed to invoke scripts with and ask for help. This absolutely needs to become easier to self-discover. Hiding something in extra scripts isn't enough. Remember, the audience is somebody who knows about RPM packaging. The expectation should be to go to the spec file and figure out the rest on their own. That's hard enough for OpenJDK spec files already. We don't need to make it even harder by introducing 3 levels of indirection ;-)
If update_packages.sh satisfies *your* work-flow, then it should be possible to massage that script to output the parameters used to generate a sourcetarball. After that it's a matter of adding that comment.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #7 from Severin Gehwolf sgehwolf@redhat.com --- https://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #8 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to Severin Gehwolf from comment #7)
https://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build.
That built successfully.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #9 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #8)
(In reply to Severin Gehwolf from comment #7)
https://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build.
That built successfully.
Of course it does. I told you :)
Working on other hints now. TY!
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #10 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #1)
This:
%global _privatelibs libjsoundalsa[.]so.*|libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so. *|libjli[.]so.*|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so. *|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so. *|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so. *|libjaas_unix[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so. *|libjimage[.]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so. *|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so. *|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so. *|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[. ]so.*|lib[.]so\(SUNWprivate_.*
Should be this:
%global _privatelibs libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so.*|libjli[.]so. *|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so. *|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so. *|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so. *|libjaas[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so.*|libjimage[. ]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so. *|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so. *|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so. *|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[. ]so.*
lib.so provides isn't generated for JDK 10+, libjaas_unix.so is no longer in JDK 11, etc.
Fixed. You have some sript to generate this befor buid? Or from build itslef? Used.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #11 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #2)
Patch6: systemLcmsAndJpgFixFor-f0aeede1b855.patch
We should start sticking to a better naming scheme for patches. This one is an upstream bug and, thus, we need to create an upstream bug if there doesn't exist one yet. In this case it's: https://bugs.openjdk.java.net/browse/JDK-8205616
Suggested naming conventions for upstream bugs:
JDK-<id>-some-name-with-hyphens.patch
Similarly for downstream-only patches, we need to have a RHBZ bug for each patch. Suggested naming convention:
RHBZ-<id>-some-name-with-hyphens.patch
Sticking to some agreed upon naming convention makes archaeology for patches easier. What's more, it gets easier to track the upstream status for them (if any).
fixed
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #12 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #6)
(In reply to jiri vanek from comment #5)
(In reply to Severin Gehwolf from comment #3)
Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz Source8: systemtap-tapset-3.6.0pre02.tar.xz
Each of these sources should have a comment preceding them how *exactly* the tarball was generated. I've been asked before by other fedora contributors how our sources are generated. When being asked I mostly don't remember myself and need to go digging. If every source was preceded by a comment where it came from those issues go away. Example:
# Generated by: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk bash
This kind of comment should not be necessary.
It's absolutely necessary.
Those valueas are exactly for this purpose hardcoded in update_package.sh
Yet, nothing mentions "update_package.sh" in Source{0,1,8} comments. So for somebody new to the package, why would they look at update_package.sh? They wouldn't. There is "generate_source_tarball.sh", "generate_tarballs.sh" and "update_package.sh" as auxiliary scripts. Knowing nothing about a specific work-flow one is lost which one to use for which tarball source. Then by the time they've looked at the third script they are giving up trying to figure out the exact parameters one is supposed to invoke scripts with and ask for help. This absolutely needs to become easier to self-discover. Hiding something in extra scripts isn't enough. Remember, the audience is somebody who knows about RPM packaging. The expectation should be to go to the spec file and figure out the rest on their own. That's hard enough for OpenJDK spec files already. We don't need to make it even harder by introducing 3 levels of indirection ;-)
If update_packages.sh satisfies *your* work-flow, then it should be possible to massage that script to output the parameters used to generate a sourcetarball. After that it's a matter of adding that comment.
Fixed. All three scripts needs a bit of tweeking. Will tune them during next update pf sources
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #13 from jiri vanek jvanek@redhat.com --- https://src.fedoraproject.org/rpms/java-openjdk/c/1ae669ee38b50409dd95eb5be6...
Spec URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk.spec SRPM URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk-11.0.ea.19-1...
updated
Hopefully scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27901945
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #14 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #9)
(In reply to Severin Gehwolf from comment #8)
(In reply to Severin Gehwolf from comment #7)
https://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build.
That built successfully.
Of course it does. I told you :)
FYI: An official package review requires for the proposed SRPM to build on at least one architecture. We should at least provide some evidence of that. Sorry, but "I am telling you it builds" isn't good enough as evidence. Hence the above build and comment.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #15 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #10)
Fixed. You have some sript to generate this befor buid? Or from build itslef? Used.
I've used something like this:
$ grep 'lib.*.so' java-11-openjdk.spec \ | grep ^% | cut -d'/' -f4 | sed 's/./[.]/g' \ | sed 's/]so/]so.*/g' > private_libs.txt <verify it looks sane> $ echo $(cat private_libs.txt) | sed 's/ /|/g' libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so.*|libjli[.]so.*|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so.*|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so.*|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so.*|libjaas[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so.*|libjimage[.]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so.*|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so.*|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so.*|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[.]so.*
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #16 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #14)
(In reply to jiri vanek from comment #9)
(In reply to Severin Gehwolf from comment #8)
(In reply to Severin Gehwolf from comment #7)
https://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build.
That built successfully.
Of course it does. I told you :)
FYI: An official package review requires for the proposed SRPM to build on at least one architecture. We should at least provide some evidence of that. Sorry, but "I am telling you it builds" isn't good enough as evidence. Hence the above build and comment.
Yup. I know. I Was not expecting "ready to go" proof of all arches to soon.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #17 from Severin Gehwolf sgehwolf@redhat.com --- # to regenerate source0 and source1(shenandaoh hotspot) run update_package.sh # update_package.sh contains hardcoded repos, revisions, tags, and projects to regenerate the source archives # at the end it sed specfile and sources to match those new names # FIXME adapt the script to work better on shenandoah hotspot (After the jdk10 and removal of forest). Current source1 was done by manual delete # FIXME: adapt the sed to new specfile and sources or drop those parts # next update will be used to tweek those two files Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz
I'm missing a short and concise:
# Use this to generate the source tarball: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk \ # bash generate_source_tarball.sh
# Shenandoah HotSpot # current name used with tip and bleading edge may be incorrect Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz
With JDK 11 I'm doubtful we should continue with the different hotspot for shenandoah arches approach. One single tarball for all arches would be better. We'll discuss this elsewhere. I'd suggest to leave shenandoah out for now and add it back in when the upstream repos are ready.
# Systemtap tapsets. Zipped up to keep it small # Use 'generate_tarballs.sh' to generate the following tarballs # They are based on code contained in the IcedTea7 project # The script have hardcoded url and revision # FIXME discover what exactly is current systemtap from or # FIXME current systemtap is not working, new version is necessary Source8: systemtap-tapset-3.6.0pre02.tar.xz
All of those comments above suggest we don't have a good way to regenerate each tarball. I have a feeling that update_package.sh has too many responsibilities. Ideally there would be one script or one command per tarball doing just that: producing one source tarball in a reproducible way!
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #18 from Severin Gehwolf sgehwolf@redhat.com --- Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed (FE-Legal clarification)
Issues: ======= - Package java-11-openjdk provides "java" and "jre". Intentional? Are we ready for people requiring "java" to get JDK-11? - Some licenses in sources are not listed in "License" field. I'm blocking FE-Legal for a review. Note that I've asked about NTP license here:
https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/... - Please remove no longer needed defattr in %files. See below. - Some descriptions/summaries exceed 79 characters. See:
https://fedoraproject.org/wiki/Common_Rpmlint_issues#description-line-too-lo... Please fix descriptions/summaries. See rpmlint output. - There are many typos in the spec file. Please run: $ hunspell java-11-openjdk.spec and fix them.
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. Note: See rpmlint output. Note from reviewer: Only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory. Verify they are not in ld path. Note from reviewer: Not in ld path. [x]: Package does not contain any libtool archives (.la)
Generic: [ ]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [ ]: License field in the package spec file matches the actual license. Note: licensecheck output file attached. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: Package must own all directories that it creates. [-]: Package does not own files or directories owned by other packages. [x]: %build honors applicable compiler flags or justifies otherwise. Note from reviewer: Relevant compiler flags/linker flags are passed to the OpenJDK build. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: %config files are marked noreplace or the reason is justified. Note: No (noreplace) in %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64/conf/management/jmxremote.password.template %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64/conf/accessibility.properties %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64-slowdebug/conf/management/jmxremote.password.template %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64-slowdebug/conf/accessibility.properties Note from reviewer: This seems OK as those are password templates and properties not expected to be changed by the user. [!]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [!]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package contains icons. Note: icons in java-11-openjdk, java-11-openjdk-slowdebug [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: No %config files under /usr. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
Java: [x]: Javadoc documentation files are generated and included in -javadoc subpackage [x]: Javadoc subpackages should not have Requires: jpackage-utils
Maven: [-]: If package contains pom.xml files install it (including metadata) even when building with ant [x]: Old add_to_maven_depmap macro is not being used
===== SHOULD items =====
Generic: [x]: Uses parallel make %{?_smp_mflags} macro. Note: Uses parallel make via other means and OpenJDK build support. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [!]: Final provides and requires are sane (see attachments). [x]: Fully versioned dependency in subpackages if applicable. [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: Scriptlets must be sane, if used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define buildoutputdir() %{expand:openjdk/build%{?1}}, %define uniquejavadocdir() %{expand:%{fullversion}%{?1}}, %define uniquesuffix() %{expand:%{fullversion}.%{_arch}%{?1}}, %define etcjavadir() %{expand:%{etcjavasubdir}/%{uniquesuffix -- %{?1}}}, %define sdkdir() %{expand:%{uniquesuffix -- %{?1}}}, %define jrelnk() %{expand:jre-%{javaver}-%{origin}-%{version}-%{release}.%{_arch}%{?1}}, %define sdkbindir() %{expand:%{_jvmdir}/%{sdkdir -- %{?1}}/bin}, %define jrebindir() %{expand:%{_jvmdir}/%{sdkdir -- %{?1}}/bin}, %define post_script() %{expand:, %define post_headless() %{expand:, %define postun_script() %{expand:, %define postun_headless() %{expand:, %define posttrans_script() %{expand:, %define post_devel() %{expand:, %define postun_devel() %{expand:, %define posttrans_devel() %{expand:, %define post_javadoc() %{expand:, %define postun_javadoc() %{expand:, %define post_javadoc_zip() %{expand:, %define postun_javadoc_zip() %{expand:, %define files_jre() %{expand:, %define files_jre_headless() %{expand:, %define files_devel() %{expand:, %define files_jmods() %{expand:, %define files_demo() %{expand:, %define files_src() %{expand:, %define files_javadoc() %{expand:, %define files_javadoc_zip() %{expand:, %define java_rpo() %{expand:, %define java_headless_rpo() %{expand:, %define java_devel_rpo() %{expand:, %define java_jmods_rpo() %{expand:, %define java_demo_rpo() %{expand:, %define java_javadoc_rpo() %{expand:, %define java_src_rpo() %{expand: Note from reviewer: These need to be %define as they should be expanded when being used. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: SourceX is a working URL.
Java: [x]: Packages are noarch unless they use JNI Note: java-11-openjdk* packages are arch specific. [x]: Package uses upstream build method (ant/maven/etc.)
===== EXTRA items =====
Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 703918080 bytes in /usr/share java-11-openjdk-javadoc-zip- slowdebug-11.0.ea.19-1.fc28.x86_64.rpm:50954240, java-11-openjdk- javadoc-zip-11.0.ea.19-1.fc28.x86_64.rpm:50954240, java-11-openjdk- javadoc-slowdebug-11.0.ea.19-1.fc28.x86_64.rpm:300718080, java-11 -openjdk-javadoc-11.0.ea.19-1.fc28.x86_64.rpm:300718080 See:
http://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Package_Review_Guid... Note from reviewer: Due to graal and AOT being available only on certain architectures, javadocs are architecture dependent. [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #19 from Severin Gehwolf sgehwolf@redhat.com --- Created attachment 1455277 --> https://bugzilla.redhat.com/attachment.cgi?id=1455277&action=edit Full review.txt with rpmlint output.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #20 from Severin Gehwolf sgehwolf@redhat.com --- Created attachment 1455279 --> https://bugzilla.redhat.com/attachment.cgi?id=1455279&action=edit licensecheck output.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #21 from Severin Gehwolf sgehwolf@redhat.com --- Created attachment 1455281 --> https://bugzilla.redhat.com/attachment.cgi?id=1455281&action=edit Patch with suggested license changes.
This patch needs a legal review.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Severin Gehwolf sgehwolf@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |182235 (FE-Legal)
--- Comment #22 from Severin Gehwolf sgehwolf@redhat.com --- Blocking FE-Legal so as to get input on changes of patch in comment 21.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #23 from Severin Gehwolf sgehwolf@redhat.com --- $ cut -d':' -f2 review-java-11-openjdk/licensecheck.out | sort | uniq Apache GPL (v2) Apache (v2.0) Apache (v2.0) GENERATED FILE BSD (2 clause) BSD (3 clause) BSD (3 clause) GENERATED FILE BSD (3 clause) GPL (v2) BSD (4 clause) CC0 GPL (v2) CDDL Freetype Freetype GENERATED FILE GENERATED FILE GPL (v2) GPL (v2) GENERATED FILE GPL (v2 or later) GPL (v2) (with incorrect FSF address) GPL (v2) (with incorrect FSF address) GENERATED FILE ISC ISC MIT (old) LGPL (v2.1 or later) MIT (CMU, retain warranty disclaimer) MIT (old) MIT/X11 (BSD like) MIT/X11 (BSD like) GPL (v2) *No copyright* Apache (v2.0) *No copyright* Apache (v2.0) GENERATED FILE *No copyright* Apache (v2.0) GPL *No copyright* CC0 *No copyright* GENERATED FILE *No copyright* GPL *No copyright* GPL (v2) *No copyright* MIT/X11 (BSD like) *No copyright* NTP *No copyright* Public domain GPL (v2) *No copyright* Public domain GPL (v2) GENERATED FILE *No copyright* UNKNOWN NTP NTP (legal disclaimer) Public domain GPL (v2) UNKNOWN zlib/libpng zlib/libpng MIT/X11 (BSD like)
All licenses should be accounted for except NTP. See comment 21.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #24 from Severin Gehwolf sgehwolf@redhat.com --- CDDL is from the hotspot ideal graph visualizer: $ grep CDDL review-java-11-openjdk/licensecheck.out /var/lib/mock/fedora-28-x86_64/root/builddir/build/BUILD/java-11-openjdk-11.0.ea.19-1.fc28.x86_64/openjdk/src/utils/IdealGraphVisualizer/View/src/com/sun/hotspot/igv/view/actions/CustomizablePanAction.java: CDDL
That's not part of the binary packages.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@redhat.com Blocks|182235 (FE-Legal) |
--- Comment #25 from Tom "spot" Callaway tcallawa@redhat.com --- Licensing patch looks correct. Lifting FE-Legal.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Severin Gehwolf sgehwolf@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(tcallawa@redhat.c | |om)
--- Comment #26 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to Tom "spot" Callaway from comment #25)
Licensing patch looks correct. Lifting FE-Legal.
Thanks, Tom! Have you seen this? https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/...
Is it OK to use NTP as a licence identifier. It's not listed in the "Good Licenses" list.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #27 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #17)
# to regenerate source0 and source1(shenandaoh hotspot) run update_package.sh # update_package.sh contains hardcoded repos, revisions, tags, and projects to regenerate the source archives # at the end it sed specfile and sources to match those new names # FIXME adapt the script to work better on shenandoah hotspot (After the jdk10 and removal of forest). Current source1 was done by manual delete # FIXME: adapt the sed to new specfile and sources or drop those parts # next update will be used to tweek those two files Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz
I'm missing a short and concise:
# Use this to generate the source tarball: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk \ # bash generate_source_tarball.sh
Yes because I disagree with that approach. Those flags do not have sense without wider context. Thats why I wont to tune update_package.sh to be more straightforward and not doing redundant things.
# Shenandoah HotSpot # current name used with tip and bleading edge may be incorrect Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz
With JDK 11 I'm doubtful we should continue with the different hotspot for shenandoah arches approach. One single tarball for all arches would be better. We'll discuss this elsewhere. I'd suggest to leave shenandoah out for now and add it back in when the upstream repos are ready.
Yup. I have noticed your conversation. It would be really nice to have only one source tarball. Still we agreed to have shenndaoh in LTS versions. What is the purpose of not having it on since start? What is the moment to add it? Forking? Will Shenandoah team be her i time? I would match rather keep Shenandoah hotspot for 64b intel and arm, even on tip, rather then waiting for some moment i future to add it. By having it in will allow me to track it, and to ping Shenandoah team in time.
# Systemtap tapsets. Zipped up to keep it small # Use 'generate_tarballs.sh' to generate the following tarballs # They are based on code contained in the IcedTea7 project # The script have hardcoded url and revision # FIXME discover what exactly is current systemtap from or # FIXME current systemtap is not working, new version is necessary Source8: systemtap-tapset-3.6.0pre02.tar.xz
All of those comments above suggest we don't have a good way to regenerate
Nope, the regeneration of tapset is deterministic, but was not done properly in one moment. IIRC, there was issue found and several chaotic pulls without updating the clonning file.
each tarball. I have a feeling that update_package.sh has too many responsibilities. Ideally there would be one script or one command per tarball doing just that: producing one source tarball in a reproducible way!
So the steps I wont t do: - rename generate_tarballs.sh to regenerate_systemtap.sh + tune the script to point to current valid locations + get rid of the icon and desktop file as ithave nothing common now + we need to agree on systemtap versioning and releaseing (see stalled thread on java-team) - rename generate_source_tarball.sh to generate_openjdk_tarball.sh + keep it as it is - modidy update_package.sh + maybe rename it to generate-sources.sh + simplyfy its logic. ++ stop touching specfile ++ stop touching sources + generate also ystemtap (if necessary) and so on - its reason really si to replace comment you suggest, by pushed, right-away for use script.
Changes to update_package I would like to keep for next update of sources
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #28 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #23)
$ cut -d':' -f2 review-java-11-openjdk/licensecheck.out | sort | uniq Apache GPL (v2) Apache (v2.0) Apache (v2.0) GENERATED FILE BSD (2 clause) BSD (3 clause) BSD (3 clause) GENERATED FILE BSD (3 clause) GPL (v2) BSD (4 clause) CC0 GPL (v2) CDDL Freetype Freetype GENERATED FILE GENERATED FILE GPL (v2) GPL (v2) GENERATED FILE GPL (v2 or later) GPL (v2) (with incorrect FSF address) GPL (v2) (with incorrect FSF address) GENERATED FILE ISC ISC MIT (old) LGPL (v2.1 or later) MIT (CMU, retain warranty disclaimer) MIT (old) MIT/X11 (BSD like) MIT/X11 (BSD like) GPL (v2) *No copyright* Apache (v2.0) *No copyright* Apache (v2.0) GENERATED FILE *No copyright* Apache (v2.0) GPL *No copyright* CC0 *No copyright* GENERATED FILE *No copyright* GPL *No copyright* GPL (v2) *No copyright* MIT/X11 (BSD like) *No copyright* NTP *No copyright* Public domain GPL (v2) *No copyright* Public domain GPL (v2) GENERATED FILE *No copyright* UNKNOWN NTP NTP (legal disclaimer) Public domain GPL (v2) UNKNOWN zlib/libpng zlib/libpng MIT/X11 (BSD like)
All licenses should be accounted for except NTP. See comment 21.
This was the state for lder JDK8 and jdk7 before. Gnu_andrew changed those to current state in rhel, and his arguemntatnion on that is pretty good.
Area you sure you wont blindly add all from those list?
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #29 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #27)
(In reply to Severin Gehwolf from comment #17)
# to regenerate source0 and source1(shenandaoh hotspot) run update_package.sh # update_package.sh contains hardcoded repos, revisions, tags, and projects to regenerate the source archives # at the end it sed specfile and sources to match those new names # FIXME adapt the script to work better on shenandoah hotspot (After the jdk10 and removal of forest). Current source1 was done by manual delete # FIXME: adapt the sed to new specfile and sources or drop those parts # next update will be used to tweek those two files Source0: jdk-jdk-jdk-%{majorver}+%{buildver}.tar.xz
I'm missing a short and concise:
# Use this to generate the source tarball: # $ VERSION="jdk-11+19" PROJECT_NAME=jdk REPO_NAME=jdk \ # bash generate_source_tarball.sh
Yes because I disagree with that approach. Those flags do not have sense without wider context. Thats why I wont to tune update_package.sh to be more straightforward and not doing redundant things.
# Shenandoah HotSpot # current name used with tip and bleading edge may be incorrect Source1: jdk-shenandoah-jdk-ac148db384ee.tar.xz
With JDK 11 I'm doubtful we should continue with the different hotspot for shenandoah arches approach. One single tarball for all arches would be better. We'll discuss this elsewhere. I'd suggest to leave shenandoah out for now and add it back in when the upstream repos are ready.
Yup. I have noticed your conversation. It would be really nice to have only one source tarball. Still we agreed to have shenndaoh in LTS versions. What is the purpose of not having it on since start? What is the moment to add it? Forking? Will Shenandoah team be her i time? I would match rather keep Shenandoah hotspot for 64b intel and arm, even on tip, rather then waiting for some moment i future to add it. By having it in will allow me to track it, and to ping Shenandoah team in time.
It seems too risky to keep this without by-in from Shenandoah folks. This has the potential to break x86_64 and aarch64 in strange ways.
# Systemtap tapsets. Zipped up to keep it small # Use 'generate_tarballs.sh' to generate the following tarballs # They are based on code contained in the IcedTea7 project # The script have hardcoded url and revision # FIXME discover what exactly is current systemtap from or # FIXME current systemtap is not working, new version is necessary Source8: systemtap-tapset-3.6.0pre02.tar.xz
All of those comments above suggest we don't have a good way to regenerate
Nope, the regeneration of tapset is deterministic, but was not done properly in one moment. IIRC, there was issue found and several chaotic pulls without updating the clonning file.
each tarball. I have a feeling that update_package.sh has too many responsibilities. Ideally there would be one script or one command per tarball doing just that: producing one source tarball in a reproducible way!
So the steps I wont t do:
- rename generate_tarballs.sh to regenerate_systemtap.sh
- tune the script to point to current valid locations
- get rid of the icon and desktop file as ithave nothing common now
- we need to agree on systemtap versioning and releaseing (see stalled
thread on java-team)
- rename generate_source_tarball.sh to generate_openjdk_tarball.sh
- keep it as it is
- modidy update_package.sh
++ stop touching specfile ++ stop touching sources
- maybe rename it to generate-sources.sh
- simplyfy its logic.
HUGE +1
- generate also ystemtap (if necessary)
and so on
- its reason really si to replace comment you suggest, by pushed,
right-away for use script.
OK. Those are reasonable steps. Just to be clear on the expectations, which I'll make a requirement for this review to pass:
- For each source tarball there need to be clear instructions as to how to generate it. That includes all needed parameters to get the exact same tarball (reproducible tarballs) in a comment. I'll be testing it for each tarball individually. - There need to be switches to the script to just generate one tarball or alternatively document steps as mentioned above, VERSION="jdk-11+19"PROJECT_NAME=jdk REPO_NAME=jdk bash generate_source...,
Changes to update_package I would like to keep for next update of sources
Like I said, if you are fond of update_package.sh, then this needs to get fixed immediately (or it'll never get fixed).
FWIW, with mono-repository these would also be possible instructions for the main tarball:
$ wget http://hg.openjdk.java.net/jdk/jdk/archive/jdk-11+19.tar.bz2 $ tar -xf jdk-jdk-11+19.tar.bz2 $ cd jdk-jdk-11+19 $ rm -rf src/jdk.crypto.ec/share/native/libsunec/impl $ patch -Np1 < %{SOURCEX} $ cd .. $ tar -cJf jdk-jdk-11+19.tar.xz jdk-jdk-11+19
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #30 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #28)
(In reply to Severin Gehwolf from comment #23)
$ cut -d':' -f2 review-java-11-openjdk/licensecheck.out | sort | uniq Apache GPL (v2) Apache (v2.0) Apache (v2.0) GENERATED FILE BSD (2 clause) BSD (3 clause) BSD (3 clause) GENERATED FILE BSD (3 clause) GPL (v2) BSD (4 clause) CC0 GPL (v2) CDDL Freetype Freetype GENERATED FILE GENERATED FILE GPL (v2) GPL (v2) GENERATED FILE GPL (v2 or later) GPL (v2) (with incorrect FSF address) GPL (v2) (with incorrect FSF address) GENERATED FILE ISC ISC MIT (old) LGPL (v2.1 or later) MIT (CMU, retain warranty disclaimer) MIT (old) MIT/X11 (BSD like) MIT/X11 (BSD like) GPL (v2) *No copyright* Apache (v2.0) *No copyright* Apache (v2.0) GENERATED FILE *No copyright* Apache (v2.0) GPL *No copyright* CC0 *No copyright* GENERATED FILE *No copyright* GPL *No copyright* GPL (v2) *No copyright* MIT/X11 (BSD like) *No copyright* NTP *No copyright* Public domain GPL (v2) *No copyright* Public domain GPL (v2) GENERATED FILE *No copyright* UNKNOWN NTP NTP (legal disclaimer) Public domain GPL (v2) UNKNOWN zlib/libpng zlib/libpng MIT/X11 (BSD like)
All licenses should be accounted for except NTP. See comment 21.
This was the state for lder JDK8 and jdk7 before. Gnu_andrew changed those to current state in rhel, and his arguemntatnion on that is pretty good.
Well, this is JDK 11, not JDK 8 or 7 :) No more Java EE and corba in JDK 11: http://openjdk.java.net/jeps/320, harfbuzz has been added: http://openjdk.java.net/jeps/258, freetype sources have been added (yet, we don't build them).
Area you sure you wont blindly add all from those list?
No, not blindly. This was in reference to the suggested patch in comment 21 (which should be added once we have NTP clarification). I.e. the current license list as specified by "License" in the spec accounts for all of the above except for NTP. Hence, my email to the legal list for clarification. A reviewer is supposed to check licenses in sources. I've run licensecheck on them and this came of it. Then I've manually verified suspicious items. Does that clear things up?
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #31 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #18)
Package Review
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed (FE-Legal clarification)
Issues:
- Package java-11-openjdk provides "java" and "jre". Intentional? Are we ready for people requiring "java" to get JDK-11?
For jre, yes, for sdk, no. This is aligned with java-openjdk, and up to now no issue caused. The versionless sdk provides are the onse which have weight, and those are not here - search for commented out provides which makes the crucial differences.
From other part, maybe you will recall the issue with to much commented out provides when we were adding jdk7 to rhel6. That was quite disaster.
- Some licenses in sources are not listed in "License" field. I'm blocking FE-Legal for a review. Note that I've asked about NTP license here:
Ok. this will be fixed by your patch as you write above
https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/ thread/2QXHMTZ47DMMARJVI6PUMSYUPVFAGLCV/
- Please remove no longer needed defattr in %files. See below.
- Some descriptions/summaries exceed 79 characters. See:
This is so evil law... Its 21century. 99% monitors have width of 160 chars aat *minimum*. Will be fixed by best effort
https://fedoraproject.org/wiki/Common_Rpmlint_issues#description-line-too- long Please fix descriptions/summaries. See rpmlint output.
- There are many typos in the spec file. Please run: $ hunspell java-11-openjdk.spec and fix them.
Funny. Manual correction by three people stay its place during java-openjdk review.... Running it now.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #32 from jiri vanek jvanek@redhat.com ---
It seems too risky to keep this without by-in from Shenandoah folks. This has the potential to break x86_64 and aarch64 in strange ways.
Are they really out? I'm really afraid of leaving (again) behind. Can we keep it in untill the review is finished, so all is done with it in mind? If the shenandoah repo is still not accptable at that time, Then I will bow and remove it. Hmm?
So the steps I wont t do:
- rename generate_tarballs.sh to regenerate_systemtap.sh
- tune the script to point to current valid locations
- get rid of the icon and desktop file as ithave nothing common now
- we need to agree on systemtap versioning and releaseing (see stalled
thread on java-team)
- rename generate_source_tarball.sh to generate_openjdk_tarball.sh
- keep it as it is
- modidy update_package.sh
++ stop touching specfile ++ stop touching sources
- maybe rename it to generate-sources.sh
- simplyfy its logic.
++ prit a bit of what is ahppening
HUGE +1
Thanx for adding courage :)
- generate also systemtap (if necessary)
and so on
- its reason really si to replace comment you suggest, by pushed,
right-away for use script.
OK. Those are reasonable steps. Just to be clear on the expectations, which I'll make a requirement for this review to pass:
- For each source tarball there need to be clear instructions as to how to generate it. That includes all needed parameters to get the exact same tarball (reproducible tarballs) in a comment. I'll be testing it for each tarball individually.
- There need to be switches to the script to just generate one tarball or alternatively document steps as mentioned above, VERSION="jdk-11+19"PROJECT_NAME=jdk REPO_NAME=jdk bash generate_source...,
zzzz. I can survive comment saying what envrionmet variable to set to what :(
Changes to update_package I would like to keep for next update of sources
Like I said, if you are fond of update_package.sh, then this needs to get fixed immediately (or it'll never get fixed).
ok.
It should not be my workflow (although it grown into it), it should be genereic helper. If it do not serve that, then it should be removed. Still I like it *much more* then comment with "VERSION=..."
FWIW, with mono-repository these would also be possible instructions for the main tarball:
$ wget http://hg.openjdk.java.net/jdk/jdk/archive/jdk-11+19.tar.bz2 $ tar -xf jdk-jdk-11+19.tar.bz2 $ cd jdk-jdk-11+19 $ rm -rf src/jdk.crypto.ec/share/native/libsunec/impl $ patch -Np1 < %{SOURCEX} $ cd .. $ tar -cJf jdk-jdk-11+19.tar.xz jdk-jdk-11+19
The monolitic repo have huge performance problems when cloned. If this will be faster I will swithc to it. Thanx for reminding this approach.
btw - you highligh the reproducible sources - you mean after unpack, right? Or do you wont to tkae similar approach as https://src.fedoraproject.org/cgit/rpms/java-1.8.0-openjdk.git/tree/repackRe... did? Is it even possible with tar.xz?
Thanx for review!
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Severin Gehwolf sgehwolf@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment|0 |1 #1455281 is| | obsolete| | Flags|needinfo?(tcallawa@redhat.c | |om) |
--- Comment #33 from Severin Gehwolf sgehwolf@redhat.com --- Created attachment 1455380 --> https://bugzilla.redhat.com/attachment.cgi?id=1455380&action=edit Patch with suggested license changes.
Updated license change patch in-line with fedora legal feedback.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #34 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #32)
It seems too risky to keep this without by-in from Shenandoah folks. This has the potential to break x86_64 and aarch64 in strange ways.
Are they really out? I'm really afraid of leaving (again) behind. Can we keep it in untill the review is finished, so all is done with it in mind? If the shenandoah repo is still not accptable at that time, Then I will bow and remove it. Hmm?
It's been suggested that http://hg.openjdk.java.net/shenandoah/jdk is the Shenandoah dev forest and we should not be using it. We should be using http://hg.openjdk.java.net/shenandoah/jdk11 once jdk/jdk11 has been forked. Either way, we'll be changing sources in an update:
jdk/jdk => jdk/jdk11
or
jdk/jdk => shenandoah/jdk11
We should just use jdk/jdk now for all arches and move to shenandoah/jdk11 once it exists.
There shouldn't be any shenandoah specific things in the spec file as far as I understand it.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #35 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #10)
(In reply to Severin Gehwolf from comment #1)
This:
%global _privatelibs libjsoundalsa[.]so.*|libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so. *|libjli[.]so.*|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so. *|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so. *|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so. *|libjaas_unix[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so. *|libjimage[.]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so. *|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so. *|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so. *|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[. ]so.*|lib[.]so\(SUNWprivate_.*
Should be this:
%global _privatelibs libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjawt[.]so.*|libjli[.]so. *|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libjsig[.]so. *|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so. *|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so. *|libjaas[.]so.*|libjava[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so.*|libjimage[. ]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so. *|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so. *|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so. *|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libverify[.]so.*|libzip[. ]so.*
lib.so provides isn't generated for JDK 10+, libjaas_unix.so is no longer in JDK 11, etc.
Fixed. You have some sript to generate this befor buid? Or from build itslef? Used.
According to: https://bugzilla.redhat.com/show_bug.cgi?id=1590796#c14
this needs to change to the following in order to not break dependencies for other packages:
libsplashscreen[.]so.*|libawt_xawt[.]so.*|libjli[.]so.*|libattach[.]so.*|libawt[.]so.*|libextnet[.]so.*|libawt_headless[.]so.*|libdt_socket[.]so.*|libfontmanager[.]so.*|libinstrument[.]so.*|libj2gss[.]so.*|libj2pcsc[.]so.*|libj2pkcs11[.]so.*|libjaas[.]so.*|libjavajpeg[.]so.*|libjdwp[.]so.*|libjimage[.]so.*|libjsound[.]so.*|liblcms[.]so.*|libmanagement[.]so.*|libmanagement_agent[.]so.*|libmanagement_ext[.]so.*|libmlib_image[.]so.*|libnet[.]so.*|libnio[.]so.*|libprefs[.]so.*|librmi[.]so.*|libsaproc[.]so.*|libsctp[.]so.*|libsunec[.]so.*|libunpack[.]so.*|libzip[.]so.*
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #36 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to Severin Gehwolf from comment #18)
Package Review
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
Issues:
[...]
- Some licenses in sources are not listed in "License" field.
Please apply patch https://bugzilla.redhat.com/attachment.cgi?id=1455380 to fix this.
[...]
Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. Note: licensecheck output file attached.
[...]
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #37 from jiri vanek jvanek@redhat.com --- jdk11 havebeen forked.
It's been suggested that http://hg.openjdk.java.net/shenandoah/jdk is the Shenandoah dev forest and we should not be using it. We should be using http://hg.openjdk.java.net/shenandoah/jdk11 once jdk/jdk11 has been forked. Either way, we'll be changing sources in an update:
jdk/jdk => jdk/jdk11
or
jdk/jdk => shenandoah/jdk11
Shenadoah was not. I guess it will anytime soon. Once forked, is it ok enough tobe used?
We should just use jdk/jdk now for all arches and move to shenandoah/jdk11 once it exists.
There shouldn't be any shenandoah specific things in the spec file as far as I understand it.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #38 from jiri vanek jvanek@redhat.com ---
$ wget http://hg.openjdk.java.net/jdk/jdk/archive/jdk-11+19.tar.bz2 $ tar -xf jdk-jdk-11+19.tar.bz2 $ cd jdk-jdk-11+19 $ rm -rf src/jdk.crypto.ec/share/native/libsunec/impl $ patch -Np1 < %{SOURCEX} $ cd .. $ tar -cJf jdk-jdk-11+19.tar.xz jdk-jdk-11+19
The script is known to be used on local repos, so the clone will be kept.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #39 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #37)
jdk11 havebeen forked.
It's been suggested that http://hg.openjdk.java.net/shenandoah/jdk is the Shenandoah dev forest and we should not be using it. We should be using http://hg.openjdk.java.net/shenandoah/jdk11 once jdk/jdk11 has been forked. Either way, we'll be changing sources in an update:
jdk/jdk => jdk/jdk11
or
jdk/jdk => shenandoah/jdk11
Shenadoah was not. I guess it will anytime soon. Once forked, is it ok enough tobe used?
Once shenandoah/jdk11 exists, we should be sure equivalent jdk-11+BB tags exist and then we should use that tree on all arches. It's my understanding that shenandoah support will be built in by default on all arches that support it and left out on arches which don't.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #40 from jiri vanek jvanek@redhat.com --- All issues you pickd up should be fixed. Thank you for your review! Spec URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk.spec SRPM URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk-11.0.ea.20-1... Fedora Account System Username: jvanek
Work repo - https://src.fedoraproject.org/rpms/java-openjdk/tree/java-11-openjdk updated
Scracth build: https://koji.fedoraproject.org/koji/taskinfo?taskID=28070907 As it is based on +20 of shenandoah project My local build on shenandoah-arch keep running in time of this posting
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #41 from jiri vanek jvanek@redhat.com ---
Scracth build: https://koji.fedoraproject.org/koji/taskinfo?taskID=28070907 As it is based on +20 of shenandoah project My local build on shenandoah-arch keep running in time of this posting
Failed in PRM files part: https://src.fedoraproject.org/rpms/java-openjdk/c/a8520e7d109cdd5bb99159ac9d...
New one: https://koji.fedoraproject.org/koji/taskinfo?taskID=28091196
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #42 from Severin Gehwolf sgehwolf@redhat.com --- Please also add this to the License comment clarifying public_suffix_list.dat:
diff --git a/java-11-openjdk.spec b/java-11-openjdk.spec index 5ca02ed..c50a311 100644 --- a/java-11-openjdk.spec +++ b/java-11-openjdk.spec @@ -861,6 +861,7 @@ Group: Development/Languages # - JPEG library (IJG), zlib & libpng (zlib), giflib (MIT), harfbuzz (ISC), # - freetype (FTL), jline (BSD) and LCMS (MIT) # - jquery (MIT), jdk.crypto.cryptoki PKCS 11 wrapper (RSA) +# - public_suffix_list.dat from publicsuffix.org (MPLv2.0) # The test code includes copies of NSS under the Mozilla Public License v2.0 # The PCSClite headers are under a BSD with advertising license # The elliptic curve cryptography (ECC) source code is licensed under the LGPLv2.1 or any later version
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #43 from jiri vanek jvanek@redhat.com --- Done: https://src.fedoraproject.org/rpms/java-openjdk/c/58db6624a43260e79c79b87f7f...
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #44 from Severin Gehwolf sgehwolf@redhat.com --- ---------------------------- # Temporarily disable slowdebug build for Aarch64 since it does not # bootcycle. See JDK-8204331. %ifnarch %{arm} %{aarch64} %global include_debug_build 1 %else ----------------------------
JDK-8204331 is fixed for jdk11+20. Please re-enable the slowdebug build for aarch64 again. Thanks.
---------------------------- [!]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed ----------------------------
Not yet fixed.
---------------------------- # remove redundant *diz and *debuginfo files find images/%{jdkimage} -iname '*.diz' -exec rm {} ; find images/%{jdkimage} -iname '*.debuginfo' -exec rm {} ; ----------------------------
---^ This is a no-op on JDK 11. Seems to stem from JDK 8 times. Please remove.
---------------------------- # FIXME: remove SONAME entries from demo DSOs. See # https://bugzilla.redhat.com/show_bug.cgi?id=436497 ----------------------------
I believe this is fixed with private_libs regexp. Please remove this comment.
---------------------------- - simplified and celared update_package.sh ----------------------------
Typo: cleared not celared.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #45 from Severin Gehwolf sgehwolf@redhat.com --- Isn't Shenandoah expected to be present? At least on x86_64? Seems not (from the scratch build)
<mock-chroot> sh-4.4# /usr/lib/jvm/java-11-openjdk/bin/java -version openjdk version "11-ea" 2018-09-25 OpenJDK Runtime Environment (build 11-ea+20) OpenJDK 64-Bit Server VM (build 11-ea+20, mixed mode, sharing) <mock-chroot> sh-4.4# /usr/lib/jvm/java-11-openjdk/bin/java -XX:+UseShenandoahGC -version Unrecognized VM option 'UseShenandoahGC' Error: Could not create the Java Virtual Machine. Error: A fatal exception has occurred. Program will exit.
I'd suggest to add a test like this to %check - for x86_64 and aarch64 - so that we don't regress on this unexpectedly.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #46 from jiri vanek jvanek@redhat.com ---
[!]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed
Not yet fixed.
Sorry. was forgotten unintentionally. Will fix it, and elaborate on all other hints. Tahnx!
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #47 from jiri vanek jvanek@redhat.com --- All new issues should be fixed. Thank you for your review! Spec URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk.spec SRPM URL: https://jvanek.fedorapeople.org/java-11-openjdk/java-11-openjdk-11.0.ea.20-1... Fedora Account System Username: jvanek
Work repo - https://src.fedoraproject.org/rpms/java-openjdk/tree/java-11-openjdk updated
Scracth build: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096115 My local build on shenandoah-arch passed
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #48 from Severin Gehwolf sgehwolf@redhat.com --- -# Temporarily disable slowdebug build for Aarch64 since it does not -# bootcycle. See JDK-8204331. -%ifnarch %{arm} %{aarch64} %global include_debug_build 1 %else %global include_debug_build 0 @@ -69,9 +66,6 @@ %else %global include_debug_build 0 %endif -%else -%global include_debug_build 0 -%endif
--^ This will enable slowdebug builds on ARM 32 bit, which was previously disabled since it takes a very long time to build. What you perhaps wanted was to revert the first hunk of this: https://src.fedoraproject.org/rpms/java-openjdk/raw/bc90ba687ed9f3e0f6b3948f...
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #49 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #48)
-# Temporarily disable slowdebug build for Aarch64 since it does not -# bootcycle. See JDK-8204331. -%ifnarch %{arm} %{aarch64} %global include_debug_build 1 %else %global include_debug_build 0 @@ -69,9 +66,6 @@ %else %global include_debug_build 0 %endif -%else -%global include_debug_build 0 -%endif
--^ This will enable slowdebug builds on ARM 32 bit, which was previously disabled since it takes a very long time to build. What you perhaps wanted was to revert the first hunk of this: https://src.fedoraproject.org/rpms/java-openjdk/raw/ bc90ba687ed9f3e0f6b3948f90ce0682ccef4bd3
Facepalm
// lets measures how long that scratch will take...
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #50 from jiri vanek jvanek@redhat.com --- spec and srpm updated
new scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096637
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #51 from Severin Gehwolf sgehwolf@redhat.com --- +# Temporarily disable slowdebug build for Aarch64 since it does not +# bootcycle. See JDK-8204331. +%ifnarch %{arm} %global include_debug_build 1 %else
Now that comment is wrong. Please just remove it.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #52 from jiri vanek jvanek@redhat.com --- Sorry. Done
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #53 from Severin Gehwolf sgehwolf@redhat.com --- (In reply to jiri vanek from comment #50)
spec and srpm updated
new scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096637
As to the s390x failure I've been told that s390x builders moved to new hardware and some builds failed due to that. I think it's safe to ignore this one. The aarch64 failure is real, though :(
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #54 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #53)
(In reply to jiri vanek from comment #50)
spec and srpm updated
new scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096637
As to the s390x failure I've been told that s390x builders moved to new
Oh. I see. I thought it was cancelled after aarch64 failed. Thanx.
hardware and some builds failed due to that. I think it's safe to ignore this one. The aarch64 failure is real, though :(
Yup. Developers pinged.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #55 from Severin Gehwolf sgehwolf@redhat.com --- Looks good to me. The AArch 64 build failure is nothing the package review can change.
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. Note: See rpmlint output. Note from reviewer: Only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory. Verify they are not in ld path. Note from reviewer: Not in ld path. [x]: Package does not contain any libtool archives (.la)
Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: licensecheck output file attached. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: Package must own all directories that it creates. [-]: Package does not own files or directories owned by other packages. [x]: %build honors applicable compiler flags or justifies otherwise. Note from reviewer: Relevant compiler flags/linker flags are passed to the OpenJDK build. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: %config files are marked noreplace or the reason is justified. Note: No (noreplace) in %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64/conf/management/jmxremote.password.template %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64/conf/accessibility.properties %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64-slowdebug/conf/management/jmxremote.password.template %config
/etc/java/java-11-openjdk/java-11-openjdk-11.0.ea.19-1.fc28.x86_64-slowdebug/conf/accessibility.properties Note from reviewer: This seems OK as those are password templates and properties not expected to be changed by the user. [x]: Each %files section contains %defattr if rpm < 4.4 [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package contains icons. Note: icons in java-11-openjdk, java-11-openjdk-slowdebug [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: No %config files under /usr. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
Java: [x]: Javadoc documentation files are generated and included in -javadoc subpackage [x]: Javadoc subpackages should not have Requires: jpackage-utils
Maven: [-]: If package contains pom.xml files install it (including metadata) even when building with ant [x]: Old add_to_maven_depmap macro is not being used
===== SHOULD items =====
Generic: [x]: Uses parallel make %{?_smp_mflags} macro. Note: Uses parallel make via other means and OpenJDK build support. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane. [x]: Fully versioned dependency in subpackages if applicable. [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: Scriptlets must be sane, if used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define buildoutputdir() %{expand:openjdk/build%{?1}}, %define uniquejavadocdir() %{expand:%{fullversion}%{?1}}, %define uniquesuffix() %{expand:%{fullversion}.%{_arch}%{?1}}, %define etcjavadir() %{expand:%{etcjavasubdir}/%{uniquesuffix -- %{?1}}}, %define sdkdir() %{expand:%{uniquesuffix -- %{?1}}}, %define jrelnk() %{expand:jre-%{javaver}-%{origin}-%{version}-%{release}.%{_arch}%{?1}}, %define sdkbindir() %{expand:%{_jvmdir}/%{sdkdir -- %{?1}}/bin}, %define jrebindir() %{expand:%{_jvmdir}/%{sdkdir -- %{?1}}/bin}, %define post_script() %{expand:, %define post_headless() %{expand:, %define postun_script() %{expand:, %define postun_headless() %{expand:, %define posttrans_script() %{expand:, %define post_devel() %{expand:, %define postun_devel() %{expand:, %define posttrans_devel() %{expand:, %define post_javadoc() %{expand:, %define postun_javadoc() %{expand:, %define post_javadoc_zip() %{expand:, %define postun_javadoc_zip() %{expand:, %define files_jre() %{expand:, %define files_jre_headless() %{expand:, %define files_devel() %{expand:, %define files_jmods() %{expand:, %define files_demo() %{expand:, %define files_src() %{expand:, %define files_javadoc() %{expand:, %define files_javadoc_zip() %{expand:, %define java_rpo() %{expand:, %define java_headless_rpo() %{expand:, %define java_devel_rpo() %{expand:, %define java_jmods_rpo() %{expand:, %define java_demo_rpo() %{expand:, %define java_javadoc_rpo() %{expand:, %define java_src_rpo() %{expand: Note from reviewer: These need to be %define as they should be expanded when being used. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: SourceX is a working URL.
Java: [x]: Packages are noarch unless they use JNI Note: java-11-openjdk* packages are arch specific. [x]: Package uses upstream build method (ant/maven/etc.)
===== EXTRA items =====
Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 703918080 bytes in /usr/share java-11-openjdk-javadoc-zip- slowdebug-11.0.ea.19-1.fc28.x86_64.rpm:50954240, java-11-openjdk- javadoc-zip-11.0.ea.19-1.fc28.x86_64.rpm:50954240, java-11-openjdk- javadoc-slowdebug-11.0.ea.19-1.fc28.x86_64.rpm:300718080, java-11 -openjdk-javadoc-11.0.ea.19-1.fc28.x86_64.rpm:300718080 See:
http://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Package_Review_Guid... Note from reviewer: Due to graal and AOT being available only on certain architectures, javadocs are architecture dependent. [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Severin Gehwolf jerboaa@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jerboaa@gmail.com Flags| |fedora-review+
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #56 from jiri vanek jvanek@redhat.com --- Thanx
https://pagure.io/releng/fedora-scm-requests/issue/7347
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
jiri vanek jvanek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #57 from jiri vanek jvanek@redhat.com --- f27+f28 branches https://pagure.io/releng/fedora-scm-requests/issue/7348 https://pagure.io/releng/fedora-scm-requests/issue/7349
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #58 from jiri vanek jvanek@redhat.com --- (In reply to Severin Gehwolf from comment #53)
(In reply to jiri vanek from comment #50)
spec and srpm updated
new scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096637
As to the s390x failure I've been told that s390x builders moved to new hardware and some builds failed due to that. I think it's safe to ignore this one. The aarch64 failure is real, though :(
https://koji.fedoraproject.org/koji/taskinfo?taskID=28130952
with changes to hopefully fix aarch64
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #59 from jiri vanek jvanek@redhat.com --- hi Sewerin!
Acording to "https://pagure.io/releng/fedora-scm-requests/issue/7347" : The review is not approved by the assignee of the Bugzilla bug
Somebody (obviously) do not like your double account. Probably you have to reassign to yourself, to your second account.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Severin Gehwolf jerboaa@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|sgehwolf@redhat.com |jerboaa@gmail.com
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #60 from jiri vanek jvanek@redhat.com --- (In reply to jiri vanek from comment #58)
(In reply to Severin Gehwolf from comment #53)
(In reply to jiri vanek from comment #50)
spec and srpm updated
new scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096637
As to the s390x failure I've been told that s390x builders moved to new hardware and some builds failed due to that. I think it's safe to ignore this one. The aarch64 failure is real, though :(
https://koji.fedoraproject.org/koji/taskinfo?taskID=28130952
with changes to hopefully fix aarch64
green.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #61 from Severin Gehwolf jerboaa@gmail.com --- The review above is valid. I'm the same person. However, only this account has Fedora related groups so that I can set fedora_review+.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #62 from Mohan Boddu mboddu@bhujji.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/java-11-openjdk
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #63 from jiri vanek jvanek@redhat.com --- Package is mostly built. Waiting for s390x or ppc64 on mostof the builds. Will do updates asap
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #64 from Fedora Update System updates@fedoraproject.org --- java-11-openjdk-11.0.ea.22-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-baca028a2f
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #65 from Fedora Update System updates@fedoraproject.org --- java-11-openjdk-11.0.ea.22-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-18ba63a547
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #66 from Fedora Update System updates@fedoraproject.org --- java-11-openjdk-11.0.ea.22-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-18ba63a547
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #67 from Fedora Update System updates@fedoraproject.org --- java-11-openjdk-11.0.ea.22-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-baca028a2f
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2018-07-30 13:24:09
--- Comment #68 from Fedora Update System updates@fedoraproject.org --- java-11-openjdk-11.0.ea.22-1.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1594313
--- Comment #69 from Fedora Update System updates@fedoraproject.org --- java-11-openjdk-11.0.ea.22-1.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org