RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Igor Ignatyev]

LGTM

-- Igor

> On Oct 11, 2018, at 5:54 PM, Hamlin Li <huaming.li at oracle.com> wrote:
> 
> Hi Igor,
> 
> It's updated in place http://cr.openjdk.java.net/~mli/8186610/webrev.01/ <http://cr.openjdk.java.net/~mli/8186610/webrev.01/>, please help to review again.
> 
> Thank you
> 
> -Hamlin
> 
> On 2018/10/12 2:51 AM, Igor Ignatyev wrote:
>> Hi Hamlin,
>> 
>> the key point here, ModuleTargetHelper.java is not a "library" class, it's a part of tools/jlink/plugins/SystemModuleDescriptors/ tests.
>> 
>> thanks for updating webrev, I have one nit --- given ModuleUtils is the only class in jdk/test/lib/module package, I doubt we need to introduce this package, ModuleUtils can be put into j.t.l.util package.
>> 
>> -- Igor.
>> 
>>> On Oct 10, 2018, at 11:31 PM, Hamlin Li <huaming.li at oracle.com <mailto:huaming.li at oracle.com>> wrote:
>>> 
>>> Thank you clarifying Igor.
>>> 
>>> Moving ModuleTargetHelper to local folder has a drawback: it's hard for future maintainer to found it if they need to use it in other places, that make it an "invisible" library class.
>>> Although I don't fully agree with you, I have updated the webrev as you suggested: http://cr.openjdk.java.net/~mli/8186610/webrev.01/ <http://cr.openjdk.java.net/%7Emli/8186610/webrev.01/>
>>> Thank you
>>> 
>>> -Hamlin
>>> 
>>> On 2018/10/11 11:38 AM, Igor Ignatyev wrote:
>>>> b/c this will make test library modularization[1] somewhat troublesome, also I ain't sure if ModuleTargetHelper really needs to be placed into the top-level library regardless of its dependency on non-public API. "promoting" test library class to the top-level library comes w/ increased maintenance costs, the parent task[2] explains that in more details. 
>>>> 
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8211358 <https://bugs.openjdk.java.net/browse/JDK-8211358>
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8211290 <https://bugs.openjdk.java.net/browse/JDK-8211290>
>>>> 
>>>> -- Igor
>>>> 
>>>>> On Oct 10, 2018, at 8:26 PM, Hamlin Li <huaming.li at oracle.com <mailto:huaming.li at oracle.com>> wrote:
>>>>> 
>>>>> Hi Igor,
>>>>> 
>>>>> Would you please clarify your concern further? I mean why ModuleTargetHelper can be put to top level when it use non-public APIs e.g. jdk.internal.module.*?
>>>>> 
>>>>> Thank you
>>>>> 
>>>>> -Hamlin
>>>>> 
>>>>> On 2018/10/11 11:08 AM, Igor Ignatyev wrote:
>>>>>> Hi Hamlin,
>>>>>> 
>>>>>> as ModuleTargetHelper uses non-public API, I'd prefer not to have in a common test library, and 8211976 suggests moving it closer to tests. could you please explain why you decided to put it into the library instead?
>>>>>> 
>>>>>> Thanks,
>>>>>> -- Igor
>>>>>> 
>>>>>> ----- Original Message -----
>>>>>> From: huaming.li at oracle.com <mailto:huaming.li at oracle.com>
>>>>>> To: core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>
>>>>>> Sent: Wednesday, October 10, 2018 7:40:34 PM GMT -08:00 US/Canada Pacific
>>>>>> Subject: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary
>>>>>> 
>>>>>> Would you please review the following patch?
>>>>>> 
>>>>>> bug:
>>>>>> 
>>>>>>    https://bugs.openjdk.java.net/browse/JDK-8186610 <https://bugs.openjdk.java.net/browse/JDK-8186610>
>>>>>> 
>>>>>>    https://bugs.openjdk.java.net/browse/JDK-8211976 <https://bugs.openjdk.java.net/browse/JDK-8211976>
>>>>>> 
>>>>>> webrev: http://cr.openjdk.java.net/~mli/8186610/ <http://cr.openjdk.java.net/%7Emli/8186610/>
>>>>>> 
>>>>>> 
>>>>>> Thank you
>>>>>> 
>>>>>> -Hamlin
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>