On Tue, 26 Mar 2024 19:46:00 GMT, Phil Race <p...@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 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 .. `#` is the line comment marker in makefiles. What you see here is a common pattern in our makefiles, typically used in multi-line lists. The trailing # is a trick to avoid having a special treatment on the last line (without a ``), which is all too easy to forget, and very hard to debug. This particular list is very short, so `LIBJAWT_EXTRA_HEADER_DIRS := include` would have yielded the same result, and I would normally have chosen that syntax, but in this case the list will be conditionally appended to just below, so in a conceptual sense the list is much longer, but the limited syntax in makefiles does not allow for us to express this in a better way. > 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. Here, as in many places, there have been attempts at keeping the lists sorted, but unfortunately this has not been kept 100%. :-( The extraction of -ljava is essential for the upcoming JDK_LIB rewrite. Once again, if you have any suggestions to what tests you think I should run, let me know! ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540223963 PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540225111