Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
Looks good. On 9/26/2019 5:15 AM, Kevin Rushforth wrote: Looks good. -- Kevin On 9/26/2019 3:57 AM, Andy Herrick wrote: Revised [3] to remove the change to CreateJmods.gmk since it is not needed as the module will be resolved because it provides an implementation of ToolProvider. [3] http://cr.openjdk.java.net/~herrick/8230649/webrev.03/ /Andy On 9/25/2019 10:33 AM, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ /Andy
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
Looks good. -- Kevin On 9/26/2019 3:57 AM, Andy Herrick wrote: Revised [3] to remove the change to CreateJmods.gmk since it is not needed as the module will be resolved because it provides an implementation of ToolProvider. [3] http://cr.openjdk.java.net/~herrick/8230649/webrev.03/ /Andy On 9/25/2019 10:33 AM, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ /Andy
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
Looks good. - Alexey On 9/26/2019 6:57 AM, Andy Herrick wrote: Revised [3] to remove the change to CreateJmods.gmk since it is not needed as the module will be resolved because it provides an implementation of ToolProvider. [3] http://cr.openjdk.java.net/~herrick/8230649/webrev.03/ /Andy On 9/25/2019 10:33 AM, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ /Andy
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
Revised [3] to remove the change to CreateJmods.gmk since it is not needed as the module will be resolved because it provides an implementation of ToolProvider. [3] http://cr.openjdk.java.net/~herrick/8230649/webrev.03/ /Andy On 9/25/2019 10:33 AM, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ /Andy
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
That seems fine then. Thanks. -- Kevin On 9/25/2019 8:29 AM, Andy Herrick wrote: I can will remove the makefile change then. The warning is going the same place as the help output (Log.info()) which is stdout unless a different stream is passed to ToolProvider. So if we want a more detailed message in Help text we can either show the more detailed message in addition to the message shown in all cases, or specifically exclude the generic message in the Help case. /Andy On 9/25/19 11:18 AM, Kevin Rushforth wrote: On 9/25/2019 7:44 AM, Alan Bateman wrote: On 25/09/2019 15:33, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ I assume the change to CreateJmods.gmk is not needed as the module will be resolved because it provides an implementation of ToolProvider. I was going to point out the same thing as Alan. The rest looks good, although an explicit informational line in the --help message might be nice, in case log warnings are going somewhere else. -- Kevin
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
I can will remove the makefile change then. The warning is going the same place as the help output (Log.info()) which is stdout unless a different stream is passed to ToolProvider. So if we want a more detailed message in Help text we can either show the more detailed message in addition to the message shown in all cases, or specifically exclude the generic message in the Help case. /Andy On 9/25/19 11:18 AM, Kevin Rushforth wrote: On 9/25/2019 7:44 AM, Alan Bateman wrote: On 25/09/2019 15:33, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ I assume the change to CreateJmods.gmk is not needed as the module will be resolved because it provides an implementation of ToolProvider. I was going to point out the same thing as Alan. The rest looks good, although an explicit informational line in the --help message might be nice, in case log warnings are going somewhere else. -- Kevin
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
On 9/25/2019 7:44 AM, Alan Bateman wrote: On 25/09/2019 15:33, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ I assume the change to CreateJmods.gmk is not needed as the module will be resolved because it provides an implementation of ToolProvider. I was going to point out the same thing as Alan. The rest looks good, although an explicit informational line in the --help message might be nice, in case log warnings are going somewhere else. -- Kevin
Re: RFR: JDK-8230649 : Make jpackage tool an experimental feature
On 25/09/2019 15:33, 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). These are the code changes to make jpackage an experimental feature. [1] https://bugs.openjdk.java.net/browse/JDK-8230649 [2] http://cr.openjdk.java.net/~herrick/8230649/webrev.02/ I assume the change to CreateJmods.gmk is not needed as the module will be resolved because it provides an implementation of ToolProvider. -Alan
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
On 9/16/2019 6:06 AM, Alan Bateman wrote: On 16/09/2019 13:56, Kevin Rushforth wrote: In that case, it may be better to issue any warnings about it being experimental directly from jpackage instead of relying on the incubating modules. Incubating modules are incubating non-final APIs so I don't think it's the right solution here. I suspect what is needed is a combination of a warning message (from the tool) and documentation to make it clear that that the feature and CLI options aren't yet final. Yes, that's what I was thinking as well, now that it's clear that the incubating module approach isn't the right one to use. -- Kevin
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
On 16/09/2019 13:56, Kevin Rushforth wrote: In that case, it may be better to issue any warnings about it being experimental directly from jpackage instead of relying on the incubating modules. Incubating modules are incubating non-final APIs so I don't think it's the right solution here. I suspect what is needed is a combination of a warning message (from the tool) and documentation to make it clear that that the feature and CLI options aren't yet final. -Alan.
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
In that case, it may be better to issue any warnings about it being experimental directly from jpackage instead of relying on the incubating modules. -- Kevin On 9/15/2019 1:05 PM, Andy Herrick wrote: yes - result of this change is as you suggest, everything shows this warning, so we will need further discussions as to what it might mean to make jpackage "experimental". /Andy On 9/15/2019 12:35 PM, Alan Bateman wrote: On 15/09/2019 14:58, 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 is the at least the first small set of changes that need to be make to make jpackage an experimental package. - Add flags to the build so the module jdk.jpackage is built as an experimental module. - Modify test programs to tolerate the warning emitted when jpackage is run. I think this change will need discussion. Can you provide a summary on what you mean by "experimental package"? I remember seeing Mark's comment go by where he suggested that the tool should be an experimental feature but I'm not sure if this translates to a warning or a configure option. I see the JIRA issue references the JEP for Incubating Modules but I'm not sure that it makes sense here as jdk.jpackage doesn't export an API and will eagerly participate in service binding because it `provides java.util.spi.ToolProvider`. There are subtle issues around incubating modules that want to provide services that were not worked out in the JDK 9 time frame. In this case, java.base uses ToolProvider so jdk.jpackage will be resolved when it is observable. I assum `java -version` will print a warning and that will not be welcomed. -Alan
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
yes - result of this change is as you suggest, everything shows this warning, so we will need further discussions as to what it might mean to make jpackage "experimental". /Andy On 9/15/2019 12:35 PM, Alan Bateman wrote: On 15/09/2019 14:58, 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 is the at least the first small set of changes that need to be make to make jpackage an experimental package. - Add flags to the build so the module jdk.jpackage is built as an experimental module. - Modify test programs to tolerate the warning emitted when jpackage is run. I think this change will need discussion. Can you provide a summary on what you mean by "experimental package"? I remember seeing Mark's comment go by where he suggested that the tool should be an experimental feature but I'm not sure if this translates to a warning or a configure option. I see the JIRA issue references the JEP for Incubating Modules but I'm not sure that it makes sense here as jdk.jpackage doesn't export an API and will eagerly participate in service binding because it `provides java.util.spi.ToolProvider`. There are subtle issues around incubating modules that want to provide services that were not worked out in the JDK 9 time frame. In this case, java.base uses ToolProvider so jdk.jpackage will be resolved when it is observable. I assum `java -version` will print a warning and that will not be welcomed. -Alan
Re: RFR: JDK-8230649: Make jpackage tool an experimental feature
On 15/09/2019 14:58, 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 is the at least the first small set of changes that need to be make to make jpackage an experimental package. - Add flags to the build so the module jdk.jpackage is built as an experimental module. - Modify test programs to tolerate the warning emitted when jpackage is run. I think this change will need discussion. Can you provide a summary on what you mean by "experimental package"? I remember seeing Mark's comment go by where he suggested that the tool should be an experimental feature but I'm not sure if this translates to a warning or a configure option. I see the JIRA issue references the JEP for Incubating Modules but I'm not sure that it makes sense here as jdk.jpackage doesn't export an API and will eagerly participate in service binding because it `provides java.util.spi.ToolProvider`. There are subtle issues around incubating modules that want to provide services that were not worked out in the JDK 9 time frame. In this case, java.base uses ToolProvider so jdk.jpackage will be resolved when it is observable. I assum `java -version` will print a warning and that will not be welcomed. -Alan
