On 04/09/2020 21:55, Aleksei Voitylov wrote:
Alan,
here is a simpler version which, for the two tests in question, refers
to a local helper class with a native method:
http://cr.openjdk.java.net/~avoitylov/webrev.8247589.07/
If there is a preference to avoid JNI, there is yet another alternative:
split the two launcher tests in question into tests with @requires
vm.musl | os.family = "aix" and with @requires !vm.musl & os.family !=
"aix".
The download side of using JNI in these tests is that it complicates the
setup a bit for those that run jtreg directly and/or just build the JDK
and not the test libraries. You could reduce this burden a bit by
limiting the load library/isMusl check to Linux only, meaning isMusl
would not be called on other platforms.
The alternative you suggest above might indeed be better. I assume you
don't mean splitting the tests but rather just adding a second @test
description so that the vm.musl case runs the test with a system
property that allows the test know the expected load library path behavior.
The updated comment in java_md.c in this looks good. A minor comment on
Platform.isBusybox is Files.isSymbolicLink returning true implies that
the link exists so no need to check for exists too. Also the
if-then-else style for the new class in ProcessBuilder/Basic.java is
inconsistent with the rest of the test so it stands out.
Given the repo transition this weekend then I assume you'll create a PR
for the final review at least. Also I see JEP 386 hasn't been targeted
yet but I assume Boris, as owner, will propose-to-target and wait for it
to be targeted before it is integrated.
-Alan
