On Fri, 11 Aug 2023 01:49:57 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> On OSX, don't require that sudo be used to launch SA tools if developer mode
> is enabled. More details are in the CR.
>
> Due to this change, the following tests are no longer being skipped if the
> host has developer mode is enabled. They previously required running as root
> because if sudo was used some files were created with root ownership and
> could not be deleted:
>
> serviceability/sa/ClhsdbDumpclass.java
> serviceability/sa/TestClassDump.java
>
> I tested on my home system (M1) with developer mode enabled and also
> disabled. Also tested with CI, which appears to have developer mode disabled
> on most hosts, but there are a few where it is enabled.
This seems reasonable in principle but I would be concerned about the overhead
of exec'ing another process here. How many test actually use this? Is this
query only ever executed per VM lifetime? (otherwise we should cache the
result).
Thanks.
test/lib/jdk/test/lib/SA/SATestUtils.java line 98:
> 96: private static boolean developerModeEnabled() {
> 97: List<String> cmd = new ArrayList<String>();
> 98: cmd.add("DevToolsSecurity");
Is this tool likely to be in the path?
test/lib/jdk/test/lib/SA/SATestUtils.java line 113:
> 111: String err = new String(p.getErrorStream().readAllBytes());
> 112: System.out.print("DevToolsSecurity stdout: " + out);
> 113: if (out.equals("")) System.out.println();
This looks odd - why not just an unconditional println?
-------------
PR Review: https://git.openjdk.org/jdk/pull/15238#pullrequestreview-1573032069
PR Review Comment: https://git.openjdk.org/jdk/pull/15238#discussion_r1290919192
PR Review Comment: https://git.openjdk.org/jdk/pull/15238#discussion_r1290919017
