RFR: 8329086: Clean up java.desktop native compilation [v3]
Magnus Ihse Bursie
ihse at openjdk.org
Wed Mar 27 10:39:35 UTC 2024
On Tue, 26 Mar 2024 23:38:07 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:
>> The code, prior to this PR, includes these lines:
>>
>> # The fast floor code loses precision.
>> LCMS_CFLAGS=-DCMS_DONT_USE_FAST_FLOOR -DCMS_NO_HALF_SUPPORT
>>
>>
>> This will overwrite the value of `LCMS_CFLAGS` as given in `spec.gmk` by configure. It is normally empty, but if you run `configure --with-lcms=system`, then the configure script will query `pkg-config` about the value needed for `CFLAGS` for LCMS. On my test system it was still empty, but there is no guarantee that it will be so. (And if we truly believed that would be the case, we wouldn't export LCMS_CFLAGS in spec.gmk...)
>>
>> Hence, it is a bug to replace this value. I am pretty certain that the intention here was to add `-DCMS_DONT_USE_FAST_FLOOR -DCMS_NO_HALF_SUPPORT` to the compilation, in addition to any value LCMS_CFLAGS would have from configure.
>
>> If we can assume that LCMS_CFLAGS from configure is always empty when building liblcms from source
>
> It is. It is only set in configure if we set USE_EXTERNAL_LCMS to true.
>
>> I assume that LCMS_CFLAGS is only ever populated with -I paths pointing to the header files of an external liblcms.
>
> Presumably; we don't even set it ourselves, but rely on pkg-config. In theory it could also include e.g. -D values. In practice, it was empty in my test machine.
>
>> 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.
>
> Yes, that is correctly understood.
>
> I'll do it in a separate followup then.
I created https://bugs.openjdk.org/browse/JDK-8329173 to fix this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540846886
More information about the build-dev
mailing list