Re: RFR: JDK-8185734: [Windows] Structured Exception Catcher missing around gtest execution
On Sun, 13 Dec 2020 11:07:42 GMT, Thomas Stuefe wrote: > Hi, > > May I have reviews please for the following patch. > > At the moment, if a crash happens on Windows in gtests, the gtest SEH handler > may be invoked instead of our error handler, and we just see a > one-line-warning "SEH happened blabala". No hs-err file. > > Even worse, if a crash happens inside the VM as part of the gtests, if our > SEH handler does not get involved, this may interfere with VM functionality - > e.g. SafeFetch. > > Whether or not our SEH handler gets involved currently depends on arbitrary > factors: > - whether the fault happens in a VM or Java thread - which have a > __try/__except around their start function - or whether the fault happens > directly in the thread running the test > - Faults in generated code are not handled on x86 but are okay x64 (where a > SEH handler is registered for the code cache region) or on aarch64 (which > uses VEH). > > This patch consists of two parts > > A) It surrounds the gtestlauncher main function with a SEH catcher. For that > to work I also need to export the SEH handler from the hotspot. Note: It is > difficult to place the SEH catcher: SEH is mutually exclusive with C++ > exceptions, and since googletest uses C++ exceptions to communicate test > conditions, the only place to put those __try/__except is really up here, at > the entry of the gtestlauncher main() function. > > B) This is unfortunately not sufficient since googletest uses its own SEH > catcher to wrap each test (see gtest.cc). Since that catcher sits below our > catcher on the stack, it superimposes ours. > > In JBS, @kimbarrett suggested to build gtests with GTEST_HAS_SEH switched off > to prevent gtest from using SEH. Unfortunately that won't work since the use > of death tests means we need SEH. If we switch GTEST_HAS_SEH off, the death > tests don't build. I also do not like this suggestion since this > configuration may have a higher chance of bitrotting upstream. > > The solution I found is to switch off exception catching from user code as > described in [3] using `--gtest_catch_exceptions=0` or the environment > variable `GTEST_CATCH_EXCEPTIONS=0`. Since we do not use C++ exceptions in > the hotspot, this is fine. > > The only drawback is that this cannot be done from within the gtestlauncher > itself. Setting the environment variable in the main function - or even > during dynamic initialization - does not work because the gtestlauncher > itself parses all arguments as part of dynamic initialization. So I did the > next best thing and specified `--gtest_catch_exceptions=0` at the places > where we run the gtests. This is not perfect, but better than nothing. > > Testing: manually on Windows x64, x86, GH actions (Linux errors seem > unrelated to this patch). > > > > Notes: > - If we owned the googletest code - forked it off like we did before - (B) > would be very simple to solve by changing the default for > `gtest_catch_exceptions` to 1. I still believe maintaining a fork of > googletest would have many benefits. > > - Using VEH would have saved us from using __try/__except here, so we would > have not needed (A). ATM we use VEH on aarch, SEH on x64+x32. Uniformly > switching to VEH has been discussed several times in the past, the last > attempt has been by @luhenry (see [1], [2]), but this has not yet > materialized. > > [1] > https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040228.html > [2] > https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/040967.html > [3] > https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#disabling-catching-test-thrown-exceptions src/hotspot/os/windows/os_windows.cpp line 515: > 513: } > 514: > 515: JNIEXPORT I understand if you do not want to start a code cleanup, but it looks like this should be in an external .hpp file, which could then have been included by all the .cpp files, instead of having the declaration repeated. - PR: https://git.openjdk.java.net/jdk/pull/1757
Re: RFR: JDK-8185734: [Windows] Structured Exception Catcher missing around gtest execution
On Sun, 13 Dec 2020 11:07:42 GMT, Thomas Stuefe wrote: > Hi, > > May I have reviews please for the following patch. > > At the moment, if a crash happens on Windows in gtests, the gtest SEH handler > may be invoked instead of our error handler, and we just see a > one-line-warning "SEH happened blabala". No hs-err file. > > Even worse, if a crash happens inside the VM as part of the gtests, if our > SEH handler does not get involved, this may interfere with VM functionality - > e.g. SafeFetch. > > Whether or not our SEH handler gets involved currently depends on arbitrary > factors: > - whether the fault happens in a VM or Java thread - which have a > __try/__except around their start function - or whether the fault happens > directly in the thread running the test > - Faults in generated code are not handled on x86 but are okay x64 (where a > SEH handler is registered for the code cache region) or on aarch64 (which > uses VEH). > > This patch consists of two parts > > A) It surrounds the gtestlauncher main function with a SEH catcher. For that > to work I also need to export the SEH handler from the hotspot. Note: It is > difficult to place the SEH catcher: SEH is mutually exclusive with C++ > exceptions, and since googletest uses C++ exceptions to communicate test > conditions, the only place to put those __try/__except is really up here, at > the entry of the gtestlauncher main() function. > > B) This is unfortunately not sufficient since googletest uses its own SEH > catcher to wrap each test (see gtest.cc). Since that catcher sits below our > catcher on the stack, it superimposes ours. > > In JBS, @kimbarrett suggested to build gtests with GTEST_HAS_SEH switched off > to prevent gtest from using SEH. Unfortunately that won't work since the use > of death tests means we need SEH. If we switch GTEST_HAS_SEH off, the death > tests don't build. I also do not like this suggestion since this > configuration may have a higher chance of bitrotting upstream. > > The solution I found is to switch off exception catching from user code as > described in [3] using `--gtest_catch_exceptions=0` or the environment > variable `GTEST_CATCH_EXCEPTIONS=0`. Since we do not use C++ exceptions in > the hotspot, this is fine. > > The only drawback is that this cannot be done from within the gtestlauncher > itself. Setting the environment variable in the main function - or even > during dynamic initialization - does not work because the gtestlauncher > itself parses all arguments as part of dynamic initialization. So I did the > next best thing and specified `--gtest_catch_exceptions=0` at the places > where we run the gtests. This is not perfect, but better than nothing. > > Testing: manually on Windows x64, x86, GH actions (Linux errors seem > unrelated to this patch). > > > > Notes: > - If we owned the googletest code - forked it off like we did before - (B) > would be very simple to solve by changing the default for > `gtest_catch_exceptions` to 1. I still believe maintaining a fork of > googletest would have many benefits. > > - Using VEH would have saved us from using __try/__except here, so we would > have not needed (A). ATM we use VEH on aarch, SEH on x64+x32. Uniformly > switching to VEH has been discussed several times in the past, the last > attempt has been by @luhenry (see [1], [2]), but this has not yet > materialized. > > [1] > https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040228.html > [2] > https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/040967.html > [3] > https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#disabling-catching-test-thrown-exceptions Build changes look good. I left a note on the hotspot code; feel free to ignore it if you want. :) I agree with your assessment that restoring gtest to the code base would make life simpler. I'm not sure what needs to be done to do that; I suppose it needs someone to write and implement a JBS feature, and to publicly acknowledge that they intent to spend the time needed to keep it up to date with upstream going forward. (I also agree with your assessment on VEH replacing SEH, as my recollection of working with the runtime system on JRockit was that VEH was superior for the needs of a VM, and much closer to the posix signal model.) - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1757
Re: RFR: JDK-8185734: [Windows] Structured Exception Catcher missing around gtest execution
On Tue, 15 Dec 2020 08:16:51 GMT, Magnus Ihse Bursie wrote: >> Hi, >> >> May I have reviews please for the following patch. >> >> At the moment, if a crash happens on Windows in gtests, the gtest SEH >> handler may be invoked instead of our error handler, and we just see a >> one-line-warning "SEH happened blabala". No hs-err file. >> >> Even worse, if a crash happens inside the VM as part of the gtests, if our >> SEH handler does not get involved, this may interfere with VM functionality >> - e.g. SafeFetch. >> >> Whether or not our SEH handler gets involved currently depends on arbitrary >> factors: >> - whether the fault happens in a VM or Java thread - which have a >> __try/__except around their start function - or whether the fault happens >> directly in the thread running the test >> - Faults in generated code are not handled on x86 but are okay x64 (where a >> SEH handler is registered for the code cache region) or on aarch64 (which >> uses VEH). >> >> This patch consists of two parts >> >> A) It surrounds the gtestlauncher main function with a SEH catcher. For that >> to work I also need to export the SEH handler from the hotspot. Note: It is >> difficult to place the SEH catcher: SEH is mutually exclusive with C++ >> exceptions, and since googletest uses C++ exceptions to communicate test >> conditions, the only place to put those __try/__except is really up here, at >> the entry of the gtestlauncher main() function. >> >> B) This is unfortunately not sufficient since googletest uses its own SEH >> catcher to wrap each test (see gtest.cc). Since that catcher sits below our >> catcher on the stack, it superimposes ours. >> >> In JBS, @kimbarrett suggested to build gtests with GTEST_HAS_SEH switched >> off to prevent gtest from using SEH. Unfortunately that won't work since the >> use of death tests means we need SEH. If we switch GTEST_HAS_SEH off, the >> death tests don't build. I also do not like this suggestion since this >> configuration may have a higher chance of bitrotting upstream. >> >> The solution I found is to switch off exception catching from user code as >> described in [3] using `--gtest_catch_exceptions=0` or the environment >> variable `GTEST_CATCH_EXCEPTIONS=0`. Since we do not use C++ exceptions in >> the hotspot, this is fine. >> >> The only drawback is that this cannot be done from within the gtestlauncher >> itself. Setting the environment variable in the main function - or even >> during dynamic initialization - does not work because the gtestlauncher >> itself parses all arguments as part of dynamic initialization. So I did the >> next best thing and specified `--gtest_catch_exceptions=0` at the places >> where we run the gtests. This is not perfect, but better than nothing. >> >> Testing: manually on Windows x64, x86, GH actions (Linux errors seem >> unrelated to this patch). >> >> >> >> Notes: >> - If we owned the googletest code - forked it off like we did before - (B) >> would be very simple to solve by changing the default for >> `gtest_catch_exceptions` to 1. I still believe maintaining a fork of >> googletest would have many benefits. >> >> - Using VEH would have saved us from using __try/__except here, so we would >> have not needed (A). ATM we use VEH on aarch, SEH on x64+x32. Uniformly >> switching to VEH has been discussed several times in the past, the last >> attempt has been by @luhenry (see [1], [2]), but this has not yet >> materialized. >> >> [1] >> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040228.html >> [2] >> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/040967.html >> [3] >> https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#disabling-catching-test-thrown-exceptions > > src/hotspot/os/windows/os_windows.cpp line 515: > >> 513: } >> 514: >> 515: JNIEXPORT > > I understand if you do not want to start a code cleanup, but it looks like > this should be in an external .hpp file, which could then have been included > by all the .cpp files, instead of having the declaration repeated. I agree, but I would like to leave this out for now. I think at some point we may switch to Vectored Exception Handling for all Windows architectures, then we can remove this hideous coding in the gtest launcher main function again. And remove the export. - PR: https://git.openjdk.java.net/jdk/pull/1757
Re: RFR: JDK-8185734: [Windows] Structured Exception Catcher missing around gtest execution
On Tue, 15 Dec 2020 08:20:54 GMT, Magnus Ihse Bursie wrote: > Build changes look good. > > I left a note on the hotspot code; feel free to ignore it if you want. :) > > I agree with your assessment that restoring gtest to the code base would make > life simpler. I'm not sure what needs to be done to do that; I suppose it > needs someone to write and implement a JBS feature, and to publicly > acknowledge that they intent to spend the time needed to keep it up to date > with upstream going forward. Yes, and unfortunately I cannot do this - I think this has to be driven from inside Oracle. > > (I also agree with your assessment on VEH replacing SEH, as my recollection > of working with the runtime system on JRockit was that VEH was superior for > the needs of a VM, and much closer to the posix signal model.) I also agree. @luhenry 's work convinced me, lets see when it bears fruit. Cheers, Thomas - PR: https://git.openjdk.java.net/jdk/pull/1757
Integrated: JDK-8185734: [Windows] Structured Exception Catcher missing around gtest execution
On Sun, 13 Dec 2020 11:07:42 GMT, Thomas Stuefe wrote: > Hi, > > May I have reviews please for the following patch. > > At the moment, if a crash happens on Windows in gtests, the gtest SEH handler > may be invoked instead of our error handler, and we just see a > one-line-warning "SEH happened blabala". No hs-err file. > > Even worse, if a crash happens inside the VM as part of the gtests, if our > SEH handler does not get involved, this may interfere with VM functionality - > e.g. SafeFetch. > > Whether or not our SEH handler gets involved currently depends on arbitrary > factors: > - whether the fault happens in a VM or Java thread - which have a > __try/__except around their start function - or whether the fault happens > directly in the thread running the test > - Faults in generated code are not handled on x86 but are okay x64 (where a > SEH handler is registered for the code cache region) or on aarch64 (which > uses VEH). > > This patch consists of two parts > > A) It surrounds the gtestlauncher main function with a SEH catcher. For that > to work I also need to export the SEH handler from the hotspot. Note: It is > difficult to place the SEH catcher: SEH is mutually exclusive with C++ > exceptions, and since googletest uses C++ exceptions to communicate test > conditions, the only place to put those __try/__except is really up here, at > the entry of the gtestlauncher main() function. > > B) This is unfortunately not sufficient since googletest uses its own SEH > catcher to wrap each test (see gtest.cc). Since that catcher sits below our > catcher on the stack, it superimposes ours. > > In JBS, @kimbarrett suggested to build gtests with GTEST_HAS_SEH switched off > to prevent gtest from using SEH. Unfortunately that won't work since the use > of death tests means we need SEH. If we switch GTEST_HAS_SEH off, the death > tests don't build. I also do not like this suggestion since this > configuration may have a higher chance of bitrotting upstream. > > The solution I found is to switch off exception catching from user code as > described in [3] using `--gtest_catch_exceptions=0` or the environment > variable `GTEST_CATCH_EXCEPTIONS=0`. Since we do not use C++ exceptions in > the hotspot, this is fine. > > The only drawback is that this cannot be done from within the gtestlauncher > itself. Setting the environment variable in the main function - or even > during dynamic initialization - does not work because the gtestlauncher > itself parses all arguments as part of dynamic initialization. So I did the > next best thing and specified `--gtest_catch_exceptions=0` at the places > where we run the gtests. This is not perfect, but better than nothing. > > Testing: manually on Windows x64, x86, GH actions (Linux errors seem > unrelated to this patch). > > > > Notes: > - If we owned the googletest code - forked it off like we did before - (B) > would be very simple to solve by changing the default for > `gtest_catch_exceptions` to 1. I still believe maintaining a fork of > googletest would have many benefits. > > - Using VEH would have saved us from using __try/__except here, so we would > have not needed (A). ATM we use VEH on aarch, SEH on x64+x32. Uniformly > switching to VEH has been discussed several times in the past, the last > attempt has been by @luhenry (see [1], [2]), but this has not yet > materialized. > > [1] > https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040228.html > [2] > https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/040967.html > [3] > https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#disabling-catching-test-thrown-exceptions This pull request has now been integrated. Changeset: 568dc29b Author:Thomas Stuefe URL: https://git.openjdk.java.net/jdk/commit/568dc29b Stats: 17 lines in 5 files changed: 17 ins; 0 del; 0 mod 8185734: [Windows] Structured Exception Catcher missing around gtest execution Reviewed-by: dholmes, ihse - PR: https://git.openjdk.java.net/jdk/pull/1757
Re: RFR: 8247957: remove doclint support for HTML 4 [v2]
On Tue, 15 Dec 2020 07:26:15 GMT, Yoshiki Sato wrote: >> HTML4 is no longer supported in javadoc. >> >> doclint needs to drop HTML4 support as well. >> The changes consist of: >> * Removing jdk.javadoc.internal.doclint.HtmlVersion and its references. >> * Sorting out supported tags and attributes in HTML5 (including fix >> incorrect permission of valign in TH, TR, TD, THEAD and TBODY) >> * Modifying test code and expected outputs to be checked in HTML5 > > Yoshiki Sato has updated the pull request incrementally with one additional > commit since the last revision: > > 8257204 and 8256313 > 8257204: Remove usage of -Xhtmlversion option from javac > 8256313: JavaCompilation.gmk needs to be updated not to use > --doclint-format html5 option Build change look fine. I have not looked at the other changes. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/893
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:22:04 GMT, Jonathan Gibbons wrote: >> This is for JDK16, as a precursor to fixing JDK-8258002. >> >> While it is good to be using localized strings in the generated output, the >> significance for JDK-8258002 is that the strings are now obtained from a >> resource file, and not hardcoded in JavaScript file itself. >> >> The source file `search.js` is renamed to `search.js.template`, and (unlike >> other template files) is copied as-is into the generated image. The values >> in the template are resolved when javadoc is executed, depending on the >> locale in use at the time. Because of the change in the file's extension, >> two makefiles are updated to accommodate the new extension: one is for the >> "interim" javadoc used to generate the API docs; the other is for the >> primary javadoc in the main JDK image. > > make/CompileInterimLangtools.gmk line 77: > >> 75: Standard.java, \ >> 76: EXTRA_FILES := >> $(BUILDTOOLS_OUTPUTDIR)/gensrc/$1.interim/module-info.java, \ >> 77: COPY := .gif .png .xml .css .js .js.template .txt >> javax.tools.JavaCompilerTool, \ > > Build-folk: it would be nice if this macro could use `$(jdk.javadoc_COPY)` > instead of having to duplicate entries. > (Future RFE?) I agree. The entire design of CompileJavaModules.gmk needs to be updated. I've been procrastinating on cleaning this up, maybe it's time to get going on it... - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:19:59 GMT, Jonathan Gibbons wrote: > This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk16/pull/16
RFR: 8258407: Split up CompileJavaModules.gmk into make/modules/$M/Java.gmk
Right now `CompileJavaModules.gmk` contains two different part: one part with the functionality needed to compile a java module, and one part were all special requirements for all modules are listed. The second part should be removed from `CompileJavaModules.gmk`, and instead listed directly for each individual module in `make/modules/$M/Java.gmk`. I used a special-written shell script to automatically extract the module-specific part from CompileJavaModules.gmk into the respective Java.gmk files, to avoid risking any hard-to-detect copy/paste errors. After this I did a `sed -i` to remove the module-specific prefix. All this makes me confident that I have correctly moved the variables (I realize this is hard to verify from the patch). - Commit messages: - $($(MODULE)_COPY_EXTRA) target is not used anymore - Remove module prefix from java variables - Remove debug code mistakenly pushed - Move some more module-specific stuff to Java.gmk files - Only include the module Java.gmk file we need - First step: break out Java settings to separate files Changes: https://git.openjdk.java.net/jdk/pull/1779/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1779&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258407 Stats: 2118 lines in 47 files changed: 1556 ins; 548 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/1779.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1779/head:pull/1779 PR: https://git.openjdk.java.net/jdk/pull/1779
Re: RFR: 8258407: Split up CompileJavaModules.gmk into make/modules/$M/Java.gmk [v2]
> Right now `CompileJavaModules.gmk` contains two different part: one part with > the functionality needed to compile a java module, and one part were all > special requirements for all modules are listed. > > The second part should be removed from `CompileJavaModules.gmk`, and instead > listed directly for each individual module in `make/modules/$M/Java.gmk`. > > I used a special-written shell script to automatically extract the > module-specific part from CompileJavaModules.gmk into the respective Java.gmk > files, to avoid risking any hard-to-detect copy/paste errors. After this I > did a `sed -i` to remove the module-specific prefix. All this makes me > confident that I have correctly moved the variables (I realize this is hard > to verify from the patch). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Rename DISABLED_WARNINGS to DISABLED_WARNINGS_java to avoid overwriting the global variable - Changes: - all: https://git.openjdk.java.net/jdk/pull/1779/files - new: https://git.openjdk.java.net/jdk/pull/1779/files/09b5bee7..1e1cb858 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1779&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1779&range=00-01 Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/1779.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1779/head:pull/1779 PR: https://git.openjdk.java.net/jdk/pull/1779
Re: RFR: 8258407: Split up CompileJavaModules.gmk into make/modules/$M/Java.gmk [v3]
> Right now `CompileJavaModules.gmk` contains two different part: one part with > the functionality needed to compile a java module, and one part were all > special requirements for all modules are listed. > > The second part should be removed from `CompileJavaModules.gmk`, and instead > listed directly for each individual module in `make/modules/$M/Java.gmk`. > > I used a special-written shell script to automatically extract the > module-specific part from CompileJavaModules.gmk into the respective Java.gmk > files, to avoid risking any hard-to-detect copy/paste errors. After this I > did a `sed -i` to remove the module-specific prefix. All this makes me > confident that I have correctly moved the variables (I realize this is hard > to verify from the patch). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Final bug fixes... - Changes: - all: https://git.openjdk.java.net/jdk/pull/1779/files - new: https://git.openjdk.java.net/jdk/pull/1779/files/1e1cb858..a869f4b0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1779&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1779&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1779.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1779/head:pull/1779 PR: https://git.openjdk.java.net/jdk/pull/1779
RFR: 8258411: Move module set configuration from Modules.gmk to conf dir
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. - Commit messages: - 8258411: Move module set configuration from Modules.gmk to conf dir Changes: https://git.openjdk.java.net/jdk/pull/1781/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1781&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258411 Stats: 348 lines in 2 files changed: 179 ins; 162 del; 7 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
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir
On Tue, 15 Dec 2020 16:11:45 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. I really dislike patch as it mixes up several things in module-sets.conf. If you really need to move configuration out of Modules.gmk (and I see no reason to do this) then please look at separating out the static mapping of modules to class loaders, the modules used for the interim builds, and the modules used to create API docs. - Changes requested by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1781
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir
On Tue, 15 Dec 2020 16:53:46 GMT, Alan Bateman 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. > > I really dislike patch as it mixes up several things in module-sets.conf. If > you really need to move configuration out of Modules.gmk (and I see no reason > to do this) then please look at separating out the static mapping of modules > to class loaders, the modules used for the interim builds, and the modules > used to create API docs. @AlanBateman It's not really more mixed-up than it was previously in Modules.gmk, since I lifted the code mostly unchanged from there. But sure, I can split it up further; I agree that it might make sense to do so. - PR: https://git.openjdk.java.net/jdk/pull/1781
RFR: 8258420: Move URL configuration from Docs.gmk to conf dir
In `Docs.gmk` there are some hard-coded links to online URL documentation and bug reporting locations. These should not reside in the make file per se, but instead move to the `make/conf` directory. - Commit messages: - 8258420: Move URL configuration from Docs.gmk to conf dir Changes: https://git.openjdk.java.net/jdk/pull/1785/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1785&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258420 Stats: 39 lines in 2 files changed: 33 ins; 6 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1785.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1785/head:pull/1785 PR: https://git.openjdk.java.net/jdk/pull/1785
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&pr=1781&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1781&range=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
Re: RFR: 8258420: Move URL configuration from Docs.gmk to conf dir
On Tue, 15 Dec 2020 17:50:46 GMT, Magnus Ihse Bursie wrote: > In `Docs.gmk` there are some hard-coded links to online URL documentation and > bug reporting locations. These should not reside in the make file per se, but > instead move to the `make/conf` directory. This looks okay me to but I can't help thinking that javadoc.conf should also be the place to list the root modules for javadoc (PR 1781 puts them in oddly named module-sets-docs.conf). - PR: https://git.openjdk.java.net/jdk/pull/1785
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]
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 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: 8258420: Move URL configuration from Docs.gmk to conf dir
On Tue, 15 Dec 2020 19:19:28 GMT, Alan Bateman wrote: >> In `Docs.gmk` there are some hard-coded links to online URL documentation >> and bug reporting locations. These should not reside in the make file per >> se, but instead move to the `make/conf` directory. > > This looks okay me to but I can't help thinking that javadoc.conf should also > be the place to list the root modules for javadoc (PR 1781 puts them in oddly > named module-sets-docs.conf). Maybe, but if anything I think they should rather merge with the upcoming `conf/branding.conf` (see JDK-8258426). I think that once we got all configuration collected in one place, it's easier to see how and if we can unify them. At this stage, I'd like to concentrate on just splitting out the configuration part from the makefiles. - PR: https://git.openjdk.java.net/jdk/pull/1785
RFR: 8258426: Split up autoconf/version-numbers and move it to conf dir
The file `make/autoconf/version-numbers` is plagued by a two-fold problem: first of all, it's a configuration file, not a part of autoconf, so it should reside in `make/conf` instead of `make/autoconf`. Secondly, contrary to the name, it does not only contain version numbers, but also the branding properties (company name, product name, etc). This should be split out into a separate branding configuration file. This is the last patch in the series of moving configuration into `make/conf`. - Commit messages: - 8258426: Split up autoconf/version-numbers and move it to conf dir Changes: https://git.openjdk.java.net/jdk/pull/1786/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1786&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258426 Stats: 151 lines in 7 files changed: 81 ins; 55 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/1786.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1786/head:pull/1786 PR: https://git.openjdk.java.net/jdk/pull/1786
Re: RFR: 8257620: Do not use objc_msgSend_stret to get macOS version
On Wed, 2 Dec 2020 17:34:00 GMT, Anton Kozlov wrote: > Please review a small change that replaces use of objc_msgSend_stret in macOS > platform code with pure ObjC code. It's also a prerequisite for macOS/AArch64 > support, where objc_msgSend_stret is not available. Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1569
Re: RFR: 8258426: Split up autoconf/version-numbers and move it to conf dir [v2]
> The file `make/autoconf/version-numbers` is plagued by a two-fold problem: > first of all, it's a configuration file, not a part of autoconf, so it should > reside in `make/conf` instead of `make/autoconf`. > > Secondly, contrary to the name, it does not only contain version numbers, but > also the branding properties (company name, product name, etc). This should > be split out into a separate branding configuration file. > > This is the last patch in the series of moving configuration into `make/conf`. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Also update submit.yml - Changes: - all: https://git.openjdk.java.net/jdk/pull/1786/files - new: https://git.openjdk.java.net/jdk/pull/1786/files/b40db3af..bcd3b338 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1786&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1786&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1786.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1786/head:pull/1786 PR: https://git.openjdk.java.net/jdk/pull/1786
Re: RFR: 8257733: Move module-specific data from make to respective module [v3]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [ ] java.base > - [ ] java.desktop > - [x] jdk.compiler > - [ ] java.se Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Move macosxicons from share to macosx - Changes: - all: https://git.openjdk.java.net/jdk/pull/1611/files - new: https://git.openjdk.java.net/jdk/pull/1611/files/f0047704..00dc61c1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=01-02 Stats: 1 line in 2 files changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
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: 8257733: Move module-specific data from make to respective module [v4]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [ ] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie 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 eight additional commits since the last revision: - Update comment refering to "make" dir - Move new symbols to jdk.compiler - Merge branch 'master' into shuffle-data - Move macosxicons from share to macosx - Move to share/data, and move jdwp.spec to java.se - Update references in test - Step 2: Update references - First stage, move actual data files - Changes: - all: https://git.openjdk.java.net/jdk/pull/1611/files - new: https://git.openjdk.java.net/jdk/pull/1611/files/00dc61c1..68b252b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=02-03 Stats: 38451 lines in 948 files changed: 28535 ins; 6611 del; 3305 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v2]
On Thu, 10 Dec 2020 23:07:52 GMT, Naoto Sato wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Move to share/data, and move jdwp.spec to java.se > > Reviewed changes to `characterdata`, `charsetmapping`, `cldr`, `currency`, > `lsrdata`, `tzdata`, and `unicodedata` with minor comment. Looks good overall. I think this is almost ready to be pushed, but I'd like to have someone from the client team review the changes for java.desktop as well. @prrace Any change you can have a look at this? > test/jdk/java/util/Locale/LSRDataTest.java line 57: > >> 55: // path to the lsr file from the make folder, this test relies on the >> 56: // relative path to the file in the make folder, considering >> 57: // test and make will always exist in the same jdk layout > > The comment refers to the "make" folder. (line 55 as well). Fixed. Thank you for noticing! - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8258411: Move module set configuration from Modules.gmk to conf dir [v3]
> 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: Remove JRE_TOOL_MODULES and inline jdk.jdwp.agent in JRE definition - Changes: - all: https://git.openjdk.java.net/jdk/pull/1781/files - new: https://git.openjdk.java.net/jdk/pull/1781/files/df445d6e..ef9d53cb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1781&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1781&range=01-02 Stats: 7 lines in 3 files changed: 0 ins; 6 del; 1 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
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
RFR: 8258445: Move make/templates to make/data
The `templates` directory in `make` is an odd bird. It actually contains data files that the license checker needs, so it should move to `make/data`. - Commit messages: - 8258445: Move make/templates to make/data Changes: https://git.openjdk.java.net/jdk/pull/1790/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1790&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258445 Stats: 5 lines in 4 files changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/1790.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1790/head:pull/1790 PR: https://git.openjdk.java.net/jdk/pull/1790
RFR: 8258447: Move make/hotspot/hotspot.script to make/scripts
The hotspot launcher script is misplaced among the hotspot make files. It should move to make/scripts (and get a proper extension). - Commit messages: - 8258447: Move make/hotspot/hotspot.script to make/scripts Changes: https://git.openjdk.java.net/jdk/pull/1791/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1791&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258447 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1791.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1791/head:pull/1791 PR: https://git.openjdk.java.net/jdk/pull/1791
RFR: 8258449: Move make/hotspot/symbols to make/data
The "symbol" files in make/hotspot/symbols are a list of exported symbols from libjvm. As such, they are an essential data source for the build system, and they should reside in make/data, not mixed with the hotspot make source code. - Commit messages: - 8258449: Move make/hotspot/symbols to make/data Changes: https://git.openjdk.java.net/jdk/pull/1793/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1793&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258449 Stats: 6 lines in 7 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/1793.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1793/head:pull/1793 PR: https://git.openjdk.java.net/jdk/pull/1793
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: 8258447: Move make/hotspot/hotspot.script to make/scripts
On Tue, 15 Dec 2020 23:28:25 GMT, Magnus Ihse Bursie wrote: > The hotspot launcher script is misplaced among the hotspot make files. It > should move to make/scripts (and get a proper extension). Can someone from hotspot please confirm if this script is still needed/used? - PR: https://git.openjdk.java.net/jdk/pull/1791
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: 8247957: remove doclint support for HTML 4 [v2]
On Tue, 15 Dec 2020 09:20:44 GMT, Magnus Ihse Bursie wrote: >> Yoshiki Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8257204 and 8256313 >> 8257204: Remove usage of -Xhtmlversion option from javac >> 8256313: JavaCompilation.gmk needs to be updated not to use >> --doclint-format html5 option > > Build change look fine. I have not looked at the other changes. I found an issue when called from javac, so will convert to draft until the issue resolved. - PR: https://git.openjdk.java.net/jdk/pull/893
