Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Alexandre (Shura) Iline]

> On May 5, 2016, at 12:12 PM, Jonathan Gibbons  
> wrote:
> 
> There is potentially a separate discussion here, as to whether javac should 
> "force" the provision of a jar-fs provider.
> 
> Strictly speaking, javac does not inherently require it.  You can use javac 
> just fine, with files in the system image, and source and class files in the 
> default file system.  But I suspect most folk would be suprised if they ca 
> across an image for which javac could not read jar files.

What would definitely help is a error message which specifies the jar name. At 
least in my case that would, as I would not miss the message.

Shura

> 
> -- Jon
> 
> On 05/05/2016 12:01 PM, Alexandre (Shura) Iline wrote:
>>> On May 5, 2016, at 11:42 AM, Alexandre (Shura) Iline 
>>>  wrote:
>>> Whether the java.tools API behavior is correct is a separate matter. I am 
>>> planning to create a standalone test case and take it with javac ppl.
>> I take this ^^ back, as the error was there all along: 
>> "java.nio.file.ProviderNotFoundException: no provider found for .jar files”
>> 
>> The fix is valid, then, is and waiting for a review.
>> 
>> Shura
>> 
>>> Thank you.
>>> 
>>> Shura
>>> 
 On May 4, 2016, at 8:30 AM, Chris Hegarty  wrote:
 
 On 4 May 2016, at 14:32, Alan Bateman  wrote:
> On 04/05/2016 11:24, Chris Hegarty wrote:
>> :
>> The tests cause compilation of test library classes, but only some tests
>> actually use the methods that provoke compilation. Similar to above, 
>> tests
>> that don’t actually compile anything could depend on just java.compiler.
>> 
>> This is all to fragile and may cause problems with future refactoring. I
>> think we should add the same set of @moduels to all these tests, rather
>> than an individual set determined by intimate knowledge of the inner
>> workings of the test.
>> 
>> @modules java.compiler
>>  jdk.compiler
>>  jdk.zipfs
>>  jdk.jartool
>> 
>> with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.
>> 
> or we could move the tests into their own MultiRelease sub-directory and 
> create a TEST.properties with a module key. That would allow these tests 
> to drop @modules, except the test that uses the HTTP server.
 I think that would be better.
 
 -Chris.
> 

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Chris Hegarty]

On 05/05/16 19:42, Alexandre (Shura) Iline wrote:

Chris, could you please take another look:
http://cr.openjdk.java.net/~shurailine/8151914/webrev.02/


This looks ok to me Shura, maybe just 'mrjar' for the directory
name?

Since there are file moves can you please prepare the changeset,
and I will push it for you.

-Chris.



What I have discovered is that jdk.zipfs was used to access jars on the 
classpath, which were JTreg jars: jtreg.jar, testing.jar, etc. Cleaning the 
class path through the environment removed dependency on the zipfs.

Whether the java.tools API behavior is correct is a separate matter. I am 
planning to create a standalone test case and take it with javac ppl.

Thank you.

Shura


On May 4, 2016, at 8:30 AM, Chris Hegarty  wrote:

On 4 May 2016, at 14:32, Alan Bateman  wrote:


On 04/05/2016 11:24, Chris Hegarty wrote:

:
The tests cause compilation of test library classes, but only some tests
actually use the methods that provoke compilation. Similar to above, tests
that don’t actually compile anything could depend on just java.compiler.

This is all to fragile and may cause problems with future refactoring. I
think we should add the same set of @moduels to all these tests, rather
than an individual set determined by intimate knowledge of the inner
workings of the test.

@modules java.compiler
   jdk.compiler
   jdk.zipfs
   jdk.jartool

with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.


or we could move the tests into their own MultiRelease sub-directory and create 
a TEST.properties with a module key. That would allow these tests to drop 
@modules, except the test that uses the HTTP server.


I think that would be better.

-Chris.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Jonathan Gibbons]

There is potentially a separate discussion here, as to whether javac 
should "force" the provision of a jar-fs provider.


Strictly speaking, javac does not inherently require it.  You can use 
javac just fine, with files in the system image, and source and class 
files in the default file system.  But I suspect most folk would be 
suprised if they ca across an image for which javac could not read jar 
files.


-- Jon

On 05/05/2016 12:01 PM, Alexandre (Shura) Iline wrote:

On May 5, 2016, at 11:42 AM, Alexandre (Shura) Iline 
 wrote:
Whether the java.tools API behavior is correct is a separate matter. I am 
planning to create a standalone test case and take it with javac ppl.

I take this ^^ back, as the error was there all along: 
"java.nio.file.ProviderNotFoundException: no provider found for .jar files”

The fix is valid, then, is and waiting for a review.

Shura


Thank you.

Shura


On May 4, 2016, at 8:30 AM, Chris Hegarty  wrote:

On 4 May 2016, at 14:32, Alan Bateman  wrote:

On 04/05/2016 11:24, Chris Hegarty wrote:

:
The tests cause compilation of test library classes, but only some tests
actually use the methods that provoke compilation. Similar to above, tests
that don’t actually compile anything could depend on just java.compiler.

This is all to fragile and may cause problems with future refactoring. I
think we should add the same set of @moduels to all these tests, rather
than an individual set determined by intimate knowledge of the inner
workings of the test.

@modules java.compiler
  jdk.compiler
  jdk.zipfs
  jdk.jartool

with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.


or we could move the tests into their own MultiRelease sub-directory and create 
a TEST.properties with a module key. That would allow these tests to drop 
@modules, except the test that uses the HTTP server.

I think that would be better.

-Chris.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Alexandre (Shura) Iline]

> On May 5, 2016, at 11:42 AM, Alexandre (Shura) Iline 
>  wrote:
> Whether the java.tools API behavior is correct is a separate matter. I am 
> planning to create a standalone test case and take it with javac ppl.
I take this ^^ back, as the error was there all along: 
"java.nio.file.ProviderNotFoundException: no provider found for .jar files”

The fix is valid, then, is and waiting for a review.

Shura

> 
> Thank you.
> 
> Shura
> 
>> On May 4, 2016, at 8:30 AM, Chris Hegarty  wrote:
>> 
>> On 4 May 2016, at 14:32, Alan Bateman  wrote:
>>> 
>>> On 04/05/2016 11:24, Chris Hegarty wrote:
 :
 The tests cause compilation of test library classes, but only some tests
 actually use the methods that provoke compilation. Similar to above, tests
 that don’t actually compile anything could depend on just java.compiler.
 
 This is all to fragile and may cause problems with future refactoring. I
 think we should add the same set of @moduels to all these tests, rather
 than an individual set determined by intimate knowledge of the inner
 workings of the test.
 
 @modules java.compiler
  jdk.compiler
  jdk.zipfs
  jdk.jartool
 
 with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.
 
>>> or we could move the tests into their own MultiRelease sub-directory and 
>>> create a TEST.properties with a module key. That would allow these tests to 
>>> drop @modules, except the test that uses the HTTP server.
>> 
>> I think that would be better.
>> 
>> -Chris.
> 

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Alexandre (Shura) Iline]

Chris, could you please take another look:
http://cr.openjdk.java.net/~shurailine/8151914/webrev.02/

What I have discovered is that jdk.zipfs was used to access jars on the 
classpath, which were JTreg jars: jtreg.jar, testing.jar, etc. Cleaning the 
class path through the environment removed dependency on the zipfs. 

Whether the java.tools API behavior is correct is a separate matter. I am 
planning to create a standalone test case and take it with javac ppl.

Thank you.

Shura

> On May 4, 2016, at 8:30 AM, Chris Hegarty  wrote:
> 
> On 4 May 2016, at 14:32, Alan Bateman  wrote:
>> 
>> On 04/05/2016 11:24, Chris Hegarty wrote:
>>> :
>>> The tests cause compilation of test library classes, but only some tests
>>> actually use the methods that provoke compilation. Similar to above, tests
>>> that don’t actually compile anything could depend on just java.compiler.
>>> 
>>> This is all to fragile and may cause problems with future refactoring. I
>>> think we should add the same set of @moduels to all these tests, rather
>>> than an individual set determined by intimate knowledge of the inner
>>> workings of the test.
>>> 
>>> @modules java.compiler
>>>   jdk.compiler
>>>   jdk.zipfs
>>>   jdk.jartool
>>> 
>>> with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.
>>> 
>> or we could move the tests into their own MultiRelease sub-directory and 
>> create a TEST.properties with a module key. That would allow these tests to 
>> drop @modules, except the test that uses the HTTP server.
> 
> I think that would be better.
> 
> -Chris.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Alexandre (Shura) Iline]

This makes sense - I will move the tests into a subduer, put the dependencies 
into a TEST.properties file.

jdk.zipfs has the code needed for access jars - the tests are failing without 
that dependency.

Shura
> On May 4, 2016, at 8:30 AM, Chris Hegarty  wrote:
> 
> On 4 May 2016, at 14:32, Alan Bateman  wrote:
>> 
>> On 04/05/2016 11:24, Chris Hegarty wrote:
>>> :
>>> The tests cause compilation of test library classes, but only some tests
>>> actually use the methods that provoke compilation. Similar to above, tests
>>> that don’t actually compile anything could depend on just java.compiler.
>>> 
>>> This is all to fragile and may cause problems with future refactoring. I
>>> think we should add the same set of @moduels to all these tests, rather
>>> than an individual set determined by intimate knowledge of the inner
>>> workings of the test.
>>> 
>>> @modules java.compiler
>>>   jdk.compiler
>>>   jdk.zipfs
>>>   jdk.jartool
>>> 
>>> with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.
>>> 
>> or we could move the tests into their own MultiRelease sub-directory and 
>> create a TEST.properties with a module key. That would allow these tests to 
>> drop @modules, except the test that uses the HTTP server.
> 
> I think that would be better.
> 
> -Chris.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Alan Bateman]

On 04/05/2016 11:24, Chris Hegarty wrote:

:
The tests cause compilation of test library classes, but only some tests
actually use the methods that provoke compilation. Similar to above, tests
that don’t actually compile anything could depend on just java.compiler.

This is all to fragile and may cause problems with future refactoring. I
think we should add the same set of @moduels to all these tests, rather
than an individual set determined by intimate knowledge of the inner
workings of the test.

  @modules java.compiler
jdk.compiler
jdk.zipfs
jdk.jartool

with the addition of jdk.httpserver for MultiReleaseJarHttpProperties.

or we could move the tests into their own MultiRelease sub-directory and 
create a TEST.properties with a module key. That would allow these tests 
to drop @modules, except the test that uses the HTTP server.


-Alan

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Alan Bateman]

On 04/05/2016 09:40, Chris Hegarty wrote:

On 3 May 2016, at 16:10, Chris Hegarty  wrote:


Can you please take a look on:
http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/

Taking another look over this before sponsoring….

The test library dependency seems to be on java.compiler, and not jdk.compiler,
right ?  Also, I see no reason for the addition of jdk.zipfs.

java.compiler is just the API module, the runtime image needs to have 
jdk.compiler linked in to be useful. So I think @modules jdk.compiler is 
right here as otherwise the test will be selected when testing a "JRE". 
java. javac needs the zipfs provider in order to compiler against 
libraries that are packaged as JAR/zip files so I assume this is why it 
listed here.


-Alan.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Chris Hegarty]

On 3 May 2016, at 16:10, Chris Hegarty  wrote:

>>> 
>>> Can you please take a look on:
>>> http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/

Taking another look over this before sponsoring….

The test library dependency seems to be on java.compiler, and not jdk.compiler,
right ?  Also, I see no reason for the addition of jdk.zipfs.

-Chris.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Chris Hegarty]

On 2 May 2016, at 22:48, Steve Drach  wrote:

> Looks fine to me,

+1.

-Chris.

> although I am not an official reviewer.  Thanks for doing this.
> 
>> On May 2, 2016, at 1:03 PM, Alexandre (Shura) Iline 
>>  wrote:
>> 
>> Hi,
>> 
>> Can you please take a look on:
>> http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/
>> ?
>> 
>> Thank you
>> 
>> Shura
>> 
> 

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Steve Drach]

Looks fine to me, although I am not an official reviewer.  Thanks for doing 
this.

> On May 2, 2016, at 1:03 PM, Alexandre (Shura) Iline 
>  wrote:
> 
> Hi,
> 
> Can you please take a look on:
> http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/
> ?
> 
> Thank you
> 
> Shura
> 

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

[Mandy Chung]

> On May 2, 2016, at 1:03 PM, Alexandre (Shura) Iline 
>  wrote:
> 
> Hi,
> 
> Can you please take a look on:
> http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/
> ?

Looks okay to me.

Mandy