Re: RFR 8171138: Remove FileCopierPlugin

[Mandy Chung]

> On Dec 14, 2016, at 8:35 PM, Sundararajan Athijegannathan 
>  wrote:
> 
> Hi Mandy,
> 
> Thanks for your review. I updated to handle TOP entries in accept callback 
> uniformly: http://cr.openjdk.java.net/~sundar/8171138/webrev.01/index.html

395 // Copy TOP files of the "java.base" module (only)
396 if ("java.base".equals(file.moduleName())) {

It may be better to throw an InternalError for any entry other than 
“java.base”.

Otherwise looks good.  You can fix this before you push.  No need for a new 
webrev.

Mandy

Re: RFR 8171138: Remove FileCopierPlugin

[Sundararajan Athijegannathan]

Hi Mandy,

Thanks for your review. I updated to handle TOP entries in accept 
callback uniformly: 
http://cr.openjdk.java.net/~sundar/8171138/webrev.01/index.html


Thanks,
-Sundar

On 14/12/16, 10:51 PM, Mandy Chung wrote:

On Dec 14, 2016, at 9:02 AM, Sundararajan 
Athijegannathan  wrote:

Please review http://cr.openjdk.java.net/~sundar/8171138/webrev.00/ for 
https://bugs.openjdk.java.net/browse/JDK-8171138

Piggybacking cleanup: release file generation ("TOP" entry type handling) moved 
to accept(ResourcePoolEntry)


The removal of FileCopierPlugin looks fine.

The image builder writes out all entries from the ResourcePool.
`release` file is already an entry added to ResourcePool and the content
is known when the entry is created.  The image builder could just write out 
`release` file in the same way as all other entries (i.e. with 
ResourcePoolEntry::contents).  A related question is whether we want to keep 
TOP type going forward.  You could either leave the `release` file generation 
change out, leave it as is, or write the content when the entry is created in 
ReleaseInfoPlugin (i.e. storeRelease method can also be removed).

Mandy

Re: 8170987: Module system implementation refresh (12/2016)

[Mandy Chung]

> 
> The webrevs with the changes for this update are here:
> 
>   http://cr.openjdk.java.net/~alanb/8170987/1

I have pushed the change to rename jdk.crypto.pkcs11 and jdk.pack200
and dropped java.compact$N.  So module-info.java changes will not be 
needed when you sync with jdk9/dev.

I reviewed all changes except javac/javadoc changes.  Looks good in 
general. 

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

115 /**
116 * Returns {@code true} if the given name is a legal binary name.
117 */
118 public static boolean isJavaIdentifier(String name) {

Not sure if it’s intended to have the javadoc for isJavaIdentifier 
method be the same as isBinaryName.

When we use —-module-version for user modules, the runtime will load
regex.  The system modules jlink plugin uses the cached version if
JDK modules to be compiled with —0module-version in the future.
This might be something we should look at in the future for performance.

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

  64 throw new RuntimeException("cannot add deprecated to " + 
value);

This comment applies to ModuleResoluton::with* methods.  This should
probably be an InternalError?  

 108 return String.valueOf(value);

Nit: since you override toString method, might be helpful to print
an informative description.  

src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java

1102 if (value.equals("deprecated"))
1103 return (new ModuleResolution(0)).withDeprecated();
1104 else if (value.equals("deprecated-for-removal"))
1105 return (new 
ModuleResolution(0)).withDeprecatedForRemoval();
1106 else if (value.equals("incubating"))
1107 return (new ModuleResolution(0)).withIncubating();

Why not passing the flag to ModuleResolution constructor?  Similar
statement is also in sun/tools/jar/GNUStyleOptions.java.

I was wondering if jmod describe and jar —-print-module-descriptor should
print all optional attributes.  While the module resolution is of limited
use, it would be handy to print all optional attributes, if present rather
than having to run javap.

It’s okay to follow up as a separate JBS issue if we want to do that.

test/jdk/modules/incubator/ImageModules.java
  @modules jdk.jlink jdk.jartool are missing.  I have fixed it.

Mandy

hg: jigsaw/jake/langtools: Add support for reading module resolution flags

[jonathan . gibbons]

Changeset: 63af48cfc1c9
Author:jjg
Date:  2016-12-14 17:05 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/63af48cfc1c9

Add support for reading module resolution flags

! src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/util/Names.java

hg: jigsaw/jake/jdk: Fix up @modules in incubator and jlink plugin tests

[mandy . chung]

Changeset: 3d3212804cd8
Author:mchung
Date:  2016-12-14 17:00 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/3d3212804cd8

Fix up @modules in incubator and jlink plugin tests

! test/jdk/modules/incubator/ImageModules.java
! test/tools/jlink/plugins/SystemModuleDescriptors/CompiledVersionTest.java
! test/tools/jlink/plugins/SystemModuleDescriptors/UserModuleTest.java

hg: jigsaw/jake/jdk: fix formatting nit

[mandy . chung]

Changeset: e0558b224fdf
Author:mchung
Date:  2016-12-14 16:44 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/e0558b224fdf

fix formatting nit

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

Re: 8170987: Module system implementation refresh (12/2016)

[Claes Redestad]

Hi,

I took a quick pass over the jdk changes. It generally looks very good,
but I've got some comments:

MethodHandles.Lookup.dropLookupMode: The javadoc doesn't really roll
of the tongue here. Maybe "Creates a new lookup from the current one
where the given lookup mode has been dropped. ..." for starters?

ModuleDescriptor$Builder: should automatic be moved into a constructor
and automatic(boolean) removed for consistency with other boolean
attributes? My gut feeling tells me that
Builder.module("name").automatic(true) is non-sensical (not to mention
Builder.automaticModule("name").automatic(false)). It probably makes
no sense to export it through the JLMA bridge, but could avoid that
by adding a new private constructor called by the current.

WARNING could be a local anonymous class inside
printStackTraceIfExposedReflectively. ;-) A more noticeable cleanup
would be to move these methods to jdk/internal/reflect/Reflection.java
where there's now what appears to be code duplication (although the
printed messages diverge).

I see the Checks.isJavaIdentifier has been reworked, which should also
resolve the correctness issues we found here[1]. Good!

In ClassWriter.java there's a comment line that seems to have been
removed by mistake.

Thanks!

/Claes

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

On 2016-12-14 22:46, Alan Bateman wrote:

Folks on jigsaw-dev will be aware that we are on yet another mission to
bring the changes accumulated in the jake forest to jdk9/dev. The plan
this time is to bring the changes to jdk9/dev to make jdk-9+150.

The changes in this update are mostly for JSR 376 issues
#VersionedDependences and #ModuleNameCharacters and so involve updates
to the binary form of the module declaration. There is also some small
changes left over from #IndirectQualifiedReflectiveAccess that we didn't
include in the last refresh.

This update has the implementation of Incubator Modules (JEP 11 [1]),
everything except the javac support. This was initially planned to push
to jdk9/dev but was re-routed to jake to avoid needing re-work when
merged with the changes in jake.

There is a bit of refactoring in the implementation in this update. We
expect to do more on than, plus lots of clean-up, once all the feature
work is out of way.

The webrevs with the changes for this update are here:

   http://cr.openjdk.java.net/~alanb/8170987/1

They are currently based on jdk-9+148 and will be re-based for jdk9/dev
later this week.

One review note this time is to ignore the changes in ModuleBootstrap
for DEBUG_ADD_OPENS, that is the only change in this webrev that is not
proposed to move to jdk9/dev.

-Alan

[1] http://openjdk.java.net/jeps/11

Re: 8170987: Module system implementation refresh (12/2016)

[Lois Foltan]

On 12/14/2016 4:46 PM, Alan Bateman wrote:
Folks on jigsaw-dev will be aware that we are on yet another mission 
to bring the changes accumulated in the jake forest to jdk9/dev. The 
plan this time is to bring the changes to jdk9/dev to make jdk-9+150.


The changes in this update are mostly for JSR 376 issues 
#VersionedDependences and #ModuleNameCharacters and so involve updates 
to the binary form of the module declaration. There is also some small 
changes left over from #IndirectQualifiedReflectiveAccess that we 
didn't include in the last refresh.


This update has the implementation of Incubator Modules (JEP 11 [1]), 
everything except the javac support. This was initially planned to 
push to jdk9/dev but was re-routed to jake to avoid needing re-work 
when merged with the changes in jake.


There is a bit of refactoring in the implementation in this update. We 
expect to do more on than, plus lots of clean-up, once all the feature 
work is out of way.


The webrevs with the changes for this update are here:

   http://cr.openjdk.java.net/~alanb/8170987/1


I have reviewed the hotspot changes, looks good.
Lois



They are currently based on jdk-9+148 and will be re-based for 
jdk9/dev later this week.


One review note this time is to ignore the changes in ModuleBootstrap 
for DEBUG_ADD_OPENS, that is the only change in this webrev that is 
not proposed to move to jdk9/dev.


-Alan

[1] http://openjdk.java.net/jeps/11

8170987: Module system implementation refresh (12/2016)

[Alan Bateman]

Folks on jigsaw-dev will be aware that we are on yet another mission to 
bring the changes accumulated in the jake forest to jdk9/dev. The plan 
this time is to bring the changes to jdk9/dev to make jdk-9+150.


The changes in this update are mostly for JSR 376 issues 
#VersionedDependences and #ModuleNameCharacters and so involve updates 
to the binary form of the module declaration. There is also some small 
changes left over from #IndirectQualifiedReflectiveAccess that we didn't 
include in the last refresh.


This update has the implementation of Incubator Modules (JEP 11 [1]), 
everything except the javac support. This was initially planned to push 
to jdk9/dev but was re-routed to jake to avoid needing re-work when 
merged with the changes in jake.


There is a bit of refactoring in the implementation in this update. We 
expect to do more on than, plus lots of clean-up, once all the feature 
work is out of way.


The webrevs with the changes for this update are here:

   http://cr.openjdk.java.net/~alanb/8170987/1

They are currently based on jdk-9+148 and will be re-based for jdk9/dev 
later this week.


One review note this time is to ignore the changes in ModuleBootstrap 
for DEBUG_ADD_OPENS, that is the only change in this webrev that is not 
proposed to move to jdk9/dev.


-Alan

[1] http://openjdk.java.net/jeps/11

hg: jigsaw/jake/jdk: ModuleReference equals/hc should not use reader and hash suppliers

[alan . bateman]

Changeset: 78a3778bd7b2
Author:alanb
Date:  2016-12-14 20:55 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/78a3778bd7b2

ModuleReference equals/hc should not use reader and hash suppliers

! src/java.base/share/classes/jdk/internal/module/ModulePatcher.java
! src/java.base/share/classes/jdk/internal/module/ModuleReferenceImpl.java
! src/java.base/share/classes/jdk/internal/module/ModuleReferences.java
! src/java.base/share/classes/jdk/internal/module/SystemModuleFinder.java

hg: jigsaw/jake/jdk: DEBUG_ADD_OPENS: Further message tweaks

[mark . reinhold]

Changeset: d435ff62fe7e
Author:mr
Date:  2016-12-14 11:58 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/d435ff62fe7e

DEBUG_ADD_OPENS: Further message tweaks

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

hg: jigsaw/jake/jdk: Improve warnings for invalid module names at jlink/run time

[alan . bateman]

Changeset: d9e238be50b9
Author:alanb
Date:  2016-12-14 19:25 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/d9e238be50b9

Improve warnings for invalid module names at jlink/run time

! src/java.base/share/classes/jdk/internal/module/Checks.java
! src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
! src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

Re: --add-opens on an automatic module ?

[Kevin Rushforth]

Alan Bateman wrote:

On 14/12/2016 17:21, David Hill wrote:



I have yet another gradle exception:

java.lang.IllegalAccessException: class 
org.gradle.groovy.scripts.internal.GradleResolveVisitor cannot access 
a member of class org.codehaus.groovy.control.ClassNodeResolver with 
modifiers "protected static final"


Gradle is trying to use a jar it is packaged with likely 
(groovy-all-2.4.7.jar)


A quick search online did not show me how to specify a (presumably) 
automatic module in the


   --add-opens=module/package=consumer

argument.
Automatic modules open all their packages and so the --add-opens here 
with be a no-op.


As regards the IllegalAccessException then I don't know the types 
involved here so know if 
org.gradle.groovy.scripts.internal.GradleResolveVisitor is a sub-type 
of org.codehaus.groovy.control.ClassNodeResolver or not. It's possible 
this is nothing to do with modules but running with 
-Dsun.reflect.debugModuleAccessChecks=true might reveal something useful.




We are already running with "-Dsun.reflect.debugModuleAccessChecks=true" 
and I don't see anything obvious. I do note that when I see this, which 
isn't very often, it seems only to be a warning; I haven't noticed any 
problems as a result. As to whether or not it has anything to do with 
modules, you might be right. It did only start happening with jdk-9+148 
though (we didn't get with build 147 prior to switching).


-- Kevin

hg: jigsaw/jake/jdk: DEBUG_ADD_OPENS: s/RESOLVED/ALL-RESOLVED/; tweak warning messages

[mark . reinhold]

Changeset: b2442ace41dd
Author:mr
Date:  2016-12-14 11:15 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/b2442ace41dd

DEBUG_ADD_OPENS: s/RESOLVED/ALL-RESOLVED/; tweak warning messages

! src/java.base/share/classes/java/lang/reflect/AccessibleObject.java
! src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java

RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

[Uwe Schindler]

Hi Chris,

looks good to me. I already created a patch / branch of Apache Lucene that uses 
your proposed API and removes the Runnable hack. You can see it and check it 
out on Github:



For your info the issue description and description is:


If you would like to test it against your patch, check out this branch, run 
"ant ivy-bootstrap" and then cd into the lucene/core dirto execute:
$ ant test -Dtestcase=TestMmapDirectory

Those tests should run without any error message (disabled test because 
unmapping not supported):

[junit4] Started J0 PID(3464@localhost).
   [junit4] Suite: org.apache.lucene.store.TestMmapDirectory
   [junit4] OK  0.07s | TestMmapDirectory.testIllegalEOF
   [junit4] OK  0.01s | TestMmapDirectory.testShort
   [junit4] OK  0.06s | TestMmapDirectory.testRandomInt
   [junit4] OK  0.02s | TestMmapDirectory.testFsyncDoesntCreateNewFiles
   [junit4] OK  0.01s | TestMmapDirectory.testSetOfStrings
   [junit4] OK  0.01s | TestMmapDirectory.testLargeWrites
   [junit4] OK  0.01s | TestMmapDirectory.testMapOfStrings
   [junit4] OK  0.01s | TestMmapDirectory.testSliceOfSlice
   [junit4] OK  0.01s | TestMmapDirectory.testByte
   [junit4] OK  0.01s | TestMmapDirectory.testVInt
   [junit4] OK  0.01s | TestMmapDirectory.testChecksum
   [junit4] OK  0.02s | TestMmapDirectory.testZLong
   [junit4] OK  0.06s | TestMmapDirectory.testRandomShort
   [junit4] OK  0.01s | TestMmapDirectory.testRename
   [junit4] OK  0.10s | TestMmapDirectory.testCreateTempOutput
   [junit4] OK  0.01s | TestMmapDirectory.testZInt
   [junit4] OK  0.01s | TestMmapDirectory.testIndexOutputToString
   [junit4] OK  0.01s | TestMmapDirectory.testDetectClose
   [junit4] OK  0.03s | TestMmapDirectory.testListAllIsSorted
   [junit4] OK  0.01s | TestMmapDirectory.testDoubleCloseDirectory
   [junit4] OK  0.00s | TestMmapDirectory.testDirectoryFilter
   [junit4] OK  0.11s | TestMmapDirectory.testPendingDeletions
   [junit4] OK  0.04s | TestMmapDirectory.testCopyFromDestination
   [junit4] OK  0.01s | TestMmapDirectory.testDeleteFile
   [junit4] OK  0.04s | TestMmapDirectory.testCopyBytesWithThreads
   [junit4] OK  0.03s | TestMmapDirectory.testRandomByte
   [junit4] IGNORED 0.00s | TestMmapDirectory.testAceWithThreads
   [junit4]> Cause: Annotated @Ignore(This test is for JVM testing 
purposes. There are no guarantees that it may not fail with SIGSEGV!)
   [junit4] OK  0.02s | TestMmapDirectory.testNoDir
   [junit4] OK  0.01s | TestMmapDirectory.testSeekBeyondEndOfFile
   [junit4] OK  0.13s | TestMmapDirectory.testRandomLong
   [junit4] OK  0.01s | TestMmapDirectory.testDoubleCloseInput
   [junit4] OK  0.01s | TestMmapDirectory.testSliceOutOfBounds
   [junit4] OK  2.75s | TestMmapDirectory.testThreadSafety
   [junit4] OK  0.01s | TestMmapDirectory.testSeekToEndOfFile
   [junit4] OK  0.00s | TestMmapDirectory.testLong
   [junit4] OK  0.01s | TestMmapDirectory.testCopyFrom
   [junit4] OK  0.00s | TestMmapDirectory.testString
   [junit4] OK  0.01s | TestMmapDirectory.testSeekToEOFThenBack
   [junit4] OK  0.00s | TestMmapDirectory.testInt
   [junit4] OK  0.01s | TestMmapDirectory.testSeekPastEOF
   [junit4] OK  0.02s | TestMmapDirectory.testCopyBytes
   [junit4] OK  0.01s | TestMmapDirectory.testVLong
   [junit4] OK  0.01s | TestMmapDirectory.testDoubleCloseOutput
   [junit4] Completed [1/1] in 5.14s, 43 tests, 1 skipped

With Java 9 build 148 it currently looks like this:

   [junit4] Suite: org.apache.lucene.store.TestMmapDirectory
   [junit4] IGNOR/A 0.04s | TestMmapDirectory.testShort
   [junit4]> Assumption #1: test requires a jre that supports unmapping: 
Unmapping is not supported on this platform, because internal Java APIs are not 
compatible to this Lucene version: 
java.lang.reflect.InaccessibleObjectException: Unable to make public 
jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible: module 
java.base does not "opens java.nio" to unnamed module @45c7ad7f

Don't care about the Groovy error printed later, this one is known, but does 
not affect testing.

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Wednesday, December 14, 2016 12:58 PM
> To: Peter Levart ; Core-Libs-Dev  d...@openjdk.java.net>; jigsaw-dev ; Uwe
> Schindler 
> Subject: Re: Java 9 build 148 causes trouble in Apache
> Lucene/Solr/Elasticsearch
> 
> Webrev updated in-place.
>

Re: --add-opens on an automatic module ?

[Alan Bateman]

On 14/12/2016 17:21, David Hill wrote:



I have yet another gradle exception:

java.lang.IllegalAccessException: class 
org.gradle.groovy.scripts.internal.GradleResolveVisitor cannot access 
a member of class org.codehaus.groovy.control.ClassNodeResolver with 
modifiers "protected static final"


Gradle is trying to use a jar it is packaged with likely 
(groovy-all-2.4.7.jar)


A quick search online did not show me how to specify a (presumably) 
automatic module in the


   --add-opens=module/package=consumer

argument.
Automatic modules open all their packages and so the --add-opens here 
with be a no-op.


As regards the IllegalAccessException then I don't know the types 
involved here so know if 
org.gradle.groovy.scripts.internal.GradleResolveVisitor is a sub-type of 
org.codehaus.groovy.control.ClassNodeResolver or not. It's possible this 
is nothing to do with modules but running with 
-Dsun.reflect.debugModuleAccessChecks=true might reveal something useful.


-Alan

Re: --add-opens on an automatic module ?

[Claes Redestad]

Hi,

is this something that is logged when you are using 
-Dsun.reflect.debugModuleAccessChecks=true? In that case this could be

a pre-existing exception that is/was being swallowed.

(I might be wrong, but my assumption it that automatic modules - if
such are involved - are implicitly open)

Thanks!

/Claes

On 2016-12-14 18:21, David Hill wrote:


I have yet another gradle exception:

java.lang.IllegalAccessException: class
org.gradle.groovy.scripts.internal.GradleResolveVisitor cannot access a
member of class org.codehaus.groovy.control.ClassNodeResolver with
modifiers "protected static final"

Gradle is trying to use a jar it is packaged with likely
(groovy-all-2.4.7.jar)

A quick search online did not show me how to specify a (presumably)
automatic module in the

   --add-opens=module/package=consumer

argument.

Re: RFR 8171138: Remove FileCopierPlugin

[Jim Laskey (Oracle)]

+1

> On Dec 14, 2016, at 1:02 PM, Sundararajan Athijegannathan 
>  wrote:
> 
> Please review http://cr.openjdk.java.net/~sundar/8171138/webrev.00/ for 
> https://bugs.openjdk.java.net/browse/JDK-8171138
> 
> Piggybacking cleanup: release file generation ("TOP" entry type handling) 
> moved to accept(ResourcePoolEntry)
> 
> Thanks,
> -Sundar

--add-opens on an automatic module ?

[David Hill]

I have yet another gradle exception:

java.lang.IllegalAccessException: class 
org.gradle.groovy.scripts.internal.GradleResolveVisitor cannot access a member of class 
org.codehaus.groovy.control.ClassNodeResolver with modifiers "protected static 
final"

Gradle is trying to use a jar it is packaged with likely (groovy-all-2.4.7.jar)

A quick search online did not show me how to specify a (presumably) automatic 
module in the

   --add-opens=module/package=consumer

argument.

--
David Hill
Java Embedded Development

"A man's feet should be planted in his country, but his eyes should survey the 
world."
-- George Santayana (1863 - 1952)

Re: RFR 8171138: Remove FileCopierPlugin

[Mandy Chung]

> On Dec 14, 2016, at 9:02 AM, Sundararajan Athijegannathan 
>  wrote:
> 
> Please review http://cr.openjdk.java.net/~sundar/8171138/webrev.00/ for 
> https://bugs.openjdk.java.net/browse/JDK-8171138
> 
> Piggybacking cleanup: release file generation ("TOP" entry type handling) 
> moved to accept(ResourcePoolEntry)
> 

The removal of FileCopierPlugin looks fine.

The image builder writes out all entries from the ResourcePool.
`release` file is already an entry added to ResourcePool and the content
is known when the entry is created.  The image builder could just write out 
`release` file in the same way as all other entries (i.e. with 
ResourcePoolEntry::contents).  A related question is whether we want to keep 
TOP type going forward.  You could either leave the `release` file generation 
change out, leave it as is, or write the content when the entry is created in 
ReleaseInfoPlugin (i.e. storeRelease method can also be removed).

Mandy

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[Alan Bateman]

On 14/12/2016 16:51, Rony G. Flatscher wrote:


Thanks to everyone who commented and/or shed some light about the situation for 
objects that need to
invoke/access protected members in superclasses. The conclusion is, that 
nothing will change as
protected members are still regarded to be public members for subclasses or 
objects of subclasses.
In my case that is fine as I have adhered to allowing access to public members 
(and protected ones
from subclasses or subclass instances) in the Java bridge.

Being in the process of tidying up various development threads in the beta 
version of the bridge
(including adding javax.script/jsr-223 support) of the past two years, I want 
to make sure that the
GA-version scheduled for April will be able to be fully usable with Java 9 that 
will appear
thereafter (currently in July). Therefore I have been lurking around jdk9-dev 
and jigsaw-dev and
reading the informal description
Oracle publish weekly builds [1] of JDK 9 so it might be best to just 
run your tests to see if hit any issues.


-Alan

[1] https://jdk9.java.net/download/

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[Rony G. Flatscher]

Thanks to everyone who commented and/or shed some light about the situation for 
objects that need to
invoke/access protected members in superclasses. The conclusion is, that 
nothing will change as
protected members are still regarded to be public members for subclasses or 
objects of subclasses.
In my case that is fine as I have adhered to allowing access to public members 
(and protected ones
from subclasses or subclass instances) in the Java bridge.

Being in the process of tidying up various development threads in the beta 
version of the bridge
(including adding javax.script/jsr-223 support) of the past two years, I want 
to make sure that the
GA-version scheduled for April will be able to be fully usable with Java 9 that 
will appear
thereafter (currently in July). Therefore I have been lurking around jdk9-dev 
and jigsaw-dev and
reading the informal description I was unsure how non-public members get 
treated in this particular
case. Or with other words, there is nothing I have to take into account for 
and/or need to change
because of #AwkwardStrongEncapsulation which is great! :)

Of course, other tooling, JVM languages and other bridges may meet different 
needs and employ
different techniques, so it has also been interesting to see and to learn about 
them and what
changes for them with Java 9, what problems they have to master and what means 
are being explored to
do so. (And BTW it has been very interesting and motivating to see the 
constructive communication
culture in these lists from all involved parties!)

---rony


On 14.12.2016 09:07, Alan Bateman wrote:
>
>
> On 14/12/2016 07:30, Peter Levart wrote:
>> Hi David,
>>
>> On 12/14/2016 07:17 AM, David Holmes wrote:
 But let me explain why .setAccessible(true) can't be allowed for
 protected members in general.
>>>
>>> I'm confused as to what is being argued for/against here. 
>>
>> Rony asked why .setAccessible(true) can't be used for protected members even 
>> if called from a
>> subclass of the member's declaring class.
> Right, I think it would be helpful if Rory could paste in a stack trace from 
> where setAccessible
> is failing so that there are specifics to discuss.
>
> -Alan

RFR 8171138: Remove FileCopierPlugin

[Sundararajan Athijegannathan]

Please review http://cr.openjdk.java.net/~sundar/8171138/webrev.00/ for 
https://bugs.openjdk.java.net/browse/JDK-8171138


Piggybacking cleanup: release file generation ("TOP" entry type 
handling) moved to accept(ResourcePoolEntry)


Thanks,
-Sundar

Re: Review Request: JDK-8171201 & JDK-8171202: Drop java.compact$N aggregator modules

[Alan Bateman]

On 14/12/2016 16:19, Mandy Chung wrote:

:
Fixed.



All looks good to me.

-Alan

hg: jigsaw/jake/jdk: 3 new changesets

[alan . bateman]

Changeset: 47dc888f6f6c
Author:alanb
Date:  2016-12-14 16:08 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/47dc888f6f6c

javadoc clarification

! src/java.base/share/classes/java/lang/module/ModuleDescriptor.java

Changeset: c2f37a058fac
Author:alanb
Date:  2016-12-14 16:19 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/c2f37a058fac

Temporary warning for modules with names ending in digits

! src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
! src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java

Changeset: 64aff73e90cf
Author:alanb
Date:  2016-12-14 16:15 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/64aff73e90cf

Lookup::dropLookupMode
Contributed-by: alan.bate...@oracle.com, john.r.r...@oracle.com

! src/java.base/share/classes/java/lang/invoke/MethodHandles.java
+ test/java/lang/invoke/DropLookupModeTest.java
! 
test/java/lang/invoke/MethodHandles/privateLookupIn/test/p/PrivateLookupInTests.java

Re: Review Request: JDK-8171201 & JDK-8171202: Drop java.compact$N aggregator modules

[Mandy Chung]

> On Dec 14, 2016, at 12:03 AM, Alan Bateman  wrote:
> 
> On 14/12/2016 06:49, Mandy Chung wrote:
> 
>> JDK-8171201: Drop java.compact$N aggregator modules
>> JDK-8171202: Rename jdk.crypto.pkcs11 and jdk.pack200 to end with Java 
>> letters
>> 
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171201%2b8171202/webrev.00/
>> 
>> 
> The hg moves are showing up as new files in the webrev - is that because the 
> webrev is on several repos?
> 

Not sure and it might be webrev on several repos.

The number of lines changed is zero which is correct.

> In the LimitModsTest test then it might be simpler to leave java.scripting 
> out of the test.

Sure.

$ hg diff limitmods/LimitModsTest.java
diff --git a/test/tools/launcher/modules/limitmods/LimitModsTest.java 
b/test/tools/launcher/modules/limitmods/LimitModsTest.java
--- a/test/tools/launcher/modules/limitmods/LimitModsTest.java
+++ b/test/tools/launcher/modules/limitmods/LimitModsTest.java
@@ -24,7 +24,7 @@
 /**
  * @test
  * @library /lib/testlibrary
- * @modules java.desktop java.compact1 jdk.compiler
+ * @modules java.desktop java.logging jdk.compiler
  * @build LimitModsTest CompilerUtils jdk.testlibrary.*
  * @run testng LimitModsTest
  * @summary Basic tests for java --limit-modules
@@ -83,13 +83,12 @@
 assertTrue(exitValue == 0);
 
 
-// java --limit-modules java.compact1 --list-modules
-exitValue = executeTestJava("--limit-modules", "java.compact1", 
"--list-modules")
+// java --limit-modules java.logging --list-modules
+exitValue = executeTestJava("--limit-modules", "java.logging", 
"--list-modules")
 .outputTo(System.out)
 .errorTo(System.out)
 .shouldContain("java.base")
 .shouldContain("java.logging")
-.shouldContain("java.compact1")
 .shouldNotContain("java.xml")
 .getExitValue();
 

> Also in the java.se aggregator then it looks like the module comment has a 
> line break after "The module defining" and can probably be fixed as part of 
> this edit.
> 

Fixed.

/**
 * Defines the core Java SE API.
 * 
 * The modules defining CORBA and Java EE APIs are not required by
 * this module, but they are required by {@code java.se.ee}.
 */

Mandy

hg: jigsaw/jake/jdk: Additional test coverage in ImageModules.java

[chris . hegarty]

Changeset: ae2572dcd89a
Author:chegar
Date:  2016-12-14 16:01 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/ae2572dcd89a

Additional test coverage in ImageModules.java

! test/jdk/modules/incubator/ImageModules.java

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[Alan Bateman]

On 14/12/2016 14:30, Jochen Theodorou wrote:


:

I don't intend to get access to hidden API after all... just exported.
If you only interested in accessing the types and members that you have 
access to in these exported packages then you don't need setAccessible 
of course. We have seen a few cases, not many, where code uses 
setAccessible for performance reasons but I don't know if this is what 
you are thinking about here.



Which leaves us with: you want this change and I am unhappy with it. I 
don't see a technical reason for the limit.
Strong encapsulation has always been a goal of this project. If you let 
setAccessible be used to suppress access and get at any member of any 
type then it makes a mockery of that. The initial attempt at a 
compromise in 2015 was to have it fail when attempting to break into any 
member of any type in non-exported packages. That is what has been in 
JDK 9 for some time. The problem with this compromise is that it makes 
it impossible to encapsulate the internal implementation that is in 
exported package - if you read the JSR issue then you'll see the example 
of code breaking into java.lang.invoke.MethodHandles.Lookup to get at 
its private constructor and using that to create full power lookups with 
a lookup classes in non-exported packages. You'll also see the 
unworkable concern that module authors will be forced to move all their 
internals to non-exported packages. This is what 
#AwkwardStrongEncapsulation is about.  Hopefully it's a bit clearer now.


-Alan

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[Jochen Theodorou]

On 13.12.2016 23:17, Peter Levart wrote:
[...]

You might have access to a protected method, but you can not delegate
that access to a 3rd party unless you make the Method object
.setAccessible(true) and pass it to the 3rd party as a capability. (I
recommend using MethodHandle(s) for such delegation of rights instead of
reflection though).


that is unlikely to happen


But let me explain why .setAccessible(true) can't be allowed for
protected members in general.

Jigsaw establishes strong encapsulation. What that means is that even
without a SecurityManager present, code should not be allowed to gain
access to a member beyond what is allowed by accessibility rules of Java
language unless that member is in a class in an open package or such
access is willingly delegated to code by some other code.


I am aware of this. For me it is more a question of how far strong 
encapsulation is supposed to go and you can understand strong 
encapsulation in a module system in different ways as well.


I don't intend to get access to hidden API after all... just exported.

[...]

You can't perform the access check for a protected
instance member without knowing the 'targetClass' (the runtime class of
the target instance).


sure, I already commented on this part with: why do access checks for 
this at all? Your answer is because of strong encapsulation and my 
comment for that is that you guys are maybe overdoing it a bit. Which 
leaves us with: you want this change and I am unhappy with it. I don't 
see a technical reason for the limit.


bye Jochen

Re: RFR 8171070: Test ModuleNamesOrderTest.java fails

[Jim Laskey (Oracle)]

+1

> On Dec 14, 2016, at 9:37 AM, Alan Bateman  wrote:
> 
> On 14/12/2016 13:44, Sundararajan Athijegannathan wrote:
> 
>> Please review http://cr.openjdk.java.net/~sundar/8171070/webrev.00/ for 
>> https://bugs.openjdk.java.net/browse/JDK-8171070
> Looks okay.
> 
> -Alan

Re: RFR 8171070: Test ModuleNamesOrderTest.java fails

[Alan Bateman]

On 14/12/2016 13:44, Sundararajan Athijegannathan wrote:

Please review http://cr.openjdk.java.net/~sundar/8171070/webrev.00/ 
for https://bugs.openjdk.java.net/browse/JDK-8171070

Looks okay.

-Alan

RFR 8171070: Test ModuleNamesOrderTest.java fails

[Sundararajan Athijegannathan]

Please review http://cr.openjdk.java.net/~sundar/8171070/webrev.00/ for 
https://bugs.openjdk.java.net/browse/JDK-8171070


Thanks,
-Sundar

Re: Review Request: JDK-8171201 & JDK-8171202: Drop java.compact$N aggregator modules

[Erik Joelsson]

Build changes look good to me.

/Erik


On 2016-12-14 07:49, Mandy Chung wrote:

JDK-8171201: Drop java.compact$N aggregator modules
JDK-8171202: Rename jdk.crypto.pkcs11 and jdk.pack200 to end with Java letters

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171201%2b8171202/webrev.00/

Compact Profiles were introduced in SE 8.  The java.compact$N
aggregator module in SE 9 module graph gives unnecessary
importance going forward.  We want people who care about
small systems to think about modules, not profiles.

In addition, the compact profile builds use these aggregator
modules but those builds still have to pull in the necessary
service providers manually, so they aren't a complete solution.

We propose to drop java.compact1, java.compact2, java.compact3
aggregator modules.

This patch also includes the change for JDK-8171202 to rename
jdk.crypto.pkcs11 to jdk.crypto.token and jdk.pack200 to jdk.pack
to prepare the stronger constraint that module names must end in
Java letters [1]

Mandy
[1] 
http://mail.openjdk.java.net/pipermail/jpms-spec-observers/2016-December/000682.html

hg: jigsaw/jake/langtools: Missing target modules of qualified exports should produce a warning rather than an error.

[jan . lahoda]

Changeset: 5c6049f251e7
Author:jlahoda
Date:  2016-12-14 12:51 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/5c6049f251e7

Missing target modules of qualified exports should produce a warning rather 
than an error.

! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
! 
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
+ test/tools/javac/diags/examples/WarnModuleNotFound/WarnModuleNotFound.java
+ 
test/tools/javac/diags/examples/WarnModuleNotFound/modulesourcepath/m/api/Api.java
+ 
test/tools/javac/diags/examples/WarnModuleNotFound/modulesourcepath/m/module-info.java
! test/tools/javac/modules/ModuleInfoTest.java

Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

[Chris Hegarty]

Webrev updated in-place.
  http://cr.openjdk.java.net/~chegar/Unsafe_invokeCleaner/

-Chris.

On 13/12/16 21:18, Peter Levart wrote:

I think this is OK.

Just a couple of nits in test:

1. You create a static Path bob = Paths.get("bob") field, but then you
don't use it in:

  56 try (FileChannel fc = FileChannel.open(Paths.get("bob"),
CREATE, WRITE)) {

2. badBuffers could include a duplicate and a slice of a direct buffer
allocated with ByteBuffer.allocateDirect()

3. The comment in the test is referencing the old method name:

  26  * @summary Basic test for Unsafe::deallocate


Regards, Peter

On 12/13/2016 08:47 PM, Chris Hegarty wrote:

Taking into account the feedback so far, and changing the method name ( since
it is an attractive nuisance ), here is where I think we ended up.

http://cr.openjdk.java.net/~chegar/Unsafe_invokeCleaner/

If this is agreeable, I’ll file an issue in JIRA to track the code changes, and
update JEP 260.

-Chris.

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[David Holmes]

On 14/12/2016 8:15 PM, Alan Bateman wrote:

On 14/12/2016 09:56, David Holmes wrote:



Sorry I don't follow. Calling setAccessible(true), logically does
nothing except set a flag that controls whether reflective use of the
member will need to pass an access check -it doesn't (or shouldn't)
perform the access check!

Best to read the javadoc as this changed to align with modules. When
reading the javadoc then keep in mind that the JDK modules do not open
their packages for deep reflection.


Yes - thanks Alan. I've only just noticed that setAccessible has been 
updated to check the accessibility and throw an exception! That came as 
a surprise. It doesn't have all the info needed to make the decision in 
all cases.


David
-


The actual use then has the full access check. Due to the added
complication of module encapsulation we can't simply skip the access
check if setAccessible is true, because we also need to see if the
target module is "open" in the right way.

If setAccessible(true) succeeds then it means access checks are
suppressed when using that AccessibleObject is used. There are no
changes here. Having setAccessible(true) succeed but still do an access
check when using the AccessibleObject would be too significant a change
in behavior.




If the current implementation is trying to cache the result of the
accessibility check at setAccessible time then that would seem an
invalid attempt at optimisation. java.lang.reflect objects are not,
AFAIK, intended to be capabilities the way MethodHandles are.

I don't see an issue here as it is not used when the accessible flag has
been set with setAccessible(true).



BTW in this discussion when setAccessible says that it is "suppressing
default Java language access control checks" are the module related
checks considered "language access control checks" or are they
considered something outside the language?

The former.

-Alan

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[Alan Bateman]

On 14/12/2016 09:56, David Holmes wrote:



Sorry I don't follow. Calling setAccessible(true), logically does 
nothing except set a flag that controls whether reflective use of the 
member will need to pass an access check -it doesn't (or shouldn't) 
perform the access check!
Best to read the javadoc as this changed to align with modules. When 
reading the javadoc then keep in mind that the JDK modules do not open 
their packages for deep reflection.


The actual use then has the full access check. Due to the added 
complication of module encapsulation we can't simply skip the access 
check if setAccessible is true, because we also need to see if the 
target module is "open" in the right way.
If setAccessible(true) succeeds then it means access checks are 
suppressed when using that AccessibleObject is used. There are no 
changes here. Having setAccessible(true) succeed but still do an access 
check when using the AccessibleObject would be too significant a change 
in behavior.





If the current implementation is trying to cache the result of the 
accessibility check at setAccessible time then that would seem an 
invalid attempt at optimisation. java.lang.reflect objects are not, 
AFAIK, intended to be capabilities the way MethodHandles are.
I don't see an issue here as it is not used when the accessible flag has 
been set with setAccessible(true).




BTW in this discussion when setAccessible says that it is "suppressing 
default Java language access control checks" are the module related 
checks considered "language access control checks" or are they 
considered something outside the language?

The former.

-Alan

Re: hg: jigsaw/jake/langtools: enable -Xlint:module by default

[Remi Forax]

A fine idea.

Rémi

- Mail original -
> De: "jonathan gibbons" 
> À: jigsaw-dev@openjdk.java.net
> Envoyé: Mercredi 14 Décembre 2016 04:23:12
> Objet: hg: jigsaw/jake/langtools: enable -Xlint:module by default

> Changeset: 4a200166396d
> Author:jjg
> Date:  2016-12-13 19:23 -0800
> URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/4a200166396d
> 
> enable -Xlint:module by default
> 
> ! src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java

Re: Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

[Alan Bateman]

On 14/12/2016 07:30, Peter Levart wrote:

Hi David,

On 12/14/2016 07:17 AM, David Holmes wrote:

But let me explain why .setAccessible(true) can't be allowed for
protected members in general.


I'm confused as to what is being argued for/against here. 


Rony asked why .setAccessible(true) can't be used for protected 
members even if called from a subclass of the member's declaring class.
Right, I think it would be helpful if Rory could paste in a stack trace 
from where setAccessible is failing so that there are specifics to discuss.


-Alan

Re: Review Request: JDK-8171201 & JDK-8171202: Drop java.compact$N aggregator modules

[Alan Bateman]

On 14/12/2016 06:49, Mandy Chung wrote:


JDK-8171201: Drop java.compact$N aggregator modules
JDK-8171202: Rename jdk.crypto.pkcs11 and jdk.pack200 to end with Java letters

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171201%2b8171202/webrev.00/


The hg moves are showing up as new files in the webrev - is that because 
the webrev is on several repos?


In the LimitModsTest test then it might be simpler to leave 
java.scripting out of the test. Also in the java.se aggregator then it 
looks like the module comment has a line break after "The module 
defining" and can probably be fixed as part of this edit.


Otherwise looks good to me.

-Alan