RFR: 8329086: Clean up java.desktop native compilation

[Magnus Ihse Bursie]

On Tue, 26 Mar 2024 23:10:44 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> 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.
>
> 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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540237249