Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Mandy Chung 

> On Aug 22, 2016, at 2:56 PM, Steve Drach  wrote:
> 
> Is a jar tool that takes a -d option lurking somewhere?  The latest one I 
> have does not have that option.

It’s in jdk-9+132 and has been in jdk9/dev for 1.5 weeks (see JDK-8136930).  It 
was -p previously.

$ jar -h   
Usage: jar [OPTION...] [ [--release VERSION] [-C dir] files] …
:
:

 Main operation mode:

  -c, --create   Create the archive
  -i, --generate-index=FILE  Generate index information for the specified jar
 archives
  -t, --list List the table of contents for the archive
  -u, --update   Update an existing jar archive
  -x, --extract  Extract named (or all) files from the archive
  -d, --print-module-descriptor  Print the module descriptor

:

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Steve Drach 
 Er, I thought the plan was for the set of concealed packages to be the 
 same. It's okay for the ConcealedPackages in the base section to include 
 "empty" packages.
>>> 
>>> I wasn’t involved with that decision.  Chris wrote that code, perhaps he 
>>> can comment.
>> 
>> This surprises me, as I have the same recollection as Alan; no additional
>> concealed packages are allowable in the versioned section.
>> 
>> I just checked the jar tool, and it does NOT add any versioned specific
>> concealed packages.
> 
> 
> If a JAR file contains an empty directory, should `q` be considered as a 
> concealed package in the base section?
> 
> $ jar -t --file hi.jar
> META-INF/
> META-INF/MANIFEST.MF
> module-info.class
> p/Hi.class
> q/
> 
> currently not:
> 
> $ jar -d --file mr.jar
> hi
>  requires mandated java.base
>  conceals p

Is a jar tool that takes a -d option lurking somewhere?  The latest one I have 
does not have that option.

hg: jigsaw/jake/langtools: 20 new changesets

2016-08-22 Thread jonathan . gibbons 
Changeset: 0f8b6aba6279
Author:ksrini
Date:  2016-08-09 07:31 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/0f8b6aba6279

8075529: Documentation in DocumentationTool.getTask(...) should mention about 
"null" parameter for doclet.
Reviewed-by: jjg

! src/java.compiler/share/classes/javax/tools/DocumentationTool.java

Changeset: d7aa2b610144
Author:jlahoda
Date:  2016-08-09 20:27 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/d7aa2b610144

8143048: Re-examine dependency on property sun.boot.class.path
Summary: Removing obsolete references to sun.boot.class.path
Reviewed-by: jjg

! src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java
! test/tools/javac/processing/messager/MessagerDiags.java

Changeset: 99470b5ea8d8
Author:jjg
Date:  2016-08-09 13:22 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/99470b5ea8d8

8160697: HTMLWriter needs perf cleanup
Reviewed-by: ksrini, bpatel

! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractIndexWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractModuleIndexWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractPackageIndexWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractTreeWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AllClassesFrameWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AnnotationTypeFieldWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AnnotationTypeOptionalMemberWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AnnotationTypeRequiredMemberWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AnnotationTypeWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConfigurationImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstantsSummaryWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriterImpl.java
+ 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Contents.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/EnumConstantWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/FieldWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/FrameOutputWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HelpWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDoclet.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/MethodWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleFrameWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleIndexFrameWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleIndexWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModulePackageIndexFrameWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NestedClassWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageFrameWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageIndexFrameWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageIndexWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageTreeWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageUseWriter.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PropertyWriterImpl.java
! 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SerializedFormWriterImpl.java
!

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Alan Bateman 



On 22/08/2016 22:04, Mandy Chung wrote:

:


If a JAR file contains an empty directory, should `q` be considered as a 
concealed package in the base section?

$ jar -t --file hi.jar
META-INF/
META-INF/MANIFEST.MF
module-info.class
p/Hi.class
q/

currently not:

$ jar -d --file mr.jar
hi
   requires mandated java.base
   conceals p


which is correct. I think the configuration to try is a MR JAR containing:

module-info.class
p/Hi.class
META-INF/versions/9/p/Hi.class
META-INF/versions/9/p/internal/Hi5.class

Then repeat with a META-INF/versions/9/module-info.class.

In all cases then I would assume p.internal should be in the set of 
concealed packages.


-Alan

-Alan

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Mandy Chung 

> On Aug 22, 2016, at 1:35 PM, Chris Hegarty  wrote:
> 
> 
>> On 22 Aug 2016, at 19:24, Steve Drach  wrote:
>>> 
>>> Er, I thought the plan was for the set of concealed packages to be the 
>>> same. It's okay for the ConcealedPackages in the base section to include 
>>> "empty" packages.
>> 
>> I wasn’t involved with that decision.  Chris wrote that code, perhaps he can 
>> comment.
> 
> This surprises me, as I have the same recollection as Alan; no additional
> concealed packages are allowable in the versioned section.
> 
> I just checked the jar tool, and it does NOT add any versioned specific
> concealed packages.


If a JAR file contains an empty directory, should `q` be considered as a 
concealed package in the base section?

$ jar -t --file hi.jar
META-INF/
META-INF/MANIFEST.MF
module-info.class
p/Hi.class
q/

currently not:

$ jar -d --file mr.jar
hi
  requires mandated java.base
  conceals p

Mandy

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Steve Drach 
 It exists purely because another class in the same versioned directory 
 depends on it.  If we are creating a versionedStream for the version that 
 the non-public class is in, it will be in finalNames, otherwise it won’t 
 be.  I believe the code is correct here.
 
 New concealed packages can be added in a versioned section of the jar file 
 created by jar tool.  Should classes in concealed packages be added to 
 finalNames or not?  Or stated differently, for jlink, should a 
 versionedStream contain entries in concealed packages?
 
>>> Er, I thought the plan was for the set of concealed packages to be the 
>>> same. It's okay for the ConcealedPackages in the base section to include 
>>> "empty" packages.
>> 
>> I wasn’t involved with that decision.  Chris wrote that code, perhaps he can 
>> comment.
> 
> This surprises me, as I have the same recollection as Alan; no additional
> concealed packages are allowable in the versioned section.
> 
> I just checked the jar tool, and it does NOT add any versioned specific
> concealed packages.

You’re right.  Apparently when I read the code this morning, I hadn’t had 
enough coffee yet ;-)

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Chris Hegarty 

> On 22 Aug 2016, at 19:24, Steve Drach  wrote:
>> ...
>>> It exists purely because another class in the same versioned directory 
>>> depends on it.  If we are creating a versionedStream for the version that 
>>> the non-public class is in, it will be in finalNames, otherwise it won’t 
>>> be.  I believe the code is correct here.
>>> 
>>> New concealed packages can be added in a versioned section of the jar file 
>>> created by jar tool.  Should classes in concealed packages be added to 
>>> finalNames or not?  Or stated differently, for jlink, should a 
>>> versionedStream contain entries in concealed packages?
>>> 
>> Er, I thought the plan was for the set of concealed packages to be the same. 
>> It's okay for the ConcealedPackages in the base section to include "empty" 
>> packages.
> 
> I wasn’t involved with that decision.  Chris wrote that code, perhaps he can 
> comment.

This surprises me, as I have the same recollection as Alan; no additional
concealed packages are allowable in the versioned section.

I just checked the jar tool, and it does NOT add any versioned specific
concealed packages.

-Chris.

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Steve Drach 
>> If there is a class in a versioned directory that is not in the base 
>> directory, then that class must not be public or protected.  It’s not part 
>> of the public interface of the multi-release jar.
> It's also a modular JAR and so if the class is not in an exported package 
> then it's okay for it to be public.

So, if a class is not in an exported package, it’s in a concealed package, is 
that right?  So, should any classes in a concealed package, public or not, be 
included in the versionedStream used in jlink?

> 
> 
>> It exists purely because another class in the same versioned directory 
>> depends on it.  If we are creating a versionedStream for the version that 
>> the non-public class is in, it will be in finalNames, otherwise it won’t be. 
>>  I believe the code is correct here.
>> 
>> New concealed packages can be added in a versioned section of the jar file 
>> created by jar tool.  Should classes in concealed packages be added to 
>> finalNames or not?  Or stated differently, for jlink, should a 
>> versionedStream contain entries in concealed packages?
>> 
> Er, I thought the plan was for the set of concealed packages to be the same. 
> It's okay for the ConcealedPackages in the base section to include "empty" 
> packages.

I wasn’t involved with that decision.  Chris wrote that code, perhaps he can 
comment.

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Alan Bateman 

On 22/08/2016 18:47, Steve Drach wrote:


:
If there is a class in a versioned directory that is not in the base directory, 
then that class must not be public or protected.  It’s not part of the public 
interface of the multi-release jar.
It's also a modular JAR and so if the class is not in an exported 
package then it's okay for it to be public.




It exists purely because another class in the same versioned directory depends 
on it.  If we are creating a versionedStream for the version that the 
non-public class is in, it will be in finalNames, otherwise it won’t be.  I 
believe the code is correct here.

New concealed packages can be added in a versioned section of the jar file 
created by jar tool.  Should classes in concealed packages be added to 
finalNames or not?  Or stated differently, for jlink, should a versionedStream 
contain entries in concealed packages?

Er, I thought the plan was for the set of concealed packages to be the 
same. It's okay for the ConcealedPackages in the base section to include 
"empty" packages.


-Alan

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

2016-08-22 Thread Steve Drach 
>> I didn’t have it right ;-)  It turns out a JarFile stream of versioned 
>> entries was more interesting than I initially thought.  Here’s another 
>> webrev.  It’s not clear to me if I should include the change to JarFile in 
>> this changeset or if it should be in a stand alone changeset.  Advice 
>> appreciated.
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8156499/webrev.02/ 
>> 
> 
> 
> I think it’s time to enumerate several test cases of a multi-release Jar 
> content, e.g. a MRJAR with a new concealed package in versioned entries but 
> not in the base entry (I believe that’s allowed?? version 9 & 10 entries that 
> makes sure versioned 10 entries are skipped.  It should also check the 
> module-info.class with different requires and ensure that the image jlink 
> created contains the module-info.class of the right version.  This would help 
> the review of this change.

Okay, I can do that.

> 
> 
> I think ModularJarArchive is the right place to add the versioned entries 
> support.

Okay.

>  JarArchive should probably be renamed to ZipArchive (that’s an existing 
> code).

Just curious, why?  Note the processing of the module-info.class.  Isn’t that 
more appropriate for a JarArchive rather than a ZipArchive?  Seems to me we 
should just change the internal instances of ZipFile to JarFile and change the 
name zipFile to jarFile for consistency.

> 
> Version of JarFile
> 
> 221 jarFile = new JarFile(file.toFile(), true, ZipFile.OPEN_READ, 
> JarFile.runtimeVersion());
> 
> JarFile::runtimeVersion is the version of the jlink tool.  You should not use 
> the version of the jlink runtime.  Instead, this should use the version of 
> java.base which will be the runtime version of the image being created.
> 
> ImageHelper::newArchive is one place where it creates JarArchive. You can 
> find java.base module from the Configuration passed to ImageHelper 
> constructor via Configuration.findModule(“java.base”) and from its module 
> descriptor.  Jlink implementation is rather over engineering at the moment.  
> You will have to find if there are other places to pass the right versoin 
> when creating ModularJarArchive.

As Alan noted, this is intended for phase 2 after he adds some apparently 
necessary code.

> 
> 
> 179 // a legal multi-release jar always has a base entry
> 
> I thought that a new class or a new concealed package can be added in a 
> versioned entry in MRJAR.  If so, those cases will not be included in the 
> finalNames.

If there is a class in a versioned directory that is not in the base directory, 
then that class must not be public or protected.  It’s not part of the public 
interface of the multi-release jar.  It exists purely because another class in 
the same versioned directory depends on it.  If we are creating a 
versionedStream for the version that the non-public class is in, it will be in 
finalNames, otherwise it won’t be.  I believe the code is correct here.

New concealed packages can be added in a versioned section of the jar file 
created by jar tool.  Should classes in concealed packages be added to 
finalNames or not?  Or stated differently, for jlink, should a versionedStream 
contain entries in concealed packages?

> 
> Have you considered adding a new method in JarFile to return the versioned 
> entry name and/or the version?  If it’s the base version, it will return the 
> same value as JarFile::getName.

There is a package-private method in JarFile that allows one to get the real 
name of a JarEntry.  It’s accessed externally by using SharedSecrets.  As far 
as i know it’s only used in two places.  The JEP-238 team decided not to make 
it a public method, although I think we could be persuaded to change our minds.

>  As we discussed the jdeps support for MRJAR offline, a tool would be 
> interested in getting the base entry name, versioned entry name, or even 
> version.

JarFile::getVersion exists.  And, as mentioned above, it possible to get a base 
name and a real name for a JarEntry.

> 
> Mandy