Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-23 Thread Mandy Chung

> On May 23, 2016, at 8:05 AM, Chris Hegarty  wrote:
> 
> Updated webrev:
>  http://cr.openjdk.java.net/~chegar/8156497.01/


Looks fine to me.

Minor comment on the test, “9” is hardcoded in a few places.  It might be good 
to prepare for the future release e.g. use Runtime.version().major() + 1 
instead.

e.g.
 257 "-C", mrjarDir.toString(), 
"META-INF/versions/9/module-info.class”,
 760 Path versionSection = metaInfDir.resolve("versions").resolve("9");

Mandy

Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-23 Thread Alan Bateman



On 23/05/2016 16:05, Chris Hegarty wrote:


D'oh. Fixed.

Updated webrev:
  http://cr.openjdk.java.net/~chegar/8156497.01/

This version looks good.

-Alan


Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-23 Thread Chris Hegarty

On 20/05/16 22:24, Alan Bateman wrote:

On 20/05/2016 16:55, Chris Hegarty wrote:

:

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

Note: while there are some new test scenarios added, which give
reasonable coverage, further tests will be added later. Steve has some
additional jar tools support coming for easier creation of MRJARS.



The checks looks right.

In checkModuleInfos then I assume you can use findFirst instead of
collecting the module-infos into a set.


Yes, of course. done


The comment in computeHashes is confusing - too many "as" :-)


Updated.


You might want to re-read the error messages to see if they can be
improved. For example "Invalid versioned module-info.class ..." might be
better as "module-info.class in versioned section ...". Another one is
"Unexpected versioned module-info.class without root module-info.class"
where it might be clearer to say "module-info.class found in versions
section of JAR file, no module-info.class found in root directory" or
something like that.


That is certainly better. done.


Looks like the copyright headers have the Classpath exception so now
might be a good time to fix those.


D'oh. Fixed.

Updated webrev:
  http://cr.openjdk.java.net/~chegar/8156497.01/

-Chris.


Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-20 Thread Alan Bateman

On 20/05/2016 16:55, Chris Hegarty wrote:

:

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

Note: while there are some new test scenarios added, which give
reasonable coverage, further tests will be added later. Steve has some
additional jar tools support coming for easier creation of MRJARS.



The checks looks right.

In checkModuleInfos then I assume you can use findFirst instead of 
collecting the module-infos into a set.


The comment in computeHashes is confusing - too many "as" :-)

You might want to re-read the error messages to see if they can be 
improved. For example "Invalid versioned module-info.class ..." might be 
better as "module-info.class in versioned section ...". Another one is 
"Unexpected versioned module-info.class without root module-info.class" 
where it might be clearer to say "module-info.class found in versions 
section of JAR file, no module-info.class found in root directory" or 
something like that.


Looks like the copyright headers have the Classpath exception so now 
might be a good time to fix those.


-Alan


Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-20 Thread Chris Hegarty

> On 20 May 2016, at 17:01, Jonathan Gibbons  
> wrote:
> 
> It would be good if (eventually) there were test cases involving the use
> of MRM jars, both via the JarFile API and jar-fs file system, perhaps
> extending to the use of MRM jars in javac as well.

I agree.

Just to note, the updated test in the webrev is already exercising the JarFile
API, indirectly. Support for Modular JARs on the module path uses the 
JarFile constructor that accepts a JarFile.Release value when opening.
The test exercises this when it verifies the newly created, or updated, JAR
file by running the module’s entry point.

But I agree, more test scenarios will need to be added.

-Chris.

> -- Jon
> 
> On 05/20/2016 08:55 AM, Chris Hegarty wrote:
>> What do you get if you mix JEP 261 [1] with JEP 238 [2]?
>> A Multi-release modular JAR.
>> 
>> This issue proposes to add support to the jar tool for creating and
>> updating modular JAR files with an optional module-info.class in the
>> versioned section.
>> 
>> MRJARs are intended to target multiple releases of the Java platform, so
>> it seems reasonable for a Multi-release modular JAR ( MRMJAR ) to be
>> able to declare a different set of dependencies on modules that are part
>> of the Java platform. The reasoning here is that these are
>> implementation details rather than parts of a module's API surface, and
>> that one may well want to change them as the JDK itself evolves. The
>> runtime already has support for loading from the versioned section.
>> 
>> Specifically, a versioned module descriptor must be identical to the
>> root module descriptor, with two exceptions:
>> 
>> - A versioned descriptor can have different non-public `requires`
>> clauses of platform ( `java.*` and `jdk.*` ) modules, and
>> 
>> - A versioned descriptor can have different `uses` clauses, even of
>> service types defined outside of `java.*` and `jdk.*` modules.
>> 
>> http://cr.openjdk.java.net/~chegar/8156497.00/
>> 
>> Note: while there are some new test scenarios added, which give
>> reasonable coverage, further tests will be added later. Steve has some
>> additional jar tools support coming for easier creation of MRJARS.
>> 
>> -Chris.
>> 
>> [1] http://openjdk.java.net/jeps/261
>> [2] http://openjdk.java.net/jeps/238
> 



Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-20 Thread Alexandre (Shura) Iline
Chris,

Does the test need to use @modules for jdk.jartool and java.compiler?

Shura

> On May 20, 2016, at 8:55 AM, Chris Hegarty  wrote:
> 
> What do you get if you mix JEP 261 [1] with JEP 238 [2]?
> A Multi-release modular JAR.
> 
> This issue proposes to add support to the jar tool for creating and
> updating modular JAR files with an optional module-info.class in the
> versioned section.
> 
> MRJARs are intended to target multiple releases of the Java platform, so
> it seems reasonable for a Multi-release modular JAR ( MRMJAR ) to be
> able to declare a different set of dependencies on modules that are part
> of the Java platform. The reasoning here is that these are
> implementation details rather than parts of a module's API surface, and
> that one may well want to change them as the JDK itself evolves. The
> runtime already has support for loading from the versioned section.
> 
> Specifically, a versioned module descriptor must be identical to the
> root module descriptor, with two exceptions:
> 
> - A versioned descriptor can have different non-public `requires`
> clauses of platform ( `java.*` and `jdk.*` ) modules, and
> 
> - A versioned descriptor can have different `uses` clauses, even of
> service types defined outside of `java.*` and `jdk.*` modules.
> 
> http://cr.openjdk.java.net/~chegar/8156497.00/
> 
> Note: while there are some new test scenarios added, which give
> reasonable coverage, further tests will be added later. Steve has some
> additional jar tools support coming for easier creation of MRJARS. 
> 
> -Chris.
> 
> [1] http://openjdk.java.net/jeps/261
> [2] http://openjdk.java.net/jeps/238



Re: RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-20 Thread Jonathan Gibbons

It would be good if (eventually) there were test cases involving the use
of MRM jars, both via the JarFile API and jar-fs file system, perhaps
extending to the use of MRM jars in javac as well.

-- Jon

On 05/20/2016 08:55 AM, Chris Hegarty wrote:

What do you get if you mix JEP 261 [1] with JEP 238 [2]?
A Multi-release modular JAR.

This issue proposes to add support to the jar tool for creating and
updating modular JAR files with an optional module-info.class in the
versioned section.

MRJARs are intended to target multiple releases of the Java platform, so
it seems reasonable for a Multi-release modular JAR ( MRMJAR ) to be
able to declare a different set of dependencies on modules that are part
of the Java platform. The reasoning here is that these are
implementation details rather than parts of a module's API surface, and
that one may well want to change them as the JDK itself evolves. The
runtime already has support for loading from the versioned section.

Specifically, a versioned module descriptor must be identical to the
root module descriptor, with two exceptions:

- A versioned descriptor can have different non-public `requires`
clauses of platform ( `java.*` and `jdk.*` ) modules, and

- A versioned descriptor can have different `uses` clauses, even of
service types defined outside of `java.*` and `jdk.*` modules.

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

Note: while there are some new test scenarios added, which give
reasonable coverage, further tests will be added later. Steve has some
additional jar tools support coming for easier creation of MRJARS.

-Chris.

[1] http://openjdk.java.net/jeps/261
[2] http://openjdk.java.net/jeps/238




RFR 8156497: Add jar tool support for Multi-release modular JARs

2016-05-20 Thread Chris Hegarty
What do you get if you mix JEP 261 [1] with JEP 238 [2]?
A Multi-release modular JAR.

This issue proposes to add support to the jar tool for creating and
updating modular JAR files with an optional module-info.class in the
versioned section.

MRJARs are intended to target multiple releases of the Java platform, so
it seems reasonable for a Multi-release modular JAR ( MRMJAR ) to be
able to declare a different set of dependencies on modules that are part
of the Java platform. The reasoning here is that these are
implementation details rather than parts of a module's API surface, and
that one may well want to change them as the JDK itself evolves. The
runtime already has support for loading from the versioned section.

Specifically, a versioned module descriptor must be identical to the
root module descriptor, with two exceptions:

- A versioned descriptor can have different non-public `requires`
clauses of platform ( `java.*` and `jdk.*` ) modules, and

- A versioned descriptor can have different `uses` clauses, even of
service types defined outside of `java.*` and `jdk.*` modules.

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

Note: while there are some new test scenarios added, which give
reasonable coverage, further tests will be added later. Steve has some
additional jar tools support coming for easier creation of MRJARS. 

-Chris.

[1] http://openjdk.java.net/jeps/261
[2] http://openjdk.java.net/jeps/238