Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v67]
On Wed, 15 May 2024 10:08:19 GMT, Pavel Rappo wrote: > I think we should add a test to verify that if `--disable-line-doc-comments` > is specified, no `///` dangling comments are reported. Added more tests for dangling doc comments. Note we cannot currently use `-Xlint:dangling-doc-comments` in javadoc itself, so the new tests are part of the javac set of tests. - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-2113453038
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v69]
> Please review a patch to add support for Markdown syntax in documentation > comments, as described in the associated JEP. > > Notable features: > > * support for `///` documentation comments in `JavaTokenizer` > * new module `jdk.internal.md` -- a private copy of the `commonmark-java` > library > * updates to `DocCommentParser` to treat `///` comments as Markdown > * updates to the standard doclet to render Markdown comments in HTML Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: update tests for dangling doc comments, per review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/43546101..bfa35bd4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=68 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=67-68 Stats: 110 lines in 9 files changed: 95 ins; 0 del; 15 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting > the use of JNI in the following ways: > > * `System::load` and `System::loadLibrary` are now restricted methods > * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods > * binding a JNI `native` method declaration to a native implementation is now > considered a restricted operation > > This PR slightly changes the way in which the JDK deals with restricted > methods, even for FFM API calls. In Java 22, the single > `--enable-native-access` was used both to specify a set of modules for which > native access should be allowed *and* to specify whether illegal native > access (that is, native access occurring from a module not specified by > `--enable-native-access`) should be treated as an error or a warning. More > specifically, an error is only issued if the `--enable-native-access flag` is > used at least once. > > Here, a new flag is introduced, namely > `illegal-native-access=allow/warn/deny`, which is used to specify what should > happen when access to a restricted method and/or functionality is found > outside the set of modules specified with `--enable-native-access`. The > default policy is `warn`, but users can select `allow` to suppress the > warnings, or `deny` to cause `IllegalCallerException` to be thrown. This > aligns the treatment of restricted methods with other mechanisms, such as > `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. > > Some changes were required in the package-info javadoc for > `java.lang.foreign`, to reflect the changes in the command line flags > described above. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.org/jdk/pull/19213/files - new: https://git.openjdk.org/jdk/pull/19213/files/daf729f4..1c45e5d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19213=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19213=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19213.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19213/head:pull/19213 PR: https://git.openjdk.org/jdk/pull/19213
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v5]
On Wed, 15 May 2024 10:40:34 GMT, Maurizio Cimadamore
wrote:
>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting
>> the use of JNI in the following ways:
>>
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is
>> now considered a restricted operation
>>
>> This PR slightly changes the way in which the JDK deals with restricted
>> methods, even for FFM API calls. In Java 22, the single
>> `--enable-native-access` was used both to specify a set of modules for which
>> native access should be allowed *and* to specify whether illegal native
>> access (that is, native access occurring from a module not specified by
>> `--enable-native-access`) should be treated as an error or a warning. More
>> specifically, an error is only issued if the `--enable-native-access flag`
>> is used at least once.
>>
>> Here, a new flag is introduced, namely
>> `illegal-native-access=allow/warn/deny`, which is used to specify what
>> should happen when access to a restricted method and/or functionality is
>> found outside the set of modules specified with `--enable-native-access`.
>> The default policy is `warn`, but users can select `allow` to suppress the
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This
>> aligns the treatment of restricted methods with other mechanisms, such as
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>>
>> Some changes were required in the package-info javadoc for
>> `java.lang.foreign`, to reflect the changes in the command line flags
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Refine warning text for JNI method binding
src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 871:
> 869: return IllegalNativeAccess.WARN;
> 870: } else {
> 871: fail("Value specified to --illegal-access not recognized:"
Typo in the message, should be --illegal-native-access.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601898238
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v68]
> Please review a patch to add support for Markdown syntax in documentation > comments, as described in the associated JEP. > > Notable features: > > * support for `///` documentation comments in `JavaTokenizer` > * new module `jdk.internal.md` -- a private copy of the `commonmark-java` > library > * updates to `DocCommentParser` to treat `///` comments as Markdown > * updates to the standard doclet to render Markdown comments in HTML Jonathan Gibbons has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 93 commits: - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - Remove `--no-fonts` from `MISSING_IN_MAN_PAGE` - Update javadoc.1 troff man page - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - address review feedback, to improve testing of changes to Elements - update copyright years - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - update commonmark-java from 0.21.0 to 0.22.0 - ... and 83 more: https://git.openjdk.org/jdk/compare/61aff6db...43546101 - Changes: https://git.openjdk.org/jdk/pull/16388/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16388=67 Stats: 26438 lines in 242 files changed: 25744 ins; 257 del; 437 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote:
>> WIP
>>
>> This changeset contains hsdis for Windows/gcc Port. It supports both the
>> binutils and capstone backends, though the LLVM backend is left out due to
>> compatibility issues encountered during the build. Currently, which gcc
>> distributions are supported is still to be clarified, as several, ranging
>> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason,
>> the build system hack in place at the moment to compile the binutils backend
>> on Windows is still left in place, for now.
>
> Julian Waters has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains two commits:
>
> - Add check for compiler in configure
> - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
Hi Julian, sorry for not getting back to you.
The problem from my PoV is that this is a very large and intrusive patch in the
build of the actual product, for a claimed problem in the tangential hsdis
library. I have not understood a pressing business need to get a Windows/gcc
port for hsdis, which means I can't really prioritize trying to understand this
patch.
As you know, the build system does not currently really separate between "the
OS is Windows" and "the toolchain is Microsoft". I understand that you want to
fix that, and in theory I support it. However, you must also realize that
making a complete fix of this requires a lot of work, for something that there
is no clearly defined need. (After all, cl.exe works fine to create the build,
has no apparent issues, and is as far as an "official" compiler for Windows as
you can get.) That makes it fall squarely in the WIBNIs bucked ("wouldn't it be
nice if...").
If you can fix just the hsdis build without changing anything outside the hsdis
Makefiles, that would be a different story. Such a change would be limited in
scope, easy to say it will not affect the product proper, and be easier to
review.
-
PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112546029
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v28]
On Wed, 15 May 2024 11:55:39 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 with a new target base due to a > merge or a rebase. The pull request now contains 108 commits: > > - Merge branch 'master' into jdk-8311302-jmodless-link > - Simplify JLINK_JDK_EXTRA_OPTS var handling > - Only add runtime track files for linkable runtimes > - Generate the runtime image link diff at jlink time > >But only do that once the plugin-pipeline has run. The generation is >enabled with the hidden option '--generate-linkable-runtime' and >the resource pools available at jlink time are being used to generate >the diff. > - Merge branch 'master' into jdk-8311302-jmodless-link > - Use shorter name for the build tool > - Review feedback from Erik J. > - Remove dependency on CDS which isn't needed anymore > - Fix coment > - Fix comment > - ... and 98 more: https://git.openjdk.org/jdk/compare/957eb611...8cca277e Thanks! Now I am 100% happy with the build changes. :) - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14787#pullrequestreview-2057984229
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Add check for compiler in configure > - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis Magnus? Erik? You guys there? :( - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112465392
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v28]
> 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 b eing 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.internal.vm.compiler.manage... Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 108 commits: - Merge branch 'master' into jdk-8311302-jmodless-link - Simplify JLINK_JDK_EXTRA_OPTS var handling - Only add runtime track files for linkable runtimes - Generate the runtime image link diff at jlink time But only do that once the plugin-pipeline has run. The generation is enabled with the hidden option '--generate-linkable-runtime' and the resource pools available at jlink time are being used to generate the diff. - Merge branch 'master' into jdk-8311302-jmodless-link - Use shorter name for the build tool - Review feedback from Erik J. - Remove dependency on CDS which isn't needed anymore - Fix coment - Fix comment - ... and 98 more: https://git.openjdk.org/jdk/compare/957eb611...8cca277e - Changes: https://git.openjdk.org/jdk/pull/14787/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14787=27 Stats: 3424 lines in 36 files changed: 3272 ins; 110 del; 42 mod Patch: https://git.openjdk.org/jdk/pull/14787.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14787/head:pull/14787 PR: https://git.openjdk.org/jdk/pull/14787
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]
On Wed, 15 May 2024 10:34:01 GMT, Maurizio Cimadamore wrote: > I don't fully agree that this option is not module related (which is why I > gave it that name). The very definition of illegal native access is related > to native access occurring from a module that is outside a specific set. So I > think it's 50/50 as to whether this option is module-related or not. Of > course I can fix the code if there's something clearly better. It maps here to a jdk.module.* property so I suppose it's okay. The functions were introduced to create jdk.module.* properties where the values were a set module names or a module path. Maybe these functions should be renamed at some point (not here) as they are more widely useful now. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601421535
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v5]
> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting > the use of JNI in the following ways: > > * `System::load` and `System::loadLibrary` are now restricted methods > * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods > * binding a JNI `native` method declaration to a native implementation is now > considered a restricted operation > > This PR slightly changes the way in which the JDK deals with restricted > methods, even for FFM API calls. In Java 22, the single > `--enable-native-access` was used both to specify a set of modules for which > native access should be allowed *and* to specify whether illegal native > access (that is, native access occurring from a module not specified by > `--enable-native-access`) should be treated as an error or a warning. More > specifically, an error is only issued if the `--enable-native-access flag` is > used at least once. > > Here, a new flag is introduced, namely > `illegal-native-access=allow/warn/deny`, which is used to specify what should > happen when access to a restricted method and/or functionality is found > outside the set of modules specified with `--enable-native-access`. The > default policy is `warn`, but users can select `allow` to suppress the > warnings, or `deny` to cause `IllegalCallerException` to be thrown. This > aligns the treatment of restricted methods with other mechanisms, such as > `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. > > Some changes were required in the package-info javadoc for > `java.lang.foreign`, to reflect the changes in the command line flags > described above. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Refine warning text for JNI method binding - Changes: - all: https://git.openjdk.org/jdk/pull/19213/files - new: https://git.openjdk.org/jdk/pull/19213/files/0d21bf99..daf729f4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19213=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19213=03-04 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19213.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19213/head:pull/19213 PR: https://git.openjdk.org/jdk/pull/19213
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]
On Wed, 15 May 2024 06:15:35 GMT, Alan Bateman wrote: >> So my recollection/understanding is that we use this mechanism to convert >> module-related `--` flags passed to the VM into system properties that the >> Java code can then read, but we set them up such that you are not allowed to >> specify them directly via `-D`. Is the question here whether this new >> property should be in the `jdk.module` namespace? > > That's my recollection too. The usage here isn' related to modules which > makes me wonder if this function should be renamed (not by this PR of course) > of if we should be using PropertyList_unique_add (with AddProperty, > WriteableProperty, InternalProperty) instead. There will be further GNU style > options coming that will likely need to map to an internal system property in > the same way. I don't fully agree that this option is not module related (which is why I gave it that name). The very definition of illegal native access is related to native access occurring from a module that is outside a specific set. So I think it's 50/50 as to whether this option is module-related or not. Of course I can fix the code if there's something clearly better. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601386336
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v67]
On Tue, 7 May 2024 17:31:29 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 91 commits: > > - Merge remote-tracking branch 'upstream/master' into > 8298405.doclet-markdown-v3 > - Remove `--no-fonts` from `MISSING_IN_MAN_PAGE` > - Update javadoc.1 troff man page > - Merge remote-tracking branch 'upstream/master' into > 8298405.doclet-markdown-v3 > - address review feedback, to improve testing of changes to Elements > - update copyright years > - Merge remote-tracking branch 'upstream/master' into > 8298405.doclet-markdown-v3 > - update commonmark-java from 0.21.0 to 0.22.0 > - Remove links to `jdk.javadoc` module from `java.compiler` module` > - Suppress warnings building tests > - ... and 81 more: https://git.openjdk.org/jdk/compare/524aaad9...cc12140a I think we should add a test to verify that if `--disable-line-doc-comments` is specified, no `///` dangling comments are reported. - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-2112100605
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v4]
On Wed, 15 May 2024 07:55:27 GMT, ExE Boss wrote: > Note that this line is still not entirely correct, as for code like: You are correct - the message is however consistent with what written in JEP 472. I'll discuss with @pron - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601335120
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v26]
On Wed, 8 May 2024 22:36:51 GMT, Magnus Ihse Bursie 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 105 commits: >> >> - Generate the runtime image link diff at jlink time >> >>But only do that once the plugin-pipeline has run. The generation is >>enabled with the hidden option '--generate-linkable-runtime' and >>the resource pools available at jlink time are being used to generate >>the diff. >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Use shorter name for the build tool >> - Review feedback from Erik J. >> - Remove dependency on CDS which isn't needed anymore >> - Fix coment >> - Fix comment >> - Fix typo >> - Revert some now unneded build changes >> - Follow build tools naming convention >> - ... and 95 more: https://git.openjdk.org/jdk/compare/23a72a1f...67aea4ca > > make/Images.gmk line 100: > >> 98: >> 99: ifeq ($(JLINK_PRODUCE_RUNTIME_LINK_JDK), true) >> 100: JLINK_JDK_EXTRA_OPTS := $(JLINK_JDK_EXTRA_OPTS) >> --generate-linkable-runtime > > I just noticed this slight improvement: > > Suggestion: > > JLINK_JDK_EXTRA_OPTS += --generate-linkable-runtime Thanks, fixed. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1601322116
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v27]
> 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 b eing 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.internal.vm.compiler.manage... Severin Gehwolf has updated the pull request incrementally with two additional commits since the last revision: - Simplify JLINK_JDK_EXTRA_OPTS var handling - Only add runtime track files for linkable runtimes - Changes: - all: https://git.openjdk.org/jdk/pull/14787/files - new: https://git.openjdk.org/jdk/pull/14787/files/67aea4ca..be30a182 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14787=26 - incr: https://webrevs.openjdk.org/?repo=jdk=14787=25-26 Stats: 11 lines in 2 files changed: 5 ins; 5 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14787.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14787/head:pull/14787 PR: https://git.openjdk.org/jdk/pull/14787
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v4]
On Tue, 14 May 2024 18:10:28 GMT, Maurizio Cimadamore
wrote:
>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting
>> the use of JNI in the following ways:
>>
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is
>> now considered a restricted operation
>>
>> This PR slightly changes the way in which the JDK deals with restricted
>> methods, even for FFM API calls. In Java 22, the single
>> `--enable-native-access` was used both to specify a set of modules for which
>> native access should be allowed *and* to specify whether illegal native
>> access (that is, native access occurring from a module not specified by
>> `--enable-native-access`) should be treated as an error or a warning. More
>> specifically, an error is only issued if the `--enable-native-access flag`
>> is used at least once.
>>
>> Here, a new flag is introduced, namely
>> `illegal-native-access=allow/warn/deny`, which is used to specify what
>> should happen when access to a restricted method and/or functionality is
>> found outside the set of modules specified with `--enable-native-access`.
>> The default policy is `warn`, but users can select `allow` to suppress the
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This
>> aligns the treatment of restricted methods with other mechanisms, such as
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>>
>> Some changes were required in the package-info javadoc for
>> `java.lang.foreign`, to reflect the changes in the command line flags
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Address review comments
>Improve warning for JNI methods, similar to what's described in JEP 472
>Beef up tests
> - Address review comments
src/java.base/share/classes/java/lang/Module.java line 334:
> 332: System.err.printf("""
> 333: WARNING: A native method in %s has been bound
> 334: WARNING: %s has been called by %s in %s
Note that this line is still not entirely correct, as for code like:
// in module a:
package a;
import b.Foo;
public class Foo {
public static void main(String... args) {
System.load("JNI library implementing Java_b_Bar_nativeMethod");
Bar.nativeMethod();
}
}
// in module b:
package b;
public class Bar {
public static native void nativeMethod();
}
It’ll show `Bar` as the caller of `Bar::nativeMethod()`, even though the caller
is `Foo` in this case, which is why I initially suggested just omitting the
caller from **JNI** linkage warnings.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601140578
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v3]
On Wed, 15 May 2024 00:54:43 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/arguments.cpp line 2271:
>>
>>> 2269: } else if (match_option(option, "--illegal-native-access=",
>>> )) {
>>> 2270: if (!create_module_property("jdk.module.illegal.native.access",
>>> tail, InternalProperty)) {
>>> 2271: return JNI_ENOMEM;
>>
>> I think it would be helpful to get guidance on if this is the right way to
>> add this system property, only because this one not a "module property". The
>> configuration (WriteableProperty + InternalProperty) look right.
>
> So my recollection/understanding is that we use this mechanism to convert
> module-related `--` flags passed to the VM into system properties that the
> Java code can then read, but we set them up such that you are not allowed to
> specify them directly via `-D`. Is the question here whether this new
> property should be in the `jdk.module` namespace?
That's my recollection too. The usage here isn' related to modules which makes
me wonder if this function should be renamed (not by this PR of course) of if
we should be using PropertyList_unique_add (with AddProperty,
WriteableProperty, InternalProperty) instead. There will be further GNU style
options coming that will likely need to map to an internal system property in
the same way.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1601002132
