RFR JDK-8173777: Merge javac -Xmodule into javac--patch-module

[Robert Field]

JShell changes to the memory file manager are unchanged in this update 
and so still look good.

-Robert


On 02/08/17 12:46, Jan Lahoda wrote:
> Hi,
>
> Based on some (offline) feedback and further testing, I've updated the 
> langtools patch. The new webrev is here:
> http://cr.openjdk.java.net/~jlahoda/8173777/langtools.02/
>
> Delta webrev compared to .01:
> http://cr.openjdk.java.net/~jlahoda/8173777/langtools.02/delta/webrev/index.html 
>
>
> The changes in this updated version:
> -the previous version didn't allow the same file to be under both 
> --patch-module and --module-source-path. The updated version allows 
> this, and the --patch-module will prevail. This is currently only 
> allowed when both --patch-module and --module-source-path agree on the 
> name of the module, but this could be trivially removed if desired.
>
> -for incremental compilation, the previous version required that the 
> class output is also specified in --patch-module. The updated version 
> will automatically use class output to read classfiles for patch 
> modules (by virtue of ModuleSymbol.patchOutputLocation).
>
> -the previous version didn't (always) report an error for module-info 
> on --patch-module in multi-module mode. This should be fixed now, and 
> there should be an error for module-info on patch location even in 
> multi-module mode.
>
> I was looking at explicit uses of -Xmodule: in other repositories, and 
> there are some in the hotspot repository. All these appear to use a 
> utility from the top-level repository:
> test/lib/jdk/test/lib/InMemoryJavaCompiler.java
> (there are other tests that use @compile/module in various 
> repositories, including hotspot, but these need to be coordinated with 
> jtreg). For now, I propose to tweak the utility to interpret -Xmodule: 
> itself:
> http://cr.openjdk.java.net/~jlahoda/8173777/top-level.00/
>
> Eventually, we could stop using -Xmodule: inside this utility and pass 
> the module name explicitly, e.g. encoded in the className, but for now 
> this seems acceptable to me.
>
> Feedback is welcome.
>
> Thanks!
>
> Jan
>
>
> On 7.2.2017 19:11, Jan Lahoda wrote:
>> Hi,
>>
>> Thanks.
>>
>> I did two small updates to the patches:
>> -for langtools repository, I added unwrapping to
>> ClientCodeWrapper.WrappedJavaFileManager.getLocationForModule - the
>> incoming JavaFileObject is unwrapped, so that we don't pass our wrappers
>> to the client's code. Direct link:
>> http://cr.openjdk.java.net/~jlahoda/8173777/langtools.01/src/jdk.compiler/share/classes/com/sun/tools/javac/api/ClientCodeWrapper.java.udiff.html 
>>
>>
>>
>> -for jdk repository, I fixed the:
>> test/javax/xml/jaxp/common/8035437/run.sh
>> to use a platform specific path separator. This is based on code from
>> jdk/test/java/lang/instrument/appendToClassLoaderSearch/CommonSetup.sh
>> Direct link:
>> http://cr.openjdk.java.net/~jlahoda/8173777/jdk.02/test/javax/xml/jaxp/common/8035437/run.sh.udiff.html 
>>
>>
>>
>> (Sorry that I missed this before.)
>>
>> Updated webrevs:
>> -langtools repository:
>> http://cr.openjdk.java.net/~jlahoda/8173777/langtools.01/
>> -jdk repository:
>> http://cr.openjdk.java.net/~jlahoda/8173777/jdk.02/
>>
>> Thanks,
>>     Jan
>>
>> On 6.2.2017 21:42, Alan Bateman wrote:
>>>
>>>
>>> On 06/02/2017 18:41, Jan Lahoda wrote:
>>>> I've updated the jdk repository webrev to wrap long lines:
>>>> http://cr.openjdk.java.net/~jlahoda/8173777/jdk.01/
>>>>
>>>> Jan
>>> This looks okay to me.
>>>
>>> -Alan
>>>