Re: RFR: JDK-8315575: Retransform of record class with record component annotation fails with CFE
On Fri, 8 Mar 2024 02:54:49 GMT, Alex Menkov wrote: > RecordComponent class has _attributes_count field. > The only user of the field is JvmtiClassFileReconstituter. Incorrect value of > the field causes producing incorrect data for Record attribute. > Parsing Record attribute ClassFileParser skips unknown attributes and may > skip RuntimeInvisibleAnnotations/RuntimeInvisibleTypeAnnotations. > The fix updates ClassFileParser to calculate correct attributes_count. > > Testing: > - tier1,tier2,hs-tier5-svc; > - redefineClasses/retransformClasses tests: >- test/jdk/java/lang/instrument >- test/hotspot/jtreg/serviceability/jvmti/RedefineClasses >- test/hotspot/jtreg/vmTestbase/nsk/jvmti/RedefineClasses >- test/hotspot/jtreg/vmTestbase/nsk/jvmti/RetransformClasses test/jdk/java/lang/instrument/RetransformRecordAnnotation.java line 27: > 25: * @test > 26: * @bug 8315575 > 27: * @summary test that records with invisible annotation can be > retfansformed Typo `retfansformed`. - PR Review Comment: https://git.openjdk.org/jdk/pull/18161#discussion_r1518468461
Re: RFR: 8327138: Clean up status management in cdsConfig.hpp and CDS.java [v5]
On Thu, 7 Mar 2024 22:59:38 GMT, Calvin Cheung wrote: >> Ioi Lam has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains five additional commits >> since the last revision: >> >> - Merge branch 'master' into 8327138-clean-up-cdsConfig-and-CDS-java >> - @dholmes-ora comments -- white spaces and copyright >> - more alignment >> - fixed alignments >> - 8327138: Clean up status management in cdsConfig.hpp and CDS.java > > Marked as reviewed by ccheung (Reviewer). Thanks @calvinccheung @matias9927 @dholmes-ora for the review. - PR Comment: https://git.openjdk.org/jdk/pull/18095#issuecomment-1986716664
Integrated: 8327138: Clean up status management in cdsConfig.hpp and CDS.java
On Sat, 2 Mar 2024 01:18:06 GMT, Ioi Lam wrote: > A few clean ups: > > 1. Rename functions like "`s_loading_full_module_graph()` to > `is_using_full_module_graph()`. The meaning of "loading" is not clear: it > might be interpreted as to cover only the period where the artifact is being > loaded, but not the period after the artifact is completely loaded. However, > the function is meant to cover both periods, so "using" is a more precise > term. > > 2. The cumbersome sounding `disable_loading_full_module_graph()` is changed > to `stop_using_full_module_graph()`, etc. > > 3. The status of `is_using_optimized_module_handling()` is moved from > metaspaceShared.hpp to cdsConfig.hpp, to be consolidated with other types of > CDS status. > > 4. The status of CDS was communicated to the Java class > `jdk.internal.misc.CDS` by ad-hoc native methods. This is now changed to a > single method, `CDS.getCDSConfigStatus()` that returns a bit field. That way > we don't need to add a new native method for each type of status. > > 5. `CDS.isDumpingClassList()` was a misnomer. It's changed to > `CDS.isLoggingLambdaFormInvokers()`. This pull request has now been integrated. Changeset: 761ed250 Author:Ioi Lam URL: https://git.openjdk.org/jdk/commit/761ed250ec4b0d92d091a0c316b6d5028986a019 Stats: 219 lines in 19 files changed: 58 ins; 58 del; 103 mod 8327138: Clean up status management in cdsConfig.hpp and CDS.java Reviewed-by: ccheung, matsaave - PR: https://git.openjdk.org/jdk/pull/18095
Re: RFR: 8327138: Clean up status management in cdsConfig.hpp and CDS.java [v5]
> A few clean ups: > > 1. Rename functions like "`s_loading_full_module_graph()` to > `is_using_full_module_graph()`. The meaning of "loading" is not clear: it > might be interpreted as to cover only the period where the artifact is being > loaded, but not the period after the artifact is completely loaded. However, > the function is meant to cover both periods, so "using" is a more precise > term. > > 2. The cumbersome sounding `disable_loading_full_module_graph()` is changed > to `stop_using_full_module_graph()`, etc. > > 3. The status of `is_using_optimized_module_handling()` is moved from > metaspaceShared.hpp to cdsConfig.hpp, to be consolidated with other types of > CDS status. > > 4. The status of CDS was communicated to the Java class > `jdk.internal.misc.CDS` by ad-hoc native methods. This is now changed to a > single method, `CDS.getCDSConfigStatus()` that returns a bit field. That way > we don't need to add a new native method for each type of status. > > 5. `CDS.isDumpingClassList()` was a misnomer. It's changed to > `CDS.isLoggingLambdaFormInvokers()`. Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - Merge branch 'master' into 8327138-clean-up-cdsConfig-and-CDS-java - @dholmes-ora comments -- white spaces and copyright - more alignment - fixed alignments - 8327138: Clean up status management in cdsConfig.hpp and CDS.java - Changes: - all: https://git.openjdk.org/jdk/pull/18095/files - new: https://git.openjdk.org/jdk/pull/18095/files/c8f08f7f..25c19bb4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18095=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18095=03-04 Stats: 82460 lines in 929 files changed: 7471 ins; 72858 del; 2131 mod Patch: https://git.openjdk.org/jdk/pull/18095.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18095/head:pull/18095 PR: https://git.openjdk.org/jdk/pull/18095
Re: RFR: 8314250: CDS dump error message: Invoker type parameter must start and end with Object: L3I_L [v2]
On Fri, 8 Mar 2024 06:51:21 GMT, Calvin Cheung wrote: >> To avoid the CDS dump error message, a fix is during dumping a classlist, >> check if an invoker can be archived. >> If not, don't write the invoker info into the classlist, i.e. don't call >> `logLambdaFormInvoker()`. While generating holder classes (in >> `generateHolderClasses()`), don't add the `MethodType` to the `invokerTypes` >> if will fail the check in the `build()` method which would result in a >> `RuntimeException`. >> >> Also updated the `MethodHandlesInvokersTest.java` under >> `appcds/methodHandles` and `appcds/dynamicArchive/methodHandles` to check >> that the "Failed to generate LambdaForm holder classes" error is not in the >> output; >> >> Passed tiers 1 - 3 testing. > > Calvin Cheung has updated the pull request incrementally with one additional > commit since the last revision: > > @iklam comments and copyright update LGTM! Please don't forget to address Ioi's nit before integration, thanks. - Marked as reviewed by matsaave (Committer). PR Review: https://git.openjdk.org/jdk/pull/17953#pullrequestreview-1925970472
Integrated: 8325567: jspawnhelper without args fails with segfault
On Mon, 4 Mar 2024 21:19:26 GMT, Elif Aslan wrote: > This change is intended to address the segmentation fault issue that occurs > when jspawnhelper is called without arguments,. > There is a new test added to verify the behavior in such cases. > > `[ec2-user@ip-172-16-0-10 jdk]$ make CONF=linux-x86_64-server-fastdebug test > TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java` > > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR >jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java > 1 1 0 0 > == > TEST SUCCESS This pull request has now been integrated. Changeset: 26274709 Author:Elif Aslan Committer: Evgeny Astigeevich URL: https://git.openjdk.org/jdk/commit/262747094670b00ac63463a059074afa9b81d8a4 Stats: 62 lines in 2 files changed: 62 ins; 0 del; 0 mod 8325567: jspawnhelper without args fails with segfault Co-authored-by: Vladimir Petko Reviewed-by: eastigeevich, rriggs, shade, vpetko - PR: https://git.openjdk.org/jdk/pull/18112
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v8]
On Fri, 8 Mar 2024 17:24:24 GMT, Elif Aslan wrote: >> This change is intended to address the segmentation fault issue that occurs >> when jspawnhelper is called without arguments,. >> There is a new test added to verify the behavior in such cases. >> >> `[ec2-user@ip-172-16-0-10 jdk]$ make CONF=linux-x86_64-server-fastdebug test >> TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java` >> >> >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >>jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java >> 1 1 0 0 >> == >> TEST SUCCESS > > Elif Aslan has updated the pull request incrementally with one additional > commit since the last revision: > > Address comments lgtm - Marked as reviewed by eastigeevich (Committer). PR Review: https://git.openjdk.org/jdk/pull/18112#pullrequestreview-1925932313
Re: RFR: JDK-8327631: Update IANA Language Subtag Registry to Version 2024-03-07
On Thu, 7 Mar 2024 23:31:46 GMT, Justin Lu wrote: > Please review this PR which is a trivial update to the IANA sub tag registry > to version 2024-03-07. Tests pass as expected after update. > > Associated announcement -> > https://mm.icann.org/pipermail/ietf-languages-announcements/2024-March/90.html Marked as reviewed by iris (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18159#pullrequestreview-1925817696
Re: RFR: 8327705: Remove mention of "applet" from java.text package description
On Fri, 8 Mar 2024 19:54:31 GMT, Naoto Sato wrote: > Removing left over "applet" mention in the package-info doc. Marked as reviewed by jlu (Committer). - PR Review: https://git.openjdk.org/jdk/pull/18173#pullrequestreview-1925828046
Re: RFR: 8327705: Remove mention of "applet" from java.text package description
On Fri, 8 Mar 2024 19:54:31 GMT, Naoto Sato wrote: > Removing left over "applet" mention in the package-info doc. Marked as reviewed by iris (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18173#pullrequestreview-1925813600
Re: RFR: 8327705: Remove mention of "applet" from java.text package description
On Fri, 8 Mar 2024 19:54:31 GMT, Naoto Sato wrote: > Removing left over "applet" mention in the package-info doc. bye bye - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18173#pullrequestreview-1925798262
Re: RFR: 8327705: Remove mention of "applet" from java.text package description
On Fri, 8 Mar 2024 19:54:31 GMT, Naoto Sato wrote: > Removing left over "applet" mention in the package-info doc. Farewell applets. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18173#pullrequestreview-1925759402
RFR: 8327705: Remove mention of "applet" from java.text package description
Removing left over "applet" mention in the package-info doc. - Commit messages: - initial commit Changes: https://git.openjdk.org/jdk/pull/18173/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18173=00 Issue: https://bugs.openjdk.org/browse/JDK-8327705 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18173.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18173/head:pull/18173 PR: https://git.openjdk.org/jdk/pull/18173
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v8]
On Fri, 8 Mar 2024 17:24:24 GMT, Elif Aslan wrote: >> This change is intended to address the segmentation fault issue that occurs >> when jspawnhelper is called without arguments,. >> There is a new test added to verify the behavior in such cases. >> >> `[ec2-user@ip-172-16-0-10 jdk]$ make CONF=linux-x86_64-server-fastdebug test >> TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java` >> >> >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >>jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java >> 1 1 0 0 >> == >> TEST SUCCESS > > Elif Aslan has updated the pull request incrementally with one additional > commit since the last revision: > > Address comments LGTM - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18112#pullrequestreview-1925740150
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v12]
On Fri, 8 Mar 2024 17:19:41 GMT, Severin Gehwolf wrote: >> I tried out the latest commit (a797ea69). >> >> The output "The default module path, '$java.home/jmods' not present. Use >> --verbose to show the full list of plugin options applied" is bit confusing >> as it looks like jlink failed but it actually succeeded. Blowing away the >> generated image and retrying with --verbose tripped this assert >> >> java.lang.AssertionError: handling of scratch options failed >> at >> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.logPackagedModuleEquivalent(JlinkTask.java:675) >> at >> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImageProvider(JlinkTask.java:581) >> at >> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImage(JlinkTask.java:430) >> at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:302) >> at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:56) >> at jdk.jlink/jdk.tools.jlink.internal.Main.main(Main.java:34) >> Caused by: jdk.tools.jlink.internal.TaskHelper$BadArgs: >> (...my-original-jdk-directory..)/build/linux-x64/images/jdk/jmods already >> exists >> at >> jdk.jlink/jdk.tools.jlink.internal.TaskHelper.newBadArgs(TaskHelper.java:730) >> at >> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.lambda$static$12(JlinkTask.java:183) >> at >> jdk.jlink/jdk.tools.jlink.internal.TaskHelper$Option.process(TaskHelper.java:177) >> at >> jdk.jlink/jdk.tools.jlink.internal.TaskHelper$OptionsHelper.handleOptions(TaskHelper.java:600) >> at >> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.logPackagedModuleEquivalent(JlinkTask.java:672) >> ... 5 more >> >> I haven't dug into this yet but I'm puzzled that the file path to where the >> original build was created is in the exception messages, is that recorded? > >> > @AlanBateman @mlchung I've now pushed an update of this patch which now >> > uses a build-time approach as discussed elsewhere. In order to produce a >> > linkable runtime JDK image, one needs to set --enable-runtime-link-image >> > at configure time. >> >> What is the rationale for introducing a special configure flag for this >> functionality? I've tried to look though all comments in this PR, and read >> the JBS issue and CSR, but I have not found any motivation for this. I'm >> sorry if it's there and I missed it, but this is a huge PR with a lot of >> discussion. > > Sorry, yes this was part of a meeting discussion we had outside this PR. My > understanding is that by default the produced jimage isn't runtime-link > enabled. We (Red Hat) would turn it on in our builds, though. @AlanBateman or > @mlchung might have more details. I think it was a requirement to get this > patch in. At least for the initial contribution. > >> In general, it is better not to introduce variants of the build like this. >> The testing matrix just explodes a bit more. And my understanding from the >> discussion is that this functionality is likely to be turned on anyway, >> otherwise you'll build a crippled jlink without full functionality. > > I would be happy if this could be on by default. For now, I think though we > need to work on the premise that whether or not the resulting JDK image is > suitable for runtime linking (without jmods) is a build-time config decision. > Therefore we need the configure option. @jerboaa thanks for the update. First to recap the revised proposal (based on Alan's notes shared with me earlier): The JDK build is capable of producing a JDK run-time image that does not include packaged modules and the new JDK image is capable to create custom run-time images (call it "linkable" JDK image for now). To reconstitute to the original module content, the new JDK image conceptually needs to contain the "diffs" from the original packaged packaged. This makes it possible for the jlink plugins to run "as if" the resources for each module were coming from the original packaged module. The new image also has the checksums of at least the user-editable configuration files so that edits can be detected. The revised proposal has a few limitations: 1. The "linkable" JDK image can only be created by the JDK build. 2. The "linkable" JDK image is created from the JDK image produced by the JDK build today. It contains the same set of modules, there is no possibility to combine its generation with other jlink options or code transformations. 3. The "linkable" JDK image cannot create a run-time image that contains the "jdk.jlink" module. 4. The "linkable" JDK image only reconstitutes classes/resources to the original module bits. Other plugins such as man pages and dedup legal files are not reconstituted. These limitations are trade-off to make for a much simpler approach. It removes the issues of creating a linkable JDK, or creating another image using a linkable JDK, while at the same time executing a pipeline of plugins that do other transformations. Here
Re: RFR: JDK-8327631: Update IANA Language Subtag Registry to Version 2024-03-07
On Thu, 7 Mar 2024 23:31:46 GMT, Justin Lu wrote: > Please review this PR which is a trivial update to the IANA sub tag registry > to version 2024-03-07. Tests pass as expected after update. > > Associated announcement -> > https://mm.icann.org/pipermail/ietf-languages-announcements/2024-March/90.html LGTM - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18159#pullrequestreview-1925720318
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Tue, 27 Feb 2024 15:23:09 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Only show runtime image suffix for JDK modules This PR is about having jlink to produce a run-time image without needing packaged modules (jmod files). The motivation is that jmod files are huge in particular when native debug symbols are included. The previous proposal to support jlink to create a run-time image without jmod files got too complicated and how plugins should revert/apply the transformation and how the users know what transformations are done in the resulting image. The primary motivation for this is to produce OpenJDK run-time image without packaged modules (right now we called it _linkable JDK image_ but need to ponder on the name more) and jlink from such _linkable JDK image_ is capable of creating custom run-time image. Per our offline discussion, the revised proposal is to add an "option" in the JDK build that can produce the linkable JDK image that does not include the packaged modules. The build option can be a configure option or a make target that would need advice from the build team. Such build option would create a run-time image adjacent to `/images/jdk` (for example `/images/linkable-jdk`). No proposal to change the default, i.e. the linkable image is not built by default. This PR is to place the infrastructure and support in place. @magicus is a make target more appropriate for this? - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-1986252706
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes Build changes look good to me, but needs review and verification from the AIX folks. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18172#pullrequestreview-1925621925
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]
On Fri, 8 Mar 2024 10:10:23 GMT, Magnus Ihse Bursie wrote: >> I think what matters for this test test most is which platforms we build >> `jspawnhelper` for, and those platforms indeed are: >> >> >> ifeq ($(call isTargetOs, macosx aix linux), true) >> $(eval $(call SetupJdkExecutable, BUILD_JSPAWNHELPER, \ >> >> >> So I'd say we just add `(os.family == "mac")` here. I would find it a bit >> weird to ask for `os.family != "windows"`, which would implicitly rely on >> exhaustiveness of current os family types. > > Hm, that is not ideal code in the makefile. `jspawnhelper` is called from > `src/java.base/unix/classes/java/lang/ProcessImpl.java`, so it is relied upon > for all Unix implementation. Granted, this is currently the same as the list > "macosx aix linux", but it would be better to express the same logic in the > makefile as in the code. JDK-8327675 was just integrated, which replaces the build logic for jspawnhelper to check for "unix" instead of enumerating all our unixes. I suggest the same pattern should be used here. - PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1518152090
Integrated: 8327167: Clarify the handling of Leap year by Calendar
On Thu, 7 Mar 2024 19:28:13 GMT, Naoto Sato wrote: > A simple doc update to include a leap day example in the `Calendar` class. This pull request has now been integrated. Changeset: 87b40c6a Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/87b40c6ad2b0fa972fa6c5699a52045e82e0c7ef Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod 8327167: Clarify the handling of Leap year by Calendar Reviewed-by: bpb, joehw, lancea, jlu, iris, rriggs - PR: https://git.openjdk.org/jdk/pull/18158
Re: RFR: 8327460: Compile tests with the same visibility rules as product code [v2]
On Wed, 6 Mar 2024 13: 43: 00 GMT, Magnus Ihse Bursie wrote: >> Currently, our symbol visibility handling for tests are sloppy; we only handle it properly on Windows. We need to bring it up to the same levels as
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.
Report Suspicious
ZjQcmQRYFpfptBannerEnd
On Wed, 6 Mar 2024 13:43:00 GMT, Magnus Ihse Bursie wrote:
>> Currently, our symbol visibility handling for tests are sloppy; we only handle it properly on Windows. We need to bring it up to the same levels as product code. This is a prerequisite for [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is a building block for Hermetic Java.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision:
>
> Update line number for dereference_null in TestDwarf
Thanks, this looks good to me.
test/jdk/java/foreign/CallGeneratorHelper.java line 216:
> 214: if (header) {
> 215: System.out.println(
> 216: "#include \"export.h\"\n"
We don't generate these header files any more, so the changes to this file are not really needed.
-
Marked as reviewed by jvernee (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18135#pullrequestreview-1925503653
PR Review Comment: https://git.openjdk.org/jdk/pull/18135#discussion_r1518086954
Integrated: JDK-8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string
On Sat, 2 Mar 2024 00: 34: 32 GMT, Justin Lu wrote: > Please review this PR and corresponding CSR which prevents an OutOfMemoryError by restricting the initial maximum fraction digits for an empty pattern DecimalFormat. ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. Report Suspicious ZjQcmQRYFpfptBannerEnd On Sat, 2 Mar 2024 00:34:32 GMT, Justin Lu wrote: > Please review this PR and corresponding CSR which prevents an OutOfMemoryError by restricting the initial maximum fraction digits for an empty pattern DecimalFormat. > > For an empty String pattern DecimalFormat, the maximum fraction digits is initialized to `Integer.MAX_VALUE`. When toPattern() is invoked, StringBuilder internally doubles capacity attempting to append Integer.MAX_VALUE digits until OOME occurs. CSR covers potential behavioral compatibility changes. This pull request has now been integrated. Changeset: 6efdaf8d Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/6efdaf8ddf2940bcd5f96e114fe05b951ace313b Stats: 259 lines in 3 files changed: 201 ins; 34 del; 24 mod 8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/18094
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Fri, 8 Mar 2024 17: 35: 45 GMT, Severin Gehwolf wrote: >> make/Images. gmk line 144: >> >>> 142: OUTPUT_DIR := $(JDK_IMAGE_DIR), \ >>> 143: SUPPORT_DIR := $(JDK_RUN_TIME_IMAGE_SUPPORT_DIR), ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. Report Suspicious ZjQcmQRYFpfptBannerEnd On Fri, 8 Mar 2024 17:35:45 GMT, Severin Gehwolf wrote: >> make/Images.gmk line 144: >> >>> 142: OUTPUT_DIR := $(JDK_IMAGE_DIR), \ >>> 143: SUPPORT_DIR := $(JDK_RUN_TIME_IMAGE_SUPPORT_DIR), \ >>> 144: PRE_COMMAND := $(RM) -r $(JDK_IMAGE_DIR), \ >> >> This looks scary! Why the rm? Are you sure you are not racing with some other rule that tries to read from the JDK_IMAGE_DIR? > > That was modelled similar to `jdk_jlink` target. It also does the removal. When building with `--enable-runtime-link-image`, the flow is: > > > 1. Link the initial jdk image (current `images/jdk`). Output is $(RL_INTERMEDIATE_IMAGE_DIR) > 2. Generate the diff to the packaged modules (target `generate_jimage_diff`) > 3. Do the final link creating a runtime linkable jimage with `--create-linkabel-runtime` into `JDK_IMAGE_DIR`. > > > All three steps should have appropriate dependencies on each other. So I don't think there is a race. If you see one please let me know! Thanks. Why the rm? Because jlink refuses to run if the output dir already exists. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1518047758
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Fri, 8 Mar 2024 16: 51: 11 GMT, Magnus Ihse Bursie wrote: >> Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: >> >> Only show runtime ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. Report Suspicious ZjQcmQRYFpfptBannerEnd On Fri, 8 Mar 2024 16:51:11 GMT, Magnus Ihse Bursie wrote: >> Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: >> >> Only show runtime image suffix for JDK modules > > make/Images.gmk line 144: > >> 142: OUTPUT_DIR := $(JDK_IMAGE_DIR), \ >> 143: SUPPORT_DIR := $(JDK_RUN_TIME_IMAGE_SUPPORT_DIR), \ >> 144: PRE_COMMAND := $(RM) -r $(JDK_IMAGE_DIR), \ > > This looks scary! Why the rm? Are you sure you are not racing with some other rule that tries to read from the JDK_IMAGE_DIR? That was modelled similar to `jdk_jlink` target. It also does the removal. When building with `--enable-runtime-link-image`, the flow is: 1. Link the initial jdk image (current `images/jdk`). Output is $(RL_INTERMEDIATE_IMAGE_DIR) 2. Generate the diff to the packaged modules (target `generate_jimage_diff`) 3. Do the final link creating a runtime linkable jimage with `--create-linkabel-runtime` into `JDK_IMAGE_DIR`. All three steps should have appropriate dependencies on each other. So I don't think there is a race. If you see one please let me know! Thanks. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1518046993
Re: RFR: 8327486: java/util/Properties/PropertiesStoreTest.java fails "Text 'xxx' could not be parsed at index 20" after 8174269 [v2]
On Fri, 8 Mar 2024 02: 41: 06 GMT, SendaoYan wrote: >> Date. toString() uses Locale. US explicitly for printing the time zone, so replace Locale. ROOT to Locale. US in this testcase for fix the test failure. >> ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. Report Suspicious ZjQcmQRYFpfptBannerEnd On Fri, 8 Mar 2024 02:41:06 GMT, SendaoYan wrote: >> Date.toString() uses Locale.US explicitly for printing the time zone, so replace Locale.ROOT to Locale.US in this testcase for fix the test failure. >> >> This testcase fixed has been verified. >> >> Only change the testcase, risk is low. > > SendaoYan has updated the pull request incrementally with one additional commit since the last revision: > > 1. modify copyright year to 2024; 2. delete unmatch comment Changes requested by naoto (Reviewer). test/jdk/java/util/Properties/PropertiesStoreTest.java line 59: > 57: private static final String DATE_FORMAT_PATTERN = "EEE MMM dd HH:mm:ss zzz "; > 58: // use a neutral locale, since when the date comment was written by Properties.store(...), > 59: // it internally calls the Date.toString() which always writes in a locale insensitive manner Instead of blindly removing the comment, I'd suggest reflecting the fact that `Date.toString()` uses `Locale.US` for time zone names. - PR Review: https://git.openjdk.org/jdk/pull/18155#pullrequestreview-1925420427 PR Review Comment: https://git.openjdk.org/jdk/pull/18155#discussion_r1518034168
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Fri, 8 Mar 2024 16:52:33 GMT, Magnus Ihse Bursie wrote: >> Severin Gehwolf has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Only show runtime image suffix for JDK modules > > make/Images.gmk line 96: > >> 94: >> 95: ifeq ($(JLINK_KEEP_PACKAGED_MODULES), true) >> 96: ifeq ($(JLINK_PRODUCE_RUNTIME_LINK_JDK), true) > > I don't get it. Why don't you use the JDK_LINK_OUTPUT_DIR from just above? Makes sense. I can fold both `JLINK_JDK_EXTRA_OPTS` into one. `JLINK_JDK_EXTRA_OPTS := --keep-packaged-modules $(JDK_LINK_OUTPUT_DIR)/jmods` > make/Images.gmk line 104: > >> 102: endif >> 103: >> 104: > > Is this extra line intentional? No. I can remove it. Thanks. > make/Images.gmk line 128: > >> 126: JDK_RUN_TIME_IMAGE_SUPPORT_DIR := >> $(SUPPORT_OUTPUTDIR)/images/jlink-run-time >> 127: JDK_JMODS_DIFF := >> $(JDK_RUN_TIME_IMAGE_SUPPORT_DIR)/jimage_packaged_modules_diff.data >> 128: JLINK_RUNTIME_CREATE_EXTRA += >> --create-linkable-runtime=$(JDK_JMODS_DIFF) > > My understanding is that the `--create-linkable-runtime` is the primary > argument for the jlink invocation in jlink_runtime_jdk. As such, I think it > would be much better if this was inlined in place in the jlink_runtime_jdk > COMMAND. The "extra" makes it sound like it is optional. > > In contrast, the value of JLINK_RUNTIME_CREATE_EXTRA that is set above for > JLINK_KEEP_PACKAGED_MODULES is indeed "extra". Your understanding is correct. Will fix. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1518027506 PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1518028421 PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1518030091
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v8]
> This change is intended to address the segmentation fault issue that occurs > when jspawnhelper is called without arguments,. > There is a new test added to verify the behavior in such cases. > > `[ec2-user@ip-172-16-0-10 jdk]$ make CONF=linux-x86_64-server-fastdebug test > TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java` > > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR >jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java > 1 1 0 0 > == > TEST SUCCESS Elif Aslan has updated the pull request incrementally with one additional commit since the last revision: Address comments - Changes: - all: https://git.openjdk.org/jdk/pull/18112/files - new: https://git.openjdk.org/jdk/pull/18112/files/c82fb77b..ef321d2c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18112=07 - incr: https://webrevs.openjdk.org/?repo=jdk=18112=06-07 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18112.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18112/head:pull/18112 PR: https://git.openjdk.org/jdk/pull/18112
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v12]
On Fri, 8 Dec 2023 15:44:07 GMT, Alan Bateman wrote: >> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 46 commits: >> >> - Add @enablePreview for JImageValidator that uses classfile API >> - Fix SystemModulesPlugin after merge >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Don't show the verbose hint when already verbose >> - Use '_files' over '_resources' as the suffix for listing resources >> - Remove the hidden option hint. >> >>Also adjust the messages being printed when performing >>a run-time image link. >> - Localize messages, switch expression >> - Rename RunImageArchive => JRTArchive and RunImageLinkException => >> RuntimeImageLinkException >> >>Also moved the stamp file to jdk.jlink module. The resources files per >>module now get unconditionally created (empty if no resources not in the >>jimage). >> - First round of addressing review feedback. >> >>- Share resource names (JlinkTask and JlinkResourcesListPlugin) >>- Exclude resources in JlinkResourcesListPlugin the same way >> as done for other plugins. >> - Rename AddRunImageResourcesPlugin => JlinkResourcesListPlugin >> - ... and 36 more: https://git.openjdk.org/jdk/compare/87516e29...a797ea69 > > I tried out the latest commit (a797ea69). > > The output "The default module path, '$java.home/jmods' not present. Use > --verbose to show the full list of plugin options applied" is bit confusing > as it looks like jlink failed but it actually succeeded. Blowing away the > generated image and retrying with --verbose tripped this assert > > java.lang.AssertionError: handling of scratch options failed > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.logPackagedModuleEquivalent(JlinkTask.java:675) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImageProvider(JlinkTask.java:581) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImage(JlinkTask.java:430) > at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:302) > at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:56) > at jdk.jlink/jdk.tools.jlink.internal.Main.main(Main.java:34) > Caused by: jdk.tools.jlink.internal.TaskHelper$BadArgs: > (...my-original-jdk-directory..)/build/linux-x64/images/jdk/jmods already > exists > at > jdk.jlink/jdk.tools.jlink.internal.TaskHelper.newBadArgs(TaskHelper.java:730) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.lambda$static$12(JlinkTask.java:183) > at > jdk.jlink/jdk.tools.jlink.internal.TaskHelper$Option.process(TaskHelper.java:177) > at > jdk.jlink/jdk.tools.jlink.internal.TaskHelper$OptionsHelper.handleOptions(TaskHelper.java:600) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.logPackagedModuleEquivalent(JlinkTask.java:672) > ... 5 more > > I haven't dug into this yet but I'm puzzled that the file path to where the > original build was created is in the exception messages, is that recorded? > > @AlanBateman @mlchung I've now pushed an update of this patch which now > > uses a build-time approach as discussed elsewhere. In order to produce a > > linkable runtime JDK image, one needs to set --enable-runtime-link-image at > > configure time. > > What is the rationale for introducing a special configure flag for this > functionality? I've tried to look though all comments in this PR, and read > the JBS issue and CSR, but I have not found any motivation for this. I'm > sorry if it's there and I missed it, but this is a huge PR with a lot of > discussion. Sorry, yes this was part of a meeting discussion we had outside this PR. My understanding is that by default the produced jimage isn't runtime-link enabled. We (Red Hat) would turn it on in our builds, though. @AlanBateman or @mlchung might have more details. I think it was a requirement to get this patch in. At least for the initial contribution. > In general, it is better not to introduce variants of the build like this. > The testing matrix just explodes a bit more. And my understanding from the > discussion is that this functionality is likely to be turned on anyway, > otherwise you'll build a crippled jlink without full functionality. I would be happy if this could be on by default. For now, I think though we need to work on the premise that whether or not the resulting JDK image is suitable for runtime linking (without jmods) is a build-time config decision. Therefore we need the configure option. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-1986098525
Re: RFR: 8327460: Compile tests with the same visibility rules as product code [v2]
On Wed, 6 Mar 2024 13:43:00 GMT, Magnus Ihse Bursie wrote: >> Currently, our symbol visibility handling for tests are sloppy; we only >> handle it properly on Windows. We need to bring it up to the same levels as >> product code. This is a prerequisite for >> [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is >> a building block for Hermetic Java. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Update line number for dereference_null in TestDwarf @JornVernee Are you okay with this solution? No JNI in foreign tests. - PR Comment: https://git.openjdk.org/jdk/pull/18135#issuecomment-1986062755
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Tue, 27 Feb 2024 15:23:09 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Only show runtime image suffix for JDK modules make/Images.gmk line 128: > 126: JDK_RUN_TIME_IMAGE_SUPPORT_DIR := > $(SUPPORT_OUTPUTDIR)/images/jlink-run-time > 127: JDK_JMODS_DIFF := > $(JDK_RUN_TIME_IMAGE_SUPPORT_DIR)/jimage_packaged_modules_diff.data > 128: JLINK_RUNTIME_CREATE_EXTRA += > --create-linkable-runtime=$(JDK_JMODS_DIFF) My understanding is that the `--create-linkable-runtime` is the primary argument for the jlink invocation in jlink_runtime_jdk. As such, I think it would be much better if this was inlined in place in the jlink_runtime_jdk COMMAND. The "extra" makes it sound like it is optional. In contrast, the value of JLINK_RUNTIME_CREATE_EXTRA that is set above for JLINK_KEEP_PACKAGED_MODULES is indeed "extra". - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1517986887
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Tue, 27 Feb 2024 15:23:09 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Only show runtime image suffix for JDK modules make/Images.gmk line 96: > 94: > 95: ifeq ($(JLINK_KEEP_PACKAGED_MODULES), true) > 96: ifeq ($(JLINK_PRODUCE_RUNTIME_LINK_JDK), true) I don't get it. Why don't you use the JDK_LINK_OUTPUT_DIR from just above? make/Images.gmk line 104: > 102: endif > 103: > 104: Is this extra line intentional? make/Images.gmk line 144: > 142: OUTPUT_DIR := $(JDK_IMAGE_DIR), \ > 143: SUPPORT_DIR := $(JDK_RUN_TIME_IMAGE_SUPPORT_DIR), \ > 144: PRE_COMMAND := $(RM) -r $(JDK_IMAGE_DIR), \ This looks scary! Why the rm? Are you sure you are not racing with some other rule that tries to read from the JDK_IMAGE_DIR? - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1517984170 PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1517983052 PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1517982726
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v12]
On Fri, 8 Dec 2023 15:44:07 GMT, Alan Bateman wrote: >> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 46 commits: >> >> - Add @enablePreview for JImageValidator that uses classfile API >> - Fix SystemModulesPlugin after merge >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Don't show the verbose hint when already verbose >> - Use '_files' over '_resources' as the suffix for listing resources >> - Remove the hidden option hint. >> >>Also adjust the messages being printed when performing >>a run-time image link. >> - Localize messages, switch expression >> - Rename RunImageArchive => JRTArchive and RunImageLinkException => >> RuntimeImageLinkException >> >>Also moved the stamp file to jdk.jlink module. The resources files per >>module now get unconditionally created (empty if no resources not in the >>jimage). >> - First round of addressing review feedback. >> >>- Share resource names (JlinkTask and JlinkResourcesListPlugin) >>- Exclude resources in JlinkResourcesListPlugin the same way >> as done for other plugins. >> - Rename AddRunImageResourcesPlugin => JlinkResourcesListPlugin >> - ... and 36 more: https://git.openjdk.org/jdk/compare/87516e29...a797ea69 > > I tried out the latest commit (a797ea69). > > The output "The default module path, '$java.home/jmods' not present. Use > --verbose to show the full list of plugin options applied" is bit confusing > as it looks like jlink failed but it actually succeeded. Blowing away the > generated image and retrying with --verbose tripped this assert > > java.lang.AssertionError: handling of scratch options failed > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.logPackagedModuleEquivalent(JlinkTask.java:675) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImageProvider(JlinkTask.java:581) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.createImage(JlinkTask.java:430) > at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:302) > at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:56) > at jdk.jlink/jdk.tools.jlink.internal.Main.main(Main.java:34) > Caused by: jdk.tools.jlink.internal.TaskHelper$BadArgs: > (...my-original-jdk-directory..)/build/linux-x64/images/jdk/jmods already > exists > at > jdk.jlink/jdk.tools.jlink.internal.TaskHelper.newBadArgs(TaskHelper.java:730) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.lambda$static$12(JlinkTask.java:183) > at > jdk.jlink/jdk.tools.jlink.internal.TaskHelper$Option.process(TaskHelper.java:177) > at > jdk.jlink/jdk.tools.jlink.internal.TaskHelper$OptionsHelper.handleOptions(TaskHelper.java:600) > at > jdk.jlink/jdk.tools.jlink.internal.JlinkTask.logPackagedModuleEquivalent(JlinkTask.java:672) > ... 5 more > > I haven't dug into this yet but I'm puzzled that the file path to where the > original build was created is in the exception messages, is that recorded? > @AlanBateman @mlchung I've now pushed an update of this patch which now uses > a build-time approach as discussed elsewhere. In order to produce a linkable > runtime JDK image, one needs to set --enable-runtime-link-image at configure > time. What is the rationale for introducing a special configure flag for this functionality? I've tried to look though all comments in this PR, and read the JBS issue and CSR, but I have not found any motivation for this. I'm sorry if it's there and I missed it, but this is a huge PR with a lot of discussion. In general, it is better not to introduce variants of the build like this. The testing matrix just explodes a bit more. And my understanding from the discussion is that this functionality is likely to be turned on anyway, otherwise you'll build a crippled jlink without full functionality. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-1986042463
Re: RFR: 8327167: Clarify the handling of Leap year by Calendar
On Thu, 7 Mar 2024 19:28:13 GMT, Naoto Sato wrote: > A simple doc update to include a leap day example in the `Calendar` class. Marked as reviewed by rriggs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18158#pullrequestreview-1925260836
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes Build changes look ok, but I'm not an AIX developer - Marked as reviewed by jwaters (Committer). PR Review: https://git.openjdk.org/jdk/pull/18172#pullrequestreview-1925243852
Re: RFR: 8314250: CDS dump error message: Invoker type parameter must start and end with Object: L3I_L [v2]
On Fri, 8 Mar 2024 06:51:21 GMT, Calvin Cheung wrote: >> To avoid the CDS dump error message, a fix is during dumping a classlist, >> check if an invoker can be archived. >> If not, don't write the invoker info into the classlist, i.e. don't call >> `logLambdaFormInvoker()`. While generating holder classes (in >> `generateHolderClasses()`), don't add the `MethodType` to the `invokerTypes` >> if will fail the check in the `build()` method which would result in a >> `RuntimeException`. >> >> Also updated the `MethodHandlesInvokersTest.java` under >> `appcds/methodHandles` and `appcds/dynamicArchive/methodHandles` to check >> that the "Failed to generate LambdaForm holder classes" error is not in the >> output; >> >> Passed tiers 1 - 3 testing. > > Calvin Cheung has updated the pull request incrementally with one additional > commit since the last revision: > > @iklam comments and copyright update Looks good. Just a small nit. src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java line 28: > 26: package java.lang.invoke; > 27: > 28: //import jdk.internal.misc.CDS; This line can be removed. - Marked as reviewed by iklam (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17953#pullrequestreview-1925195828 PR Review Comment: https://git.openjdk.org/jdk/pull/17953#discussion_r1517911192
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes Also, I believe that the `HOTSPOT_TOOLCHAIN_TYPE=xlc` quirk actually is bad. This means that the clang functionality in `compilerWarnings_gcc.hpp` (where the `_gcc` is hotspot-speak for "clang or gcc") is being ignored, and it means that globalDefinitions_xlc.hpp is in big parts a direct copy of globalDefinitions_gcc.hpp, but apparently lagging in some fixes that has been made in that file. And it means a lot of lines like this: #if defined(TARGET_COMPILER_gcc) || defined(TARGET_COMPILER_xlc) But cleaning up that is left as an exercise to the AIX team; my goal here just primarily to get rid of the old xlc stuff from the build system. - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1985939418
Re: RFR: 8327701: Remove the xlc toolchain [v2]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain is no longer supported, and should be removed. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Revert SEARCH_PATH changes - Changes: - all: https://git.openjdk.org/jdk/pull/18172/files - new: https://git.openjdk.org/jdk/pull/18172/files/9f4a059d..53a05019 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18172=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18172=00-01 Stats: 7 lines in 1 file changed: 0 ins; 5 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18172.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18172/head:pull/18172 PR: https://git.openjdk.org/jdk/pull/18172
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:44:48 GMT, Magnus Ihse Bursie wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes make/autoconf/toolchain.m4 line 444: > 442: COMPILER_NAME=$2 > 443: SEARCH_LIST="$3" > 444: SEARCH_PATH="$PATH" I am note 100% sure about this; I think the intention of some of the old code amounted to this, but it was not entirely clear. But, then again, I think this is a good idea, and I think we should do this on *all* platforms -- search the toolchain path before the normal path. - PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1517884839
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:41:16 GMT, Magnus Ihse Bursie wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Revert SEARCH_PATH changes > > make/autoconf/toolchain.m4 line 444: > >> 442: COMPILER_NAME=$2 >> 443: SEARCH_LIST="$3" >> 444: SEARCH_PATH="$PATH" > > I am note 100% sure about this; I think the intention of some of the old code > amounted to this, but it was not entirely clear. > > But, then again, I think this is a good idea, and I think we should do this > on *all* platforms -- search the toolchain path before the normal path. Hm, as I write this, it strikes me as odd that we should not do this already. And of course we do, in `TOOLCHAIN_PRE_DETECTION`, so this snippet snippet is actually redundant. - PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1517889315
Re: RFR: 8327701: Remove the xlc toolchain
On Fri, 8 Mar 2024 15:29:58 GMT, Magnus Ihse Bursie wrote: > As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain is no longer supported, and should be removed. make/autoconf/toolchain.m4 line 420: > 418: # Remove "Thread model:" and further details from the version > string, and > 419: # collapse into a single line > 420: COMPILER_VERSION_STRING=`$ECHO $COMPILER_VERSION_OUTPUT | \ These changes are not strictly needed, but it makes printing the clang version nicer, and compensates for the removal of the extra version information that was printed by some of the removed code. - PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1517880241
RFR: 8327701: Remove the xlc toolchain
As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect clang by another name, and it uses the clang toolchain in the JDK build. Thus the old xlc toolchain is no longer supported, and should be removed. - Commit messages: - 8327701: Remove the xlc toolchain Changes: https://git.openjdk.org/jdk/pull/18172/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18172=00 Issue: https://bugs.openjdk.org/browse/JDK-8327701 Stats: 333 lines in 19 files changed: 25 ins; 267 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/18172.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18172/head:pull/18172 PR: https://git.openjdk.org/jdk/pull/18172
Re: RFR: 8327701: Remove the xlc toolchain
On Fri, 8 Mar 2024 15:29:58 GMT, Magnus Ihse Bursie wrote: > As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain is no longer supported, and should be removed. make/autoconf/flags-cflags.m4 line 687: > 685: PICFLAG="-fPIC" > 686: PIEFLAG="-fPIE" > 687: elif test "x$TOOLCHAIN_TYPE" = xclang && test "x$OPENJDK_TARGET_OS" = > xaix; then Just a remark: This code has never been executed, since running with clang on any OS would hit the branch above. Also, the code is syntactically incorrect, missing a trailing `"`. make/autoconf/flags-cflags.m4 line 695: > 693: -D$FLAGS_CPU_LEGACY" > 694: > 695: if test "x$FLAGS_CPU_BITS" = x64; then A wise man said: "If the code and the comments contradict each other, then probably both are wrong". I am here assuming that the comment claiming that this is only needed by xlc is correct. If it turns out that this is needed even with clang on AIX, we'll have to restore the test and update the comment to this fact. make/autoconf/flags.m4 line 324: > 322: AC_DEFUN([FLAGS_SETUP_TOOLCHAIN_CONTROL], > 323: [ > 324: # COMPILER_TARGET_BITS_FLAG : option for selecting 32- or 64-bit > output COMPILER_TARGET_BITS_FLAG and COMPILER_BINDCMD_FILE_FLAG was introduced just for xlc, so they are not needed anymore. COMPILER_COMMAND_FILE_FLAG was introduced to support ancient versions of gcc, and then used by xlc as well. It is not needed for gcc anymore, so I remove it. - PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1517871818 PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1517873282 PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1517874782
Re: RFR: 7036144: GZIPInputStream readTrailer uses faulty available() test for end-of-stream [v8]
> `GZIPInputStream`, when looking for a concatenated stream, relies on what the > underlying `InputStream` says is how many bytes are `available()`. But this > is inappropriate because `InputStream.available()` is just an estimate and is > allowed (for example) to always return zero. > > The fix is to ignore what's `available()` and just proceed and see what > happens. If fewer bytes are available than required, the attempt to extend to > another stream is canceled just as it was before, e.g., when the next stream > header couldn't be read. Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision: Back-out Javadoc addition; to be added in a separate issue. - Changes: - all: https://git.openjdk.org/jdk/pull/17113/files - new: https://git.openjdk.org/jdk/pull/17113/files/d557d8c6..04072c19 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17113=07 - incr: https://webrevs.openjdk.org/?repo=jdk=17113=06-07 Stats: 19 lines in 1 file changed: 0 ins; 19 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17113.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17113/head:pull/17113 PR: https://git.openjdk.org/jdk/pull/17113
Re: RFR: 8327675: jspawnhelper should be built on all unix platforms
On Fri, 8 Mar 2024 13:58:25 GMT, Erik Joelsson wrote: > Looks good, but why are the tests failing? **macos-x64 / test (langtools/tier1)** fails when installing homebrew: Error: Failure while executing; `/usr/bin/env /usr/local/Homebrew/Library/Homebrew/shims/shared/curl --disable --cookie /dev/null --globoff --user-agent Homebrew/4.2.9\ (Macintosh;\ Intel\ Mac\ OS\ X\ 13.6.4)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --remote-time --output /Users/runner/Library/Caches/Homebrew/api/formula.jws.json --location --disable --cookie /dev/null --globoff --show-error --user-agent Homebrew/4.2.9\ (Macintosh;\ Intel\ Mac\ OS\ X\ 13.6.4)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --compressed --speed-limit 100 --speed-time 5 [https://formulae.brew.sh/api/formula.jws.json`](https://formulae.brew.sh/api/formula.jws.json%60) exited with 28. Here's the output: curl: (28) Operation too slow. Less than 100 bytes/sec transferred the last 5 seconds **linux-cross-compile / build (riscv64)** fails when installing the sysroot: W: Failure while configuring base packages. This will be re-attempted up to five times. W: See /home/runner/work/jdk/jdk/sysroot/debootstrap/debootstrap.log for details (possibly the package libpng16-16:riscv64 is at fault) **linux-x86 / test (jdk/tier1 part 2)** fails on [java/util/Collections/RotateHuge](https://github.com/magicus/jdk/actions/runs/8201858400#summary-22432420637): (路 ???) Test failures summary These tests reported errors: [java/util/Collections/RotateHuge](https://github.com/magicus/jdk/actions/runs/8201858400#user-content-java_util_collections_rotatehuge) I'd say that's just GHA for ya... - PR Comment: https://git.openjdk.org/jdk/pull/18165#issuecomment-1985779398
Integrated: 8327675: jspawnhelper should be built on all unix platforms
On Fri, 8 Mar 2024 10:17:08 GMT, Magnus Ihse Bursie wrote: > We should match the building of jspawnhelper in > make/modules/java.base/Launcher.gmk with the usage for all unix platforms in > src/java.base/unix/classes/java/lang/ProcessImpl.java. > > Granted, the list of OSes in the makefile amounts to the current list of all > Unix OSes in the JDK, but there is no need to have this logical disparity. > > This was inspired by the discovery in > https://github.com/openjdk/jdk/pull/18112#discussion_r1517455696. This pull request has now been integrated. Changeset: 585a9584 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/585a95844144da53bc43f5b6383e7c907bff7047 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8327675: jspawnhelper should be built on all unix platforms Reviewed-by: shade, stuefe, erikj - PR: https://git.openjdk.org/jdk/pull/18165
Re: RFR: 8327675: jspawnhelper should be built on all unix platforms
On Fri, 8 Mar 2024 10:17:08 GMT, Magnus Ihse Bursie wrote: > We should match the building of jspawnhelper in > make/modules/java.base/Launcher.gmk with the usage for all unix platforms in > src/java.base/unix/classes/java/lang/ProcessImpl.java. > > Granted, the list of OSes in the makefile amounts to the current list of all > Unix OSes in the JDK, but there is no need to have this logical disparity. > > This was inspired by the discovery in > https://github.com/openjdk/jdk/pull/18112#discussion_r1517455696. Looks good, but why are the tests failing? - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18165#pullrequestreview-1924937739
Re: RFR: 8327501: Common ForkJoinPool prevents class unloading in some cases
On Wed, 6 Mar 2024 22:58:54 GMT, Viktor Klang wrote: > The common ForkJoinPool creating threads as a result of submitting tasks is > preventing class unloading when the thread construction is initiated from a > class loaded in a separate classloader. This fix avoids that when no > SecurityManager is configured. @trancexpress I defer all credit to @AlanBateman. :) - PR Comment: https://git.openjdk.org/jdk/pull/18144#issuecomment-1985742052
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v4]
On Thu, 7 Mar 2024 21:53:07 GMT, Jan Lahoda wrote: >> Currently, JDK modules load by the bootstrap and platform ClassLoaders are >> automatically granted the native access. I am working on an upgrade of JLine >> inside the `jdk.internal.le` module, and I would like to replace the current >> native bindings with FFM-based bindings (which are now somewhat provided by >> JLine). But, for that, native access is needed for the `jdk.internal.le` >> module. We could possibly move the module to the platform ClassLoader, but >> it seems it might be better to have more control over which modules have the >> native access. >> >> This patch introduces an explicit list of modules that will automatically be >> granted the native access. Note this patch is not yet intended to change the >> end behavior - the list of modules granted native access is supposed to be >> the same as modules in the boot and platform ClassLoaders. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Adjusting javadoc as suggested. Thanks for all the feedback and comments! I've filled: https://bugs.openjdk.org/browse/JDK-8327686 to cover the cleanup of the modules with native access enabled, and: https://bugs.openjdk.org/browse/JDK-8327688 to allow to configure the modules with native access at link time. I can try to take a look at either of those, if desired. - PR Comment: https://git.openjdk.org/jdk/pull/18106#issuecomment-1985530868
Integrated: 8327218: Add an ability to specify modules which should have native access enabled
On Mon, 4 Mar 2024 13:52:13 GMT, Jan Lahoda wrote: > Currently, JDK modules load by the bootstrap and platform ClassLoaders are > automatically granted the native access. I am working on an upgrade of JLine > inside the `jdk.internal.le` module, and I would like to replace the current > native bindings with FFM-based bindings (which are now somewhat provided by > JLine). But, for that, native access is needed for the `jdk.internal.le` > module. We could possibly move the module to the platform ClassLoader, but it > seems it might be better to have more control over which modules have the > native access. > > This patch introduces an explicit list of modules that will automatically be > granted the native access. Note this patch is not yet intended to change the > end behavior - the list of modules granted native access is supposed to be > the same as modules in the boot and platform ClassLoaders. This pull request has now been integrated. Changeset: 27a03e0d Author:Jan Lahoda URL: https://git.openjdk.org/jdk/commit/27a03e0dc3e08094aebc3524f68617f7e7fb5c5d Stats: 132 lines in 9 files changed: 106 ins; 9 del; 17 mod 8327218: Add an ability to specify modules which should have native access enabled Co-authored-by: Maurizio Cimadamore Reviewed-by: mcimadamore, erikj, alanb, ihse - PR: https://git.openjdk.org/jdk/pull/18106
Re: RFR: 8327675: jspawnhelper should be built on all unix platforms
On Fri, 8 Mar 2024 10:17:08 GMT, Magnus Ihse Bursie wrote: > We should match the building of jspawnhelper in > make/modules/java.base/Launcher.gmk with the usage for all unix platforms in > src/java.base/unix/classes/java/lang/ProcessImpl.java. > > Granted, the list of OSes in the makefile amounts to the current list of all > Unix OSes in the JDK, but there is no need to have this logical disparity. > > This was inspired by the discovery in > https://github.com/openjdk/jdk/pull/18112#discussion_r1517455696. +1 - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18165#pullrequestreview-1924593320
Re: RFR: 8327675: jspawnhelper should be built on all unix platforms
On Fri, 8 Mar 2024 10:17:08 GMT, Magnus Ihse Bursie wrote: > We should match the building of jspawnhelper in > make/modules/java.base/Launcher.gmk with the usage for all unix platforms in > src/java.base/unix/classes/java/lang/ProcessImpl.java. > > Granted, the list of OSes in the makefile amounts to the current list of all > Unix OSes in the JDK, but there is no need to have this logical disparity. > > This was inspired by the discovery in > https://github.com/openjdk/jdk/pull/18112#discussion_r1517455696. Looks good! - Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18165#pullrequestreview-1924580539
RFR: 8327675: jspawnhelper should be built on all unix platforms
We should match the building of jspawnhelper in make/modules/java.base/Launcher.gmk with the usage for all unix platforms in src/java.base/unix/classes/java/lang/ProcessImpl.java. Granted, the list of OSes in the makefile amounts to the current list of all Unix OSes in the JDK, but there is no need to have this logical disparity. This was inspired by the discovery in https://github.com/openjdk/jdk/pull/18112#discussion_r1517455696. - Commit messages: - 8327675: jspawnhelper should be built on all unix platforms Changes: https://git.openjdk.org/jdk/pull/18165/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18165=00 Issue: https://bugs.openjdk.org/browse/JDK-8327675 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18165.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18165/head:pull/18165 PR: https://git.openjdk.org/jdk/pull/18165
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]
On Fri, 8 Mar 2024 09:19:43 GMT, Aleksey Shipilev wrote: >> Yes indeed, it is used for all Unix OSes (that is, everything but Windows). > > I think what matters for this test test most is which platforms we build > `jspawnhelper` for, and those platforms indeed are: > > > ifeq ($(call isTargetOs, macosx aix linux), true) > $(eval $(call SetupJdkExecutable, BUILD_JSPAWNHELPER, \ > > > So I'd say we just add `(os.family == "mac")` here. I would find it a bit > weird to ask for `os.family != "windows"`, which would implicitly rely on > exhaustiveness of current os family types. Hm, that is not ideal code in the makefile. `jspawnhelper` is called from `src/java.base/unix/classes/java/lang/ProcessImpl.java`, so it is relied upon for all Unix implementation. Granted, this is currently the same as the list "macosx aix linux", but it would be better to express the same logic in the makefile as in the code. - PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1517516188
Integrated: 8326096: Deprecate getTotalIn, getTotalOut methods of java.util.zip.Inflater, java.util.zip.Deflater
On Mon, 19 Feb 2024 18:35:58 GMT, Eirik Bjørsnøs wrote: > Please review this PR which proposes that we officially deprecate the > following four methods in the `java.util.zip` package: > > * `Inflater.getTotalIn()` > * `Inflater.getTotalOut()` > * `Deflater.getTotalIn()` > * `Deflater.getTotalOut()` > > Since these legacy methods return `int`, they cannot safely return the number > of bytes processed without the risk of losing information about the > magnitude or even sign of the returned value. > > The corresponding methods `getBytesRead()` and `getBytesWritten()` methods > introduced in Java 5 return `long`, and should be used instead when obtaining > this information. > > Unrelated to the deprecation itself, the documentation currently does not > specify what these methods are expected to return when the number of > processed bytes is greater than `Integer.MAX_VALUE`. This PR aims to clarify > this in the API specification. This pull request has now been integrated. Changeset: d0d4912c Author:Eirik Bjørsnøs URL: https://git.openjdk.org/jdk/commit/d0d4912c3bbc06e9a9c5273308d5f4ef7bac1b24 Stats: 28 lines in 2 files changed: 16 ins; 0 del; 12 mod 8326096: Deprecate getTotalIn, getTotalOut methods of java.util.zip.Inflater, java.util.zip.Deflater Co-authored-by: Jaikiran Pai Reviewed-by: lancea, alanb - PR: https://git.openjdk.org/jdk/pull/17919
Re: RFR: 8326096: Deprecate getTotalIn, getTotalOut methods of java.util.zip.Inflater, java.util.zip.Deflater [v12]
On Sat, 2 Mar 2024 18:51:16 GMT, Eirik Bjørsnøs wrote: >> Please review this PR which proposes that we officially deprecate the >> following four methods in the `java.util.zip` package: >> >> * `Inflater.getTotalIn()` >> * `Inflater.getTotalOut()` >> * `Deflater.getTotalIn()` >> * `Deflater.getTotalOut()` >> >> Since these legacy methods return `int`, they cannot safely return the >> number of bytes processed without the risk of losing information about the >> magnitude or even sign of the returned value. >> >> The corresponding methods `getBytesRead()` and `getBytesWritten()` methods >> introduced in Java 5 return `long`, and should be used instead when >> obtaining this information. >> >> Unrelated to the deprecation itself, the documentation currently does not >> specify what these methods are expected to return when the number of >> processed bytes is greater than `Integer.MAX_VALUE`. This PR aims to clarify >> this in the API specification. > > Eirik Bjørsnøs has updated the pull request incrementally with one additional > commit since the last revision: > > Make the new specification text an @implSpec Thanks a lot to Jai, Lance & Alan for all your help in getting this PR across the finish line! - PR Comment: https://git.openjdk.org/jdk/pull/17919#issuecomment-1985362189
Re: RFR: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes [v4]
> Please consider this PR which suggests we rename `ZipEntry.extraAttributes` > to `ZipEntry.externalFileAttributes`. > > This field was introduced in > [JDK-8218021](https://bugs.openjdk.org/browse/JDK-8218021), originally under > the name `ZipEntry.posixPerms`. > [JDK-8250968](https://bugs.openjdk.org/browse/JDK-8250968) later renamed the > field to `ZipEntry.extraAttributes` and extended its semantics to hold the > full two-byte value of the `external file attributes` field, as defined by > `APPNOTE.TXT` > > The name `extraAttributes` is misleading. It has nothing to do with the > `extra field` (an unrelated structure defined in `APPNOTE.TXT`), although the > name indicates it does. > > To prevent confusion and make life easier for future maintainers, I suggest > we rename this field to `ZipEntry.externalFileAttributes` and update related > methods, parameters and comments accordingly. > > While this change is a straightforward renaming, reviewers should consider > whether it carries its weight, especially considering it might complicate > future backports. > > As a note to reviewers, this PR includes the following intended updates: > > - Rename `ZipEntry.extraAttributes` and any references to this field to > `ZipEntry.externalFileAttributes` > - Update `JavaUtilZipFileAccess` to similarly rename methods to > `setExternalFileAttributes` and `getExternalFileAttributes` > - Rename the field `JarSigner.extraAttrsDetected` to > `JarSigner.externalFileAttrsDetected` > - Rename a local variable in `JarSigner.writeEntry` to > `externalFileAttributes` > - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to > `externalFileAttributesDetected` > - Rename resource string key names in `s.s.t.jarsigner.Resources` from > `extra.attributes.detected` to `external.file.attributes.detected` > - Rename method `SymlinkTest.verifyExtraAttrs` to > `verifyExternalFileAttributes`, also updated two references to 'extra > attributes' in this method > - Updated copyright in all affected files > > If the resource file changes should be dropped and instead handled via > `msgdop` updates, let me know and I can revert the non-default files. > > I did a search across the code base to find 'extraAttrs', 'extra.attr' after > these updates, and found nothing related to zip/jar. The `zip` and `jar` > tests run clean: > > > make test TEST="test/jdk/java/util/jar" > make test TEST="test/jdk/java/util/zip" Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision: Rename SymlinkTest.verifyExternalAttrs to verifyExternalFileAttributes - Changes: - all: https://git.openjdk.org/jdk/pull/16952/files - new: https://git.openjdk.org/jdk/pull/16952/files/10db9803..243887b4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16952=03 - incr: https://webrevs.openjdk.org/?repo=jdk=16952=02-03 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16952.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16952/head:pull/16952 PR: https://git.openjdk.org/jdk/pull/16952
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]
On Fri, 8 Mar 2024 07:35:27 GMT, Magnus Ihse Bursie wrote: >> test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 29: >> >>> 27: * @test >>> 28: * @bug 8325567 >>> 29: * @requires (os.family == "linux") | (os.family == "aix") >> >> Unless I'm mistaken, jspawn helper is used on Mac as well. > > Yes indeed, it is used for all Unix OSes (that is, everything but Windows). I think what matters for this test test most is which platforms we build `jspawnhelper` for, and those platforms indeed are: ifeq ($(call isTargetOs, macosx aix linux), true) $(eval $(call SetupJdkExecutable, BUILD_JSPAWNHELPER, \ So I'd say we just add `(os.family == "mac")` here. I would find it a bit weird to ask for `os.family != "windows"`, which would implicitly rely on exhaustiveness of current os family types. - PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1517455696
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]
On Thu, 7 Mar 2024 20:04:13 GMT, Vladimir Petko wrote: > I wonder if it would make sense to add a test with a correct argument format? I would say that is what current jspawnhelper tests already test, and it is also tested implicitly with `Process` tests. Let's keep this test for testing that warning messages are printed on most common cases of misuse -- that is why `JspawnhelperWarnings` is a good name. - PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1517458419
Integrated: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase
On Wed, 6 Mar 2024 09:26:25 GMT, Matthias Baesken wrote: > We define the RESTARTABLE macro again and again at a lot of places in the JDK > native codebase. This could be centralized to avoid repeating it again and > again ! This pull request has now been integrated. Changeset: fb4610e6 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/fb4610e6b7a339d0a95a99d6e113e3ddda291561 Stats: 185 lines in 18 files changed: 69 ins; 106 del; 10 mod 8327444: simplify RESTARTABLE macro usage in JDK codebase Reviewed-by: gli, clanger, alanb, dholmes, bpb - PR: https://git.openjdk.org/jdk/pull/18132
Re: RFR: 8327460: Compile tests with the same visibility rules as product code [v2]
On Wed, 6 Mar 2024 13:53:06 GMT, Magnus Ihse Bursie wrote: > I am currently running tier 4-10 This is now done. A handful of tests failed, but all are due to transient environmental problems, known errors, or seemingly intermittent product errors -- none are due to symbol visibility. So I'd argue that this is one of the most well-tested PRs posted ever. :-) - PR Comment: https://git.openjdk.org/jdk/pull/18135#issuecomment-1985232111
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v4]
On Thu, 7 Mar 2024 21:53:07 GMT, Jan Lahoda wrote: >> Currently, JDK modules load by the bootstrap and platform ClassLoaders are >> automatically granted the native access. I am working on an upgrade of JLine >> inside the `jdk.internal.le` module, and I would like to replace the current >> native bindings with FFM-based bindings (which are now somewhat provided by >> JLine). But, for that, native access is needed for the `jdk.internal.le` >> module. We could possibly move the module to the platform ClassLoader, but >> it seems it might be better to have more control over which modules have the >> native access. >> >> This patch introduces an explicit list of modules that will automatically be >> granted the native access. Note this patch is not yet intended to change the >> end behavior - the list of modules granted native access is supposed to be >> the same as modules in the boot and platform ClassLoaders. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Adjusting javadoc as suggested. Marked as reviewed by alanb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18106#pullrequestreview-1924302699
