Re: RFR [9] 8168149: Examine the behavior of jmod command-line options - repeating vs last one wins

[Mandy Chung]

> On Jan 5, 2017, at 5:14 AM, Chris Hegarty  wrote:
> 
> 
>> I wonder if it would be more helpful if it fails when a non-repeating option 
>> is specified more than once for a packaging tool.  Otherwise, the only way 
>> to find out if the command-line is correct is to list the content after the 
>> JMOD file is created.  OptionSpec::value throws an exception if the option 
>> is specified more than once.
> 
> Right. I was following the existing longstanding precedent of
> last-one-wins for JDK tool options. If I read your comment
> correctly you are suggesting that packaging tools do not
> follow this, correct?  This will need further discussion, and 
> maybe a clarification in JEP 293 "Guidelines for JDK
> Command-Line Tool Options”.

jmod is a new JDK 9 tool that we should consider what’s the right thing to do 
with the new tool-specific options that specify the location of the content to 
be packaged such as —-cmds, —-libs, etc.

The discussion would be around `—-module-path` whether JDK tools accepting 
these options is required to follow the last-one-wins convention (which I have 
no objection to make them last-one-wins for consistency while I’m not too 
concern for it to be different for jmod tool).  I agree that JEP 293 should 
cover this guideline.  

Mandy

Re: Invoking default methods from a Proxy's InvocationHandler in JDK9

[Mandy Chung]

> On Jan 2, 2017, at 11:20 PM, Peter Levart  wrote:
> 
> Hi Matthew,
> 
> On 01/03/2017 04:28 AM, Matthew Hall wrote:
>> I'm a member of the JDBI [1] project, an open source SQL access library
>> atop JDBC.
>> 
>> A major part of our API provides implementations of declarative interfaces
>> defined by users (similar to MyBatis). Interface methods may be default (in
>> which case the default method implementation is used) or abstract (in which
>> case JDBI provides the implementation).
>> 
>> We're using JDK 8, and we use java.lang.reflect.Proxy and InvocationHandler
>> to provide declarative interface implementations for our users.
>> 
>> Prior to release of JDK 8, it appears that the subject of enhancing Proxies
>> for default methods was discussed [2], but ultimately did not make it into
>> the release.
>> 
>> The root of the problem is that the generated Proxy class overrides all
>> methods in the proxy interfaces. This means that the interface default
>> methods are now superclass methods to the proxy class, which makes them
>> un-invokable from outside code--including the InvocationHandler.
>> 
>> As far as we can tell, the _only_ way to invoke a default method--on a
>> proxy, from an InvocationHandler--is to resort to some horrible
>> setAccessible shenanigans [3].
>> 
>> Specifically, we have to:
>> 1. Call Constructor.setAccessible(true) to make the private constructor
>> MethodHandles.Lookup(Class lookupClass, int allowedModes) accessible,
>> 2. Invoke that private constructor to instantiate a MethodHandles.Lookup
>> with private (!) access to all the things,
>> 3. Call Lookup.unreflectSpecial to get the MethodHandle for the default
>> method, and
>> 4. Invoke the method through the MethodHandle.
>> 
>> This is ugly, but it works--for now. If JDK 9 takes away access to
>> setAccessible, it may cease to work.
>> 
>> I found some discussion about this last summer [4], but it's hard to tell
>> if anything came out of the discussion.
>> 
>> Is there anything on the roadmap for JDK9 to allow a proxy's
>> InvocationHandler to invoke default methods on proxies? Are there any
>> existing proposals I should be aware of?
> 
> I have created a prototype last year:
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-June/041629.html
> 
> But I think the JDK 9 train has already left the station. So perhaps in JDK 
> 10...


https://bugs.openjdk.java.net/browse/JDK-8159746 is the JBS issue for this.

One other possibility is to fix proxy generated class not to override default 
methods but there would require more investigation to tease out before we can 
be certain if this can make JDK 9.

Mandy

Re: Review Request: JDK-8168836 Minor clean up on warning/error messages on --add-exports and --add-reads

[Mandy Chung]

> On Jan 3, 2017, at 1:04 PM, David Holmes  wrote:
> 
> Hi Mandy,
> 
> 
>> 
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8168836/webrev.00/
>> 
>> This patch improves the warning/error message to include the option name, 
>> emit a warning if unknown module is specified with —-patch-module be 
>> consistent with the options.
>> 
>> Mandy
> 
> As a result of this change we now reject an empty classpath with an error:
> 
> /java/re/jdk/9/promoted/all/150/binaries/linux-x64/bin/java -cp "" Hello
> Error: -cp requires class path specification
> 
> was that intentional? We've accepted an empty -cp value for a very long time.


No this is not intentional.  This line seems to be the cause of the regression:
  1250 jboolean has_arg = value != NULL && JLI_StrLen(value) > 0;

I’ll fix it.  I had a test checking on empty classpath that passed.  I’ll look 
into it how this test case is missed.

http://hg.openjdk.java.net/jdk9/dev/jdk/file/d27bab22ff62/test/tools/launcher/modules/classpath/JavaClassPathTest.java

Thanks for finding it.
Mandy

Re: RFR [9] 8168149: Examine the behavior of jmod command-line options - repeating vs last one wins

[Mandy Chung]

> On Dec 22, 2016, at 9:11 AM, Chris Hegarty  wrote:
> 
> Most options for the jmod tool should be last one wins, to be consistent
> with the JDK tool convention, 8168149 [1]. Excludes is the only
> repeatable option.
> 
> Given the existing usage of JOpt Simple, the most straight forward way
> to achieve the last-one-wins behaviour is to drop the
> withValuesSeparatedBy() from the OptionSpec have have the ValueConverters
> themselves do the separation, if any. That way all options can be made
> repeatable and the last element of the list of the option’s values will
> be the final one on the command line.
> 
> http://cr.openjdk.java.net/~chegar/8168149.00/

This looks okay.

I wonder if it would be more helpful if it fails when a non-repeating option is 
specified more than once for a packaging tool.  Otherwise, the only way to find 
out if the command-line is correct is to list the content after the JMOD file 
is created.  OptionSpec::value throws an exception if the option is specified 
more than once.

Mandy

Re: RFR 8170289: Re-examine entry point support in jlink

[Mandy Chung]

 282 throw new RuntimeException(module + " does not have 
main class: " + mainClassName);

- Throwing IllegalArgumentException would probably be better.

 101 err.launcher.value.format:launcher value should be of form 
=: {0}

- should this message include optional main class; something like 
=[/]

This validation is done after the image is created.  Can you file a JBS issue 
to separate the launcher file creation and the validation from image creation?  
That can be improved in the future.

Otherwise, looks okay.  No need to send a new webrev.  

Mandy
P.S. There is some issue to access cr.openjdk.java.net. I got a copy of 
webrev.02 from Sundar to review.

> On Dec 18, 2016, at 7:29 AM, Sundararajan Athijegannathan 
> <sundararajan.athijegannat...@oracle.com> wrote:
> 
> Updated it: http://cr.openjdk.java.net/~sundar/8170289/webrev.02
> 
> Thanks,
> -Sundar
> 
> On 17/12/16, 12:33 AM, Mandy Chung wrote:
>>> On Dec 16, 2016, at 8:36 AM, Sundararajan 
>>> Athijegannathan<sundararajan.athijegannat...@oracle.com>  wrote:
>>> 
>>> Hi,
>>> 
>>> Please review http://cr.openjdk.java.net/~sundar/8170289/webrev.01/ for 
>>> https://bugs.openjdk.java.net/browse/JDK-8170289
>> 
>>  273 Optional  mainClass = 
>> ModuleDescriptor.read(stream).mainClass();
>>  274 if (mainClass.isPresent()) {
>>  275 mainClassName = mainClass.get();
>>  276 }
>> 
>> This should set mainClassName only if the main class is not specified
>> in the -—launcher option.  One may want to create launchers for
>> multiple entry points.
>> 
>> I think it should validate if the main class is present in the image.
>> If not found, it should output an error.
>> 
>> Something to be considered in a future release - the existing implementation
>> creates the launcher scripts as a special case in DefaultImageBuilder.
>> It seems cleaner to keep DefaultImageBuilder just for the image creation,
>> i.e. simply write out entries of the ResourcePool to the image.
>> The launchers could be added to the ResourcePool entries to the
>> corresponding module by one builtin plugin implementation.
>> 
>> For this issue, keeping the change to minimal is good.
>> 
>> Mandy
>> 
>> 

Re: Review Request: JDK-8168836 Minor clean up on warning/error messages on --add-exports and --add-reads

[Mandy Chung]

> On Dec 19, 2016, at 11:39 AM, Mandy Chung <mandy.ch...@oracle.com> wrote:
> 
> 
>> On Dec 19, 2016, at 9:45 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
>> 
>> 
>> 
>> On 19/12/2016 01:44, Mandy Chung wrote:
>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8168836/webrev.00/
>>> 
>>> This patch improves the warning/error message to include the option name, 
>>> emit a warning if unknown module is specified with —-patch-module be 
>>> consistent with the options.
>>> 
>> The implementation update looks good.  As --patch-modules is a slow path 
>> then you could use cf.findModule(mn).ifPresent(…)
> 
> Fixed.

Having a second thought, I prefer to keep the if statement testing isPresent(). 
 Or I could do something like  
cf.findModule(mn).orElse(warnUnknownModule(PATCH_MODULE, mn)) but 
warnUnknownModule method would need to return ResolvedModule.

Mandy

Re: RFR: 8171400: Move checking of duplicate packages in the boot layer to link time

[Mandy Chung]

tools/launcher/modules/patch/systemmodules/PatchSystemModules.java needs to be 
updated since ModuleBootstrap now depends on this new method:

diff --git 
a/test/tools/launcher/modules/patch/systemmodules/src1/java.base/jdk/internal/modules/SystemModules.java
 
b/test/tools/launcher/modules/patch/systemmodules/src1/java.base/jdk/internal/modules/SystemModules.java
--- 
a/test/tools/launcher/modules/patch/systemmodules/src1/java.base/jdk/internal/modules/SystemModules.java
+++ 
b/test/tools/launcher/modules/patch/systemmodules/src1/java.base/jdk/internal/modules/SystemModules.java
@@ -29,4 +29,8 @@
  */
 public final class SystemModules {
 public static final String[] MODULE_NAMES = new String[0];
+
+public static boolean hasSplitPackages() {
+return true;
 }
+}

Since this fix has been pushed, I will fix this with a separate issue.

Mandy

> On Dec 19, 2016, at 4:30 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> this patch adds a check to see if there are any split packages in the system
> modules at link time, and uses this information to enable us to safely skip
> a runtime check during bootstrap for the common case that there are none
> of the sort.
> 
> Webrev[1]: http://cr.openjdk.java.net/~redestad/8171400/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8171400
> 
> This removes a chunk of the module system bootstrap overhead, and also
> amends a small issue where PACKAGES_IN_BOOT_LAYER would be wrong in the
> presence of split packages.
> 
> Thanks!
> 
> /Claes
> 
> [1] Since cr.openjdk.java.net is down I've also attached the raw patch.
> 

Re: [9] Review request: JDK-8170485: Switch to building JavaFX with new module-info syntax

[Mandy Chung]

> On Dec 6, 2016, at 8:10 AM, Kevin Rushforth  
> wrote:
> 
> Chien & Dave,
> 
> Please review the preliminary webrev to allow building JavaFX with jdk-9+148 
> and later:
> 
> https://bugs.openjdk.java.net/browse/JDK-8170485
> http://cr.openjdk.java.net/~kcr/8170485/webrev.00/
> 
> The details are in the JBS issue and also in the "HEADS-UP" message [1] I 
> sent yesterday.
> 
> As indicated in JBS, I will update the webrev later this week to bump the 
> minimum build to 148 once 148 is promoted and available on java.net. No other 
> changes are anticipated (unless you find anything while reviewing it).

Looks good. 

Mandy

Re: RFR: 8171373: Reduce copying during initialization of ModuleHashes

[Mandy Chung]

+1

Mandy

> On Dec 16, 2016, at 4:45 PM, Claes Redestad <claes.redes...@oracle.com> wrote:
> 
> Chris, Mandy,
> 
> thanks for reviewing!
> 
> I've made nameToHash final and added a comment, updated in place.
> 
> I'll push this tomorrow unless I hear any objections.
> 
> /Claes
> 
> On 2016-12-16 18:52, Mandy Chung wrote:
>> 
>>> On Dec 16, 2016, at 8:00 AM, Claes Redestad <claes.redes...@oracle.com> 
>>> wrote:
>>> 
>>> Hi,
>>> 
>>> recent changes to split out ModuleHashes from ModuleDescriptor caused a 
>>> small/tiny increase in HashMap creation, resize and copying:
>>> 
>>> Webrev: http://cr.openjdk.java.net/~redestad/8171373/webrev.01/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8171373
>> 
>> I like the optimization for one single module (i.e. java.base) containing 
>> the module hashes which is the common case.
>> 
>> Can you add a comment in SystemModuleFinder about the common case and avoid 
>> creating a new HashMap unless there is more than one module containing 
>> ModuleHashes.
>> 
>> Builder::nameToHash can now be final.
>> 
>> Mandy
>> 

Re: RFR 8170618: jmod should validate if any exported or open package is missing

[Mandy Chung]

> On Dec 21, 2016, at 4:08 AM, Sundararajan Athijegannathan 
>  wrote:
> 
> Please review http://cr.openjdk.java.net/~sundar/8170618/webrev.00/ for 
> https://bugs.openjdk.java.net/browse/JDK-8170618

thanks for fixing this Sundar.

Minor comment on the test: the bug id's already listed in @bug.  You
can take out the bug id from line 117.  line 132 should be broken into
multiple lines.

Otherwise, looks good.
Mandy

Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Mandy Chung]

> On Mar 23, 2017, at 1:07 PM, Igor Ignatyev  wrote:
> 
> Mandy/Daniel,
> 
> yes, jdk.management.agent does require java.management, but not transitively. 
> Shura and I have discussed that and agreed that in such cases a test should 
> declare dependency explicitly, otherwise it can start to fail when some of 
> transitive requires (which are not a part of the contract) are changed.
> 
> I used jdeps with the post-proceccing which makes reduction similar to 
> list-reduced-deps. I have run 'jdeps --list-reduced-deps' on classes from 
> sun/management/jmxremote/bootstrap/CustomLauncherTest.java test run, and it 
> showed the same:
>>   java.management
>>   jdk.attach
>>   jdk.management.agent/jdk.internal.agent
>>   unnamed module: 
>> /tmp/run/jdk/sun/management/jmxremote/bootstrap/JTwork-sun-management-jmxremote-bootstrap-CustomLauncherTest-java_0/classes
> 

It’s good you are using `jdeps --list-reduced-deps`.  It includes 
java.management because the test references the exported APIs from 
java.management.

I agree that @modules should list java.management even though the test 
selection would work properly without it.

Mandy

Re: 8174823: Module system implementation refresh (3/2017)

[Mandy Chung]

> On Mar 21, 2017, at 2:42 PM, Alan Bateman  wrote:
> 
> We have been accumulating changes in the jake forest for the last few weeks 
> to align with the proposals in JSR 376 and also to pick up API changes and 
> bug fixes. It's time to bring these changes into jdk9/dev. The issue to bring 
> these changes into jdk9/dev in bulk has been approved via the FC extension 
> process (still in use during JDK 9 RDP2).
> 
> A summary of the main changes is listed in JDK-8174823 [1], and the webrevs 
> with the changes to bring to jdk9/dev are here:
>   http://cr.openjdk.java.net/~alanb/8174823/1/

I reviewed all repos and the changes look fine.

Mandy

Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

[Mandy Chung]

Jon and I work on this patch as the first step to enable module
graphs for inclusion in the javadoc.  It includes a @moduleGraph
taglet, an updated GenGraphs tool, and also updates javadoc of 
module-info.java to include @moduleGraph.  I also take the 
opportunity to add simple javadoc to a few JDK modules preparing
for JEP 299 that will generate module summary page for JDK modules.

Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.00/

Here shows some sample of the javadoc with module graph images:
  
http://cr.openjdk.java.net/~mchung/jdk9/module-graph-docs/overview-summary.html

Click on a module and it will show a module graph icon. Mouse over
it will show the full size image.  java.base is a special case 
that has no hovered image since it has one node and the full
size image is already shown.

The build work to generate PNG file from .dot file will be 
done separately [1].
 
Mandy 
[1] https://bugs.openjdk.java.net/browse/JDK-8176785

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

[Mandy Chung]

> On Mar 23, 2017, at 10:47 AM, Andrey Nazarov <andrey.x.naza...@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. 

Mandy

> 
> —Andrey
> 
>> On 22 Mar 2017, at 22:11, Mandy Chung <mandy.ch...@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 [,...]  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
> 

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

[Mandy Chung]

> On Mar 23, 2017, at 11:07 AM, Mandy Chung <mandy.ch...@oracle.com> wrote:
> 
>> 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. 
> 

Having another look at jdk/tests/tools/lib/tests/*, I think it’s probably best 
to leave these new tests as is and avoid adding more helper classes until they 
are cleaned up/unified.

Mandy

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

[Mandy Chung]

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 [,...]  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

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

[Mandy Chung]

> On Mar 23, 2017, at 7:58 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
> 
> On 22/03/2017 19:11, Mandy Chung 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.
> The output for --suggest-providers looks good.
> 
> For --verbose then the "Providers: " list looks okay when used in conjunction 
> with --bind-services. I would be tempted to leave it out when --bind-services 
> is not specified because there is no service binding.
> 

When linking with explicit providers via —-add-modules, would you think it may 
be useful to see the list of providers?

> A few random comments:
> 
> - JImageTask is now using toUpperCase() which is locale specific and should 
> be changed.
> 

Fixed.

> - Jlink.moduleFinder() unconditionally uses the runtime version. Not a bug 
> introduced by your changes but this method really needs to locate java.base 
> and use its version when creating the module path. We should probably create 
> an issue for that and fix it another time.
> 

I created https://bugs.openjdk.java.net/browse/JDK-8177471 to track this.

> - JlinkTask.uses then the stream() isn't needed as Set defines forEach.

Good catch.  Fixed.

Revised webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.01/

Mandy

Re: List of previously internal APIs that are now exposed publicly?

[Mandy Chung]

> On Mar 21, 2017, at 10:03 AM, Paul Deitel  wrote:
> 
> Is there list somewhere of APIs that used to be internal, but are now 
> exported from JDK 9 modules?


The exported API in jdk.xml.dom module is one case where [1] promotes 
these subpackages be supported APIs in JDK 9.
  org.w3c.dom.css
  org.w3c.dom.html
  org.w3c.dom.stylesheets
  org.w3c.dom.xpath

Do you have a specific internal API in mind? [2] lists some relevant info.

There are several JEPs integrated in JDK 9 that define APIs that should
be used to replace some internal APIs e.g. JEP 259, 268, 272, 276

Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8042244
[2] https://wiki.openjdk.java.net/display/JDK8/Java+Dependency+Analysis+Tool

Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Mandy Chung]

> On Mar 22, 2017, at 1:09 PM, Igor Ignatyev  wrote:
> 
> http://cr.openjdk.java.net/~iignatyev/8177374/webrev.00/index.html
>> 40 lines changed: 26 ins; 13 del; 1 mod;
> 
> Hi all,
> 
> could you please review this changeset which fixes in a few jdk_svc tests 
> which were missed by JDK-8176176[1]?
> 
> testing: :jdk_svc tests
> webrev: http://cr.openjdk.java.net/~iignatyev/8177374/webrev.00/index.html
> JBS: https://bugs.openjdk.java.net/browse/JDK-8177374


test/java/lang/management/PlatformLoggingMXBean/LoggingMXBeanTest.java
   This test should need java.management.  Where is it declared?
   Does the webrev miss some new file?

Can you keep @modules sorted? e.g. the following in a few files:
  42  * @modules jdk.management.agent/jdk.internal.agent
  43  *  java.management
  44  *  jdk.attach

Mandy

Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Mandy Chung]

> On Mar 22, 2017, at 4:09 PM, Igor Ignatyev  wrote:
> 
> Hi Mandy,
>  
> I've (re)sorted @modules in all the tests which I touched -- 
> http://cr.openjdk.java.net/~iignatyev/8177374/webrev.01/index.html 
> 
> 

+1

>> test/java/lang/management/PlatformLoggingMXBean/LoggingMXBeanTest.java
>>   This test should need java.management.  Where is it declared?
> it's declared in test/java/lang/management/TEST.properties which has been 
> pushed by JDK-8176176.
> 

Ah I see.  this is in jdk9/hs/jdk repo and not yet integrated in jdk9/dev/jdk.

Mandy

Re: Making jdeps aware of exported API

[Mandy Chung]

I considered adding an option to specify the packages to be exported.  It may 
work fine for simple cases.  It may take a prefix to avoid listing the packages 
individually.  It’s an idea on the list to implement.  On the other hand, there 
will be cases that we would have to edit module-info.java to include `uses` and 
remove `exports` or  qualify any export to a friend. 

One useful option, `jdeps —-check` option provides suggestion to `requires 
transitive` that may not be needed and any unused qualified exports.

After you edit module-info.java, compile it and rerun jdeps —-check that will 
analyze the exported API packages and suggest `requires` and `requires 
transitive`.  It requires a compilation step which would be needed to run as a 
module anyway.  

About jdeps —-api-only, when running on a module, it looks at the module 
descriptor and analyzes only exported API packages.  If the classes are on 
classpath (-cp), module-info.class is skipped.

Mandy

> On Mar 23, 2017, at 2:24 AM, Gunnar Morling  wrote:
> 
> Hi,
> 
> I would like to suggest to add an option to jdeps for describing the
> intended exported API of descriptors created by
> --generate-module-info. This would have two benefits:
> 
> * Limiting the number of exported packages in the generated descriptor
> to the intended public API
> * Adding the "transitive" modifiers only to those "requires"
> statements whose types are used in the exported API
> 
> While it's relatively simple to rework descriptors after generation
> and remove any unwanted exports, that's not the case for removing
> superfluous "transitive" modifiers. Identifying them would require to
> parse the input JAR once more to find out whether a given dependence
> is used in public/protected signatures of the exported API or not.
> 
> If the intended exported API could be passed to jdeps - e.g. in form
> of regular expressions or some other kind of exclude/include patterns
> - no further post-processing would be needed for generating
> descriptors with a well-defined API.
> 
> I had a look at the --api-only option, but this seems only to be about
> public/protected access and not the exported packages of a module.
> 
> Thanks for your consideration,
> 
> --Gunnar

Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Mandy Chung]

> On Mar 23, 2017, at 7:33 AM, Daniel Fuchs  wrote:
> 
> Hi Igor,
> 
> small nit:
> 
>  42  * @modules java.management
>  43  *  jdk.attach
>  44  *  jdk.management.agent/jdk.internal.agent
> 
> I don't think java.management needs to be specified as
> a dependency when the test requires jdk.management.agent,
> because jdk.management.agent already requires java.management.

That’s true.

Igor - How do you analyze the dependency?  Are you using jdeps 
—-list-reduced-deps?

Mandy

hg: jigsaw/jake/jdk: jlink generated release file should not contain the date/timestamp

[mandy . chung]

Changeset: 13a7d45fdf76
Author:mchung
Date:  2017-03-29 10:30 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/13a7d45fdf76

jlink generated release file should not contain the date/timestamp

! 
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java

hg: jigsaw/jake/hotspot: Fix Graal/JVMCI to call Module::getPackages with right signature

[mandy . chung]

Changeset: c1d6cbb4a84e
Author:mchung
Date:  2017-03-29 13:09 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/c1d6cbb4a84e

Fix Graal/JVMCI to call Module::getPackages with right signature

! 
src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.services/src/jdk/vm/ci/services/Services.java
! 
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.test/src/org/graalvm/compiler/test/JLModule.java

Re: 8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens

[Mandy Chung]

> On Mar 23, 2017, at 12:16 PM, Alan Bateman  wrote:
> 
> I'd like to get jdk9/dev updated to align with the latest proposal for 
> --add-exports/--add-opens to not emit warnings. I've put the webrev with the 
> changes here:
> 
>http://cr.openjdk.java.net/~alanb/8177474/webrev/index.html

Looks good to me.

Mandy

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

[Mandy Chung]

> On Mar 23, 2017, at 11:40 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
> 
> On 23/03/2017 17:04, Mandy Chung wrote:
> 
>> :
>> When linking with explicit providers via —-add-modules, would you think it 
>> may be useful to see the list of providers?
> For the most part then this will just print out the providers specified to 
> --add-modules,

and all of their transitive dependencies. Basically listing the providers and 
used by the modules linked in the resulting image provides you the information 
and avoid the need to run `java —list-modules m1,m2,m3 to inspect the module 
descriptor, if it wants to confirm what providers are in the image.

> unless you are thinking about the less common case of a module that both 
> exports an API and is a service provider module at the same time?

Not really in this context.

Mandy

Re: RFR [9] 8176772: jar tool support to report automatic module names

[Mandy Chung]

> On Mar 16, 2017, at 6:09 AM, Chris Hegarty  wrote:
> 
> Updated webrev:
>  http://cr.openjdk.java.net/~chegar/8176772.01/

Looks good.

thanks
Mandy

hg: jigsaw/jake/jdk: Minor javadoc update of Module::getPackages

[mandy . chung]

Changeset: b77bacbfb8bf
Author:mchung
Date:  2017-03-18 14:01 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/b77bacbfb8bf

Minor javadoc update of Module::getPackages

! src/java.base/share/classes/java/lang/reflect/Module.java

Re: RFR [9] 8176772: jar tool support to report automatic module names

[Mandy Chung]

> On Mar 15, 2017, at 8:31 AM, Chris Hegarty  wrote:
> 
> 
>> On 15 Mar 2017, at 14:42, Alan Bateman  wrote:
>> 
>> On 15/03/2017 14:21, Chris Hegarty wrote:
>> 
>>> :
>>> 
>>> Webrev:
>>> 
>>>  http://cr.openjdk.java.net/~chegar/8176772.00/
>>>  https://bugs.openjdk.java.net/browse/JDK-8176772
>>> 
>> If findAll returns an empty set then the error.unable.derive.automodule 
>> message might be more appropriate. Otherwise looks good to me.
> 
> That would be better. Updated in-place.
>  http://cr.openjdk.java.net/~chegar/8176772.00/

Is there any reason why this option does not print the synthesized descriptor?  
I understand that the issue is specific to show the derived module name.  But 
it’d be nice for this option to print the descriptor consistent with a normal 
module.

Mandy

Re: RFR [9] 8176772: jar tool support to report automatic module names

[Mandy Chung]

> On Mar 15, 2017, at 10:12 AM, Chris Hegarty  wrote:
> 
>> 
>> Is there any reason why this option does not print the synthesized 
>> descriptor?  I understand that the issue is specific to show the derived 
>> module name.  But it’d be nice for this option to print the descriptor 
>> consistent with a normal module.
> 
> Unless I am mistaken, the synthesised descriptor does not contain
> any more information. What are you expecting?

provides and main class, if present. 

The modifier of ModuleDescriptor will have AUTOMATIC and so all packages are 
exported and open implicitly and not shown in the output which is okay.

Mandy

Review Request: JDK-8177980: ResourceBundle.getBundle throws NoClassDefFoundError when fails to define a class

[Mandy Chung]

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8177980/webrev.00/

This is a simple fix.  ResourceBundle::getBundle should handle LinkageError 
that may be thrown by Class.forName when finding a class.

Mandy

Re: Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

> On Apr 4, 2017, at 12:42 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
> 
> On 03/04/2017 19:41, Mandy Chung wrote:
> 
>> Webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8175819/webrev.00/
>> 
> I went through the updates to jlink, assuming test SystemModulesTest will be 
> aligned to the recent mails.
> 

It’s already aligned. I updated webrev.00 in place yesterday.  Maybe you need 
to reload it if you happened opening the old version.

> In DefaultImageBuilder.storeFiles then 
> map(ResourcePoolModule::osName).orElse(null) would be cleaner.
> 

OK.

> I'm sure Volker or someone maintaining the AIX port will ask for 
> jdk.tools.jlink.internal.Platform to be extended to handle that platform.

I was wondering if Volker would ask that. Anyway, I have added Platform::AIX 
constant.

Will send out a new webrev.

Mandy

Re: Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

> On Apr 3, 2017, at 4:10 PM, mark.reinh...@oracle.com wrote:
> 
> 2017/4/3 14:50:52 -0700, mandy.ch...@oracle.com:
>>> On Apr 3, 2017, at 2:39 PM, mark.reinh...@oracle.com wrote:
>>> 2017/4/3 13:35:30 -0700, si...@cjnash.com:
 ...
 
 I am not sure why we would change to osx for Mac when the Mac developers
 have recently dropped the Mac OS X terminology and changed it to macOS.
>>> 
>>> Agreed -- we should change OS_NAME from "Darwin" to "macos”.
>> 
>> OK.  Should the bundle names be updated to reflect this change?
> 
> Probably not worth doing at this late stage in 9; let's fix these in 10.

That's what I was thinking too.  I created a JBS issue target for 10:

Mandy

hg: jigsaw/jake/nashorn: Update jtreg required version to 4.2 b07

[mandy . chung]

Changeset: f1d6cca3853b
Author:mchung
Date:  2017-04-04 22:09 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/f1d6cca3853b

Update jtreg required version to 4.2 b07

! test/TEST.ROOT

hg: jigsaw/jake/langtools: Update jtreg required version to 4.2 b07

[mandy . chung]

Changeset: a564c0126103
Author:mchung
Date:  2017-04-04 22:09 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/a564c0126103

Update jtreg required version to 4.2 b07

! test/TEST.ROOT

hg: jigsaw/jake/jdk: Update jtreg required version to 4.2 b07

[mandy . chung]

Changeset: 3f059be20ccb
Author:mchung
Date:  2017-04-04 22:09 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/3f059be20ccb

Update jtreg required version to 4.2 b07

! test/TEST.ROOT

hg: jigsaw/jake/jaxp: Update jtreg required version to 4.2 b07

[mandy . chung]

Changeset: d44354fed774
Author:mchung
Date:  2017-04-04 22:09 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/d44354fed774

Update jtreg required version to 4.2 b07

! test/TEST.ROOT

hg: jigsaw/jake/hotspot: Update jtreg required version to 4.2 b07

[mandy . chung]

Changeset: b0076c96fadc
Author:mchung
Date:  2017-04-04 22:09 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/b0076c96fadc

Update jtreg required version to 4.2 b07

! test/TEST.ROOT

Re: RFR 8178323: Add negative tests for bind services Jlink feature

[Mandy Chung]

> On Apr 10, 2017, at 12:30 PM, Andrey Nazarov  
> wrote:
> 
> Mandy, thanks for review. I’ve updated patch
> http://cr.openjdk.java.net/~anazarov/8178323/webrev.01/webrev/ 
> 
> 

I suggest to add m4/p4.S to make it clear it’s a service type and m4 provides 
p4.S with  p4.Impl.
noOneUsesProvider will call —suggest-providers p4.S instead.

 239 static JLink run(boolean expectSuccess, String... options) {
Instead of adding a new parameter, what about adding a new 
runWithNonZeroExitCode method?

> See my comments inline
>> 
>> 
>> line 159-161: formatting nit: can you add spaces to align with the first
>> argument in line 158.  Same comment to SuggestProviders.java line 180-182
>> an 198-200.
> Fixed

line 216-219.

> Actually there are two issues here: 
> https://bugs.openjdk.java.net/browse/JDK-8178404 
> 
> https://bugs.openjdk.java.net/browse/JDK-8178405 
> 
Not really a bug but it’d be helpful to show that the provider exists but the 
service is not used in this case.
Mandy

Re: Issue with JavaFX and Jigsaw

[Mandy Chung]

It may be useful to point to the javadoc where it specifies to require the 
module to export the packages unconditionally.

It would be a good RFE to relax the exports to at least javafx.beans.

Mandy

> On Apr 10, 2017, at 3:56 PM, Kevin Rushforth <kevin.rushfo...@oracle.com> 
> wrote:
> 
> Sorry for the delay in responding.
> 
> I added a simple test program to the JBS bug that shows the same behavior as 
> the application and also an evaluation of the bug.
> 
> The short version is that JavaFX beans is (mostly) working as expected, 
> except for the misleading exception message. In JDK 9 it is required that any 
> object that is reflected on by JavaFX beans, specifically the items passed to 
> TableView, which are accessed via a PropertyValueFactory, will need to be in 
> a package that is exported unconditionally. In JDK 10 we can look into 
> relaxing this requirement such that the package only needs to be exported to 
> javafx.beans.
> 
> I do think we need to make the exception message less confusing in JDK 9 and 
> also document the requirement in the appropriate places (at least in 
> TableView and probably in a couple of javafx.beans.property.adapter classes).
> 
> Comments?
> 
> -- Kevin
> 
> 
> Mandy Chung wrote:
>> 
>> Hi Trisha,
>> 
>> Thanks for the report and stack trace.  I created
>> https://bugs.openjdk.java.net/browse/JDK-8177566 
>> <https://bugs.openjdk.java.net/browse/JDK-8177566> for further
>> investigation.
>> 
>> Mandy
>> 
>> 
>>   
>>> On Mar 24, 2017, at 2:34 PM, Trisha Gee <trisha@gmail.com> 
>>> <mailto:trisha@gmail.com> wrote:
>>> 
>>> Hi,
>>> 
>>> I was chatting to Alex Buckley at DevoxxUS about my experiences migrating a
>>> project to using Java 9 modules (specifically, this project:
>>> https://github.com/trishagee/sense-nine 
>>> <https://github.com/trishagee/sense-nine>) and mentioned some surprising
>>> behaviour in a module that uses JavaFX.  I've been asked to send the
>>> details so people can take a look at it.
>>> 
>>> My module-info.java is here:
>>> https://github.com/trishagee/sense-nine/blob/master/src/com.mechanitis.demo.sense.client/module-info.java
>>>  
>>> <https://github.com/trishagee/sense-nine/blob/master/src/com.mechanitis.demo.sense.client/module-info.java>
>>> Note that I have to export two additional packages (mood and user), and I
>>> did not expect (or really want) to.  This is because I was getting this
>>> error if I did not:
>>> 
>>> java.lang.reflect.InvocationTargetException
>>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>>> Method)
>>> at
>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>> at
>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>> at java.base/java.lang.reflect.Method.invoke(Method.java:547)
>>> at
>>> javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplicationWithArgs(LauncherImpl.java:482)
>>> at
>>> javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication(LauncherImpl.java:381)
>>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>>> Method)
>>> at
>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>> at
>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>> at java.base/java.lang.reflect.Method.invoke(Method.java:547)
>>> at
>>> java.base/sun.launcher.LauncherHelper$FXHelper.main(LauncherHelper.java:912)
>>> Caused by: java.lang.RuntimeException: Exception in Application start method
>>> at
>>> javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:982)
>>> at
>>> javafx.graphics/com.sun.javafx.application.LauncherImpl.lambda$launchApplication$2(LauncherImpl.java:200)
>>> at java.base/java.lang.Thread.run(Thread.java:844)
>>> Caused by: java.lang.RuntimeException: *java.lang.IllegalAccessException:
>>> class sun.reflect.misc.Trampoline cannot access class
>>> com.mechanitis.demo.sense.client.user.TwitterUser (in module
>>> com.mechanitis.demo.sense.client) because module
>>> com.mechanitis.demo.sense.client does not export
>>> com.mechanitis.demo.sense.client.user to unnamed module @779d6cc6*
>>> at
>>&g

Re: jna-platform, xalan, batik-script InvalidModuleDescriptorException

[Mandy Chung]

> On Apr 10, 2017, at 2:49 PM, Rahman USTA  wrote:
> 
> I try to adopt my application to Java 9. It use several 3rd party
> dependency from Maven. I get the following exceptions Because I'm not  the
> author of this libraries, how can I adopt this libraries to my Jigsaw
> project ?
> 

I suggest you contact the maintainer of this library to publish
a version that can be run as a module.  These libraries fail to be
loaded as automatic modules because they reference classes that are
not in the local module.

> Thanks.
> 
> 1) Error occurred during initialization of boot layer
> java.lang.module.FindException: Unable to derive module descriptor for
> \.m2\repository\net\java\dev\jna\jna-platform\4.4.0\jna-platform-4.4.0.jar
> Caused by: java.lang.module.InvalidModuleDescriptorException: Main-Class
> com.sun.jna.Native not in module
> 

com.sun.jna.Native is in a different artifact net.java.dev.jna:jna:4.4.0.
When you place it in the module path as an automatic module, the main class
is expected to be local in the same module.  Since it’s in a different
module, it fails to load jna-platform-4.4.0.jar as automatic module.

jdeps -jdkinternals shows that it depends on an internal API. 

jna-platform-4.4.0.jar -> java.desktop
   com.sun.jna.platform.WindowUtils$MacWindowUtils$1  -> 
java.awt.peer.ComponentPeerJDK internal API 
(java.desktop)

Similarly, the other InvalidModuleDescriptorException you got indicates
that it fails to load the JAR file as automatic module and reconstitute
the module descriptor.

> 2) Error occurred during initialization of boot layer
> java.lang.module.FindException: Unable to derive module descriptor for
> \.m2\repository\org\apache\xmlgraphics\batik-script\1.8\batik-script-1.8.jar
> Caused by: java.lang.module.InvalidModuleDescriptorException: Provider
> class org.apache.batik.bridge.RhinoInterpreterFactory not in module
> 

There exists META-INF/services/org.apache.batik.script.InterpreterFactory 
config file with a provider class that is not in this module.

> 3) Error occurred during initialization of boot layer
> java.lang.module.FindException: Unable to derive module descriptor for
> \.m2\repository\xalan\xalan\2.7.2\xalan-2.7.2.jar
> Caused by: java.lang.module.InvalidModuleDescriptorException: Provider
> class org.apache.bsf.BSFManager not in module
> 

Similar to batik-script case.

Mandy

> Thanks.
> 
> -- 
> Rahman USTA
> Istanbul JUG
> https://github.com/rahmanusta

Re: ResourceBundle in Java 9 Module

[Mandy Chung]

I created a JBS issue:
  https://bugs.openjdk.java.net/browse/JDK-8177980

What platform are you running on?  This works on linux but I can reproduce the 
problem on OSX (related to case-insensitive file system).

Mandy

> On Apr 3, 2017, at 8:54 AM, Rabea Gransberger  wrote:
> 
> Hello,
> 
> while migrating example code to Java 9 Modules I ran into a problem:
> 
> ResourceBundle bundle = ResourceBundle.getBundle("de.rgra.nl.messages");
> System.out.println(bundle.getString("I18n.message"));
> 
> The properties file path is:
> src\de\rgra\nl\messages.properties
> 
> The code works on Java 8 and 9 when used on the Classpath.
> 
> It fails in Java 9 when used in a module on the Modulepath with:
> 
> Exception in thread "main" java.lang.NoClassDefFoundError:
> de/rgra/nl/Messages (wrong name: de/rgra/nl/messages)
> 
> Changing it to uppercase M will work on Classpath and Modulepath
> (without changing the properties file to uppercase M):
> 
> ResourceBundle bundle = ResourceBundle.getBundle("de.rgra.nl.Messages");
> System.out.println(bundle.getString("I18n.message"));
> 
> Can anybody explain to me why this happens?
> 
> The example with run script can be found at:
> https://github.com/rgra/java9-module-refactoring/tree/master/resourcebundle
> 
> 
> 
> ---
> Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
> https://www.avast.com/antivirus
> 

Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8175819/webrev.00/

This revisits the OS name and arch in packaging JDK modules
to extend the module descriptor with ModuleTarget class file
attribute.  We considered matching with the system properties.
Linux x64 JDK can run on a system whose `os.arch` system
property value can be `amd64` or `i586` or `x86_x64`.  
Similiarly, windows x86/x64 JDK can run on a system whose 
`os.name` system property starts with “Windows” as the 
os.name property is set to "Windows XXX" for example 
"Windows Server 2012 R2”.  It might be worth considering
multiple OS arch values in ModuleTarget in the future.

JDK bundle names are revised in JDK 9.  This is a good
alternative to be consistent with $OS-$ARCH value in 
the bundle names.  This patch proposes to package JDK modules 
with OS name and arch to match the values as in JDK bundle names.
jlink will generate the `release` file and set OS_NAME and
OS_ARCH to those values.  This also proposes to drop 
OS_VERSION to align with the ModuleTarget class file attribute.

This shows the old and new value of OS_NAME/OS_ARCH properties
in the `release` file:

JDK 8   JDK 9
-   -
OS_NAME Linux   linux
SunOS   solaris
Darwin  osx
Windows windows
 
OS_ARCH i386,x86x86
i586,amd64,x86_64   x64
sparcv9 sparcv9
arm arm32
aarch64 arm64

Mandy
 

Re: Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

> On Apr 3, 2017, at 1:35 PM, Simon Nash  wrote:
> 
> On 03/04/2017 21:15, mark.reinh...@oracle.com wrote:
> 
> I am not sure why we would change to osx for Mac when the Mac developers
> have recently dropped the Mac OS X terminology and changed it to macOS.

Just to be clear, there is no plan to change the value of the
‘os.name’ system property and its value is “Mac OS X”.  
Changing the value of the system property ‘os.name’ would 
break existing applications.

In JDK 8, OS_NAME in the release file is “Darwin” which is
what this patch would change.

Mandy

Re: Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

> On Apr 11, 2017, at 6:45 AM, Peter Levart <peter.lev...@gmail.com> wrote:
> 
> Hi Mandy,
> 
> On 04/03/2017 11:50 PM, Mandy Chung wrote:
>>JDK 8   JDK 9
>>-   -
>> OS_NAMELinux   linux
>>SunOS   solaris
>>Darwin  macos
>>Windows windows
>> 
>> OS_ARCHi386,x86x86
>>i586,amd64,x86_64   amd64
> 
> i586 (i.e. 32 bit Pentium) should map to x86, not amd64 !!!

Oops… a typo of course (should be removed).  “i586” is only used as bundle 
name. os.arch does not have “i586” value.

Mandy

Re: 8177530: Module system implementation refresh (4/2017)

[Mandy Chung]

FXLauncherTest.java should fail until FX change [1] is integrated and promoted 
(target for jdk-9+164).  I suggest to rerun the test after jdk-9+164 is 
promoted and sync down to jdk9/dev and jake.

Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8177751


> On Apr 5, 2017, at 7:35 PM, Amy Lu  wrote:
> 
> Hi, Alan
> 
> I noticed test tools/launcher/FXLauncherTest.java fails with 
> jigsaw-nightly-h6277-20170404 (on Windows platform). Is that because related 
> OpenJFX changes are not yet in the mentioned build?
> 
> Thanks,
> Amy
> 
> On 4/5/17 12:28 AM, Alan Bateman wrote:
>> As I mentioned on jigsaw-dev yesterday, we have accumulated a number of 
>> changes in the jake forest and would like to bring the changes into jdk9/dev 
>> for jdk-9+165.
>> 
>> Most of the changes in this update are the move of Module and friends from 
>> java.lang.reflect to java.lang. This is mostly a mechanical update.
>> 
>> We also have the update to the derivation of automatic modules to no longer 
>> ignore trailing digits in modules names, this is to align JDK 9 with the 
>> updated proposal for issue #VersionsInModuleNames [1].
>> 
>> There are a number of smaller changes, summarized in JDK-8177530 [2].
>> 
>> The webrev with the changes is here:
>>  http://cr.openjdk.java.net/~alanb/8177530/1
>> 
>> The changes are currently based on jdk-9+163 and will be rebased before 
>> pushing to jdk9/dev.
>> 
>> The corresponding update to jtreg is already in the code-tools/jtreg repo 
>> and will be tagged (I assume as jtreg4.2-b07) before this integration. Once 
>> it is tagged then we'll rev'ing the requiredVersion in each TEST.ROOT.
>> 
>> -Alan
>> 
>> [1] 
>> http://openjdk.java.net/projects/jigsaw/spec/issues/#VersionsInModuleNames
>> [2] https://bugs.openjdk.java.net/browse/JDK-8177530
>> 
>> 
>> 
> 

Re: RFR[9] JDK-8178840: Adds FieldSetAccessibleTest.java and VerifyJimage.java to ProblemList

[Mandy Chung]

> On Apr 17, 2017, at 12:07 AM, Alan Bateman  wrote:
> 
> On 17/04/2017 03:52, sha.ji...@oracle.com wrote:
> 
>> Hi,
>> Because of JDK-8178776, 
>> java/lang/Class/getDeclaredField/FieldSetAccessibleTest.java and 
>> tools/jimage/VerifyJimage.java should be added to ProblemList.
>> 
> Temporarily excluding these tests for a few days is okay. However, we need to 
> get attention on the real issue, we should never create a run-time image 
> containing types that can't link to their super types.

Agree.  It’s fine to problem list these tests for a few days and resolve this 
soon.

Mandy

Re: RFR 8178323: Add negative tests for bind services Jlink feature

[Mandy Chung]

> On Apr 20, 2017, at 5:12 PM, Andrey Nazarov  
> wrote:
> 
> HI Mandy,
> 
> I’ve update patch 
> http://cr.openjdk.java.net/~anazarov/8178323/webrev.03/webrev/ 
> 
Nit: use 4-space indent e.g. BindServices.java. line 159 (the following lines 
will move along) and SuggestProviders.java line 255-258, 262, 273-276,  and 280.

Otherwise looks good.

You can fix the formatting before push.

Thanks
Mandy

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

Since jdk.internal.vm.compiler becomes an upgradeable module, it is not hashed 
with java.base to allow it to be upgraded and there is no integrity check.  
Such qualified export will be granted to any module named 
jdk.internal.vm.compiler at runtime.  The goal is for upgradeable modules not 
to use any internal APIs and eliminate the qualified exports.

The main thing is that jdk.vm.ci.services API would need to be guarded if it’s 
used by non-Graal modules.

Mandy

> On Apr 19, 2017, at 11:24 AM, Christian Thalinger  
> wrote:
> 
> I lost track a bit but why are we exporting jdk.vm.ci.services to the world 
> now?
> 
> module jdk.internal.vm.ci {
> -exports jdk.vm.ci.services to jdk.internal.vm.compiler;
> +exports jdk.vm.ci.services;
> 
>> On Apr 18, 2017, at 9:16 PM, Doug Simon  wrote:
>> 
>> (adding hotspot-compiler-dev)
>> 
>> Please review these changes that make jdk.internal.vm.compiler an upgradable 
>> compiler.
>> The primary requirement for this is to remove all usage of JDK internals 
>> from Graal.
>> While this most involves changes in Graal, there remain a few capabilities 
>> that must
>> be exposed via JVMCI. Namely:
>> 
>> 1. Graal needs a copy of jdk.internal.misc.VM.savedProps so that it can 
>> Graal initialize
>> can use system properties that are guaranteed not to have been modified by 
>> application
>> code (Graal initialization is lazy and may occur after application code has 
>> been run).
>> 
>> 2. Truffle needs to be able to trigger JVMCI initialization so that it can 
>> select the Graal
>> optimized implementation of the Truffle runtime.
>> 
>> These capabilities have been added to jdk.vm.ci.services.Services. A comment 
>> has
>> also been added to this class to denote the current methods to be removed
>> in a subsequent bug to update the JDK copy of Graal with the upstream 
>> version which
>> no longer uses the methods. The methods destined for removal are:
>> 
>> exportJVMCITo(Class requestor)
>> load(Class service)
>> loadSingle(Class service, boolean required)
>> 
>> The first one is no longer needed as JVMCI exports itself to Graal service 
>> providers
>> it loads via private API and Graal re-exports[1] JVMCI to any module the 
>> extends Graal.
>> The latter 2 are no longer required as Graal uses the standard 
>> ServiceLoader. This API
>> is only useful for JVMCI-8 where a hidden JVMCI class loader is used.
>> 
>> These changes have been tested against upstream Graal. To make the Graal 
>> tests pass, 2 extra
>> minor changes were required:
>> 
>> 1. In JDK-8177673, HotSpotMemoryAccessProviderImpl::checkRead was added and 
>> throws IllegalArgumentException
>> on all error paths except one which throws AssertionError. The latter was a 
>> mistake and has been
>> fixed to also throw IllegalArgumentException.
>> 2. An invalid assertion has been removed from 
>> HotSpotResolvedObjectTypeImpl::isDefinitelyResolvedWithRespectTo.
>> Up to JDK 8, it was true that all classes in java.* packages were loaded by 
>> the boot class loader.
>> This is no longer true in JDK 9 with classes like java.sql being loaded by 
>> platform class loader.
>> 
>> As hinted above, a subsequent bug will be required to pull in the latest 
>> upstream Graal and
>> remove use of JDK internal API from the jdk.aot module. The qualified 
>> exports to
>> jdk.vm.internal.compiler and jdk.aot can then be removed.
>> 
>> -Doug
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8177845
>> http://cr.openjdk.java.net/~dnsimon/8177845/
>> 
>> [1] 
>> http://openjdk.java.net/projects/jigsaw/spec/issues/#IndirectQualifiedReflectiveAccess
>> 
>> 
>> 
> 

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

> On Apr 19, 2017, at 12:07 PM, Doug Simon <doug.si...@oracle.com> wrote:
> 
> 
>> On 19 Apr 2017, at 20:01, Mandy Chung <mandy.ch...@oracle.com> wrote:
>> 
>> We should catch this issue and detect if VM.getSavedProperty is called when 
>> init level < 1 during early startup.
> 
> What are you proposing? Some extra testing of VM startup to see if this is an 
> issue? If so, can you please suggest what testing is to be performed.

This is an existing issue, not related to your change.  I will create JBS issue 
to track it.

What you have is fine.

Mandy

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

> On Apr 19, 2017, at 11:55 AM, Christian Thalinger <cthalin...@twitter.com> 
> wrote:
> 
> 
>> On Apr 19, 2017, at 8:38 AM, Mandy Chung <mandy.ch...@oracle.com> wrote:
>> 
>> Since jdk.internal.vm.compiler becomes an upgradeable module, it is not 
>> hashed with java.base to allow it to be upgraded and there is no integrity 
>> check.  Such qualified export will be granted to any module named 
>> jdk.internal.vm.compiler at runtime.  The goal is for upgradeable modules 
>> not to use any internal APIs and eliminate the qualified exports.
>> 
>> The main thing is that jdk.vm.ci.services API would need to be guarded if 
>> it’s used by non-Graal modules.
> 
> This all makes sense but where is the restriction that only 
> jdk.internal.vm.compiler can use jdk.vm.ci.services?  

It's unqualified and no restriction in this change.

Mandy

hg: jigsaw/jake/jdk: Add @spec JPMS to ThreadInfo::from

[mandy . chung]

Changeset: 31ed261df102
Author:mchung
Date:  2017-04-20 09:00 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/31ed261df102

Add @spec JPMS to ThreadInfo::from

! src/java.management/share/classes/java/lang/management/ThreadInfo.java

Re: Review Request: 8178404: jlink --suggest-providers should list providers from observable modules

[Mandy Chung]

> On Apr 18, 2017, at 7:35 AM, Alan Bateman  wrote:
> 
> For the usage message then --suggest-providers might be simpler as "Suggest 
> service providers that implement the given service type from the module 
> path". The mention of `--add-modules` could confuse readers so I'd leave it 
> out.
> 
> In passing, --bind-services currently prints "Do full service binding" and 
> maybe we should change that to something "Automaticall link in service 
> provider modules and their dependences" or something that is clearer than the 
> current text.
> 

How about this:

diff --git 
a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties 
b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
--- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
+++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
@@ -57,12 +57,12 @@
 \if specified  
 
 main.opt.bind-services=\
-\  --bind-services   Do full service binding
+\  --bind-services   Link in service provider modules 
and\n\
+\their dependences
 
 main.opt.suggest-providers=\
-\  --suggest-providers [,...]  Suggest providers of services used 
by\n\
-\the modules that would be linked, 
or\n\
-\of the given service types
+\  --suggest-providers [,...]  Suggest providers that implement 
the\n\
+\given service types from the module 
path
 
 main.command.files=\
 \  @   Read options from file
@@ -138,7 +138,7 @@
 warn.signing=WARNING: signed modular JAR {0} is currently not supported
 warn.invalid.arg=invalid classname or pathname not exist: {0}
 warn.split.package=package {0} defined in {1} {2}
-warn.unused.services=Services specified in --suggest-providers not used: {0}
+warn.provider.notfound=No provider found for service specified to 
--suggest-providers: {0}
 no.suggested.providers=--bind-services option is specified. No additional 
providers suggested.
 suggested.providers.header=Suggested providers
 providers.header=Providers


> Minor nit in JlinkTask.java L381 where the throw is indented too much, might 
> be a tab.


Will fix before I push.

Mandy

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

> On Apr 19, 2017, at 8:03 AM, Peter Levart  wrote:
> 
> Hi Alan,
> 
> On 04/19/2017 03:41 PM, Alan Bateman wrote:
>> On 19/04/2017 08:37, Peter Levart wrote:
>> 
>>> :
>>> 
>>> Note that Properties class is thread-safe, while HashMap isn't. But I think 
>>> this should not be a problem here, as during initPhase1(), as far as I 
>>> know, no threads apart from main thread are started yet (is this true?), so 
>>> anyone accessing getSavedProperty(ies) will either be the main thread 
>>> itself or a thread started afterwards, so there is a happens-before 
>>> relationship.
>> The ReferenceHandler and Finalizer threads are started early, before 
>> initPhase1. However, both should immediately block and in the case of the 
>> Finalizer, await the completion of initPhase1 before polling.
>> 
>> -Alan
> 
> Just out of curiosity, what guarantees are there that no code before 
> VM.saveAndRemoveProperties() ever executes Integer.valueOf(int) method for 
> example? Note that this call is implicitly hidden in autoboxing of int(s)...
> 
> If this happens, then class initialization of Integer.IntegerCache may fail, 
> because it is using VM.getSavedProperty():
> 
>public static String getSavedProperty(String key) {
>if (savedProps.isEmpty())
>throw new IllegalStateException("Should be non-empty if 
> initialized");
> 
>return savedProps.getProperty(key);
>}
> 
> Hm

We should catch this issue and detect if VM.getSavedProperty is called when 
init level < 1 during early startup.

Mandy

Re: Review Request: 8179025: Exclude deployment modules from FieldSetAccessibleTest.java and VerifyJimage.java

[Mandy Chung]

Have an explicit list is another alternative.  OTOH I think only deployment 
modules will name with these words though which was what I initially want to 
cover.

Since only 4 modules we are concerned about,  I updated the patch to list the 
ones needed to be excluded rather than a superset as you suggest:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8179025/webrev.01/

thanks
Mandy

> On Apr 22, 2017, at 1:22 AM, Peter Levart <peter.lev...@gmail.com> wrote:
> 
> Hi Mandy,
> 
> In order to make the FieldSetAccessibleTest more resilient to future changes 
> (i.e. adding / renaming modules), perhaps the modules to be excluded in the 
> check should be explicitly listed by their names? Currently your rule, when 
> negated, lists the following modules:
> 
> javafx.deploy
> jdk.deploy
> jdk.javaws
> jdk.plugin.dom
> jdk.plugin
> jdk.deploy.controlpanel
> jdk.plugin.server
> 
> ...which is not to much to put in a Set.of() instance.
> 
> There's no harm if future changes forget to add/change this set, but it would 
> be wrong if the rule you have now, inadvertently excludes some future module 
> that should be checked.
> 
> Regards, Peter
> 
> 
> On 04/21/2017 10:53 PM, Mandy Chung wrote:
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8179025/webrev.00/index.html 
>> <http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8179025/webrev.00/index.html>
>> 
>> These tests failed due to IAE when loading types from the deployment 
>> modules which are expected to be defined when running with javaws
>> or plugin.  This revises the tests to exclude these modules to
>> remove the tests from the problem list.  In the long term, we 
>> should look into some way not to link in these modules in the image.
>> 
>> This patch also updates JdkQualifiedExportTest.java test to take out
>> the exception for deployment modules to have qualified exports to 
>> upgradeable modules.
>> 
>> thanks
>> Mandy
> 

Review Request: 8179025: Exclude deployment modules from FieldSetAccessibleTest.java and VerifyJimage.java

[Mandy Chung]

Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8179025/webrev.00/index.html

These tests failed due to IAE when loading types from the deployment 
modules which are expected to be defined when running with javaws
or plugin.  This revises the tests to exclude these modules to
remove the tests from the problem list.  In the long term, we 
should look into some way not to link in these modules in the image.

This patch also updates JdkQualifiedExportTest.java test to take out
the exception for deployment modules to have qualified exports to 
upgradeable modules.

thanks
Mandy

Re: RFR: 8159523. Fix tests depending on absence of -limitmods in VM arguments.

[Mandy Chung]

> On Mar 8, 2017, at 5:21 PM, Alexandre (Shura) Iline 
>  wrote:
>> Also specifying the target explicitly makes the test clearer what it does.
>> 
>> .addExports(“java.base/jdk.internal.reflect”, “ALL-UNNAMED”)
> 
> Makes sense.
> 
> I hope though that you are not against having a constant for “ALL-UNNAMED”, 
> such as Task.ALL_UNNAMED. A lot of people, myself included prefer usage of 
> constants over the copy-pasted value.

I am not against defining a constant variable (I see the benefit of doing that 
to avoid typo).  However, Task is not the right class to define ALL_UNNAMED.  
Another alternative is to introduce a new Modules class to define those strings 
which seems overkill.  A typo will be caught when the test is run and it isn’t 
too bad.

>> 
>> 136 .shouldContain(Task.OutputKind.STDERR, "IllegalAccessError");
>> 164 .shouldContain(Task.OutputKind.STDOUT, "specified more than 
>> once”);
>> 
>> It may be useful to add “stdoutShouldContain” and “stderrShouldContain” 
>> method, like what ProcessTools provides.
> 
> There will be a further discussion on this - Igor will be summarizing it in a 
> separate e-mail. What he will be suggesting is 
> .stderr().shouldContain(“IllegalAccessError”) 
> or something similar.
> 

That may be fine.

> 
> Yes for filterStandardOption(…).
> I meant  ignoreStandardOption(String, int)  as a public API. Some options are 
> supported by JavaOptions, but there could be others which are not. But I 
> agree that we can hide it for now - there may never be a case for it.

Sounds good.  Add it when we identify a need for it.

> 
>> 
>> Should we remove the use of JDKToolFinder since I assume 
>> jdk.testlibrary.task will replace jdk.testlibrary.* helper classes in the 
>> future.
> 
> Probably also make it private for now. Or remove - we can re-introduce it 
> later.

I suggest to remove it as a clean start.

Mandy

Re: RFR: 8159523. Fix tests depending on absence of -limitmods in VM arguments.

[Mandy Chung]

> On Mar 6, 2017, at 5:27 PM, Alexandre (Shura) Iline 
>  wrote:
> 
> Hi,
> 
> Could you please review the suggested fox for: 
> https://bugs.openjdk.java.net/browse/JDK-8159523
> 
> There has been a bit of discussion on jigsaw-dev, but it perhaps make sense 
> to include core-libs-dev.
> 
> There have been some fixes since the review was published, so we are now at 
> revision #4:
> http://cr.openjdk.java.net/~shurailine/8159523/webrev.04/
> 

I reviewed webrev.04 which is in a good shape.  Here are my comments:

W.r.t. the JavaTask methods:

110 new JavaTask()
111 .addExports("java.base", "jdk.internal.reflect")
112 .vmOptions("-version")
113 .run();

I prefer it to use / to represent the source in the same way 
as the command-line argument to `—-add-exports` option, as Alan suggests.  Also 
specifying the target explicitly makes the test clearer what it does.

  .addExports(“java.base/jdk.internal.reflect”, “ALL-UNNAMED”)

Similiarly, module(String mid) takes the argument passed to `—-module`.

addReads(String source, String… targets)
  I suggest to require a non-empty targets.  i.e. ALL-UNNAMED must be specified.

`—-add-modules` is a repeating option and JavaTask::addModules does not support 
that.

148 .addModules("jdk.naming.dns,jdk.compiler”)

This should be “addModules(“jdk.naming.dns”, “jdk.compiler”)”

JavaTask should be extended for the new `-—add-opens` option 

classArgs(String… args) might be better to rename to mainArgs(String… args).

modulePath, upgradeModulePath, classPath - maybe just take Path arguments.  

136 .shouldContain(Task.OutputKind.STDERR, "IllegalAccessError");
164 .shouldContain(Task.OutputKind.STDOUT, "specified more than 
once”);

It may be useful to add “stdoutShouldContain” and “stderrShouldContain” method, 
like what ProcessTools provides.

ignoreStandardOptions()
ignoreStandardModuleOptions()
 It might be confusing what “standard options” are.  They are the options 
configured by jtreg and inherited from the test run.

What about “doNotInheritTestOptions” or “doNotInheritTestRuntimeOptions”?

It seems better for JavaTask constructor to take a boolean/enum flag. Instead 
of a public JavaTask constructor, what about JavaTask.newTask(Enum) or 
JavaTask.newTaskWithInheritTestRuntimeOptions() explicit factory methods? 
Either way is fine with me.

public JavaTask ignoreStandardOption(String option, int parameterCount) {
public JavaTask filterStandardOption(Function 
filter) {
  - should these methods be private?

what if classArgs and module are both called?  The builder should detect that 
and throw IAE to help troubleshooting since the main class should either be in 
a named module or just class name.

TaskError - Alan suggests to be TaskException extends RuntimeException instead.

AbstractTask.java

System.out.println("Running " + pb.command().stream().map(s -> "\"" + s + 
"\"").collect(Collectors.joining(",")));

- it would be useful to print the command-line that I can cut-n-paste to a 
shell and rerun.

i.e. no need to wrap with double quotes except -classpath and --module-path 
etc.  Use space rather than “,” as separator.

Task.Mode.* is defined to launch via different mechanism.  For java launcher, 
it has only one way.  For jar, jlink, jmod and other tools, we can now use 
java.util.ToolProvider.  It might be useful if we had a JlinkTask in the 
future.  We should either drop Task.Mode or update its comments.

Should we remove the use of JDKToolFinder since I assume jdk.testlibrary.task 
will replace jdk.testlibrary.* helper classes in the future.

I suggest the package name be “jdk.testlibrary.task” singular rather than 
plural.

Mandy

Re: java.lang.management.RuntimeMXBean not module aware

[Mandy Chung]

> 
> On Mar 7, 2017, at 12:32 AM, Volker Simonis  wrote:
> 
> Hi,
> 
> java.lang.management.RuntimeMXBean offers methods like getClassPath(),
> getLibraryPath() and even getBootClassPath() if
> isBootClassPathSupported() returns true. While
> isBootClassPathSupported() has been changed in jdk9 to always return
> false (although the VM still supports -Xbootclasspath/a) no additional
> methods have been added to RuntimeMXBean to query the new module path.
> 
> Is this intentional? I think RuntimeMXBean should contain at least a
> new method getModulePath() wich returns the VMs module path. Looking
> at the latest spec [1] I couldn't find such an addition. Will this be
> added in the final version or are there some reasons against having
> getModulePath() in RuntimeMXBean?
> 
> Exposing further information trough RuntimeMXBean like the one
> provided by the --upgrade-module-path, --add-modules, --limit-modules,
> --list-modules could be interesting/useful as well.
> 

I file https://bugs.openjdk.java.net/browse/JDK-8176314 to track this and I 
agree that certain modular runtime information such as the module graph would 
be useful for a management tool to monitor.  As Alan mentions, 
RuntimeMXBean::getInputArguments and getSystemProperties could be used to get 
the module path, upgrade module path, etc in the time being.

Mandy  

Re: mysterious _imported.marker files

[Mandy Chung]

> On Mar 4, 2017, at 7:46 AM, Alan Bateman  wrote:
> 
> On 03/03/2017 19:34, Roman Shevchenko wrote:
> 
>> I see them via JRT filesystem provider. Mostly javafx.*, but also in a 
>> couple of jdk.* modules.
>> 
> It looks like the issue is specific to the modules imported from the FX 
> forest (this includes `jdk.packager`). The build already excludes build 
> marker files when creating the packaged modules but this doesn't keep out the 
> marker files that are created for the imported modules. I've created 
> JDK-8176172 [1] to track this.
> 

The marker files are not present in javafx-exports.zip bundle (also not under  
$BUILD_OUTPUTDIR/configure-support/import-modules/modules). Looks like the 
marker files are cretead but not filtered by the jdk buld.

Mandy

Re: JDK 9 RFR of JDK-8167525: update jdk tests to remove @compile --add-modules workaround

[Mandy Chung]

> On Mar 7, 2017, at 7:13 PM, Amy Lu  wrote:
> 
> Please review the patch to remove the @compile --add-modules workaround.
> 
> In the past, tests added --add-modules to @compile (JDK-8169231) or to @run 
> (JDK-8156579) to workaround jtreg issue CODETOOLS-7901761. CODETOOLS-7901761 
> has been fixed in 4.2/b05, workaround in tests should be reverted.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8167525
> webrev: http://cr.openjdk.java.net/~amlu/8167525/webrev.00/
> 
> Note that sun/nio/cs/OLD/TestIBMDB.java was added to ProblemList.txt in 
> JDK-8169231, indicate this (JDK-8167525) as related bug. But now this test 
> pass, so removed it from ProblemList.txt

Looks good.  Thanks for cleaning this up.

Mandy

Re: Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

> On Apr 3, 2017, at 2:39 PM, mark.reinh...@oracle.com wrote:
> 
> 2017/4/3 13:35:30 -0700, si...@cjnash.com:
>> On 03/04/2017 21:15, mark.reinh...@oracle.com wrote:
>>> 2017/4/3 11:41:03 -0700, mandy.ch...@oracle.com:
 Webrev:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8175819/webrev.00/
 
 ...
 
 This shows the old and new value of OS_NAME/OS_ARCH properties
 in the `release` file:
 
   JDK 8   JDK 9
   -   -
 OS_NAME Linux   linux
   SunOS   solaris
   Darwin  osx
   Windows windows
 
 OS_ARCH i386,x86x86
   i586,amd64,x86_64   x64
   sparcv9 sparcv9
   arm arm32
   aarch64 arm64
>> 
>> I am not sure why we would change to osx for Mac when the Mac developers
>> have recently dropped the Mac OS X terminology and changed it to macOS.
> 
> Agreed -- we should change OS_NAME from "Darwin" to "macos”.

OK.  Should the bundle names be updated to reflect this change?
In any case, it is a separate issue.

   JDK 8   JDK 9
   -   -
OS_NAMELinux   linux
   SunOS   solaris
   Darwin  macos
   Windows windows

OS_ARCHi386,x86x86
   i586,amd64,x86_64   amd64
   sparcv9 sparcv9
   arm arm32
   aarch64 arm64

Mandy

Re: RFR 8178323: Add negative tests for bind services Jlink feature

[Mandy Chung]

> On Apr 7, 2017, at 8:40 AM, Andrey Nazarov  
> wrote:
> 
> Hi, 
> 
> Please review 3 negative tests for Jlink which tests new 
> bind-services/suggest-providers feature.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8178323
> 
> webrev: http://cr.openjdk.java.net/~anazarov/8178323/webrev.00/webrev/ 

Thanks for adding these tests.

BindServices.java

 155 Path dir = Paths.get("verboseNoop”);

I suggest to rename “verboseNoop” to “verboseNoBind”

line 159-161: formatting nit: can you add spaces to align with the first
argument in line 158.  Same comment to SuggestProviders.java line 180-182
an 198-200.

SuggestProviders.java
  It may be better to rename “suggestNotProvider" to “noSuggestedProvider".

  In the noOneUsesProvider test case, is m4 not observable? I think 
jlink should fail with m4 not found. When —-suggest-providers is 
specified with a service type, it’s a bug in the implementation that 
does not report it.  This should be renamed to “nonObservableModule”
instead.  We should file a bug and include this test case in the JBS report.

Mandy

Review Request JDK-8177844: Ensure non-upgradeable modules cannot be upgraded even with --patch-module

[Mandy Chung]

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8177844/webrev.01/

This fixes -—patch-module to do hash checking on the module being patched
so that it will ensure that a non-upgradeable module remains not upgradeable.

As Graal has been using —-patch-module option to disable the hash checking
to load a different version of jdk.internal.vm.compiler, it needs a 
mechanism to load Graal, as tracked by JDK-8177845.

Mandy

Re: Review Request JDK-8175819: OS name and arch in JMOD files should match the values as in the bundle name

[Mandy Chung]

> On Apr 6, 2017, at 1:09 AM, Magnus Ihse Bursie 
>  wrote:
> 
> 
> Having though this over real hard, I'd realized I need to make a plea for 
> sanity and consistency. I thought I should lay low in this discussion, but I 
> can't. Choosing "amd64" as the name for the 64-bit x86 platform is really, 
> really unfortunate and a step backwards in our effort to standardize the name 
> of this platform.
> 

I think it may be useful to see the value of `os.arch` system property on all 
platforms
at a glance.

32-bit  64-bit

linux   i386amd64   
arm aarch64
solaris amd64
sparcv9
windows x86 amd64
macos   x86_64

It's late in JDK 9.  The best is to revisit OS and architecture names in JDK 10 
on the
consistency and simplicity issue.  In the context of JDK-8175819, these are 
names to 
pass to jmod —-os-name —-os-arch options.  The values are used at resolution 
time
to avoid linking modules for different target platforms into the same image. 

As Mark suggests, we use the value of `os.arch` system property for JMOD file 
in 
JDK 9 and we can change the value in JDK 10.  No change to system property and
bundle names, as we said previously.

We should revisit OS and architecture names in JDK 10.  I have updated 
JDK-8178016 to reflect that.  Are you okay with that?

Below shows the old and new values in the `release` file and the new values
are used in JMOD files.

  JDK 8   JDK 9
  -   -
OS_NAME   Linux   linux
  SunOS   solaris
  Darwin  macos
  Windows windows

OS_ARCH   i386,x86x86
  i586,amd64,x86_64   amd64
  sparcv9 sparcv9
  arm arm
  aarch64 aarch64

Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8175819/webrev.01/index.html

Mandy

Review Request: JDK-8178286 Missing @moduleGraph in javadoc

[Mandy Chung]

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8178286/webrev.00/index.html

This adds @moduleGraph in a few more modules that will be included
in unified docs bundle.

Mandy

Re: Review Request JDK-8177844: Ensure non-upgradeable modules cannot be upgraded even with --patch-module

[Mandy Chung]

> On Apr 8, 2017, at 12:21 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
> 
> On 08/04/2017 06:50, Mandy Chung wrote:
> 
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8177844/webrev.01/
>> 
>> This fixes -—patch-module to do hash checking on the module being patched
>> so that it will ensure that a non-upgradeable module remains not upgradeable.
>> 
>> 
> The comment at L523 needs updating too, otherwise looks good.
> 

will take that out.

> Given that this can't be pushed to jdk9/dev until the issue deploying Graal 
> is fixed then I think we should just push this fix to jake and move on. It 
> will go into jdk9/dev with the next update (which I hope will be the last 
> update).

I was thinking to push this to jdk-9+166 but pushing it to jake will probably 
be a better option for Doug.

Mandy

Re: Review Request JDK-8177844: Ensure non-upgradeable modules cannot be upgraded even with --patch-module

[Mandy Chung]

No change in that regards.  The updated test [1] in the webrev shows it patches 
java.base/jdk.internal.modules.SystemModules

Before this fix, you can do —patch-module jdk.internal.vm.compiler=.jar 
—upgrade-module-path graal.jar. The reason is that —patch-module disables the 
hash checking that makes non-upgradeable module upgradeable.  After this fix, 
it fails if you attempt upgrade a patched non-upgradeable module.

[1] 
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8177844/webrev.01/test/tools/launcher/modules/patch/systemmodules/PatchSystemModules.java.frames.html

> On Apr 8, 2017, at 7:22 AM, Remi Forax <fo...@univ-mlv.fr> wrote:
> 
> Hi Mandy,
> how can I test a change in java.lang after that patch ?
> 
> Rémi
> 
> 
> On April 8, 2017 7:50:50 AM GMT+02:00, Mandy Chung <mandy.ch...@oracle.com> 
> wrote:
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8177844/webrev.01 
> <http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8177844/webrev.01>/
> 
> This fixes -—patch-module to do hash checking on the module being patched
> so that it will ensure that a non-upgradeable module remains not upgradeable.
> 
> As Graal has been using —-patch-module option to disable the hash checking
> to load a different version of jdk.internal.vm.compiler, it needs a 
> mechanism to load Graal, as tracked by JDK-8177845.
> 
> Mandy
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

hg: jigsaw/jake/jdk: 8177844: Ensure non-upgradeable modules cannot be upgraded even with --patch-module

[mandy . chung]

Changeset: b2160414fe8c
Author:mchung
Date:  2017-04-08 10:21 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/b2160414fe8c

8177844: Ensure non-upgradeable modules cannot be upgraded even with 
--patch-module
Reviewed-by: alanb

! src/java.base/share/classes/java/lang/module/Resolver.java
! test/tools/launcher/modules/patch/systemmodules/PatchSystemModules.java

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

> On Apr 18, 2017, at 3:13 PM, Doug Simon  wrote:
> 
> Please review these changes that make jdk.internal.vm.compiler an upgradable 
> compiler.
> :
> http://cr.openjdk.java.net/~dnsimon/8177845/

Thanks for making this change.  This would simplify the way to replace JDK’s 
graal with the lab graal.  A couple of comments:

jdk/internal/misc/VM.java

 161  * Note that the saved system properties do not include
 162  * the ones set by sun.misc.Version.init().

sun.misc.Version is no longer present in JDK 9.  Renamed to 
java.lang.VersionProps

jdk/vm/ci/services/Services.java
  67 Class.forName("jdk.vm.ci.runtime.JVMCI”);

JVMCI class is local in jdk.internal.vm.ci module.  An alternative may be
to provide a static initialize method rather than Class::forName.

jdk/vm/ci/hotspot/HotSpotJVMCIRuntime.java
 211 for (HotSpotJVMCIBackendFactory factory : 
ServiceLoader.load(HotSpotJVMCIBackendFactory.class)) {

This uses the thread context class loader to load providers.  Services::load 
uses 
the system class loader to load providers.  I suspect you want this to use the 
system class loader here too.

jdk/vm/ci/services/JVMCIServiceLocator.java
  78 for (JVMCIServiceLocator access : 
ServiceLoader.load(JVMCIServiceLocator.class)) {

Same comment as above. I think you want to use system class loader to load 
providers.

Mandy

Re: Allow ModuleInfoExtender to be used externally

[mandy chung]

On 8/9/17 7:39 AM, Oliver Siegmar wrote:

Hello,

I have to create a Java 9 modularized jar file programmatically.

I noticed, that the compiled module-info.class file gets modified (main-class, 
module-version and modules-hash) by the jar command line tool (using 
jdk.internal.module.ModuleInfoExtender).

Unfortunately, neither the ModuleInfoExtender itself nor the used asm-lib is 
exported to allow external use. Is it planned to change this in the final 
release of Java 9?


ModuleInfoExtender is JDK internal API and not planned to be 
exported/opened.  New version of ASM supports module-info.class. You can 
probably try out ASM 6 alpha version for now.


I think, it is a bit scary, that the binary version of module-info.class gets 
manipulated to create a jar file. Having to re-implement this functionality in 
3rd party tools (like Ant, Maven, Gradle, etc.) doesn’t make it better ;-)


To be clear, the jar tool does not change Module attribute but it may 
add optional attributes.   I don't see how it can be scary to you.


Mandy

hg: jigsaw/jake/jdk: Replace jmod --os-name/--os-arch option with --target-platform

[mandy . chung]

Changeset: c5621b8c5c91
Author:mchung
Date:  2017-04-26 19:28 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/c5621b8c5c91

Replace jmod --os-name/--os-arch option with --target-platform

! src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Platform.java
! 
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java
! src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java
! src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties
! test/tools/jlink/IntegrationTest.java
! test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java

hg: jigsaw/jake: Replace jmod --os-name/--os-arch option with --target-platform

[mandy . chung]

Changeset: 7e790234d48f
Author:mchung
Date:  2017-04-26 19:28 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/rev/7e790234d48f

Replace jmod --os-name/--os-arch option with --target-platform

! common/autoconf/generated-configure.sh
! common/autoconf/platform.m4
! common/autoconf/spec.gmk.in
! make/CreateJmods.gmk

Re: RFR: 8179434: test/java/lang/Class/getDeclaredField/FieldSetAccessibleTest.java fails due to JDK-8177845

[Mandy Chung]

Looks good.

Mandy

> On Apr 28, 2017, at 12:05 PM, Doug Simon  wrote:
> 
> Please review this small fix for a regression introduced by the change for 
> https://bugs.openjdk.java.net/browse/JDK-8177845.
> 
> http://cr.openjdk.java.net/~dnsimon/8179434/
> https://bugs.openjdk.java.net/browse/JDK-8179434
> 
> -Doug

Re: Review Request: 8179025: Exclude deployment modules from FieldSetAccessibleTest.java and VerifyJimage.java

[Mandy Chung]

> On Apr 23, 2017, at 4:09 AM, Alan Bateman <alan.bate...@oracle.com> wrote:
> 
> On 22/04/2017 16:42, Mandy Chung wrote:
> 
>> Have an explicit list is another alternative.  OTOH I think only deployment 
>> modules will name with these words though which was what I initially want to 
>> cover.
>> 
>> Since only 4 modules we are concerned about,  I updated the patch to list 
>> the ones needed to be excluded rather than a superset as you suggest:
>> 
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8179025/webrev.01/
>> 
>> 
> The updated filter looks okay to me.
> 
> In JdkQualifiedExportTest.accept then "cf.findModule(target).orElse(null) == 
> null" looks a bit odd (I assume this is what promoted Peter bringing up 
> isEmpty on core-libs-dev.  There are a dozen ways you could do this of 
> course, only alternative is:
>return cf.findModule(target).map(m -> false).orElse(true);

Indeed looks a little odd.  I prefer using isPresent since this method intends 
to return true if the target is not in the resolved graph.

   return !cf.findModule(target).isPresent();

Mandy

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

> On Apr 27, 2017, at 2:30 PM, Doug Simon <doug.si...@oracle.com> wrote:
> 
> 
>> On 27 Apr 2017, at 23:24, Mandy Chung <mandy.ch...@oracle.com> wrote:
>> 
>> 
>>> On Apr 27, 2017, at 7:47 AM, Doug Simon <doug.si...@oracle.com> wrote:
>>> 
>>> [1] http://cr.openjdk.java.net/~dnsimon/8177845.02
>> 
>> I reviewed the top repo and jdk repo change. For hotspot change,
>> I reviewed jdk.vm.ci.services/* and jdk.internal.vm.ci module-info.java.
>> 
>> make/make/CompileJavaModules.gmk
>> jdk/make/launcher/Launcher-jdk.aot.gmk
>> Reading the original jdk.internal.vm.ci module-info.java [1],
>> jdk.aot only accesses a small set of packages.  e.g. it does not
>> access jdk.vm.ci.aarch64, jdk.vm.ci.code.stack, jdk.vm.ci.common, etc.
> 
> I know but I agree with Vladimir that there's no harm in exporting everything 
> to jdk.aot to simplify continued development of this tool as support for 
> other platforms is added. As I learnt putting this patch together, missing 
> these exports when they're actually needed leads to very confusing compiler 
> error messages.

Indeed the compiler error message is very confusing and ought to be improved 
(JDK-8179359).

I have no issue to export these packages to jdk.aot, as you agree to do.

Mandy

Re: RFR: 8177845: Need a mechanism to load Graal

[Mandy Chung]

> On Apr 27, 2017, at 7:47 AM, Doug Simon  wrote:
> 
> [1] http://cr.openjdk.java.net/~dnsimon/8177845.02

I reviewed the top repo and jdk repo change. For hotspot change,
I reviewed jdk.vm.ci.services/* and jdk.internal.vm.ci module-info.java.

make/make/CompileJavaModules.gmk
jdk/make/launcher/Launcher-jdk.aot.gmk
  Reading the original jdk.internal.vm.ci module-info.java [1],
  jdk.aot only accesses a small set of packages.  e.g. it does not
  access jdk.vm.ci.aarch64, jdk.vm.ci.code.stack, jdk.vm.ci.common, etc.

 115 jdk.internal.vm.compiler \

This should be removed from PLATFORM_MODULES list.  $PLATFORM_MODULES 
already includes $UPGRADEABLE_MODULES.

Otherwise, looks good to me.

There are jdk tests verifying upgradeable modules and qualified exports.
I suggest to run through JPRT -testset core options to catch any test failure.

Mandy
[1] 
http://cr.openjdk.java.net/~dnsimon/8177845.02/hotspot/src/jdk.internal.vm.ci/share/classes/module-info.java.sdiff.html

 

Re: 8185853: Generate readability graph at link time and other startup improvements

[mandy chung]

On 8/4/17 8:48 AM, Alan Bateman wrote:
This is a patch for jdk10/jdk10 to claw back some of the regression to 
startup performance in JDK 9 for very short lived applications.


The bulk of the changes are to the "system modules" jlink plugin and 
the related code in the module system initialization. Specifically, 
the plugin now generates code to create the readability graph for 
initial modules known at link time This allows resolution to be 
skipped at startup for common cases, it also avoids needing to 
reconstitute the module descriptors for modules that will not be 
resolved (the EE modules when the initial module is the class path for 
example). The system module finder has been significantly changed as 
part of this but all the execution scenarios (images, exploded builds, 
images patched with exploded build, etc.) that are needed in 
differetnt JDK development scenarios work as before.


The patch also includes a few misc. changes to reduce the number of 
class loaded during startup and also some local tuning. There is also 
a change to the class path initialization to re-fix JDK-6760902 so 
that lookup of resources on the boot loader append path 
(-Xbootclasspath/a) is aligned with class loading in the VM in the way 
that it skips empty path elements. This follows the restoration of 
tests that Mandy pushed a few days ago with JDK-8185541.


The webrev with the changes is here:
  http://cr.openjdk.java.net/~alanb/8185853/webrev/index.html


It is good to see more optimization be done at link time that improves 
the startup.


jdk/internal/loader/ClassLoaders.java

79 if (cp.length() == 0) cp = null;

Our launcher and hotspot VM always set "java.class.path" system property 
and so it'll be non-null.  It might help the readers to check if (cp != 
null && cp.length() == 0).


jdk/internal/module/SystemModulesMap.java

128 Constructor ctor = Class.forName(cn).getConstructor();

Class.forName(Module, String) will only search the specified module.  No 
performance difference since the class exists in java.base but it may 
make it clear to the readers.


jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
   This generates one SystemModules for each module that has a main 
class.  A user may create a launcher at link time but specifies a main 
class which may be different to the one in the module descriptor.   A 
possible future enhancement is to generate a SystemModules for the main 
class if the launcher is created by jlink.


Otherwise, this patch looks good.

Mandy

Re: """error: module testng reads package test from both test and testng"""

[mandy chung]

The error message is clear because testng is not found from the module path.

Error occurred during initialization of boot layer
java.lang.module.FindException: Module testng not found, required by test

If you edit the rerun command from jtr file to add JTwork/modules to 
module path as follows:


--module-path /home/martin/ws/jdk10/jdk/
test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d/modules:/home/martin/ws/jdk10/jdk/ 


test/JTwork/modules

If it runs successfully, then the question is why jtreg includes 
JTwork/modules in javac --module-path but not at run time in your local 
run but my local jtreg works fine.  Jon may have jtreg debugging tip.


Mandy

On 8/22/17 6:18 PM, Martin Buchholz wrote:
I also tried upgrading jcommander.jar to 1.72 from 1.48, but get the 
same error.

Debugging hints?


On Tue, Aug 22, 2017 at 4:28 PM, mandy chung <mandy.ch...@oracle.com 
<mailto:mandy.ch...@oracle.com>> wrote:



On 8/22/17 3:52 PM, Martin Buchholz wrote:

:


ACTION: testng -- Failed. Unexpected exit from test [exit code: 1]
REASON: User specified action: run testng/othervm test/test.Main
TIME:   0.215 seconds
messages:
command: testng test/test.Main
reason: User specified action: run testng/othervm test/test.Main
Mode: othervm [/othervm specified]
elapsed time (seconds): 0.215
configuration:
Boot Layer
  add modules: test m4 m2 m3 m1
  module path:

/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d/modules

STDOUT:
Error occurred during initialization of boot layer
java.lang.module.FindException: Module testng not found, required
by test
STDERR:
rerun:
DISPLAY=localhost:10.0 \
HOME=/home/martin \
LANG=en_US.UTF-8 \
PATH=/bin:/usr/bin \

CLASSPATH=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d:/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX:/home/martin/jtreg-binaries/4.2-b08/lib/testng.jar:/home/martin/jtreg-binaries/4.2-b08/lib/jcommander.jar:/home/martin/jtreg-binaries/4.2-b08/lib/javatest.jar:/home/martin/jtreg-binaries/4.2-b08/lib/jtreg.jar
\

/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk/bin/java
\

-Dtest.class.path.prefix=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d:/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX
\
-Dtest.src=/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX
\
-Dtest.src.path=/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX
\

-Dtest.classes=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d
\

-Dtest.class.path=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d
\
-Dtest.vm.opts=-enablesystemassertions \
-Dtest.tool.vm.opts=-J-enablesystemassertions \
-Dtest.compiler.opts= \
-Dtest.java.opts= \

-Dtest.jdk=/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk
\

-Dcompile.jdk=/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk
\
-Dtest.timeout.factor=1.0 \
        --module-path

/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d/modules
\


> /home/martin/ws/jdk10/jdk/test/JTwork/modules is not in the
module path.  Therefore testng is not found.

I can't reproduce this error.  I downloaded testng 6.9.9 and it
works for me.  This is the jtreg version I used.

$ jtreg -version
jtreg, version 4.2 fcs b08
Installed in /java/devtools/jtreg/jtreg-4.2/lib/jtreg.jar
Running on platform version 9 from /java/re/jdk-9.jdk/Contents/Home.
Built with Java(TM) 2 SDK, Version 1.7.0-b147 on July 21, 2017.
Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights
reserved.
Use is subject to license terms.
JCov 2.0-b18 beta
TestNG (testng.jar): version 6.9.9
TestNG (jcommander.jar): version 1.72


Mandy

        --add-modules test,m4,m2,m3,m1 \
-enablesystemassertions \
com.sun.javatest.regtest.agent.MainWrapper

/home/martin/ws/jdk10/jdk/test/JTwork/java/lang/ModuleTests/addXXX/Driver.d/testng.0.jta
java/lang/ModuleTests/addXXX/Driver.java false test/test.Main

TEST RESULT: Failed. Unexpected exit from test [exit code: 1]

Re: trySetAccessible​

[mandy chung]

As described in the javadoc [1], Module::isOpen(String pn) returns true 
if this module has opened a package unconditionally. It returns true 
only if the module declares "opens pn;" unconditionally in 
module-info.java.  Since java.base does not open "java.io" 
unconditionally, so isOpen("java.io”) returns false.


Mandy
[1] 
http://download.java.net/java/jdk9/docs/api/java/lang/Module.html#isOpen-java.lang.String-


On 8/15/17 6:09 PM, Russell Gold wrote:

I am clearly missing something, here. Using build 180, I have variables:

Method method, Class callingClass.

I evaluate:

callingClass.getModule() -
-> java.lang.Module@2940 “unnamed module @f973499”
method.getDeclaringClass()
-> java.lang.Class “class java.io.ObjectInputStream”
method.getDeclaringClass().getModule()
-> java.lang.Module @2949 “module java.base"
method.getDeclaringClass().getModule().isOpen("java.io”)
-> false // which I read as saying that package “java.io” in 
module java.base is NOT open to the unnamed module

but:

method.getDeclaringClass().getModule().isOpen("java.io", 
callingClass.getModule())
-> true  // which seems to say that it IS open to that 
particular unnamed module ?!

of course, method.setAccessible(true) gives me the warning.

What am I missing?

Thanks,
Russ


On Jul 11, 2017, at 6:11 AM, Alan Bateman  wrote:

On 11/07/2017 10:16, Uwe Schindler wrote:

:
Sorry, I mixed up the parameters. So basically the "correct" code to check if 
something like java.lang.String is open to Groovy would be:

Module groovyModule = CachedClass.class.getModule(); // 
org.codehaus.groovy.reflection.CachedClass;
Class clazz = String.class; // to test if open
return clazz.getModule().isOpen(String.class.getPackage().getName(), 
groovyModule);

If I do this, it returns "true" for String, Object or any other class. So the 
behaviour is consistent.

Yes, it has to be consistent.

As an aside, you can replace getPackage().getName() with getPackageName() - 
it's more efficient and avoids the NPE when the class is an array or primitive.



I implemented this check as a single MethodHandle to a precompiled method that takes 
a Class and returns true/false if the Class is open to Groovy's module: 
https://goo.gl/XvdCQK
By this it's possible to execute this without a Java 9 at compile time. 
Unfortunately, it doesn't help, because java.base is open to unnamed module

Right, although isOpen("java.lang") will return false because the package is 
not open to all modules.



But now it is impossible for us to check, if something is not open by default.

Module::getDescriptor will return the module descriptor for named modules. So 
in this case, Object.class.getModule().getDescriptor() returns the 
ModuleDescriptor for java.base so you can inspect the packages and which are 
exported and/or opened before being changed by runtime. So there is enough in 
the API to determine which packages have been opened at runtime (either via CLI 
options or APIs).

-Alan

Re: """error: module testng reads package test from both test and testng"""

[mandy chung]

Hi Martin,

"error: module testng reads package test from both test and testng" 
indicates that a split package is detected.   Does your testng.jar 
contain classes of package "test"?   I downloaded testng from maven 
which does not have the package "test".


The testng.jar version I had bundled with jcommander classes and hence 
the error message reports com.beust.jcommander.* packages from 
jcommander and testng module.  It was an old version before testng was 
mavenized.   If I run jtreg with the testng and jcommander downloaded 
from maven, the test passes.


You can run java -p JTwork/modules --validate-modules to check if any 
split packages or other conflict.


Jon, Jan,
  I think the javac error message could be improved to make it clear 
that the package is split in multiple module.   In addition, the test 
only contains explicit modules.   It's strange that the javac reports 
that "the unnamed module reads package xxx from ".  I will create a 
JBS issue on improving the error message.


Mandy

On 8/21/17 4:06 PM, mandy chung wrote:
I can reproduce it and javac outputs the following errors.  javac 
compiles successfully if jcommander.jar is removed.   Will look into it.


error: the unnamed module reads package com.beust.jcommander from both 
jcommander and testng
error: the unnamed module reads package 
com.beust.jcommander.validators from both jcommander and testng
error: the unnamed module reads package com.beust.jcommander.internal 
from both jcommander and testng
error: the unnamed module reads package 
com.beust.jcommander.defaultprovider from both jcommander and testng
error: the unnamed module reads package 
com.beust.jcommander.converters from both jcommander and testng
error: module jcommander reads package com.beust.jcommander from both 
testng and jcommander
error: module jcommander reads package com.beust.jcommander.validators 
from both testng and jcommander
error: module jcommander reads package com.beust.jcommander.internal 
from both testng and jcommander
error: module jcommander reads package 
com.beust.jcommander.defaultprovider from both testng and jcommander
error: module jcommander reads package com.beust.jcommander.converters 
from both testng and jcommander
error: module testng reads package com.beust.jcommander from both 
jcommander and testng
error: module testng reads package com.beust.jcommander.validators 
from both jcommander and testng
error: module testng reads package com.beust.jcommander.internal from 
both jcommander and testng
error: module testng reads package 
com.beust.jcommander.defaultprovider from both jcommander and testng
error: module testng reads package com.beust.jcommander.converters 
from both jcommander and testng
/scratch/mchung/ws/jdk10/jdk10-dev/jdk/test/java/lang/ModuleTests/addXXX/test/module-info.java:23: 
error: module test reads package com.beust.jcommander from both testng 
and jcommander

module test {

Mandy

On 8/21/17 2:14 PM, Martin Buchholz wrote:
I just upgraded to 4.2-b08, although without any effect on this 
problem.  jcommander.jar is from jtreg's lib directory


code-tools/jtreg/make/Defs.gmk says:
"""
# TestNG requires jcommander, which may or may not be bundled with 
TESTNG_JAR.

# If it is not, set JCOMMANDER_JAR to an appropriate version
"""

 $ ls -l ./JTwork/modules
total 1620
-rw-r--r-- 1 martin martin   63504 Aug 21 12:27 jcommander.jar
-rw-r--r-- 1 martin martin 1589287 Aug 21 12:27 testng.jar

 $ /home/martin/jtreg-binaries/current/bin/jtreg -noreport -v:fail 
-compilejdk:/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk 
-testjdk:/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk 
java/lang/ModuleTests/addXXX

--
TEST: java/lang/ModuleTests/addXXX/Driver.java
TEST JDK: 
/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk


ACTION: build -- Failed. Compilation failed: Compilation failed
REASON: User specified action: run build test/* m1/* m2/* m3/* m4/*
TIME:   0.943 seconds
messages:
command: build test/* m1/* m2/* m3/* m4/*
reason: User specified action: run build test/* m1/* m2/* m3/* m4/*
Test directory:
  compile: test/module-info, test/test.C, test/test.Service, 
test/test.Main, m1/module-info, m1/p1.C, m2/module-info, m2/p2.C, 
m2/p2.internal.C, m3/module-info, m3/p3.C, m4/module-info, m4/p4.C

elapsed time (seconds): 0.943

ACTION: compile -- Failed. Compilation failed: Compilation failed
REASON: .class file out of date or does not exist
TIME:   0.938 seconds
messages:
command: compile 
/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX/test/module-info.java 
/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX/test/test/C.java 
/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX/test/test/Service.java 
/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX/test/test/Main.java 
/home/mar

Re: """error: module testng reads package test from both test and testng"""

[mandy chung]

On 8/22/17 3:52 PM, Martin Buchholz wrote:

:

ACTION: testng -- Failed. Unexpected exit from test [exit code: 1]
REASON: User specified action: run testng/othervm test/test.Main
TIME:   0.215 seconds
messages:
command: testng test/test.Main
reason: User specified action: run testng/othervm test/test.Main
Mode: othervm [/othervm specified]
elapsed time (seconds): 0.215
configuration:
Boot Layer
  add modules: test m4 m2 m3 m1
  module path: 
/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d/modules


STDOUT:
Error occurred during initialization of boot layer
java.lang.module.FindException: Module testng not found, required by test
STDERR:
rerun:
DISPLAY=localhost:10.0 \
HOME=/home/martin \
LANG=en_US.UTF-8 \
PATH=/bin:/usr/bin \
CLASSPATH=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d:/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX:/home/martin/jtreg-binaries/4.2-b08/lib/testng.jar:/home/martin/jtreg-binaries/4.2-b08/lib/jcommander.jar:/home/martin/jtreg-binaries/4.2-b08/lib/javatest.jar:/home/martin/jtreg-binaries/4.2-b08/lib/jtreg.jar 
\
/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk/bin/java 
\
-Dtest.class.path.prefix=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d:/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX 
\

-Dtest.src=/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX \
-Dtest.src.path=/home/martin/ws/jdk10/jdk/test/java/lang/ModuleTests/addXXX 
\
-Dtest.classes=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d 
\
-Dtest.class.path=/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d 
\

-Dtest.vm.opts=-enablesystemassertions \
-Dtest.tool.vm.opts=-J-enablesystemassertions \
        -Dtest.compiler.opts= \
        -Dtest.java.opts= \
-Dtest.jdk=/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk 
\
-Dcompile.jdk=/home/martin/ws/jdk10/build/linux-x86_64-normal-server-release/images/jdk 
\

        -Dtest.timeout.factor=1.0 \
        --module-path 
/home/martin/ws/jdk10/jdk/test/JTwork/classes/java/lang/ModuleTests/addXXX/Driver.d/modules 
\


> /home/martin/ws/jdk10/jdk/test/JTwork/modules is not in the module 
path.  Therefore testng is not found.


I can't reproduce this error.  I downloaded testng 6.9.9 and it works 
for me.  This is the jtreg version I used.


$ jtreg -version
jtreg, version 4.2 fcs b08
Installed in /java/devtools/jtreg/jtreg-4.2/lib/jtreg.jar
Running on platform version 9 from /java/re/jdk-9.jdk/Contents/Home.
Built with Java(TM) 2 SDK, Version 1.7.0-b147 on July 21, 2017.
Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
Use is subject to license terms.
JCov 2.0-b18 beta
TestNG (testng.jar): version 6.9.9
TestNG (jcommander.jar): version 1.72


Mandy

        --add-modules test,m4,m2,m3,m1 \
        -enablesystemassertions \
com.sun.javatest.regtest.agent.MainWrapper 
/home/martin/ws/jdk10/jdk/test/JTwork/java/lang/ModuleTests/addXXX/Driver.d/testng.0.jta 
java/lang/ModuleTests/addXXX/Driver.java false test/test.Main


TEST RESULT: Failed. Unexpected exit from test [exit code: 1]

Re: """error: module testng reads package test from both test and testng"""

[mandy chung]

On 8/22/17 2:55 PM, Martin Buchholz wrote:



On Tue, Aug 22, 2017 at 12:38 PM, mandy chung <mandy.ch...@oracle.com 
<mailto:mandy.ch...@oracle.com>> wrote:


Does your testng.jar contain classes of package "test"?   I
downloaded testng from maven which does not have the package "test".


Sorry, I missed that question.  Yes, for historical reasons we are 
using testng 6.9.8, and I see:


  $ ( wget -qOtestng.jar 
https://search.maven.org/remotecontent?filepath=org/testng/testng/6.9.8/testng-6.9.8.jar 
&& jar tf testng.jar | grep '^test/[^/]*.class$' | tail; rm testng.jar)

test/ReturnValueTest.class
test/SampleInheritance.class
test/SerializationTest.class
test/SimpleBaseTest.class
test/StaticTest$InnerStaticClass.class
test/StaticTest.class
test/Test1.class
test/Test2.class
test/TestHelper.class
test/TestListener.class

Including test/ is probably a packaging mistake.

Probably.  test package is not in 6.9.5 and 6.11 which I checked.

Mandy

hg: jigsaw/jake/jdk: Add @apiNote in ClassLoader::getPackage(s) about platform class loader

[mandy . chung]

Changeset: fa1e20c4e6c1
Author:mchung
Date:  2017-05-02 17:20 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/fa1e20c4e6c1

Add @apiNote in ClassLoader::getPackage(s) about platform class loader

! src/java.base/share/classes/java/lang/ClassLoader.java

Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

[Mandy Chung]

Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8020801/webrev.00/

The big hammer check disallowing MethodHandles::lookup be called by system
classes defined by the bootstrap class loader was added as defense-in-depth
to prevent this caller-sensitive method being called from JDK internal classes
via Method::invoke.  It was intended as a point fix and to be replaced
with a long-term approach.  Lookup.privateLookupIn() returns a Lookup object
and IAE is thrown if the lookup class is almost all java.* and sun.* [1].
We should fix this in JDK 9.

This patch replaces this restriction and now allow MethodHandles::lookup to
be called statically by any code.  But disallow Method::invoke of 
MethodHandles.lookup from system classes defined by the bootstrap class loader
e.g. java.base.  It is expected that no reflective call to
MethodHandles::lookup is made by the system classes and so this approach
would provide a better mechanism as a defense-in-depth.

Mandy
[1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012267.html

Re: 8178380: Module system implementation refresh (5/2017 update)

[Mandy Chung]

> On May 1, 2017, at 1:28 PM, Alan Bateman  wrote:
> 
> The webrevs with the changes is here:
>http://cr.openjdk.java.net/~alanb/8178380/1/

I have reviewed all repos.  Mostly looks good.  This is the first round of 
comments.  I will continue the review and plan to play a little more with the 
new launcher options.

src/share/vm/services/attachListener.cpp

 355 // Dynamic loading agents is not default by default

typo: s/not default/not enabled?

src/share/vm/services/diagnosticCommand.cpp
 771 loadAgentModule(CHECK);
 772 Handle loader = Handle(THREAD, SystemDictionary::java_system_loader());
 773 Klass* k = 
SystemDictionary::resolve_or_fail(vmSymbols::jdk_internal_agent_Agent(), 
loader, Handle(), true, CHECK);
 774 instanceKlassHandle ik (THREAD, k);

It’s better to refactor this to loadAgentClass that returns Klass or handle?

src/java.base/share/classes/java/lang/ClassLoader.java

 134  * platform class loader may delegate to application class loader.

“the” application class loader

 135  * In other words, classes in modules defined to the application class
 136  * loader may be visible to the platform class loader. 

It would help if this statement makes it clear that the classes in modules 
defined to the application class loader (only that are required by the 
upgradeable modules) are visible to the platform class loader.

It would also be good to mention that ClassLoader::getPackages will not 
return the packages defined to the application class loader since
application class loader is not its ancestor.

 377  * @apiNote If the parent is specified as {@code null} (for the
 378  * bootstrap class loader) then there is no guarantee that all platform
 379  * classes are visible.

What about: 

If the parent is specified as {@code null} (for the bootstrap class loader)
then there is no guarantee that all platform classes are visible.
Platform classes defined by the platform class loader and its ancestors
except bootstrap class loader are not visible to this class loader.

src/java.base/share/classes/jdk/internal/loader/BuiltinClassLoader.java

 174 /**
 175  * Register a module this this class loader. This has the effect of 
making
 176  * the types in the module visible.
 177  */
 178 public void loadModule(ModuleReference mref) {

Typo in line 175 “this this class loader”.

src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java

147 // special mode to boot with only java.base, ignores other options
 148 if (System.getProperty("jdk.module.minimumBoot") != null) {
 149 return createMinimalBootLayer();
 150 }

src/java.base/share/classes/jdk/internal/module/ModulePatcher.java
 123 try (JarFile jf = new JarFile(file.toString())) {
- what is the bug being fixed by this line?

src/java.base/share/classes/jdk/internal/module/ModulePath.java
 536 if (packages.contains(pn)) 
builder.mainClass(mainClass);
Nit: it would be consistent if this is broken into two lines.

src/java.base/share/classes/sun/launcher/LauncherHelper.java
 960 bootLayer.modules().stream()
 961 .map(m -> cf.findModule(m.getName()))
 962 .flatMap(Optional::stream)

This can be replaced with cf.modules().stream()

 988 .map(e -> Stream.concat(Stream.of(e.source()),
 989 toStringStream(e.modifiers()))
Formatting nit: line 989 should be indented more to the right

1064  * image ot be less than modules than not in the run-time image.

typo: “ot”

Should these new tracing methods drop the printToStderr argument?
This argument exists for compatibility to print help message to stderr.
These new tracing options will print to stdout.  For validate modules,
you suggested to print the errors to stderr which I agree.

src/java.base/share/classes/sun/launcher/resources/launcher.properties
  60 \--limit-modules [,...]\n\
  61 \  limit the universe of observable modules\n\

-—limit-modules is intended for testing purpose.  I wonder if 
this should be moved to —-extra-help.

  72 \  The --validate-modules option may be is useful for 
finding\n\

typo: “may be is useful”

src/jdk.jartool/share/classes/sun/tools/jar/Main.java
 619 if (dflag) {
 620 // "--describe-module/-d" does not require 
file argument(s),
 621 // but does accept --release
 622 usageError(getMsg("error.bad.dflag"));
 623 return false;
 624 }


Mandy

hg: jigsaw/jake/jdk: Minor tweak to the api note

[mandy . chung]

Changeset: 654588131ea2
Author:mchung
Date:  2017-05-03 08:59 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/654588131ea2

Minor tweak to the api note

! src/java.base/share/classes/java/lang/ClassLoader.java

hg: jigsaw/jake/jdk: 2 new changesets

[mandy . chung]

Changeset: f1ee71109b07
Author:mchung
Date:  2017-05-03 19:01 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/f1ee71109b07

Fix IntegrationTest.java test to check JAVA_FULL_VERSION

! test/tools/jlink/IntegrationTest.java

Changeset: 00902040a130
Author:mchung
Date:  2017-05-03 19:02 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/00902040a130

--show-module-resolution is not detected properly

! src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
+ test/tools/launcher/modules/showmoduleresolution/ShowModuleResolutionTest.java

Re: 8178380: Module system implementation refresh (5/2017 update)

[Mandy Chung]

> On May 1, 2017, at 1:28 PM, Alan Bateman  wrote:
> 
>http://cr.openjdk.java.net/~alanb/8178380/2/

I reviewed all repos in the new version.

src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java

 148 if (System.getProperty("jdk.module.minimumBoot") != null) {
This property can be removed after read the value, if present.

 287 if (propValue != null && Boolean.getBoolean(propValue))

It should use Boolean.parseBoolean.

I have fixed the above injake and also added a new test to verify 
-—show-module-resolution option.

Otherwise, all looks good.
Mandy

Re: #AddExportsInManifest

[Mandy Chung]

These two attributes are defined to ease migration so that executable JARs can 
run with java -jar command, as is today, to avoid adding command-line options 
to break into encapsulation.

Mandy
[1] http://openjdk.java.net/projects/jigsaw/spec/issues/#AddExportsInManifest

> On May 11, 2017, at 1:25 PM, Paul Bakker  wrote:
> 
> I'm a little confused by "...deployments work if they are dependent on JDK 
> internal APIs". What does internal JDK usage have to do with 
> opening/exporting your own packages? I would think this solves the problem 
> that some other code (e.g. a library) requires access to application code?
> Also, can you elaborate why this only applies to the unnamed module and can't 
> do the same a the --add-exports/--add-opens flags?
> 
> Note that I don't actually need these features, but I want to make sure I 
> document correctly.
> 
> Paul
> 
> 
>> On May 11, 2017, at 12:10 AM, Alan Bateman  wrote:
>> 
>> On 11/05/2017 07:51, Paul Bakker wrote:
>> 
>>> Hi Alan,
>>> 
>>> What is the reason only exports/opens to unnamed are possible?
>>> Also, are these implemented in the current jigsaw prototype? I'm having 
>>> trouble getting it to work, but that might be entirely my own doing...
>>> 
>> The attributes are defined for the main application JAR when run with `java 
>> -jar app.jar`. So there are defined to keep existing deployments work if 
>> they are dependent on JDK internal APIs.
>> 
>> The attributes have been in JDK 9 for a long time and I'm not aware of any 
>> issues. If it looks like it's not your "own doing" then send an example and 
>> we'll figure it out.
>> 
>> -Alan
> 

Re: jdeps --generate-open-module

[Mandy Chung]

> On May 15, 2017, at 12:43 PM, Sander Mak  wrote:
> 
> When executing `jdeps --generate-module-info ./out `, a module 
> descriptor exporting all packages is created. When using 
> `--generate-open-module`, a module descriptor with an open module is 
> generated, but no packages are exported. I would expect to be able to 
> generate a module descriptor that is as close to the automatic module 
> behavior of the JAR in question, as starting point for modularization. There 
> seems to be no way to generate an open module exporting all packages, which 
> is the closest to automatic module behavior as possible. What's the intended 
> use-case of `--generate-open-module` as it stands?


jdeps --generate-module-info generates a normal module exporting all packages. 
To make it an open module (that exports all packages), you can use this option 
and then add “open” keyword in the generated module-info.

They may be cases for an open module with no exports for example for frameworks 
to access reflectively.  jdeps -—generate-open-module is the option to generate 
such module-info.java.  Of course it’s easy for jdeps to generates `exports` 
for all packages in this case.  This gives the author the chance to consider 
whether a module should open all packages for reflective access as well as 
exporting all packages.

Mandy

Re: Problems with non-exported split packages (for resources/files)

[Mandy Chung]

On May 9, 2017, at 1:33 PM, Sven Strohschein  wrote:
> 
> There is a very practical example which isn't a bad practice: Resources. 
> Imagine that every Java module has its own ResourceBundle properties for 
> multi-language support (or imagine any other file which is not a Java class). 
> These files are typically placed in a "conf", "etc" or "resources" folder 
> structure and added to the classpath. It should not be necessary that every 
> module has another place for such resources, that would be a bad solution!

Can you point me to some example library that puts ResourceBundles in these 
directories?

The example resource bundles I looked at are typically local and private to a 
library/application in its own package.  In those cases, the resource bundles 
are encapsulated.  A module can load a resource bundle from another module if 
it’s open to it for access, in the same fashion as resources.

When the localized resource bundles must be distributed in multiple JAR files, 
there are several options:
1. leaving those JAR files on classpath will continue to work.
2. loading as automatic modules on modulepath will work if the package has only 
.properties file
3. migrate to service provider in modules
   Migrating from unnamed module to named module, it requires work to eliminate 
the split package [1].


> Java throws a LayerInstantiationException on starting the application when 
> multiple modules have the same resource folder structures... How should 
> resources be organized in your opinion? Should conf/etc/resources renamed and 
> restructured to common package names starting with the top level domain? 

One option is to move resources to META-INF directory and other directory whose 
name is not a valid package name [2].

> I don't want to change my projects in such a way for Jigsaw, it shouldn't be 
> necessary…
> 

Your projects should run fine if you run with -classpath as in JDK 8 unless you 
depend on JDK internal APIs.  Have you tried that?

Mandy
[1] 
http://download.java.net/java/jdk9/docs/api/java/util/ResourceBundle.html#bundleprovider
[2] 
http://mail.openjdk.java.net/pipermail/jpms-spec-experts/2016-September/000392.html

Re: 8182482: Module System spec updates

[Mandy Chung]

> On Jun 20, 2017, at 3:20 AM, Alan Bateman  wrote:
> 
> The webrev with the proposed (docs only, no implementation) changes is here:
>  http://cr.openjdk.java.net/~alanb/8182482/webrev/index.html
> 
> The ServiceLoader diffs are hard to read. It might be easier to read the 
> generated javadoc:
> http://cr.openjdk.java.net/~alanb/8182482/docs/java/util/ServiceLoader.html
> 

Looks good.

Mandy

hg: jigsaw/jake/jdk: Fix typo in launcher help message

[mandy . chung]

Changeset: 0491912f8410
Author:mchung
Date:  2017-05-25 14:43 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/0491912f8410

Fix typo in launcher help message

! src/java.base/share/classes/sun/launcher/resources/launcher.properties

hg: jigsaw/jake/hotspot: 14 new changesets

[mandy . chung]

Changeset: 507f8a7678b4
Author:mcberg
Date:  2017-05-16 12:55 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/507f8a7678b4

8178800: compiler/c2/PolynomialRoot.java fails on Xeon Phi linux host with 
UseAVX=3
Summary: upper register bank support added for novl machines that emit EVEX
Reviewed-by: kvn, thartmann

! src/cpu/x86/vm/macroAssembler_x86.cpp
! src/cpu/x86/vm/macroAssembler_x86.hpp

Changeset: 6427ba02ae4b
Author:lana
Date:  2017-05-18 16:48 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/6427ba02ae4b

Merge


Changeset: c2314cb67e28
Author:twisti
Date:  2017-05-18 16:39 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/c2314cb67e28

8180453: [JVMCI] mx eclipseinit doesn't pick up generated sources
Reviewed-by: kvn

! .mx.jvmci/mx_jvmci.py

Changeset: 34c47915ae05
Author:uvangapally
Date:  2017-05-19 15:32 +0530
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/34c47915ae05

8175845: Provide javadoc descriptions for jdk.hotspot.agent module
Summary: Added description for jdk.hotspot.agent module
Reviewed-by: mchung

! src/jdk.hotspot.agent/share/classes/module-info.java

Changeset: 8f941bab493f
Author:thartmann
Date:  2017-05-22 09:14 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/8f941bab493f

8180565: Null pointer dereferences of ConstMethod::method()
Summary: We need to check ConstMethod::method() for NULL before dereferencing.
Reviewed-by: kvn, iignatyev

! src/share/vm/oops/constMethod.cpp

Changeset: 0b218e675429
Author:thartmann
Date:  2017-05-22 09:16 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/0b218e675429

8180617: Null pointer dereference in InitializeNode::complete_stores
Summary: Fixed a missing null check on the return value of 
InitializeNode::allocation() found by Parfait.
Reviewed-by: zmajo

! src/share/vm/opto/memnode.cpp

Changeset: 1f917785fbe7
Author:thartmann
Date:  2017-05-22 09:17 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/1f917785fbe7

8180511: Null pointer dereference in Matcher::ReduceInst()
Summary: Fixed a missing null check on the return value of MachNodeGenerator() 
found by Parfait.
Reviewed-by: kvn

! src/share/vm/opto/matcher.cpp

Changeset: 286c8e80795b
Author:thartmann
Date:  2017-05-22 09:18 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/286c8e80795b

8180576: Null pointer dereference in Matcher::xform()
Summary: Fixed a missing null check on n->in(0) found by Parfait.
Reviewed-by: kvn

! src/share/vm/opto/matcher.cpp

Changeset: 31c842513336
Author:thartmann
Date:  2017-05-22 09:23 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/31c842513336

8180575: Null pointer dereference in LoadNode::Identity()
Summary: Fixed a missing null check on the return value of 
AddPNode::Ideal_base_and_offset() found by Parfait.
Reviewed-by: kvn

! src/share/vm/opto/memnode.cpp

Changeset: 79f53d722d58
Author:iignatyev
Date:  2017-05-22 15:27 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/79f53d722d58

8180793: move jdk.test.lib.wrappers.* to jdk.test.lib package
Reviewed-by: mchung

! test/compiler/codecache/stress/CodeCacheStressRunner.java
! test/compiler/codecache/stress/Helper.java
! test/compiler/compilercontrol/jcmd/StressAddJcmdBase.java
! test/compiler/whitebox/AllocationCodeBlobTest.java

Changeset: d53171650a2c
Author:iignatyev
Date:  2017-05-22 15:28 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/d53171650a2c

8180721: clean up ProblemList
Reviewed-by: sspitsyn, gtriantafill

! test/ProblemList.txt
! test/serviceability/jdwp/AllModulesCommandTest.java

Changeset: 385668275400
Author:lana
Date:  2017-05-26 00:29 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/385668275400

Added tag jdk-9+171 for changeset d53171650a2c

! .hgtags

Changeset: 408103dc579d
Author:mchung
Date:  2017-05-25 19:49 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/408103dc579d

Merge

! .hgtags
! src/jdk.hotspot.agent/share/classes/module-info.java
! test/ProblemList.txt
! test/serviceability/jdwp/AllModulesCommandTest.java

Changeset: 5ada634b8e54
Author:mchung
Date:  2017-05-25 22:15 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/5ada634b8e54

Merge

- test/runtime/modules/JVMAddModulePackage.java

hg: jigsaw/jake/nashorn: 2 new changesets

[mandy . chung]

Changeset: c8d6b740f0f7
Author:lana
Date:  2017-05-26 00:29 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/c8d6b740f0f7

Added tag jdk-9+171 for changeset fc416270a776

! .hgtags

Changeset: 3e76e17ddd2a
Author:mchung
Date:  2017-05-25 19:50 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/3e76e17ddd2a

Merge

! .hgtags

hg: jigsaw/jake/jaxws: 2 new changesets

[mandy . chung]

Changeset: 8c615099f3e3
Author:lana
Date:  2017-05-26 00:29 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxws/rev/8c615099f3e3

Added tag jdk-9+171 for changeset 139e7c786ee4

! .hgtags

Changeset: a8760921acb7
Author:mchung
Date:  2017-05-25 19:49 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxws/rev/a8760921acb7

Merge

! .hgtags

hg: jigsaw/jake/jdk: 15 new changesets

[mandy . chung]

Changeset: 78ce7067aba9
Author:bpb
Date:  2017-05-16 14:11 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/78ce7067aba9

8180431: Remove intermittent keyword from some no longer failing NIO tests
Summary: Remove "intermittent" keyword from @key tag,
Reviewed-by: alanb

! test/java/nio/channels/Selector/SelectAndClose.java
! test/java/nio/channels/Selector/WakeupAfterClose.java

Changeset: 148a2cbe49a4
Author:darcy
Date:  2017-05-16 18:23 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/148a2cbe49a4

8180452: Mark ClipCloseLoss.java as failing intermittently
Reviewed-by: serb

! test/javax/sound/sampled/Clip/ClipCloseLoss.java

Changeset: 09038ebee480
Author:mullan
Date:  2017-05-17 08:51 -0400
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/09038ebee480

8180307: Update JDK 9 Required Cipher Algorithms
Reviewed-by: valeriep

! src/java.base/share/classes/javax/crypto/Cipher.java

Changeset: 93b0c398e243
Author:bpb
Date:  2017-05-17 14:29 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/93b0c398e243

7086489: File.lastModified should accuracy as well as resolution
Summary: Add an @apiNote indicating that the last-modified time might be of 
coarser than millisecond granularity.
Reviewed-by: smarks

! src/java.base/share/classes/java/io/File.java

Changeset: 13b7c6fdf864
Author:henryjen
Date:  2017-05-17 16:26 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/13b7c6fdf864

8180447: Trailing space in JDK_JAVA_OPTIONS causes an application fail to launch
Reviewed-by: alanb, mchung, ksrini

! src/java.base/share/native/libjli/args.c
! src/java.base/share/native/libjli/emessages.h
! test/tools/launcher/ArgsEnvVar.java

Changeset: 7efb352c6765
Author:ihse
Date:  2017-05-18 09:23 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/7efb352c6765

8180426: Use standard css file for new docs bundle index.html page
Reviewed-by: mchung, erikj

+ make/data/docs-resources/resources/jdk-default.css
- make/data/docs-resources/specs/resources/jdk-default.css
! make/src/classes/build/tools/docs/docs-bundle-page.html

Changeset: 85ccf9e98ba0
Author:ihse
Date:  2017-05-18 12:00 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/85ccf9e98ba0

8180486: extLink taglet needs escaped ""
Reviewed-by: dholmes

! make/src/classes/build/tools/taglet/ExtLink.java

Changeset: ee3280639210
Author:lana
Date:  2017-05-18 16:48 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/ee3280639210

Merge

- make/data/docs-resources/specs/resources/jdk-default.css

Changeset: f6553abdefb6
Author:bchristi
Date:  2017-05-19 09:41 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/f6553abdefb6

8180633: Remove intermittent key from java/lang/ClassLoader/Assert.java
Reviewed-by: darcy, mchung

! test/java/lang/ClassLoader/Assert.java

Changeset: 490393b435bb
Author:stuefe
Date:  2017-05-21 10:52 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/490393b435bb

8180424: Another build issue on AIX after 8034174
Reviewed-by: clanger, vtewari

! src/java.base/aix/native/libnet/aix_close.c

Changeset: 698cb1c6c88e
Author:michaelm
Date:  2017-05-22 17:31 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/698cb1c6c88e

8180498: Remove httpclient internal APIs which supply ByteBuffers to read calls
Reviewed-by: chegar, dfuchs

! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncSSLConnection.java
! src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Exchange.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1Response.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpConnection.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainTunnelingConnection.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseContent.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseHeaders.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLConnection.java
! src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLDelegate.java
! 
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/SSLTunnelConnection.java
! 
test/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/ResponseHeadersTest.java

Changeset: 29bbedd4cce8
Author:mchung
Date:  2017-05-22 11:08 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/29bbedd4cce8

8180717: Upgrade the docs bundle index page
Reviewed-by: jjg, ihse

! make/src/classes/build/tools/docs/GenDocsBundlePage.java
! make/src/classes/build/tools/docs/docs-bundle-page.html
! 

hg: jigsaw/jake/corba: 2 new changesets

[mandy . chung]

Changeset: 95ed14547ca9
Author:lana
Date:  2017-05-26 00:29 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/corba/rev/95ed14547ca9

Added tag jdk-9+171 for changeset c62e5964cfcf

! .hgtags

Changeset: 117bfab0a8bd
Author:mchung
Date:  2017-05-25 19:49 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/corba/rev/117bfab0a8bd

Merge

! .hgtags

hg: jigsaw/jake/langtools: 2 new changesets

[mandy . chung]

Changeset: 65652f51a99a
Author:lana
Date:  2017-05-26 00:29 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/65652f51a99a

Added tag jdk-9+171 for changeset aae59039c1f5

! .hgtags

Changeset: 88aaba14239f
Author:mchung
Date:  2017-05-25 19:49 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/88aaba14239f

Merge

! .hgtags