Re: RFR: 8159855: Create an SPI for tools
> On Oct 7, 2016, at 2:40 PM, Jonathan Gibbons > wrote: > > Updated webrev with feedback from comments: > > * use doPrivileged within ToolProvider.findFIrst (includes adding new test) > * improve whitespace in doc comments > > Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.05/ Looks good. Thanks for adding the new test. Nit: line 68: s/nonzero/non-zero Mandy
Re: RFR: 8159855: Create an SPI for tools
Updated webrev with feedback from comments: * use doPrivileged within ToolProvider.findFIrst (includes adding new test) * improve whitespace in doc comments Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.05/ -- Jon On 10/04/2016 09:19 PM, Mandy Chung wrote: This SPI is useful and provides as a replacement to existing use of internal APIs to launch some of our tools. We will get jar, jmod, jlink and possibly other tools to convert to this SPI. ToolProvider::findFirst(String name) can find tool providers on classpath. I think it needs to wrap the for-loop (specifically iterating on providers) with doPrivileged due to the stack-based permission check. Otherwise, looks good. Mandy On Oct 4, 2016, at 4:46 PM, Jonathan Gibbons wrote: Resend with non-mostly-empty subject line! -- Jon On 10/04/2016 04:39 PM, Jonathan Gibbons wrote: Core-libs folk, Please review the following change to add a new service provider class java.util.spi.ToolProvider which can be used provide simple "command-line" access to select JDK tools, without starting a new JVM. The following tools are updated to provide access through the new SPI: javac, javadoc, javap, jdeps It is expected that additional tools will also be updated to provide access, but that will be done separately. Compiler-dev folk may wish to review the changes to the langtools repository. JBS: https://bugs.openjdk.java.net/browse/JDK-8159855 Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.03/ API: http://cr.openjdk.java.net/~jjg/8159855/api.02/java/util/spi/ToolProvider.html -- Jon
Re: RFR: 8159855: Create an SPI for tools
On 10/5/16 3:34 AM, Alan Bateman wrote: On 05/10/2016 10:54, Stephen Colebourne wrote: Interesting. How likely is it that there will be more than one tool of a given name available? The method name findFirst() seems relatively odd for the lookup operation. I'd also note that the name string to pass in are "magic". There are no constants defined for callers to use. Since there is no obvious way in the API to find the vendor, this could get tricky across JDKs. Plus how would a caller know what arguments to pass if each vendors tool differs? Just being cautious about the use case being solved. Many command line tools don't define an API and so not unheard of to find code that uses sun.tools.jar.Main.main or the main method of other tools. Also common to see code using ProcessBuilder or Runtime.exec to launch tools like jar. If the commonly used tools (jar, jdeps, ...) document their tool names in their module description then it will make it easier to run these tools in the same VM. It does mean that some tools will have both an API and a documented tool name but that shouldn't be an issue. I'm sure Jon has a lot to say on this topic, esp. with linking usage documentation and man pages. As regards constants then not clear where something like this would live as it amounts to a registry. Also many of the tools are JDK or library specific and so wouldn't have a place in the standard/Java SE docs. The firstFirst(String) method limits itself to tools that are visible via the system class loader. Sure, someone might decide to deploy lots of libraries that all claim to be the "hammer" tool but this is no different to many of the service providers mechanisms. One could have the ToolProvider define methods with the tool capabilities that would aid selection but that would complicate the API. -Alan Informally, I think the common use case for this API is to get "same VM" access to tools in the bin/ directory of the product image. That implies there is an obvious name for each tool, and that there will be documentation for the arguments that each tool that is available. Also, in general, the doc comments for a module should contain details of any services for general use, including details of how to access the service (e.g. what name to use, in this case) and how to use it (info about arguments, etc, in this case.) We are working on updating the javadoc tool to better support the provision of such information. -- Jon
Re: RFR: 8159855: Create an SPI for tools
On 05/10/2016 05:19, Mandy Chung wrote: : ToolProvider::findFirst(String name) can find tool providers on classpath. I think it needs to wrap the for-loop (specifically iterating on providers) with doPrivileged due to the stack-based permission check. Yes, that will be needed, otherwise the caller will need permission to get the system class loader and whatever other permissions are needed to locate and and load the tools. The rest looks okay to me. Personally I would space out the javadoc tags more, and indent the second/third lines more than one space but that is just minor stuff. -Alan
Re: RFR: 8159855: Create an SPI for tools
On 05/10/2016 10:54, Stephen Colebourne wrote: Interesting. How likely is it that there will be more than one tool of a given name available? The method name findFirst() seems relatively odd for the lookup operation. I'd also note that the name string to pass in are "magic". There are no constants defined for callers to use. Since there is no obvious way in the API to find the vendor, this could get tricky across JDKs. Plus how would a caller know what arguments to pass if each vendors tool differs? Just being cautious about the use case being solved. Many command line tools don't define an API and so not unheard of to find code that uses sun.tools.jar.Main.main or the main method of other tools. Also common to see code using ProcessBuilder or Runtime.exec to launch tools like jar. If the commonly used tools (jar, jdeps, ...) document their tool names in their module description then it will make it easier to run these tools in the same VM. It does mean that some tools will have both an API and a documented tool name but that shouldn't be an issue. I'm sure Jon has a lot to say on this topic, esp. with linking usage documentation and man pages. As regards constants then not clear where something like this would live as it amounts to a registry. Also many of the tools are JDK or library specific and so wouldn't have a place in the standard/Java SE docs. The firstFirst(String) method limits itself to tools that are visible via the system class loader. Sure, someone might decide to deploy lots of libraries that all claim to be the "hammer" tool but this is no different to many of the service providers mechanisms. One could have the ToolProvider define methods with the tool capabilities that would aid selection but that would complicate the API. -Alan
Re: RFR: 8159855: Create an SPI for tools
This SPI is useful and provides as a replacement to existing use of internal APIs to launch some of our tools. We will get jar, jmod, jlink and possibly other tools to convert to this SPI. ToolProvider::findFirst(String name) can find tool providers on classpath. I think it needs to wrap the for-loop (specifically iterating on providers) with doPrivileged due to the stack-based permission check. Otherwise, looks good. Mandy > On Oct 4, 2016, at 4:46 PM, Jonathan Gibbons > wrote: > > Resend with non-mostly-empty subject line! > > -- Jon > > On 10/04/2016 04:39 PM, Jonathan Gibbons wrote: >> Core-libs folk, >> >> Please review the following change to add a new service provider class >>java.util.spi.ToolProvider >> >> which can be used provide simple "command-line" access to select JDK >> tools, without starting a new JVM. >> >> The following tools are updated to provide access through the new SPI: >>javac, javadoc, javap, jdeps >> >> It is expected that additional tools will also be updated to provide access, >> but that will be done separately. >> >> Compiler-dev folk may wish to review the changes to the langtools repository. >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8159855 >> Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.03/ >> API: >> http://cr.openjdk.java.net/~jjg/8159855/api.02/java/util/spi/ToolProvider.html >> >> -- Jon >
Re: RFR: 8159855: Create an SPI for tools
This SPI is useful and provides as a replacement to existing use of internal APIs to launch some of our tools. We will get jar, jmod, jlink and possibly other tools to convert to this SPI. ToolProvider::findFirst(String name) can find tool providers on classpath. I think it needs to wrap the for-loop (specifically iterating on providers) with doPrivileged due to the stack-based permission check. Otherwise, looks good. Mandy > On Oct 4, 2016, at 4:46 PM, Jonathan Gibbons > wrote: > > Resend with non-mostly-empty subject line! > > -- Jon > > On 10/04/2016 04:39 PM, Jonathan Gibbons wrote: >> Core-libs folk, >> >> Please review the following change to add a new service provider class >>java.util.spi.ToolProvider >> >> which can be used provide simple "command-line" access to select JDK >> tools, without starting a new JVM. >> >> The following tools are updated to provide access through the new SPI: >>javac, javadoc, javap, jdeps >> >> It is expected that additional tools will also be updated to provide access, >> but that will be done separately. >> >> Compiler-dev folk may wish to review the changes to the langtools repository. >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8159855 >> Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.03/ >> API: >> http://cr.openjdk.java.net/~jjg/8159855/api.02/java/util/spi/ToolProvider.html >> >> -- Jon >
Re: RFR: 8159855: Create an SPI for tools
Resend with non-mostly-empty subject line! -- Jon On 10/04/2016 04:39 PM, Jonathan Gibbons wrote: Core-libs folk, Please review the following change to add a new service provider class java.util.spi.ToolProvider which can be used provide simple "command-line" access to select JDK tools, without starting a new JVM. The following tools are updated to provide access through the new SPI: javac, javadoc, javap, jdeps It is expected that additional tools will also be updated to provide access, but that will be done separately. Compiler-dev folk may wish to review the changes to the langtools repository. JBS: https://bugs.openjdk.java.net/browse/JDK-8159855 Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.03/ API: http://cr.openjdk.java.net/~jjg/8159855/api.02/java/util/spi/ToolProvider.html -- Jon
