Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v4]
On Thu, 10 Aug 2023 07:44:58 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust COPYRIGHT headers A belated thumbs up from me too. Thanks. - PR Review: https://git.openjdk.org/jdk/pull/15151#pullrequestreview-1571269550
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v3]
On Wed, 9 Aug 2023 11:06:04 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce buildSharedLibraryName Hi Chris and Serguei, thanks for the reviews ! I adjusted the COPYRIGHT years. - PR Comment: https://git.openjdk.org/jdk/pull/15151#issuecomment-1672693986
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v4]
> There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust COPYRIGHT headers - Changes: - all: https://git.openjdk.org/jdk/pull/15151/files - new: https://git.openjdk.org/jdk/pull/15151/files/9f9c5b25..5e8dfa79 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15151=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15151=02-03 Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v3]
On Wed, 9 Aug 2023 11:06:04 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce buildSharedLibraryName Thank you for the update. Looks good. Thanks, Serguei - Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15151#pullrequestreview-1570707364
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v3]
On Wed, 9 Aug 2023 11:06:04 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce buildSharedLibraryName Needs copyright updates, but otherwise looks good. - Marked as reviewed by cjplummer (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15151#pullrequestreview-1570274636
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v2]
On Wed, 9 Aug 2023 08:42:35 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > use Platform.sharedLibraryPrefix in more tests Hello, as suggested I introduced a Platform.buildSharedLibraryName; this can be used to simplify a lot of code in the mentioned tests. - PR Comment: https://git.openjdk.org/jdk/pull/15151#issuecomment-1671099342
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v3]
> There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Introduce buildSharedLibraryName - Changes: - all: https://git.openjdk.org/jdk/pull/15151/files - new: https://git.openjdk.org/jdk/pull/15151/files/e708bb68..9f9c5b25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15151=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15151=01-02 Stats: 50 lines in 9 files changed: 10 ins; 25 del; 15 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v2]
> There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: use Platform.sharedLibraryPrefix in more tests - Changes: - all: https://git.openjdk.org/jdk/pull/15151/files - new: https://git.openjdk.org/jdk/pull/15151/files/c99d984d..e708bb68 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15151=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15151=00-01 Stats: 9 lines in 3 files changed: 0 ins; 4 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests
On Fri, 4 Aug 2023 20:55:25 GMT, Serguei Spitsyn wrote: >> test/lib/jdk/test/lib/Platform.java line 376: >> >>> 374: } >>> 375: } >>> 376: >> >> The following tests could leverage this new API. Just look for calls to >> `Platform.sharedLibraryExt()`: >> >> test/hotspot/jtreg/runtime/signal/SigTestDriver.java >> test/hotspot/jtreg/vmTestbase/nsk/jvmti/NativeLibraryCopier.java >> test/jdk/com/sun/tools/attach/warnings/DynamicLoadWarningTest.java >> >> Perhaps a `Platform.buildSharedLibraryName()` API is worth considering. > > The fix looks good in general. > But I like the suggestion from Chris above Agreed, I'd like to see `Platform.buildSharedLibraryName()` in addition to the other methods (the latter are needed if you want to decompose a filename to get the library name (though we could also provide a function just to do that). - PR Review Comment: https://git.openjdk.org/jdk/pull/15151#discussion_r1285438334
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests
On Fri, 4 Aug 2023 18:34:52 GMT, Chris Plummer wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > test/lib/jdk/test/lib/Platform.java line 376: > >> 374: } >> 375: } >> 376: > > The following tests could leverage this new API. Just look for calls to > `Platform.sharedLibraryExt()`: > > test/hotspot/jtreg/runtime/signal/SigTestDriver.java > test/hotspot/jtreg/vmTestbase/nsk/jvmti/NativeLibraryCopier.java > test/jdk/com/sun/tools/attach/warnings/DynamicLoadWarningTest.java > > Perhaps a `Platform.buildSharedLibraryName()` API is worth considering. The fix looks good in general. But I like the suggestion from Chris above - PR Review Comment: https://git.openjdk.org/jdk/pull/15151#discussion_r1284840135
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests
On Fri, 4 Aug 2023 09:59:41 GMT, Matthias Baesken wrote: > There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Changes requested by cjplummer (Reviewer). test/lib/jdk/test/lib/Platform.java line 376: > 374: } > 375: } > 376: The following tests could leverage this new API. Just look for calls to `Platform.sharedLibraryExt()`: test/hotspot/jtreg/runtime/signal/SigTestDriver.java test/hotspot/jtreg/vmTestbase/nsk/jvmti/NativeLibraryCopier.java test/jdk/com/sun/tools/attach/warnings/DynamicLoadWarningTest.java Perhaps a `Platform.buildSharedLibraryName()` API is worth considering. - PR Review: https://git.openjdk.org/jdk/pull/15151#pullrequestreview-1563375724 PR Review Comment: https://git.openjdk.org/jdk/pull/15151#discussion_r1284731606
RFR: JDK-8313670: Simplify shared lib name handling code in some tests
There is coding e.g. in https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 that deals with shared lib naming on different OS. This code should be simplified. - Commit messages: - JDK-8313670 Changes: https://git.openjdk.org/jdk/pull/15151/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15151=00 Issue: https://bugs.openjdk.org/browse/JDK-8313670 Stats: 77 lines in 6 files changed: 13 ins; 59 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
