RFR: 8311302: Implement JEP 493: Linking Run-Time Images without JMODs [v40]
Severin Gehwolf
sgehwolf at openjdk.org
Wed Oct 30 21:33:43 UTC 2024
On Fri, 25 Oct 2024 19:59:47 GMT, Mandy Chung <mchung at openjdk.org> wrote:
> I see you add the last line in the help message.
>
> ```
> Capabilities: +run-time-image
> ```
>
> This needs some discussion/consideration how that information be conveyed.
OK. It's currently part of the JEP, though which should explain what it does. Open for suggestions, though. I've taken inspiration from JVM features which use a similar `+/-` model.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 526:
>
>> 524: }
>> 525: } catch (IOException e) {
>> 526: throw new InternalError("Failed to process run-time image resources " +
>
> For unexpected exceptions, some code throws InternalError and some throws AssertionError - better to be consistent and I think implementation typically throws InternalError. For IOException, I think `UncheckedIOException` is appropriate.
I've changed it to AssertionError, but can change it to UncheckedIOException instead. Will take another pass on this issue tomorrow.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/JmodsReader.java line 41:
>
>> 39:
>> 40: @SuppressWarnings("try")
>> 41: public class JmodsReader implements JimageDiffGenerator.ImageResource {
>
> Is JmodsReader leftover from previous implementation? It can be removed?
Removed. As is ImageReader. They were both used when reading from jmods directory a modules image respectively. Neither is the case any more and we only use `ResourcePoolReader` now.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/RuntimeImageLinkException.java line 29:
>
>> 27:
>> 28: /**
>> 29: * Exception thrown for links without packaged modules. I.e. run-image link.
>
> I found inconsistent terminologies "run-image" "run-time" "JDK run-time". Better to use consistent terminologies.
>
> For this one, maybe as simple as this:
>
> Suggestion:
>
> * Exception thrown for linking without packaged modules
Should be fixed.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/RuntimeImageLinkException.java line 38:
>
>> 36: private final IllegalArgumentException iae;
>> 37:
>> 38: public RuntimeImageLinkException(IllegalArgumentException cause) {
>
> Can you explain why `RuntimeImageLinkException` constructor takes IAE. The places throwing RILE has to create IAE. I can't see how IAE is necessary as other places would catch RILE and throw IAE or IOException instead.
>
> Can it simply take an exception message parameter? And possibly change `JlinkTask::run` in handling RILE like IAE?
>
>
> } catch (IllegalArgumentException | ResolutionException | RuntimeImageLinkException e) {
> log.println(taskHelper.getMessage("error.prefix") + " " + e.getMessage());
> if (DEBUG) {
> e.printStackTrace(log);
> }
> return EXIT_ERROR;
> ```
This was left-over from previous code. It's now just taking a string.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2448399505
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1823404901
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1823401864
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1823400499
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1823399864
More information about the build-dev
mailing list