RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences
Hi, Can you please take a look on: http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ ? Thank you Shura
On May 2, 2016, at 1:03 PM, Alexandre (Shura) Iline <alexandre.iline@oracle.com> wrote:
Hi,
Can you please take a look on: http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ ?
Looks okay to me. Mandy
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 <alexandre.iline@oracle.com> wrote:
Hi,
Can you please take a look on: http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ ?
Thank you
Shura
On 2 May 2016, at 22:48, Steve Drach <steve.drach@oracle.com> 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 <alexandre.iline@oracle.com> wrote:
Hi,
Can you please take a look on: http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ ?
Thank you
Shura
On 3 May 2016, at 16:10, Chris Hegarty <chris.hegarty@oracle.com> 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.
On 04/05/2016 09:40, Chris Hegarty wrote:
On 3 May 2016, at 16:10, Chris Hegarty <chris.hegarty@oracle.com> 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.
On 4 May 2016, at 11:07, Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 04/05/2016 09:40, Chris Hegarty wrote:
On 3 May 2016, at 16:10, Chris Hegarty <chris.hegarty@oracle.com> 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 the test depends on both the compiler API and impl, which is implicit with the 'requires public java.compiler’ in the jdk.compiler module. That’s fine, and makes sense.
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.
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. -Chris
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
On 4 May 2016, at 14:32, Alan Bateman <Alan.Bateman@oracle.com> 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.
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 <chris.hegarty@oracle.com> wrote:
On 4 May 2016, at 14:32, Alan Bateman <Alan.Bateman@oracle.com> 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.
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 <chris.hegarty@oracle.com> wrote:
On 4 May 2016, at 14:32, Alan Bateman <Alan.Bateman@oracle.com> 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.
On May 5, 2016, at 11:42 AM, Alexandre (Shura) Iline <alexandre.iline@oracle.com> 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 <chris.hegarty@oracle.com> wrote:
On 4 May 2016, at 14:32, Alan Bateman <Alan.Bateman@oracle.com> 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.
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 <chris.hegarty@oracle.com> wrote:
On 4 May 2016, at 14:32, Alan Bateman <Alan.Bateman@oracle.com> 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.
participants (5)
-
Alan Bateman
-
Alexandre (Shura) Iline
-
Chris Hegarty
-
Mandy Chung
-
Steve Drach