Re: RFR: JDK-8230519: jpackage "--package-type" values and default
Hi Andy, http://cr.openjdk.java.net/~herrick/8230519/webrev.02/test/jdk/tools/jpackage/share/ErrorTest.java.frames.html Line 90: Add space before ARG2. Looks good. Thanks, Alexander On 9/9/2019 3:25 PM, Andy Herrick wrote: Please review the updated jpackage fix for bug [1] at [3]. Functional change: If the default package type is called for, but for some reason the default package type is not supported on this machine, it will be an error (we will not try to build any other package type) Typo fixed in comment, and small fix to RuntimeTest.java. [3] http://cr.openjdk.java.net/~herrick/8230519/webrev.02/ /Andy On 9/8/2019 5:50 PM, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This fix: Adds "app-image" as a valid value for "--package-type" options, meaning build an application image instead of a package. Changes the default value of "--package-type" to a platform dependent default package type. [1] https://bugs.openjdk.java.net/browse/JDK-8230519 [2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/ Thanks Andy,
Re: RFR: JDK-8230519: jpackage "--package-type" values and default
Please review the updated jpackage fix for bug [1] at [3]. Functional change: If the default package type is called for, but for some reason the default package type is not supported on this machine, it will be an error (we will not try to build any other package type) Typo fixed in comment, and small fix to RuntimeTest.java. [3] http://cr.openjdk.java.net/~herrick/8230519/webrev.02/ /Andy On 9/8/2019 5:50 PM, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This fix: Adds "app-image" as a valid value for "--package-type" options, meaning build an application image instead of a package. Changes the default value of "--package-type" to a platform dependent default package type. [1] https://bugs.openjdk.java.net/browse/JDK-8230519 [2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/ Thanks Andy,
Re: RFR: JDK-8230519: jpackage "--package-type" values and default
On 9/9/2019 1:25 PM, Alexey Semenyuk wrote: http://cr.openjdk.java.net/~herrick/8230519/webrev.01/test/jdk/tools/jpackage/share/RuntimeTest.java.sdiff.html: --- 37 "--package-type", "app-image", 38 "--package-type", "app-image", --- Duplication by accident? yes - will fix. http://cr.openjdk.java.net/~herrick/8230519/webrev.01/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebBundler.java.sdiff.html: --- // we are just going to run "dpkg -s coreutils" ans assume Debian --- "ans" - typo? Should it be "and"? yes - will fix - Alexey On 9/8/2019 5:50 PM, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This fix: Adds "app-image" as a valid value for "--package-type" options, meaning build an application image instead of a package. Changes the default value of "--package-type" to a platform dependent default package type. [1] https://bugs.openjdk.java.net/browse/JDK-8230519 [2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/ Thanks Andy,
Re: RFR: JDK-8230519: jpackage "--package-type" values and default
On 9/9/2019 1:28 PM, Andy Herrick wrote:
On 9/9/19 1:14 PM, Kevin Rushforth wrote:
Looks good with one question:
In Arguments.java:
+ if (bundler.isDefault()) {
+ return bundler;
+ } else {
+ savedBundler = bundler;
+ }
When would there be a valid case where you loop through the list of
bundlers and don't find a default? It may be better to throw an error
in that case rather than just return the last one found.
-- Kevin
This left the possibility that if the default were not supported but
another form was, then it became the default.
On Windows, the two package types, msi and exe now require the same
tools, and on linux, deb is only the default if (among other things)
it is supported.
So that leaves only macOS. here dmg is default but required hdmiutil
and osascript.
If either of these are missing dmg is not supported, so this code
would build pkg package if --package-type not specified. Would it be
better to just get error and only build pkg if it is explicitly
specified as --package-type pkg ?
I think exiting with error if dmg tools are not available on macOS would
be better. This would be more predictable.
- Alexet
/Andy
On 9/8/2019 2:50 PM, Andy Herrick wrote:
Please review the jpackage fix for bug [1] at [2].
This is a fix for the JDK-8200758-branch branch of the open sandbox
repository (jpackage).
This fix:
Adds "app-image" as a valid value for "--package-type" options,
meaning build an application image instead of a package.
Changes the default value of "--package-type" to a platform
dependent default package type.
[1] https://bugs.openjdk.java.net/browse/JDK-8230519
[2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/
Thanks
Andy,
Re: RFR: JDK-8230519: jpackage "--package-type" values and default
http://cr.openjdk.java.net/~herrick/8230519/webrev.01/test/jdk/tools/jpackage/share/RuntimeTest.java.sdiff.html: --- 37 "--package-type", "app-image", 38 "--package-type", "app-image", --- Duplication by accident? http://cr.openjdk.java.net/~herrick/8230519/webrev.01/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebBundler.java.sdiff.html: --- // we are just going to run "dpkg -s coreutils" ans assume Debian --- "ans" - typo? Should it be "and"? - Alexey On 9/8/2019 5:50 PM, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This fix: Adds "app-image" as a valid value for "--package-type" options, meaning build an application image instead of a package. Changes the default value of "--package-type" to a platform dependent default package type. [1] https://bugs.openjdk.java.net/browse/JDK-8230519 [2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/ Thanks Andy,
Re: RFR: JDK-8230519: jpackage "--package-type" values and default
On 9/9/19 1:14 PM, Kevin Rushforth wrote:
Looks good with one question:
In Arguments.java:
+ if (bundler.isDefault()) {
+ return bundler;
+ } else {
+ savedBundler = bundler;
+ }
When would there be a valid case where you loop through the list of
bundlers and don't find a default? It may be better to throw an error
in that case rather than just return the last one found.
-- Kevin
This left the possibility that if the default were not supported but
another form was, then it became the default.
On Windows, the two package types, msi and exe now require the same
tools, and on linux, deb is only the default if (among other things) it
is supported.
So that leaves only macOS. here dmg is default but required hdmiutil
and osascript.
If either of these are missing dmg is not supported, so this code would
build pkg package if --package-type not specified. Would it be better
to just get error and only build pkg if it is explicitly specified as
--package-type pkg ?
/Andy
On 9/8/2019 2:50 PM, Andy Herrick wrote:
Please review the jpackage fix for bug [1] at [2].
This is a fix for the JDK-8200758-branch branch of the open sandbox
repository (jpackage).
This fix:
Adds "app-image" as a valid value for "--package-type" options,
meaning build an application image instead of a package.
Changes the default value of "--package-type" to a platform dependent
default package type.
[1] https://bugs.openjdk.java.net/browse/JDK-8230519
[2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/
Thanks
Andy,
Re: RFR: JDK-8230519: jpackage "--package-type" values and default
Looks good with one question:
In Arguments.java:
+ if (bundler.isDefault()) {
+ return bundler;
+ } else {
+ savedBundler = bundler;
+ }
When would there be a valid case where you loop through the list of
bundlers and don't find a default? It may be better to throw an error in
that case rather than just return the last one found.
-- Kevin
On 9/8/2019 2:50 PM, Andy Herrick wrote:
Please review the jpackage fix for bug [1] at [2].
This is a fix for the JDK-8200758-branch branch of the open sandbox
repository (jpackage).
This fix:
Adds "app-image" as a valid value for "--package-type" options,
meaning build an application image instead of a package.
Changes the default value of "--package-type" to a platform dependent
default package type.
[1] https://bugs.openjdk.java.net/browse/JDK-8230519
[2] http://cr.openjdk.java.net/~herrick/8230519/webrev.01/
Thanks
Andy,
