On Tue, 26 Mar 2024 12:51:38 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> This is a follow-up on > [JDK-8328680](https://bugs.openjdk.org/browse/JDK-8328680), making the same > kind of cleanup to java.desktop. Some code needed more special treatment > here, so there is some additional effects outside of the modules/java.desktop > directory. The code was also in worse shape than other modules, so some > additional changes to the build logic where needed. make/modules/java.desktop/lib/Awt2dLibraries.gmk line 107: > 105: > 106: LIBAWT_CFLAGS += -D__MEDIALIB_OLD_NAMES -D__USE_J2D_NAMES $(X_CFLAGS) > 107: Why is X_CFLAGS no longer needed ? make/modules/java.desktop/lib/Awt2dLibraries.gmk line 145: > 143: -delayload:gdi32.dll -delayload:imm32.dll -delayload:ole32.dll \ > 144: -delayload:shell32.dll -delayload:shlwapi.dll > -delayload:user32.dll \ > 145: -delayload:winmm.dll -delayload:winspool.drv, \ I suppose (presume?) that delayload isn't sensitive to the order ? But I do have to ask if you ran all the client automated tests, as well as making sure builds work ? And surely logical ordering related to dependencies is more important than lexical sort order ? make/modules/java.desktop/lib/Awt2dLibraries.gmk line 161: > 159: LIBS_windows := advapi32.lib comctl32.lib comdlg32.lib delayimp.lib \ > 160: gdi32.lib imm32.lib kernel32.lib ole32.lib shell32.lib > shlwapi.lib \ > 161: user32.lib uuid.lib winmm.lib winspool.lib, \ same as above, I don't think sort oder is what is important here. make/modules/java.desktop/lib/Awt2dLibraries.gmk line 569: > 567: LIBJAWT_EXTRA_HEADER_DIRS := \ > 568: include \ > 569: # A syntax question - what does the trailing # do / mean here ? Superficially I'd expect this to expand to "include #" which means there'd be a # embedded when you append the other folders below .. make/modules/java.desktop/lib/Awt2dLibraries.gmk line 848: > 846: -framework Metal \ > 847: -framework OpenGL \ > 848: -framework QuartzCore \ The re-ordering here happens to be small, and it looks like some one already might have been doing somethings lexically, but this again points to the need for running tests as well as building. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1539965731 PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1539974347 PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1539977625 PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540000977 PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540011695
