Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On 3/08/2021 2:25 am, Igor Ignatyev wrote: On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: Hi all, could you please review this big tedious and trivial(-ish) patch which moves `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? the majority of the patch is the following substitutions: - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` - `s/sun.hotspot.code/jdk.test.whitebox.code/g` - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` testing: tier1-4 Thanks, -- Igor Igor Ignatyev has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains 12 new commits since the last revision: - fixed ctw build - updated runtime/cds/appcds/JarBuilder to copy j.t.w.WhiteBox's inner class - updated requires.VMProps - updated TEST.ROOT - adjusted hotspot source - added test - moved and adjusted WhiteBox tests (test/lib-test/sun/hotspot/whitebox) - updated ClassFileInstaller to copy j.t.w.WhiteBox's inner class - removed sun/hotspot/parser/DiagnosticCommand - deprecated sun/hotspot classes disallowed s.h.WhiteBox w/ security manager - ... and 2 more: https://git.openjdk.java.net/jdk17/compare/8f12f2cf...237e8860 Hi David, This set of changes seems much more manageable to me. thank you for your review, David. Not sure about the new deprecation warnings for the old WB classes .. might that not itself trigger some failures? If not then I don't see how the deprecation warnings help with transitioning to the new WB class? the deprecation warnings (hopefully) will help people not to forget that they should use the new WB class in new tests. If the test passes it is unlikely people will actually notice these in the jtr file - and even if they see them they may just ignore them thinking they are similar to all the security manager warnings that we ignore. But as long as it does no harm. Cheers, David Thanks, -- Igor Hi Jie, Shall we also update the copyright year like test/lib/sun/hotspot/cpuinfo/CPUInfo.java? we most certainly shall, just pushed the commit that updates the copyright years in the touched files. Cheers, -- Igor - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Mon, 2 Aug 2021 15:56:39 GMT, Vladimir Kozlov wrote: > I agree with these revised changes for JDK 17. Thanks for your review, Vladimir. I'll rerun my testing before integrating (just for good luck). -- Igor - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > Igor Ignatyev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains 12 new > commits since the last revision: > > - fixed ctw build > - updated runtime/cds/appcds/JarBuilder to copy j.t.w.WhiteBox's inner class > - updated requires.VMProps > - updated TEST.ROOT > - adjusted hotspot source > - added test > - moved and adjusted WhiteBox tests (test/lib-test/sun/hotspot/whitebox) > - updated ClassFileInstaller to copy j.t.w.WhiteBox's inner class > - removed sun/hotspot/parser/DiagnosticCommand > - deprecated sun/hotspot classes >disallowed s.h.WhiteBox w/ security manager > - ... and 2 more: > https://git.openjdk.java.net/jdk17/compare/8f12f2cf...237e8860 Hi David, > This set of changes seems much more manageable to me. thank you for your review, David. > Not sure about the new deprecation warnings for the old WB classes .. might > that not itself trigger some failures? If not then I don't see how the > deprecation warnings help with transitioning to the new WB class? the deprecation warnings (hopefully) will help people not to forget that they should use the new WB class in new tests. Thanks, -- Igor Hi Jie, > Shall we also update the copyright year like > test/lib/sun/hotspot/cpuinfo/CPUInfo.java? we most certainly shall, just pushed the commit that updates the copyright years in the touched files. Cheers, -- Igor - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > Igor Ignatyev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. I agree with these revised changes for JDK 17. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > Igor Ignatyev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains 12 new > commits since the last revision: > > - fixed ctw build > - updated runtime/cds/appcds/JarBuilder to copy j.t.w.WhiteBox's inner class > - updated requires.VMProps > - updated TEST.ROOT > - adjusted hotspot source > - added test > - moved and adjusted WhiteBox tests (test/lib-test/sun/hotspot/whitebox) > - updated ClassFileInstaller to copy j.t.w.WhiteBox's inner class > - removed sun/hotspot/parser/DiagnosticCommand > - deprecated sun/hotspot classes >disallowed s.h.WhiteBox w/ security manager > - ... and 2 more: > https://git.openjdk.java.net/jdk17/compare/8f12f2cf...237e8860 Shall we also update the copyright year like test/lib/sun/hotspot/cpuinfo/CPUInfo.java? Thanks. - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > Igor Ignatyev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Hi Igor, This set of changes seems much more manageable to me. Not sure about the new deprecation warnings for the old WB classes .. might that not itself trigger some failures? If not then I don't see how the deprecation warnings help with transitioning to the new WB class? Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
On Sat, 31 Jul 2021 20:42:10 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this big tedious and trivial(-ish) patch which moves >> `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? >> >> the majority of the patch is the following substitutions: >> - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` >> - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` >> - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` >> - `s/sun.hotspot.code/jdk.test.whitebox.code/g` >> - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` >> - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` >> >> testing: tier1-4 >> >> Thanks, >> -- Igor > > Igor Ignatyev has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Vladimir, David, I've (forced) pushed a smaller version of the renaming. instead of removing `sun.hotspot` classes, it copies them to `jdk.test.whitebox` (w/ `s.h.parser.DiagnosticCommand` being removed as it's used in WhiteBox signature and it was easier to update a few tests that use it), updates hotspot code to register native methods for both `sun.hotspot.WhiteBox` and `jdk.test.whitebox.WhiteBox` classes. To make it easier and not to introduce extra dependency, I've made it impossible to use `s.h.WB` w/ a security manager enabled, otherwise there would be a dependency b/w `s.h.WB` and `j.t.w.WB$WhiteBoxPermission` or there would be 2 permissions. There are no open JDK tests that are impacted by this limitation. With minor tweaks in closed source, the patch successfully passes Oracle's tier1-4. -- Igor - PR: https://git.openjdk.java.net/jdk17/pull/290
Re: [jdk17] RFR: 8067223: [TESTBUG] Rename Whitebox API package [v2]
> Hi all, > > could you please review this big tedious and trivial(-ish) patch which moves > `sun.hotspot.WhiteBox` and related classes to `jdk.test.whitebox` package? > > the majority of the patch is the following substitutions: > - `s~sun/hotspot/WhiteBox~jdk/test/whitebox/WhiteBox~g` > - `s/sun.hotspot.parser/jdk.test.whitebox.parser/g` > - `s/sun.hotspot.cpuinfo/jdk.test.whitebox.cpuinfo/g` > - `s/sun.hotspot.code/jdk.test.whitebox.code/g` > - `s/sun.hotspot.gc/jdk.test.whitebox.gc/g` > - `s/sun.hotspot.WhiteBox/jdk.test.whitebox.WhiteBox/g` > > testing: tier1-4 > > Thanks, > -- Igor Igor Ignatyev has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains 12 new commits since the last revision: - fixed ctw build - updated runtime/cds/appcds/JarBuilder to copy j.t.w.WhiteBox's inner class - updated requires.VMProps - updated TEST.ROOT - adjusted hotspot source - added test - moved and adjusted WhiteBox tests (test/lib-test/sun/hotspot/whitebox) - updated ClassFileInstaller to copy j.t.w.WhiteBox's inner class - removed sun/hotspot/parser/DiagnosticCommand - deprecated sun/hotspot classes disallowed s.h.WhiteBox w/ security manager - ... and 2 more: https://git.openjdk.java.net/jdk17/compare/8f12f2cf...237e8860 - Changes: - all: https://git.openjdk.java.net/jdk17/pull/290/files - new: https://git.openjdk.java.net/jdk17/pull/290/files/8f12f2cf..237e8860 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=290=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=290=00-01 Stats: 3248 lines in 939 files changed: 969 ins; 0 del; 2279 mod Patch: https://git.openjdk.java.net/jdk17/pull/290.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/290/head:pull/290 PR: https://git.openjdk.java.net/jdk17/pull/290
