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