RFR: 8329086: Clean up java.desktop native compilation
Erik Joelsson
erikj at openjdk.org
Tue Mar 26 23:24:26 UTC 2024
On Tue, 26 Mar 2024 19:36:04 GMT, Phil Race <prr at openjdk.org> wrote:
>> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 280:
>>
>>> 278: # as includes, instead the system headers should be used.
>>> 279: LIBLCMS_HEADERS_FROM_SRC := false
>>> 280: # FIXME: Keep old behavior and reset LCMS_CFLAGS. This is likely a bug.
>>
>> A comment here: This code is equivalent with the old code, but it seems pretty obvious that this is a bug. I'm somewhat reluctant to changing the actual behavior in a refactor PR like this, but otoh this is a very small fix that would only affect someone running with an external lcms with non-empty CFLAGS. So if anyone thinks I should fix this right now in this PR, I can do so. Otherwise I'll file a follow-up bug and fix this in that one instead. (If nothing else, I think backporters will thank me for going that route instead.)
>
> I don't understand this. What bug ?
> The prior value LCMS_CFLAGS doesn't matter if you are not building from src.
This took me a while to decode. The old code will unconditionally override `LCMS_CFLAGS` which means whatever value it gets in configure is overwritten. That certainly seems like a bug.
Your current patch clears the variable when building with an external liblcms. I agree this is equivalent behavior for the external liblcms case. If we can assume that `LCMS_CFLAGS` from configure is always empty when building liblcms from source, then it's also equivalent when building from source. I assume that `LCMS_CFLAGS` is only ever populated with `-I` paths pointing to the header files of an external liblcms.
Am I understanding correctly that the fix you are proposing is to stop clearing `LCMS_CFLAGS` and just trust that configure sets it with the correct values if needed? I suppose doing it in a followup is probably better.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540197524
More information about the build-dev
mailing list