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.fc28.src.rpm Description: https://src.fedoraproject.org/rpms/java-openjdk/blob/java-11-openjdk/f/README.md 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
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.
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).
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 [...]
(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.
(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!
(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://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build.
(In reply to Severin Gehwolf from comment #7) > https://koji.fedoraproject.org/koji/taskinfo?taskID=27877911 scratch build. That built successfully.
(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!
(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.
(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
(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://src.fedoraproject.org/rpms/java-openjdk/c/1ae669ee38b50409dd95eb5be6c57066a0986fbd?branch=java-11-openjdk 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.fc28.src.rpm updated Hopefully scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27901945
(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.
(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.*
(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.
# 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!
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/thread/2QXHMTZ47DMMARJVI6PUMSYUPVFAGLCV/ - 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-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. ===== 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_Guidelines 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.
Created attachment 1455277 [details] Full review.txt with rpmlint output.
Created attachment 1455279 [details] licensecheck output.
Created attachment 1455281 [details] Patch with suggested license changes. This patch needs a legal review.
Blocking FE-Legal so as to get input on changes of patch in comment 21.
$ 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.
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.
Licensing patch looks correct. Lifting FE-Legal.
(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/thread/2QXHMTZ47DMMARJVI6PUMSYUPVFAGLCV/ Is it OK to use NTP as a licence identifier. It's not listed in the "Good Licenses" list.
(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
(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?
(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 > + maybe rename it to generate-sources.sh > + simplyfy its logic. > ++ stop touching specfile > ++ stop touching sources 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
(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?
(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.
> 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 > > + maybe rename it to generate-sources.sh > > + simplyfy its logic. > > ++ stop touching specfile > > ++ stop touching sources ++ 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/repackReproduciblePolycies.sh?h=f28 did? Is it even possible with tar.xz? Thanx for review!
Created attachment 1455380 [details] Patch with suggested license changes. Updated license change patch in-line with fedora legal feedback.
(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.
(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.*
(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. [...]
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.
> > $ 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.
(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.
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.fc28.src.rpm 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
> 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/a8520e7d109cdd5bb99159ac9dac6248f9d018f2?branch=java-11-openjdk New one: https://koji.fedoraproject.org/koji/taskinfo?taskID=28091196
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
Done: https://src.fedoraproject.org/rpms/java-openjdk/c/58db6624a43260e79c79b87f7fe19d73ab0954df?branch=java-11-openjdk
---------------------------- # 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.
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.
> ---------------------------- > [!]: 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!
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.fc28.src.rpm 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
-# 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
(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...
spec and srpm updated new scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=28096637
+# 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.
Sorry. Done
(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 :(
(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.
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_Guidelines 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.
Thanx https://pagure.io/releng/fedora-scm-requests/issue/7347
f27+f28 branches https://pagure.io/releng/fedora-scm-requests/issue/7348 https://pagure.io/releng/fedora-scm-requests/issue/7349
(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
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.
(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.
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+.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/java-11-openjdk
Package is mostly built. Waiting for s390x or ppc64 on mostof the builds. Will do updates asap
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
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
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
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
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.
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.