On Thu, 21 Mar 2024 15:27:06 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
>> make/Images.gmk line 131: >> >>> 129: # in FixPath call in order to avoid needing to use strip. >>> 130: RL_JIMAGE_PATH_ARG := $(call >>> FixPath,$(JDK_LINK_OUTPUT_DIR)/lib/modules) >>> 131: RL_MOD_PATH_ARG := $(call FixPath,$(IMAGES_OUTPUTDIR)/jmods) >> >> I'd much rather prefer to use strip and have proper space after commas. This >> is the approach taken elsewhere in the build system. >> >> Suggestion: >> >> RL_JIMAGE_PATH_ARG := $(strip $(call FixPath, >> $(JDK_LINK_OUTPUT_DIR)/lib/modules)) >> RL_MOD_PATH_ARG := $(strip $(call FixPath, $(IMAGES_OUTPUTDIR)/jmods)) > > Thanks! This will also likely change. I'm thinking of just generating the > diff file and putting it into `lib/` of the JDK image. That avoids needing to > call this build-time only jlink plugin and this `FixPath` magic. I see. I'm sorry if I did not fully understand how the ongoing discussions affected build source changes. Maybe I should just put the build part review of this PR on the backburner, and then you can ping me when you think you have a solution that will stick, and I can check how it holds up from a build perspective? >> make/jdk/src/jdk.unsupported_jlink_runtime/share/classes/build/tools/runtimelink/CreateLinkableRuntimePlugin.java >> line 1: >> >>> 1: /* >> >> This file does not at all fit in with the pattern of other build tools, >> which are not module-based but instead live in packages `build.tools.*` in >> the `make/jdk/src/classes` directory. We will need to find a better >> arrangement for this. >> >> First question, do this class really need to be in a separate module? (I'm >> afraid the answer is "yes" but I need to ask it anyway). > >> First question, do this class really need to be in a separate module? (I'm >> afraid the answer is "yes" but I need to ask it anyway). > > Yes, because it uses the `Plugin` ServiceLoader extension using the boot > ModuleLayer. But it's going to be a moot point because this'll get reworked > due to concerns raised by @mlchung (having the plugin pipeline running when > producing a linkable runtime jimage). Ok, fine. Will the new solution still include a build-time only tool? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1534149409 PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1534146496
