On Tue, 26 Mar 2024 19:36:04 GMT, Phil Race <p...@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. 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18486#discussion_r1540221335
