Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules

[Andrey Nazarov]

Hi Mandy
> On 23 Mar 2017, at 21:07, Mandy Chung <mandy.chung at oracle.com> wrote:
> 
> 
>> On Mar 23, 2017, at 10:47 AM, Andrey Nazarov <andrey.x.nazarov at oracle.com> wrote:
>> 
>> Hi Mandy,
>> 
>> TaskHelper.java, JLinkTest and IntegrationTest need new copyright year.
>> It’s unclear why do you need concept of “terminal” option
>> 
> 
> It’s a low risk and expedient way to support options with optional argument (-—suggest-providers) until jlink command-line processing has to be enhanced to support optional argument.  
> 
>> Instead of "copy-paste" code to run Jlink in tests ProcessTools and OutputAnalyzer can be used from standard test library test/lib/share/classes/jdk/test/lib/process
>> 
> 
> I agree that it’d be good to move it to a test library but this does not need to be the general one since it’s jlink-specific.  jlink has a test library under jdk/tests/tools/lib/tests that needs big cleanup.  At some point we should look at all jlink tests and see what makes sense. 
> 
> For these new tests, I can move the helper class to jdk/tests/tools/lib/tests. 
It would be great.

In this case I meant we can rewrite this helper class by one method e.g. http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e15c8b97a107#l4.116 <http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e15c8b97a107#l4.116>
and use asserts provided by OutputAnalyzer e.g. http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e15c8b97a107#l1.124 <http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e15c8b97a107#l1.124>
But in you case new VM is not created to run Jlink, but tests library starts new VM. So I suggest to move helper class to some common place and prevent duplication. In future we can adjust test library to run tools in different modes (API or CMD) and fix this helper class. In theory we will be able to run tools in different modes for particular test executions.

> 
> Mandy
> 
>> 
>> —Andrey
>> 
>>> On 22 Mar 2017, at 22:11, Mandy Chung <mandy.chung at oracle.com> wrote:
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.00/
>>> 
>>> This is a proposal to resolve the open issue listed in JEP 282
>>> about jlink and service binding.
>>> 
>>> jlink does not do service binding by default as it may be confusing
>>> to the users. To link in service providers, users have to determine
>>> the provider modules to be added at link time.
>>> 
>>> The proposal is to continue not to do service binding by default
>>> since full service binding may possibly cause many modules to be
>>> linked in, which would surprise many users.  Provide the following
>>> jlink options to make it easier to cope with services when linking:
>>> 
>>> $ jlink --help
>>> :
>>> --bind-services                   Do full service binding
>>> 
>>> --suggest-providers [<name>,...]  Suggest providers of services used by
>>>                                   the modules that would be linked, or
>>>                                   of the given service types
>>> 
>>> -v, --verbose                         Enable verbose tracing
>>> 
>>> Some sample output that will show with and without —-bind-services
>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/jlink.verbose.txt
>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/jlink.suggested.providers.txt
>>> - The providers are sorted by the service type name and then the 
>>> provider's module name.
>>> 
>>> When —-bind-services is specified with —-suggest-providers, no
>>> additional provider will be suggested.
>>> 
>>> Thanks
>>> Mandy
>> 
>