On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
I made a few review suggestions. Does the symptom happen on both, arm64 and x64?
test/lib/jdk/test/lib/Platform.java line 267:
> 265: // Find the path to the java binary.
> 266: String jdkPath = System.getProperty("java.home");
> 267: Path javaPath = Paths.get(jdkPath + "/bin/java");
You could do it more concisely:
Path javaPath = Paths.get(System.getProperty("java.home") + "/bin/java");
if (Files.notExists(javaPath)) {
throw new FileNotFoundException("Could not find file " +
javaPath.toAbsolutePath().toString());
test/lib/jdk/test/lib/Platform.java line 274:
> 272:
> 273: // Run codesign on the java binary.
> 274: ProcessBuilder pb = new ProcessBuilder("codesign", "--display",
> "--verbose", javaFileName);
And then here 'ProcessBuilder pb = new ProcessBuilder("codesign", "--display",
"--verbose", javaPath.toAbsolutePath().toString());`
test/lib/jdk/test/lib/Platform.java line 277:
> 275: pb.redirectErrorStream(true); // redirect stderr to stdout
> 276: Process codesignProcess = pb.start();
> 277: BufferedReader is = new BufferedReader(new
> InputStreamReader(codesignProcess.getInputStream()));
Maybe do the BufferedReader stuff in a try-with-resources and then return false
instead of potentially throwing an IOException?
test/lib/jdk/test/lib/Platform.java line 280:
> 278: String line;
> 279: while ((line = is.readLine()) != null) {
> 280: System.out.println("STDOUT: " + line);
This output is for every line seems too much. Maybe just print the lines where
you find "Info.plist=not bound" or "Info.plist entries="?
test/lib/jdk/test/lib/util/CoreUtils.java line 153:
> 151: throw new SkippedException("Cannot produce core file
> with hardened binary on OSX 10.15 and later");
> 152: }
> 153: } else {
Maybe you could do just one case here:
`else if (!Platform.hasPlistEntriesOSX()) {`...
-------------
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1491050995
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237190071
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237190832
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237191976
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237194492
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237184922
