On Tue, 26 Mar 2024 23:38:07 GMT, Magnus Ihse Bursie <i...@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
