RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758> This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues: JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/ please send feedback to core-libs-dev@openjdk.java.net /Andy Herrick
I have been using the jpackager that Johan Vos backported for OpenJDK 11. For this I have some points of improvement I would like to mention. 1) The control file for debian package does not set correct description --name test --description This is a Test Application /tmp/jdk.packager607148779833718376/linux/control Package: test Description: test The RPM gets it correctly Summary : test Description : This is a Test Application 2) Category is not set on either DEB or RPM --category Category or group of the application. --category "Some/Category/Application" Group: Unspecified Section: unknown 3) The jpackager command line should have the flag --release in addition to --version. The only way to set a different release than "1" is to supply a custom spec file for RPM and control file for DEB. package/linux/test.spec Version: 1.0.0 Release: RC1 package/linux/control Version: 1.0.0-RC1 /Sverre
On 11/10/2018 8:12 AM, Sverre Moe wrote:
I have been using the jpackager that Johan Vos backported for OpenJDK 11. For this I have some points of improvement I would like to mention.
1) The control file for debian package does not set correct description
--name test --description This is a Test Application
/tmp/jdk.packager607148779833718376/linux/control Package: test Description: test
The RPM gets it correctly Summary : test Description : This is a Test Application
2) Category is not set on either DEB or RPM --category Category or group of the application. --category "Some/Category/Application" Group: Unspecified Section: unknown
3) The jpackager command line should have the flag --release in addition to --version. The only way to set a different release than "1" is to supply a custom spec file for RPM and control file for DEB. package/linux/test.spec Version: 1.0.0 Release: RC1 package/linux/control Version: 1.0.0-RC1
I have filed issue JDK-8213941 <https://bugs.openjdk.java.net/browse/JDK-8213941> and we will be looking into it. Thanks. /Andy
/Sverre
The help output of jpackager should mention how to set the classpath for the various bundle resource files. I have not found a working solution how to set the classpath. jpackager -classpath build/deploy/ jpackager -cp build/deploy/ DEB Using default package resource [menu icon] (add package/linux/application.png to the class path to customize) Using default package resource [Menu shortcut descriptor] (add package/linux/application.desktop to the class path to customize) Using default package resource [DEB control file] (add package/linux/control to the class path to customize) RPM Using default package resource [menu icon] (add package/linux/application.png to the class path to customize) Using default package resource [Menu shortcut descriptor] (add package/linux/application.desktop to the class path to customize) Using default package resource [RPM spec file] (add package/linux/application.spec to the class path to customize) The application image can be set with the --icon argument. --icon build/deploy/package/linux/application.png Using custom package resource [menu icon] (loaded from file /home/user/workspace/application/build/deploy/package/linux/application.png) Perhaps jpackager should have an package directory argument --package-dir build/deploy /Sverre Den tor. 15. nov. 2018 kl. 15:05 skrev Andy Herrick <andy.herrick@oracle.com
:
On 11/10/2018 8:12 AM, Sverre Moe wrote:
I have been using the jpackager that Johan Vos backported for OpenJDK 11. For this I have some points of improvement I would like to mention.
1) The control file for debian package does not set correct description
--name test --description This is a Test Application
/tmp/jdk.packager607148779833718376/linux/control Package: test Description: test
The RPM gets it correctly Summary : test Description : This is a Test Application
2) Category is not set on either DEB or RPM --category Category or group of the application. --category "Some/Category/Application" Group: Unspecified Section: unknown
3) The jpackager command line should have the flag --release in addition to --version. The only way to set a different release than "1" is to supply a custom spec file for RPM and control file for DEB. package/linux/test.spec Version: 1.0.0 Release: RC1 package/linux/control Version: 1.0.0-RC1
I have filed issue JDK-8213941 <https://bugs.openjdk.java.net/browse/JDK-8213941> and we will be looking into it.
Thanks.
/Andy
/Sverre
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
Adding build-dev back .. I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application. I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version. Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ? I think we should remove this dependency even if it means losing a MacOS feature at least for now. -phil. On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
SingleInstanceService. registerSingleInstance() says If the {@code SingleInstanceListener} object is already registered, or 98 * {@code slistener} is {@code null}, then the registration is skipped. I don't see how that can be working as every call to registerSingleInstanceForId creates a new instance and assigns it to the static variable. 114 instance = new SingleInstanceImpl(); Shouldn't this be wrapped in a synchronized null check ? And what is the contract for equality of a SingleInstanceListener. The API defines it as an interface but says nothing more . And I'm not sure what is meant by the 2nd part of this below :- 36 /** 37 * This method should be implemented by the application to 38 * handle the single instance behaviour - how should the application 39 * handle the arguments when another instance of the application is 40 * invoked with params. It is phrased like a question without a question mark. Is it supposed to mean more like : 36 /** 37 * This method must be implemented by the application to 38 * handle the single instance behaviour. For example it will * need to resolve cases where the parameter list of the new * activation in conflict with the existing activation -phil. On 11/12/18, 12:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
74 75 static String getTmpDir() { 76 String os = System.getProperty("os.name").toLowerCase(); 77 if (os.contains("win")) { 78 return System.getProperty("user.home") 79 + "\\AppData\\LocalLow\\Sun\\Java\\JPackager\\tmp"; 80 } else if (os.contains("mac") || os.contains("os x")) { 81 return System.getProperty("user.home") 82 + "/Library/Application Support/Oracle/Java/JPackager/tmp"; 83 } else if (os.contains("nix") || os.contains("nux") 84 || os.contains("aix")) { 85 return System.getProperty("user.home") + "/.java/jpackager/tmp"; 86 } 87 88 return System.getProperty("java.io.tmpdir"); This seems unduly complex, and I don't understand the implication of supporting AIX .. or some unknown "Unix", when packager is targeted only at mac, linux + windows. I think its sufficient to look for the strings windows, macos and linux. And if you want a persistent storage location then default to the "unix" location if there's no match .. although I am not sure it makes sense on platforms that aren't targeted by jpackager. -phil.
-phil.
On 11/12/18, 12:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
Hello, I wonder if in the Windows temp case LocalLow is (still) the right place. If this Installer is not started in the low context it will endanger its extracted temp file by making them accessible to low integrity processes, right? And if this should search %TEMP%., %LOCALAPPDATA%\temp first. But the variables are known to have problems too, To be robust and Windows ready compliant it looks like another jpackager runtime function for SHGetKnownFolderPath would be good (pretty sure it’s already used somewhere, just not with a stable API exposed?) Gruss Bernd -- http://bernd.eckenfels.net ________________________________ Von: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> im Auftrag von Philip Race <philip.race@oracle.com> Gesendet: Montag, November 12, 2018 10:55 PM An: Andy Herrick Cc: build-dev; core-libs-dev@openjdk.java.net Betreff: Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation 74 75 static String getTmpDir() { 76 String os = System.getProperty("os.name").toLowerCase(); 77 if (os.contains("win")) { 78 return System.getProperty("user.home") 79 + "\\AppData\\LocalLow\\Sun\\Java\\JPackager\\tmp"; 80 } else if (os.contains("mac") || os.contains("os x")) { 81 return System.getProperty("user.home") 82 + "/Library/Application Support/Oracle/Java/JPackager/tmp"; 83 } else if (os.contains("nix") || os.contains("nux") 84 || os.contains("aix")) { 85 return System.getProperty("user.home") + "/.java/jpackager/tmp"; 86 } 87 88 return System.getProperty("java.io.tmpdir"); This seems unduly complex, and I don't understand the implication of supporting AIX .. or some unknown "Unix", when packager is targeted only at mac, linux + windows. I think its sufficient to look for the strings windows, macos and linux. And if you want a persistent storage location then default to the "unix" location if there's no match .. although I am not sure it makes sense on platforms that aren't targeted by jpackager. -phil.
-phil.
On 11/12/18, 12:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
On 12/11/2018 21:40, Philip Race wrote:
74 75 static String getTmpDir() { 76 String os = System.getProperty("os.name").toLowerCase(); 77 if (os.contains("win")) { 78 return System.getProperty("user.home") 79 + "\\AppData\\LocalLow\\Sun\\Java\\JPackager\\tmp"; 80 } else if (os.contains("mac") || os.contains("os x")) { 81 return System.getProperty("user.home") 82 + "/Library/Application Support/Oracle/Java/JPackager/tmp"; 83 } else if (os.contains("nix") || os.contains("nux") 84 || os.contains("aix")) { 85 return System.getProperty("user.home") + "/.java/jpackager/tmp"; 86 } 87 88 return System.getProperty("java.io.tmpdir");
This seems unduly complex, and I don't understand the implication of supporting AIX .. or some unknown "Unix", when packager is targeted only at mac, linux + windows. user.home is specified to be the user's home directory so I would think it should use that consistently everywhere. I assume "Sun" and "Oracle" can be dropped from the file location too.
-Alan
On 11/13/2018 3:39 AM, Alan Bateman wrote:
On 12/11/2018 21:40, Philip Race wrote:
74 75 static String getTmpDir() { 76 String os = System.getProperty("os.name").toLowerCase(); 77 if (os.contains("win")) { 78 return System.getProperty("user.home") 79 + "\\AppData\\LocalLow\\Sun\\Java\\JPackager\\tmp"; 80 } else if (os.contains("mac") || os.contains("os x")) { 81 return System.getProperty("user.home") 82 + "/Library/Application Support/Oracle/Java/JPackager/tmp"; 83 } else if (os.contains("nix") || os.contains("nux") 84 || os.contains("aix")) { 85 return System.getProperty("user.home") + "/.java/jpackager/tmp"; 86 } 87 88 return System.getProperty("java.io.tmpdir");
This seems unduly complex, and I don't understand the implication of supporting AIX .. or some unknown "Unix", when packager is targeted only at mac, linux + windows. user.home is specified to be the user's home directory so I would think it should use that consistently everywhere. I assume "Sun" and "Oracle" can be dropped from the file location too.
Agreed - the resulting paths will all start with System.getProperty("user.home") and the "Sun" and "Oracle" sub-directories will be removed both here and in the matching native launcher code. Added that to JDK-8213756 <https://bugs.openjdk.java.net/browse/JDK-8213756> /Andy
-Alan
Question .. why is "mac", "linux" and "windows" necessary in the package name here ? src/jdk.jpackager/macosx/classes/jdk/jpackager/internal/mac/MacAppBundler.java src/jdk.jpackager/windows/classes/jdk/jpackager/internal/builders/windows/WindowsAppImageBuilder.java src/jdk.jpackager/linux/classes/jdk/jpackager/internal/linux/LinuxRpmBundler.java There's not likely to be a clash, so is there some other reason not to want these in the same package as the shared internals like src/jdk.jpackager/share/classes/jdk/jpackager/internal/Param.java ? -phil.
I agree with this and would take it further. 1 file is in ./share/classes/jdk/jpackager/internal/builders - why not just ./share/classes/jdk/jpackager/internal 2 files are in ./share/classes/jdk/jpackager/internal/bundlers - why not just in ./share/classes/jdk/jpackager/internal 1 file is in ./linux/classes/jdk/jpackager/internal/builders/linux - why not just ./linux/classes/jdk/jpackager/internal 1 file is in ./macosx/classes/jdk/jpackager/internal/builders/mac - why not just ./macosx/classes/jdk/jpackager/internal 1 file is in ./windows/classes/jdk/jpackager/internal/builders/windows - why not just ./windows/classes/jdk/jpackager/internal then just move the associated resources - Basically put everything except Main in same package - everything would be easier to find, and we could make almost all methods package-private (the only exception would be the few things called by Main, and the ToolProvider. On 11/13/2018 2:54 PM, Phil Race wrote:
Question .. why is "mac", "linux" and "windows" necessary in the package name here ?
src/jdk.jpackager/macosx/classes/jdk/jpackager/internal/mac/MacAppBundler.java
src/jdk.jpackager/windows/classes/jdk/jpackager/internal/builders/windows/WindowsAppImageBuilder.java
src/jdk.jpackager/linux/classes/jdk/jpackager/internal/linux/LinuxRpmBundler.java
There's not likely to be a clash, so is there some other reason not to want these in the same package as the shared internals like src/jdk.jpackager/share/classes/jdk/jpackager/internal/Param.java
?
-phil.
I agree with this and would take it further. 1 file is in ./share/classes/jdk/jpackager/internal/builders - why not just ./share/classes/jdk/jpackager/internal 2 files are in ./share/classes/jdk/jpackager/internal/bundlers - why not just in ./share/classes/jdk/jpackager/internal 1 file is in ./linux/classes/jdk/jpackager/internal/builders/linux - why not just ./linux/classes/jdk/jpackager/internal 1 file is in ./macosx/classes/jdk/jpackager/internal/builders/mac - why not just ./macosx/classes/jdk/jpackager/internal 1 file is in ./windows/classes/jdk/jpackager/internal/builders/windows - why not just ./windows/classes/jdk/jpackager/internal then just move the associated resources - Basically put everything except Main in same package - everything would be easier to find, and we could make almost all methods package-private (the only exception would be the few things called by Main, and the ToolProvider. /Andy
Yes - The intent of getTmpDir() here is to match the GetTempDirectory() in launcher, which this does for the three supported platforms, but there is no need to check for the unsupported platforms. I will clean this up as you suggest as part ofJDK-8213756 <https://bugs.openjdk.java.net/browse/JDK-8213756> /Andy On 11/12/2018 4:40 PM, Philip Race wrote:
74 75 static String getTmpDir() { 76 String os = System.getProperty("os.name").toLowerCase(); 77 if (os.contains("win")) { 78 return System.getProperty("user.home") 79 + "\\AppData\\LocalLow\\Sun\\Java\\JPackager\\tmp"; 80 } else if (os.contains("mac") || os.contains("os x")) { 81 return System.getProperty("user.home") 82 + "/Library/Application Support/Oracle/Java/JPackager/tmp"; 83 } else if (os.contains("nix") || os.contains("nux") 84 || os.contains("aix")) { 85 return System.getProperty("user.home") + "/.java/jpackager/tmp"; 86 } 87 88 return System.getProperty("java.io.tmpdir");
This seems unduly complex, and I don't understand the implication of supporting AIX .. or some unknown "Unix", when packager is targeted only at mac, linux + windows.
I think its sufficient to look for the strings windows, macos and linux. And if you want a persistent storage location then default to the "unix" location if there's no match .. although I am not sure it makes sense on platforms that aren't targeted by jpackager.
-phil.
-phil.
On 11/12/18, 12:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
On 11/12/2018 3:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
1.) we could remove that functionality (I don't even really understand the use case for it), and remove the two arg registerSingleInstance method from the public interface, and remove the requires statement in module-info.java. or ... 2.) we could move that functionality to a platform dependent class, having dummy methods of setOpenFileHandler in the windows and linux platform dependent class. Then we could add a module-info.java.extra in src/jdk.jpackager.runtime/macos/classes with that requires statement. /Andy
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
On 11/12/18, 1:45 PM, Andy Herrick wrote:
On 11/12/2018 3:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
1.) we could remove that functionality (I don't even really understand the use case for it), and remove the two arg registerSingleInstance method from the public interface, and remove the requires statement in module-info.java.
I was thinking remove it as above since since even the javadoc for that method feels obliged to say it is just for macos :- 95 /** 96 * Registers {@code SingleInstanceListener} for current process. 97 * If the {@code SingleInstanceListener} object is already registered, or 98 * {@code slistener} is {@code null}, then the registration is skipped. 99 * 100 * @param slistener the listener to handle the single instance behaviour. 101 * @param setFileHandler if {@code true}, the listener is notified when the 102 * application is asked to open a list of files. If OS is not MacOS, 103 * the parameter is ignored. 104 */ yuck. If such optional functionality is needed it needs to be done via adding a "doesPlatformSupportFileHandler() method rather than saying the above. -phil.
or ...
2.) we could move that functionality to a platform dependent class, having dummy methods of setOpenFileHandler in the windows and linux platform dependent class. Then we could add a module-info.java.extra in src/jdk.jpackager.runtime/macos/classes with that requires statement.
/Andy
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
On 11/12/2018 4:54 PM, Philip Race wrote:
On 11/12/18, 1:45 PM, Andy Herrick wrote:
On 11/12/2018 3:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
1.) we could remove that functionality (I don't even really understand the use case for it), and remove the two arg registerSingleInstance method from the public interface, and remove the requires statement in module-info.java.
I was thinking remove it as above since since even the javadoc for that method feels obliged to say it is just for macos :-
95 /** 96 * Registers {@code SingleInstanceListener} for current process. 97 * If the {@code SingleInstanceListener} object is already registered, or 98 * {@code slistener} is {@code null}, then the registration is skipped. 99 * 100 * @param slistener the listener to handle the single instance behaviour. 101 * @param setFileHandler if {@code true}, the listener is notified when the 102 * application is asked to open a list of files. If OS is not MacOS, 103 * the parameter is ignored. 104 */
yuck. If such optional functionality is needed it needs to be done via adding a "doesPlatformSupportFileHandler() method rather than saying the above.
-phil.
OK - I filed JDK-8213756 <https://bugs.openjdk.java.net/browse/JDK-8213756> to handle removing this API, fixing the call to new SingleInstanceImpl() to be wrapped in synchronized null check, and fixing comments you referred to . /Andy
or ...
2.) we could move that functionality to a platform dependent class, having dummy methods of setOpenFileHandler in the windows and linux platform dependent class. Then we could add a module-info.java.extra in src/jdk.jpackager.runtime/macos/classes with that requires statement.
/Andy
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
BTW there were also a few minor grammar issues in javadoc eg 41 * the option named "-singleton" must be specified on jpackager command line. "the jpackager" 84 * Registers {@code SingleInstanceListener} for current process. and 96 * Registers {@code SingleInstanceListener} for current process. and 122 * Unregisters {@code SingleInstanceListener} for current process. 123 * If the {@code SingleInstanceListener} object is not registered, or 124 * {@code slistener} is {@code null}, then the unregistration is skipped. "the current process" There may be more like that. and as I mentioned offline, I think the correct word is "deregister" not "unregister" both in comments and method names. -phill On 11/12/18, 2:38 PM, Andy Herrick wrote:
On 11/12/2018 4:54 PM, Philip Race wrote:
On 11/12/18, 1:45 PM, Andy Herrick wrote:
On 11/12/2018 3:22 PM, Philip Race wrote:
Adding build-dev back ..
I noticed that module jdk.jpackager.runtime requires java.desktop on all platforms
So far as I can tell this is for a Mac-only support for receiving and handling file open events. Probably it only makes sense or gets used when the API is used from a running desktop application.
I might ask if we need this at all, but I definitely think it should not be required on all platforms if needed only for Mac even if we might want it on windows in a future version.
Do we not envisage cases where you need the runtime API for some kind of daemon service for which there should be a singleton ? Do you really want to force the desktop module to be dragged along in such a case ?
I think we should remove this dependency even if it means losing a MacOS feature at least for now.
1.) we could remove that functionality (I don't even really understand the use case for it), and remove the two arg registerSingleInstance method from the public interface, and remove the requires statement in module-info.java.
I was thinking remove it as above since since even the javadoc for that method feels obliged to say it is just for macos :-
95 /** 96 * Registers {@code SingleInstanceListener} for current process. 97 * If the {@code SingleInstanceListener} object is already registered, or 98 * {@code slistener} is {@code null}, then the registration is skipped. 99 * 100 * @param slistener the listener to handle the single instance behaviour. 101 * @param setFileHandler if {@code true}, the listener is notified when the 102 * application is asked to open a list of files. If OS is not MacOS, 103 * the parameter is ignored. 104 */
yuck. If such optional functionality is needed it needs to be done via adding a "doesPlatformSupportFileHandler() method rather than saying the above.
-phil.
OK - I filed JDK-8213756 <https://bugs.openjdk.java.net/browse/JDK-8213756> to handle removing this API, fixing the call to new SingleInstanceImpl() to be wrapped in synchronized null check, and fixing comments you referred to .
/Andy
or ...
2.) we could move that functionality to a platform dependent class, having dummy methods of setOpenFileHandler in the windows and linux platform dependent class. Then we could add a module-info.java.extra in src/jdk.jpackager.runtime/macos/classes with that requires statement.
/Andy
-phil.
On 11/11/18, 2:40 PM, Andy Herrick wrote:
On 11/9/2018 5:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Note: The above issues are targeted to 'internal' - meaning we expect to resolve them in the sandbox before the initial push to JDK12. Issues targeted to '12' are expected to be fixed after the initial push.
/Andy
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
Hi, A few high level comments: The JDK already has a command option parser (JoptSimple) in the module jdk.internal.opt and the System Logger. Why not use them for the argument parsing and logging? Similarly, we've been encouraging developers to use the java.nio.file APIs and get away from java.io.File. Try-with-resources could be used in a few places to improve the closing of resources. I can also see many places where the Streams functions could be used to make the code easier to read (and write). That's a missed opportunity. What granularity of comments are you looking for? Thanks, Roger On 11/09/2018 05:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
On 11/13/2018 4:08 PM, Roger Riggs wrote:
Hi,
A few high level comments:
The JDK already has a command option parser (JoptSimple) in the module jdk.internal.opt and the System Logger. Why not use them for the argument parsing and logging?
We have an RFE to convert argument parsing to joptsimple that I filed last July JDK-8208300. <https://bugs.openjdk.java.net/browse/JDK-8208300> I spent a few days at the time prototyping it, and concluded it was a multi-week project. The team decided it was not a priority for JDK12 and it is now targeted to JDK13.
Similarly, we've been encouraging developers to use the java.nio.file APIs and get away from java.io.File. Try-with-resources could be used in a few places to improve the closing of resources.
I can also see many places where the Streams functions could be used to make the code easier to read (and write). That's a missed opportunity.
What granularity of comments are you looking for?
We are looking for three types of comments: 1.) Specific show-stopper issues that would prevent you from approving the inclusion of this project in JDK12 2.) Specific small problems that could be addressed in the limited time left for JDK12. 3.) Any other problems or ideas for improvement that we should consider for JDK13 or future releases. /Andy
Thanks, Roger
On 11/09/2018 05:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
Hello, May I chime in a little on the jpackager. I have been using it with OpenJDK 11, as backported by Johan Vos from Gluon. It has worked fine, but I have noticed some flaws. 1) The control file for DEB package does not set correct description --name test --description This is a Test Application /tmp/jdk.packager607148779833718376/linux/control Package: test Description: test The RPM gets it correctly Summary : test Description : This is a Test Application 2) Category is not set on either DEB or RPM --category Category or group of the application. --category "Some/Category/Application" Group: Unspecified Section: unknown Perhaps I have misunderstood what this category is for, because I see this it is set in the generated application.desktop, but It should definitely also be set in the RPM and DEB. 3) There should be a --release flag to jpackager, at least for RPM. The only other way to set release would be to supply a custom spec file. Version: 1.0.0 Release: RC1 /Sverre Den tir. 13. nov. 2018 kl. 22:30 skrev Andy Herrick <andy.herrick@oracle.com
:
On 11/13/2018 4:08 PM, Roger Riggs wrote:
Hi,
A few high level comments:
The JDK already has a command option parser (JoptSimple) in the module jdk.internal.opt and the System Logger. Why not use them for the argument parsing and logging?
We have an RFE to convert argument parsing to joptsimple that I filed last July JDK-8208300. <https://bugs.openjdk.java.net/browse/JDK-8208300>
I spent a few days at the time prototyping it, and concluded it was a multi-week project.
The team decided it was not a priority for JDK12 and it is now targeted to JDK13.
Similarly, we've been encouraging developers to use the java.nio.file APIs and get away from java.io.File. Try-with-resources could be used in a few places to improve the closing of resources.
I can also see many places where the Streams functions could be used to make the code easier to read (and write). That's a missed opportunity.
What granularity of comments are you looking for?
We are looking for three types of comments:
1.) Specific show-stopper issues that would prevent you from approving the inclusion of this project in JDK12
2.) Specific small problems that could be addressed in the limited time left for JDK12.
3.) Any other problems or ideas for improvement that we should consider for JDK13 or future releases.
/Andy
Thanks, Roger
On 11/09/2018 05:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
Hi, Does --limit-modules work as intended, are there any tests? With a command such as: jpackager -Djlink.debug=true create-image \ --input out/artifacts \ --output out \ --limit-modules java.base \ --main-jar Hello-jar/Hello.jar \ --class Main It complains that: Exception: jdk.tools.jlink.plugin.PluginException: java.lang.module.FindException: Module jdk.compiler not found, required by jdk.jdeps After adding a few more modules it stops complaining about missing dependencies: --limit-modules java.base,jdk.compiler,jdk.internal.le,jdk.internal.ed,jdk.internal.opt \ But still, The generated runtime/bin/java --show-modules includes the full list of jdk modules. In the code: JLinkBundlerHelper.ModuleHelper ignores the limitMods argument and produces a complete set of redistributable modules. It should be possible to build a runtime with only the java.base module. What should I expect? Thanks, Roger On 11/09/2018 05:25 PM, Andy Herrick wrote:
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
I noticed in WindowsRegistry.java we're actually making calls to reg(.exe) and parsing the output/result. Is this preferred over making direct calls to the WINAPI functions via JNI? (Also, would this be a security concern if another reg.exe is in the PATH before the Windows system one?) Also: 131 ProcessBuilder security = new ProcessBuilder(buildOptions); 132 exec(security, false, false, ps); the variable name security for the ProcessBuilder seems like it may have been some leftover from refactoring... Thanks, -Andrew -----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Andy Herrick Sent: Friday, November 9, 2018 2:26 PM To: core-libs-dev@openjdk.java.net Subject: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758> This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues: JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/ please send feedback to core-libs-dev@openjdk.java.net /Andy Herrick
I don't know how much of a security concern it is, and this is a developer tool, but in general we have used JNI + API for registry queries. Perhaps because this is a non-performance sensitive developer tool there was a thought that it was OK. So I would myself have used the API and think switching should be considered. -phil. On 11/15/2018 11:02 AM, Andrew Luo wrote:
I noticed in WindowsRegistry.java we're actually making calls to reg(.exe) and parsing the output/result. Is this preferred over making direct calls to the WINAPI functions via JNI? (Also, would this be a security concern if another reg.exe is in the PATH before the Windows system one?)
Also:
131 ProcessBuilder security = new ProcessBuilder(buildOptions); 132 exec(security, false, false, ps);
the variable name security for the ProcessBuilder seems like it may have been some leftover from refactoring...
Thanks, -Andrew
-----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Andy Herrick Sent: Friday, November 9, 2018 2:26 PM To: core-libs-dev@openjdk.java.net Subject: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
This is an update to the Request For Review of the implementation of the Java Packager Tool (jpackager) as described in JEP 343: Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
This refresh renames the packages used to jdk.jpackager and jdk.jpackager.runtime, removes the JNLPConverter demo, adds an initial set of automated tests, and contains fixes to the following issues:
JDK-8213324 jpackager deletes existing app directory without warning JDK-8213166 jpackager --argument arg is broken JDK-8213163 --app-image arg does not work creating exe installers JDK-8212089 Prepare packager for localization JDK-8212537 Create method and class description comments for main functionality JDK-8213332 Create minimal automated tests for jpackager JDK-8213333 Fix issues found in jpackager with automated tests JDK-8213394 Stop using Log.info() except for expected output. JDK-8213345 Secondary Launchers broken on mac. JDK-8213156 rename packages for jpackager JDK-8213244 Fix all warnings in jpackager java code JDK-8212143 Remove native code that supports UserJvmOptionsService JDK-8213162 Association description in Inno Setup cannot contain double quotes
The following additional issues are targeted to be address in the next few weeks: JDK-8212936 Makefile and other improvements for jpackager JDK-8212164 resolve jre.list and jre.module.list JDK-8213392 Enhance --help and --version JDK-8208652 File name is not passed to main() via file association on OS X JDK-8212538 Determine standard way to determine if a Modular jar JDK-8213558 Create more unit tests
Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
please send feedback to core-libs-dev@openjdk.java.net
/Andy Herrick
participants (8)
-
Alan Bateman
-
Andrew Luo
-
Andy Herrick
-
Bernd Eckenfels
-
Phil Race
-
Philip Race
-
Roger Riggs
-
Sverre Moe