On Tue, 11 Mar 2025 19:35:45 GMT, Erik Joelsson <[email protected]> wrote:
>> Allows for future support for platforms that require different flags for
>> libiconv support.
>>
>> Sponsored-by: The FreeBSD Foundation
>
> I think this looks ok, but please wait for Magnus to have a look too.
@erikj79 Thanks for the review. I'll wait for Magnus as well, and for the tests
to pass :)
One thing I was wondering about is that with this change I think it should be
safe to just use `$(ICONV_CFLAGS)` etc when setting the main variable.
I.e. instead of:
CFLAGS := $(LIBSPLASHSCREEN_CFLAGS) \
$(GIFLIB_CFLAGS) $(LIBJPEG_CFLAGS) $(PNG_CFLAGS) $(LIBZ_CFLAGS), \
CFLAGS_aix := $(ICONV_CFLAGS), \
CFLAGS_macosx := $(ICONV_CFLAGS), \
We could just do:
CFLAGS := $(LIBSPLASHSCREEN_CFLAGS) \
$(GIFLIB_CFLAGS) $(LIBJPEG_CFLAGS) $(PNG_CFLAGS) $(LIBZ_CFLAGS) \
$(ICONV_CFLAGS), \
The reason I kept it separate for now is that it used separate assignments for
the LDFLAGS variable per platform.
However, it you consider it better to combine it into one platform neutral
assignment, I can do that instead.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23995#issuecomment-2715520647
