Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Wed, 16 Dec 2020 14:40:14 GMT, Alan Bateman wrote: > javadoc-modules.conf is probably okay although someone finding this in the > repo might initially think it's the configuration for the javadoc modules. > That plus it sets DOCS_MODULES, so maybe it should be apidocs-modules.conf. I'm okay with apidocs-modules.conf or docs-modules.conf. It's good to match the make target name which makes it easier to understand what this configuration is for. > module-loader-map.conf works as the configuration file that defines > BOOT_MODULES and PLATFORM_MODULES. I think AGGREGATOR_MODULES should be > dropped and "java.se" added to PLATFORM_MODULES. If I remember correctly, > this was separated out in JDK 9 and 10 because of the java.se.ee aggregator > module (that one was removed in Java 11 by JEP 320). module-loader-map.conf works for me too. We can drop AGGREGATOR_MODULES since java.se is the sole aggregator module in JDK now. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Wed, 16 Dec 2020 13:51:50 GMT, Magnus Ihse Bursie wrote: >> The update to JRE_MODULES in Images.gmk resolves my comment above. However, >> the naming for the configuration is still a bit odd, e.g. >> module-sets-classloaders.conf should be something like >> module-loader-map.conf as used to generate ModuleLoaderMap.java in the build. > > @AlanBateman I don't have a problem with renaming the conf files, I just did > not know what you wanted them to be called. :-) I renamed > `module-sets-classloaders.conf` to `module-loader-map.conf`. Based on this, I > rename the other two files `javadoc-modules.conf` and > `build-module-sets.conf`, respectively. I hope this is okay. Otherwise, > please just let me know what you think they should be called. Thanks for the update. javadoc-modules.conf is probably okay although someone finding this in the repo might initially think it's the configuration for the javadoc modules. That plus it sets DOCS_MODULES, so maybe it should be apidocs-modules.conf. module-loader-map.conf works as the configuration file that defines BOOT_MODULES and PLATFORM_MODULES. I think AGGREGATOR_MODULES should be dropped and "java.se" added to PLATFORM_MODULES. If I remember correctly, this was separated out in JDK 9 and 10 because of the java.se.ee aggregator module (that one was removed in Java 11 by JEP 320). We should probably look at UPGRADEABLE_MODULES while we are here. This is the modules that are overriddable by way of excluding from the hashes stored in java.base (CreateJmods.gmk). I think it's okay to leave it in module-loader-map.conf because these modules are mapped to the platform class loader. Could we just rename it to UPGRADEABLE_PLATFORM_MODULES so that its a bit clearer (in Modules.gmk) as to why they are append to PLATFORM_MODULES? - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Wed, 16 Dec 2020 10:21:08 GMT, Alan Bateman wrote: >> @mlchung The entire point of this exercise is to *not* have hard-coded lists >> of modules in make files... >> >> Having hard-coded lists have come back to bite us, time after time again. We >> try to auto-discover everything that is possible. For these sets of modules, >> however, auto-discovery is not possible since these lists *define* what we >> mean by e.g. platform modules or an interim image. > > The update to JRE_MODULES in Images.gmk resolves my comment above. However, > the naming for the configuration is still a bit odd, e.g. > module-sets-classloaders.conf should be something like module-loader-map.conf > as used to generate ModuleLoaderMap.java in the build. @AlanBateman I don't have a problem with renaming the conf files, I just did not know what you wanted them to be called. :-) I renamed `module-sets-classloaders.conf` to `module-loader-map.conf`. Based on this, I rename the other two files `javadoc-modules.conf` and `build-module-sets.conf`, respectively. I hope this is okay. Otherwise, please just let me know what you think they should be called. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Wed, 16 Dec 2020 00:14:02 GMT, Magnus Ihse Bursie wrote: >> Can any of `INTERIM_IMAGE_MODULES` , `HOTSPOT_MODULES` and >> `LANGTOOLS_MODULES` be inlined in the appropriate .gmk file? >> >> `INTERIM_IMAGE_MODULES` is for building interim image. If it has to be >> defined in a conf file, I like its name be explicit and match the target or >> makefile, for example, `interim-images.conf` or `InterimImages.conf`. >> This way I can tell what this conf file intends for. What do you think? > > @mlchung The entire point of this exercise is to *not* have hard-coded lists > of modules in make files... > > Having hard-coded lists have come back to bite us, time after time again. We > try to auto-discover everything that is possible. For these sets of modules, > however, auto-discovery is not possible since these lists *define* what we > mean by e.g. platform modules or an interim image. The update to JRE_MODULES in Images.gmk resolves my comment above. However, the naming for the configuration is still a bit odd, e.g. module-sets-classloaders.conf should be something like module-loader-map.conf as used to generate ModuleLoaderMap.java in the build. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 23:50:54 GMT, Mandy Chung wrote: >> Do you see a way to get rid of `DOCS_MODULES` but determine this set at >> build time? IIRC it was added for expediency for >> [JDK-8172312](https://bugs.openjdk.java.net/browse/JDK-8172312). This is >> the set of Java SE + JDK modules that excludes `jdk.internal.*` modules and >> `jdk.unsupported` and also platform-specific modules. (History: the docs >> bundle generated prior to JDK 9 only included platform-neutral APIs.) >> >> As for the conf file for module to class loader mapping, I actually like one >> single file `jdk-modules.conf` to enumerate all JDK modules. Currently it >> only defines the list of modules defined to boot and platform class loader >> but excludes any modules defined to application class loaders. I consider >> to enumerate all modules in this file and the build can verify if any module >> is missing. >> >> `module-sets-build.conf` is a bit awkward and I will give more thought on >> naming ideas. > > Can any of `INTERIM_IMAGE_MODULES` , `HOTSPOT_MODULES` and > `LANGTOOLS_MODULES` be inlined in the appropriate .gmk file? > > `INTERIM_IMAGE_MODULES` is for building interim image. If it has to be > defined in a conf file, I like its name be explicit and match the target or > makefile, for example, `interim-images.conf` or `InterimImages.conf`.This > way I can tell what this conf file intends for. What do you think? @mlchung The entire point of this exercise is to *not* have hard-coded lists of modules in make files... Having hard-coded lists have come back to bite us, time after time again. We try to auto-discover everything that is possible. For these sets of modules, however, auto-discovery is not possible since these lists *define* what we mean by e.g. platform modules or an interim image. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 22:58:47 GMT, Mandy Chung wrote: >> `JRE_TOOL_MODULES` started with more than one modules in JDK 9: >> >> JRE_TOOL_MODULES += \ >> jdk.jdwp.agent \ >> jdk.pack \ >> jdk.scripting.nashorn.shell \ >> # >> >> Since only `jdk.jdwp.agent` one module is left for `JRE_TOOL_MODULES`, as >> you are refactoring this file, I suggest to get rid of `JRE_TOOL_MODULES` >> and explicitly name `jdk.jdwp.agent` in `JRE_MODULES`. > > Do you see a way to get rid of `DOCS_MODULES` but determine this set at build > time? IIRC it was added for expediency for > [JDK-8172312](https://bugs.openjdk.java.net/browse/JDK-8172312). This is > the set of Java SE + JDK modules that excludes `jdk.internal.*` modules and > `jdk.unsupported` and also platform-specific modules. (History: the docs > bundle generated prior to JDK 9 only included platform-neutral APIs.) > > As for the conf file for module to class loader mapping, I actually like one > single file `jdk-modules.conf` to enumerate all JDK modules. Currently it > only defines the list of modules defined to boot and platform class loader > but excludes any modules defined to application class loaders. I consider to > enumerate all modules in this file and the build can verify if any module is > missing. > > `module-sets-build.conf` is a bit awkward and I will give more thought on > naming ideas. Can any of `INTERIM_IMAGE_MODULES` , `HOTSPOT_MODULES` and `LANGTOOLS_MODULES` be inlined in the appropriate .gmk file? `INTERIM_IMAGE_MODULES` is for building interim image. If it has to be defined in a conf file, I like its name be explicit and match the target or makefile, for example, `interim-images.conf` or `InterimImages.conf`.This way I can tell what this conf file intends for. What do you think? - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 22:47:16 GMT, Mandy Chung wrote: >> As for `JRE_TOOL_MODULES`, I understand what you mean but it is at least >> kind of a "sibling" to the others. After all, we use these sets of modules >> together to form the set of modules for the JRE: >> >> JRE_MODULES += $(filter $(ALL_MODULES), $(BOOT_MODULES) \ >> $(PLATFORM_MODULES) $(JRE_TOOL_MODULES)) >> >> So given that `BOOT_MODULES` and `PLATFORM_MODULE` has a role to play here >> as well, I think it would be odd *not* to have `JRE_TOOL_MODULES` defined at >> the same place. > > `JRE_TOOL_MODULES` started with more than one modules in JDK 9: > > JRE_TOOL_MODULES += \ > jdk.jdwp.agent \ > jdk.pack \ > jdk.scripting.nashorn.shell \ > # > > Since only `jdk.jdwp.agent` one module is left for `JRE_TOOL_MODULES`, as you > are refactoring this file, I suggest to get rid of `JRE_TOOL_MODULES` and > explicitly name `jdk.jdwp.agent` in `JRE_MODULES`. Do you see a way to get rid of `DOCS_MODULES` but determine this set at build time? IIRC it was added for expediency for [JDK-8172312](https://bugs.openjdk.java.net/browse/JDK-8172312). This is the set of JDK (non-Java SE) modules (excluding `jdk.internal.*` modules and `jdk.unsupported` and also platform-specific modules). As for the conf file for module to class loader mapping, I actually like one single file `jdk-modules.conf` to enumerate all JDK modules. Currently it only defines the list of modules defined to boot and platform class loader but excludes any modules defined to application class loaders. I consider to enumerate all modules in this file and the build can verify if any module is missing. `module-sets-build.conf` is a bit awkward and I will give more thought on naming ideas. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 20:32:05 GMT, Magnus Ihse Bursie wrote: >> I thought it was a consistent and clear naming scheme. :-) But I guess to >> each their own... >> >> Would `classloader-modules.conf`, `docs-modules.conf` and >> `build-modules.con` be better? Otherwise you'll need to come up with any >> better solutions yourself, since I'm starting to run out of ideas. > > As for `JRE_TOOL_MODULES`, I understand what you mean but it is at least kind > of a "sibling" to the others. After all, we use these sets of modules > together to form the set of modules for the JRE: > > JRE_MODULES += $(filter $(ALL_MODULES), $(BOOT_MODULES) \ > $(PLATFORM_MODULES) $(JRE_TOOL_MODULES)) > > So given that `BOOT_MODULES` and `PLATFORM_MODULE` has a role to play here as > well, I think it would be odd *not* to have `JRE_TOOL_MODULES` defined at the > same place. `JRE_TOOL_MODULES` started with more than one modules in JDK 9: JRE_TOOL_MODULES += \ jdk.jdwp.agent \ jdk.pack \ jdk.scripting.nashorn.shell \ # Since only `jdk.jdwp.agent` one module is left for `JRE_TOOL_MODULES`, as you are refactoring this file, I suggest to get rid of `JRE_TOOL_MODULES` and explicitly name `jdk.jdwp.agent` in `JRE_MODULES`. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 20:27:36 GMT, Magnus Ihse Bursie wrote: >> This looks better but I think we need to find better names for the conf >> files. Prefixing them with "module-sets" looks really strange. >> JRE_TOOL_MODULES in module-sets-classloaders.conf might also need to be >> re-examined because it is not used to generate ModuleLoaderMap. Instead it >> was defined in Modules.gmk for the legacy-jre-image build target. > > I thought it was a consistent and clear naming scheme. :-) But I guess to > each their own... > > Would `classloader-modules.conf`, `docs-modules.conf` and `build-modules.con` > be better? Otherwise you'll need to come up with any better solutions > yourself, since I'm starting to run out of ideas. As for `JRE_TOOL_MODULES`, I understand what you mean but it is at least kind of a "sibling" to the others. After all, we use these sets of modules together to form the set of modules for the JRE: JRE_MODULES += $(filter $(ALL_MODULES), $(BOOT_MODULES) \ $(PLATFORM_MODULES) $(JRE_TOOL_MODULES)) So given that `BOOT_MODULES` and `PLATFORM_MODULE` has a role to play here as well, I think it would be odd *not* to have `JRE_TOOL_MODULES` defined at the same place. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 19:28:33 GMT, Alan Bateman wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Split up module-sets.conf > > This looks better but I think we need to find better names for the conf > files. Prefixing them with "module-sets" looks really strange. > JRE_TOOL_MODULES in module-sets-classloaders.conf might also need to be > re-examined because it is not used to generate ModuleLoaderMap. Instead it > was defined in Modules.gmk for the legacy-jre-image build target. I thought it was a consistent and clear naming scheme. :-) But I guess to each their own... Would `classloader-modules.conf`, `docs-modules.conf` and `build-modules.con` be better? Otherwise you'll need to come up with any better solutions yourself, since I'm starting to run out of ideas. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
On Tue, 15 Dec 2020 18:15:28 GMT, Magnus Ihse Bursie wrote: >> The hard-coded list of modules in `make/common/Modules.gmk` has always been >> a wart in the build system. We pride ourself on using discovery instead of >> hard-coded list. In this case, it is not possible to do do auto-discovery, >> since the different module sets are configured, not determined. >> >> Thus the actual lists of module sets should move to the `make/conf` >> directory. >> >> This is the first patch in a series where I will move configuration values >> spread over the build system into the designated `make/conf` directory. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Split up module-sets.conf This looks better but I think we need to find better names for the conf files. Prefixing them with "module-sets" looks really strange. JRE_TOOL_MODULES in module-sets-classloaders.conf might also need to be re-examined because it is not used to generate ModuleLoaderMap. Instead it was defined in Modules.gmk for the legacy-jre-image build target. - PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v2]
> The hard-coded list of modules in `make/common/Modules.gmk` has always been a > wart in the build system. We pride ourself on using discovery instead of > hard-coded list. In this case, it is not possible to do do auto-discovery, > since the different module sets are configured, not determined. > > Thus the actual lists of module sets should move to the `make/conf` directory. > > This is the first patch in a series where I will move configuration values > spread over the build system into the designated `make/conf` directory. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Split up module-sets.conf - Changes: - all: https://git.openjdk.java.net/jdk/pull/1781/files - new: https://git.openjdk.java.net/jdk/pull/1781/files/9d90330a..df445d6e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1781=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1781=00-01 Stats: 417 lines in 5 files changed: 238 ins; 177 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1781.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1781/head:pull/1781 PR: https://git.openjdk.java.net/jdk/pull/1781
