RFR: 8266179: [macos] jpackage should specify architecture for produced pkg files
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2. ------------- Commit messages: - 8266179: [macos] jpackage should specify architecture for produced pkg files Changes: https://git.openjdk.java.net/jdk/pull/3807/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266179 Stats: 7 lines in 2 files changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3807.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3807/head:pull/3807 PR: https://git.openjdk.java.net/jdk/pull/3807
On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Marked as reviewed by herrick (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
The fix looks good. Would it be feasible to include an automated test for this? ------------- Marked as reviewed by kcr (Author). PR: https://git.openjdk.java.net/jdk/pull/3807
On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Marked as reviewed by asemenyuk (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Added HostArchPkgTest test to verify hostArchitectures attribute. ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision: 8266179: [macos] jpackage should specify architecture for produced pkg files [v2] ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3807/files - new: https://git.openjdk.java.net/jdk/pull/3807/files/1937e9e1..11d9a2cf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=00-01 Stats: 96 lines in 2 files changed: 94 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3807.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3807/head:pull/3807 PR: https://git.openjdk.java.net/jdk/pull/3807
On Sat, 1 May 2021 04:04:17 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v2]
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 84:
82: } 83: 84: public static void main(String[] args) throws Exception {
Please don't use direct TKit.run() call. Use jdk.jpackage.test.Annotations.Test annotation for test method. You can use SimplePackageTest jtreg test as an example ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Sat, 1 May 2021 04:04:17 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v2]
Changes requested by asemenyuk (Reviewer). test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 55:
53: 54: private static void verifyHostArch(JPackageCommand cmd) throws Exception { 55: Path distributionFile = cmd.unpackedPackageDirectory()
I think `cmd.pathToUnpackedPackageFile("/")` would be the equivalent to `cmd.unpackedPackageDirectory()`. If it is, there would be no need for changes to JPackageCommand. test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 57:
55: Path distributionFile = cmd.unpackedPackageDirectory() 56: .toAbsolutePath() 57: .getParent()
Why .getParent() is needed here? ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 15:58:56 GMT, Alexey Semenyuk <asemenyuk@openjdk.org> wrote:
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v2]
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 55:
53: 54: private static void verifyHostArch(JPackageCommand cmd) throws Exception { 55: Path distributionFile = cmd.unpackedPackageDirectory()
I think `cmd.pathToUnpackedPackageFile("/")` would be the equivalent to `cmd.unpackedPackageDirectory()`. If it is, there would be no need for changes to JPackageCommand.
Fixed.
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 84:
82: } 83: 84: public static void main(String[] args) throws Exception {
Please don't use direct TKit.run() call. Use jdk.jpackage.test.Annotations.Test annotation for test method. You can use SimplePackageTest jtreg test as an example
I will fix it. Do we have a bug or I can file one to fix other tests? We have several tests such as SigningPackageTest which uses TKit.run() call and I just copy paste it. ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 17:24:16 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 84:
82: } 83: 84: public static void main(String[] args) throws Exception {
Please don't use direct TKit.run() call. Use jdk.jpackage.test.Annotations.Test annotation for test method. You can use SimplePackageTest jtreg test as an example
I will fix it. Do we have a bug or I can file one to fix other tests? We have several tests such as SigningPackageTest which uses TKit.run() call and I just copy paste it.
No, we don't have a CR to track replacement of direct TKit.run() calls. Please feel free to file one. ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 16:01:36 GMT, Alexey Semenyuk <asemenyuk@openjdk.org> wrote:
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v2]
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 57:
55: Path distributionFile = cmd.unpackedPackageDirectory() 56: .toAbsolutePath() 57: .getParent()
Why .getParent() is needed here?
Unpacking pkg is two stage process. First we unpack it to unpacked-pkg/data which contains Distribution file, pkg background images and tar archive with app itself. Then we unpack app tar archive to unpacked-pkg/unpacked. Test needs to check unpacked-pkg/data/Distribution and cmd.pathToUnpackedPackageFile("/") returns path to unpacked-pkg/unpacked. Thus getParent() is used to move up. ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 17:30:15 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 57:
55: Path distributionFile = cmd.unpackedPackageDirectory() 56: .toAbsolutePath() 57: .getParent()
Why .getParent() is needed here?
Unpacking pkg is two stage process. First we unpack it to unpacked-pkg/data which contains Distribution file, pkg background images and tar archive with app itself. Then we unpack app tar archive to unpacked-pkg/unpacked. Test needs to check unpacked-pkg/data/Distribution and cmd.pathToUnpackedPackageFile("/") returns path to unpacked-pkg/unpacked. Thus getParent() is used to move up.
Understood. Thank you for explanation! ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision: 8266179: [macos] jpackage should specify architecture for produced pkg files [v3] ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3807/files - new: https://git.openjdk.java.net/jdk/pull/3807/files/11d9a2cf..83a59341 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=01-02 Stats: 15 lines in 2 files changed: 2 ins; 1 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/3807.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3807/head:pull/3807 PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 19:26:19 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v3]
Changes requested by asemenyuk (Reviewer). test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 93:
91: .forTypes(PackageType.MAC_PKG) 92: .addInstallVerifier(HostArchPkgTest::verifyHostArch) 93: .run();
The test is applicable only to the scenario when .pkg installer is unpacked and not when it is installed. So `PackageTest.run()` is not quite a good fit for this execution scenario as it depends on the value of `jpackage.test.action` system property (its default value is indeed to create and unpack installer, but can be overriden). The better option would be to use `PackageTest.run()` with explicit list of actions the test should perform. Suggested fix: new PackageTest() .forTypes(PackageType.MAC_PKG) .configureHelloApp() .addInstallVerifier(HostArchPkgTest::verifyHostArch) .run(PackageTest.Action.CREATE_AND_UNPACK); ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 20:20:52 GMT, Alexey Semenyuk <asemenyuk@openjdk.org> wrote:
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v3]
test/jdk/tools/jpackage/macosx/HostArchPkgTest.java line 93:
91: .forTypes(PackageType.MAC_PKG) 92: .addInstallVerifier(HostArchPkgTest::verifyHostArch) 93: .run();
The test is applicable only to the scenario when .pkg installer is unpacked and not when it is installed. So `PackageTest.run()` is not quite a good fit for this execution scenario as it depends on the value of `jpackage.test.action` system property (its default value is indeed to create and unpack installer, but can be overriden). The better option would be to use `PackageTest.run()` with explicit list of actions the test should perform. Suggested fix:
new PackageTest() .forTypes(PackageType.MAC_PKG) .configureHelloApp() .addInstallVerifier(HostArchPkgTest::verifyHostArch) .run(PackageTest.Action.CREATE_AND_UNPACK);
Sorry for the inconvenience, I didn't include this comment in my initial review. ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision: 8266179: [macos] jpackage should specify architecture for produced pkg files [v4] ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3807/files - new: https://git.openjdk.java.net/jdk/pull/3807/files/83a59341..735baf93 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3807&range=02-03 Stats: 4 lines in 1 file changed: 1 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3807.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3807/head:pull/3807 PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 22:52:21 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v4]
Marked as reviewed by asemenyuk (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 22:52:21 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v4]
I presume you've done a CI test build on machines of both architectures? ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 22:52:21 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v4]
Yes, it was tested on both x64 and arm. ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Mon, 3 May 2021 22:52:21 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
8266179: [macos] jpackage should specify architecture for produced pkg files [v4]
Marked as reviewed by kcr (Author). ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
On Fri, 30 Apr 2021 04:22:37 GMT, Alexander Matveev <almatvee@openjdk.org> wrote:
jpackage should specify architecture for produced PKG files via hostArchitectures="x86_x64 or arm64". aarch64 installer will be installable on x64 without specifying hostArchitectures which is not correct and if install on arm Mac it will request Rosetta 2. With proposed fix by setting hostArchitectures="x86_x64" if installer contains x64 binaries, it will be installable on x64 Mac and will require Rosetta 2 on arm Mac. hostArchitectures will be set to arm64 if installer contain aarch64 binaries and will gave error when run on x64 Mac and will be installable on arm Mac without triggering installation of Rosetta 2.
This pull request has now been integrated. Changeset: 2c53654b Author: Alexander Matveev <almatvee@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/2c53654bf1140c7cd243598ebdbff9ca4b9c... Stats: 101 lines in 3 files changed: 100 ins; 0 del; 1 mod 8266179: [macos] jpackage should specify architecture for produced pkg files Reviewed-by: herrick, kcr, asemenyuk ------------- PR: https://git.openjdk.java.net/jdk/pull/3807
participants (4)
-
Alexander Matveev
-
Alexey Semenyuk
-
Andy Herrick
-
Kevin Rushforth