Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken 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.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> move javaPath into caller
Hi Chris, I renamed the method and adjusted the comment.
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1609529567
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken 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.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> move javaPath into caller
Changes requested by cjplummer (Reviewer).
test/lib/jdk/test/lib/Platform.java line 276:
> 274: }
> 275:
> 276: public static boolean hasPlistEntriesOSX() throws IOException {
Although I understand why you chose this API name (it mimics the form used for
`isHardenedOSX`), I don't think it is a good choice. `isHardenedOSX` is short
for "is this a hardened OSX system". That mapping does not work with
`hasPlistEntriesOSX`, which is more like "does this OSX system have plist
entries". I would suggest `hasOSXPlistEntries`.
test/lib/jdk/test/lib/util/CoreUtils.java line 154:
> 152: }
> 153: } else {
> 154: // codesign has to add entitlements using the plist, if
> this is not present we might not generate a core file
Suggestion:
// codesign has to add entitlements using the plist. If this is
not present we might not generate a core file.
-
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1499040669
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1242519320
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1242520818
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Fri, 23 Jun 2023 08:37:16 GMT, Matthias Baesken wrote: > Chris are you okay with the latest rev. of this change? Sorry, I've been on vacation the past few days. I will have a look at it tomorrow. - PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1606661232
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken 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.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> move javaPath into caller
Hi Christoph, thanks for the review !
Chris are you okay with the latest rev. of this change?
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1603933840
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken 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.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> move javaPath into caller
Looks good to me now.
-
Marked as reviewed by clanger (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1493063728
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
> 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.
Matthias Baesken has updated the pull request incrementally with one additional
commit since the last revision:
move javaPath into caller
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14562/files
- new: https://git.openjdk.org/jdk/pull/14562/files/1b604db9..619b3578
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=01-02
Stats: 32 lines in 2 files changed: 6 ins; 23 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/14562.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14562/head:pull/14562
PR: https://git.openjdk.org/jdk/pull/14562
