Hi,

after exchanging some direct mails with Lance, I came to the conclusion that 
the current behavior to ignore file system property "multi-release" when 
"releaseVersion" is explicitly set to null is the right thing to do. So I 
updated the webrev to reinstate this functionality and added explicit comments 
as well as augmenting MultiReleaseJarTest.java a little bit to test that 
"multi-release" is ignored in the right places.

This is the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234089.1/

Best regards
Christoph


From: Langer, Christoph
Sent: Mittwoch, 13. November 2019 17:42
To: core-libs-dev@openjdk.java.net; nio-dev <nio-...@openjdk.java.net>
Subject: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and 
JarFileSystem

Hi,

can I please get reviews for this cleanup change in zipfs.

Bug: https://bugs.openjdk.java.net/browse/JDK-8234089
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234089.0/

I figured that JarFileSystemProvider is completely obsolete (please correct me 
if I'm wrong) and the reasons for having a class JarFileSystem that extends 
ZipFileSystems are very little in my opinion. I think maintainability is better 
when the few implementation details of multi release jars live in ZipFileSystem 
as well. It saves some code. The only possible drawback is that ZipFileSystem:: 
getInode would have an additional call to a lookup function, that usually is an 
identity transformation. I would hope however, that runtime impact is 
negligible.

I also fix a small bug when property "releaseVersion" is set to null in the env 
map and multi-release contains a value. In the current implementation it would 
not consider the "multi-release" value and treat the mr jar as the current 
runtime version. I had to do a small fix in MultiReleaseJarTest.java to make 
sure the properties list is cleared in every case.

The jdk/nio/zipfs tests run well after my change.

Thanks
Christoph

Reply via email to