RFR: JDK-8218431 Improved platform checking in makefiles

[Erik Joelsson]

Looks good. I have read through all of them and it does not seem like 
you got any wrong.

I note that you chose to express negatives as:

ifneq (isTargetFoo, true)

instead of:

ifeq (isTargetFoo, false)

I think I would have preferred the latter, given that the new macros 
return both true and false. I think true/false is more obvious to the 
eye than the sneaky 'n' in ifneq, but I don't have a strong opinion so 
this is fine too.

/Erik

On 2019-02-05 07:28, Magnus Ihse Bursie wrote:
> On 2019-02-05 15:49, Magnus Ihse Bursie wrote:
>> To check for various aspects of the build or target platform, we do a 
>> lot of checks like:
>>    ifeq ($(OPENJDK_BUILD_OS_ENV), windows.cygwin)
>>        ...
>>
>> The naming of those platform information variables is a bit 
>> unfortunate. Yes, we know we're building OpenJDK, so why the OPENJDK_ 
>> prefix? I've been wanting for a long time to do something about this 
>> odd prefix, and it became more urgent after the recent fix of 
>> JDK-8160926, which pushes the matter about unifying the naming of 
>> build/target variables.
>>
>> The solution in this patch is not to rename the variables per se, but 
>> to introduce an abstraction layer in terms of function calls for 
>> checking platform aspects.
>>
>> This *really* shines when it comes to testing for multiple platforms. 
>> Previously, we were required to resort to constructs such as:
>>
>>    ifneq ($(filter $(OPENJDK_TARGET_OS), windows solaris), )
>>
>> but this can now be expressed as:
>>
>>    ifeq ($(call isTargetOs, windows solaris), true)
>>
>> Or this (actually technically broken) horrible example:
>>
>>    ifneq (, $(findstring $(OPENJDK_TARGET_OS), linux macosx))
>>
>> which I had to read three times before being sure that this was the 
>> correct way to replace it:
>>
>>    ifeq ($(call isTargetOs, linux macosx), true)
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8218431
>> WebRev: 
>> http://cr.openjdk.java.net/~ihse/JDK-8218431-improved-platform-checking/webrev.01
> ... and here's an updated version that fixes a typo:
>
> http://cr.openjdk.java.net/~ihse/JDK-8218431-improved-platform-checking/webrev.02 
>
>
> /Magnus
>
>>
>> /Magnus
>>
>