On Fri, 8 Mar 2024 10:10:23 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> I think what matters for this test test most is which platforms we build 
>> `jspawnhelper` for, and those platforms indeed are:
>> 
>> 
>> ifeq ($(call isTargetOs, macosx aix linux), true)
>>   $(eval $(call SetupJdkExecutable, BUILD_JSPAWNHELPER, \
>> 
>> 
>> So I'd say we just add `(os.family == "mac")` here. I would find it a bit 
>> weird to ask for `os.family != "windows"`, which would implicitly rely on 
>> exhaustiveness of current os family types.
>
> Hm, that is not ideal code in the makefile. `jspawnhelper` is called from 
> `src/java.base/unix/classes/java/lang/ProcessImpl.java`, so it is relied upon 
> for all Unix implementation. Granted, this is currently the same as the list 
> "macosx aix linux", but it would be better to express the same logic in the 
> makefile as in the code.

JDK-8327675 was just integrated, which replaces the build logic for 
jspawnhelper to check for "unix" instead of enumerating all our unixes. I 
suggest the same pattern should be used here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1518152090

Reply via email to