On Thu, 21 Mar 2024 11:49:11 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> This is the first step of several, in which I will clean up the native > compilation code as used by modules. In this first step `java.base`, > `java.deskop`, `jdk.accessibility` and `jdk.jpackage` are left out, since > they require more work. The changes in the remaining modules are trivial by > comparison. > > The changes done here are: > > 1) A new argument `JDK_LIB` has been introduced, that is used for linking > with other libraries produced by the JDK build. As a follow-up, this will be > further cleaned up and generalized, but the goal for this change is just to > separate them out from external libraries. > > 2) The list of libraries given to `LIB` and `JDK_LIB` has been sorted in > alphabetical order. Note that this change will affect the resulting binaries > (since the order libraries are given are stored in the binary), but this > change should only be superficial. (If we have symbol clashes between > libraries, then we have problems on a whole other level...). > > 3) The code has been checked for inconsistencies and style guide errors, and > a common programming style has been applied to all `Lib.gmk` and > `Launcher.gmk` files, making sure that all parts follow best practices. > > This PR will be followed up by invidual PRs for the modules requiring not > jsut trivial cleanup (`java.base`, `java.deskop`, `jdk.accessibility` and > `jdk.jpackage`), and a PR which will unify `JDK_LIB` handling across > platforms, and automatically apply proper dependencies. Looks good, just found some indentation levels not conforming to the convention and as you are fixing style, I pointed them out. make/common/modules/LauncherCommon.gmk line 159: > 157: JDK_LIBS_windows := \ > 158: $(SUPPORT_OUTPUTDIR)/native/java.base/libjava/java.lib \ > 159: $$($1_WINDOWS_JLI_LIB), \ Indentation 4 make/modules/jdk.compiler/Launcher.gmk line 35: > 33: MAIN_CLASS := com.sun.tools.javac.Main, \ > 34: JAVA_ARGS := --add-modules ALL-DEFAULT, \ > 35: CFLAGS := -DEXPAND_CLASSPATH_WILDCARDS, \ Since you are in the area, indentation 4. make/modules/jdk.jdwp.agent/Lib.gmk line 64: > 62: EXTRA_HEADER_DIRS := \ > 63: include \ > 64: libjdwp/export, \ Indentation 4 ------------- Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18430#pullrequestreview-1953003346 PR Review Comment: https://git.openjdk.org/jdk/pull/18430#discussion_r1534366325 PR Review Comment: https://git.openjdk.org/jdk/pull/18430#discussion_r1534374945 PR Review Comment: https://git.openjdk.org/jdk/pull/18430#discussion_r1534379077