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

Reply via email to