On Thu, 20 Mar 2025 22:40:51 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
>> make/test/JtregNativeHotspot.gmk line 121: >> >>> 119: -I$(VM_TESTBASE_DIR)/nsk/stress/jni \ >>> 120: -I$(VM_TESTBASE_DIR)/vm/mlvm/share \ >>> 121: -I$(VM_TESTBASE_DIR)/vm/share \ >> >> Including these directories for every native test seems a little risky. Are >> we sure there are no header files with the same name? Can we not still add >> these only if the test needs them e.g. if it has vmTestBase in its name for >> the nsk tests? > > I don't know if it is "risky". Compare this with how all hotspot files are > compiled, with all these include directories: > > build/macosx-aarch64/hotspot/variant-server/gensrc > build/macosx-aarch64/hotspot/variant-server/gensrc/adfiles > build/macosx-aarch64/support/modules_include/java.base > build/macosx-aarch64/support/modules_include/java.base/darwin > open/src/hotspot/cpu/aarch64 > open/src/hotspot/os_cpu/bsd_aarch64 > open/src/hotspot/os/bsd > open/src/hotspot/os/posix > open/src/hotspot/os/posix/include > open/src/hotspot/share > open/src/hotspot/share/include > open/src/java.base/share/native/include > open/src/java.base/share/native/libjimage > open/src/java.base/unix/native/include > > > Are we sure that there are no header files of the same name here? We can > check, and there are none. Can we be sure that no one will add a conflicting > header? Well, yes, since then the product would not compile as expected. > > Going back to the test files. This is the complete list of all header files > present in these directories: > > test/hotspot/jtreg/vmTestbase/nsk/share/aod/aod.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jni/jni_tools.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/agent_common/agent_common.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/aod/jvmti_aod.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/Injector.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_FollowRefObjects.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/JVMTITools.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/native/native_thread.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_list.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_mutex.hpp > test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.hpp > test/hotspot/jtreg/vmTestbase/nsk/stress/jni/jnihelper.hpp > test/hotspot/jtreg/vmTestbase/vm/mlvm/share/mlvmJvmtiUtils.hpp > test/lib/jdk/test/lib/jvmti/jvmti_common.hpp > test/lib/jdk/test/lib/jvmti/jvmti_thread.hpp > > > As you can see, there are no conflicts. > > Most of the names are very specific and include some part of their specific > "package" in the name. It seems highly unlikely to me that someone would add > another header file with the same name as any of these. > > And, also, if someone were to do that after this PR, the tests would fail to > compile as expected, so it can't really happen by mistake. > > (I do note the weird fact that there is both a `jvmti_tools.hpp` a... Okay, lets give it a try. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24130#discussion_r2006936847
