RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-21 Thread Langer, Christoph
@openjdk.java.net Subject: Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem Hi Christoph, Again, thank you for your efforts here. Overall I think your latest changes look fine. I would like to suggest that for the methods that you added for MR support, that we make

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-20 Thread Lance Andersen
Hi Christoph, Again, thank you for your efforts here. Overall I think your latest changes look fine. I would like to suggest that for the methods that you added for MR support, that we make sure the 1st character in the comment is capitalized prior to your push of the changes. I know this

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-20 Thread Alan Bateman
On 20/11/2019 13:39, Langer, Christoph wrote: Hi Alan, makes sense. I’ve updated the patch: http://cr.openjdk.java.net/~clanger/webrevs/8234089.4/ The updated test looks okay. -Alan

RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-20 Thread Langer, Christoph
: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem On 20/11/2019 08:15, Langer, Christoph wrote: Hi Lance, I’ve taken care of ModulesInCustomFileSystem then, too. Updated webrev in place… If the ModulesInCustomFileSystem test really needs to be changed then the private method

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-20 Thread Alan Bateman
On 20/11/2019 08:15, Langer, Christoph wrote: Hi Lance, I’ve taken care of ModulesInCustomFileSystem then, too. Updated webrev in place… If the ModulesInCustomFileSystem test really needs to be changed then the private method that has been renamed to testZipFileSystem shoud have its

RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-20 Thread Langer, Christoph
Hi Lance, I’ve taken care of ModulesInCustomFileSystem then, too. Updated webrev in place… Cheers Christoph From: Lance Andersen Sent: Dienstag, 19. November 2019 23:42 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; nio-dev Subject: Re: RFR: 8234089: (zipfs) Remove classes

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-19 Thread Lance Andersen
Hi Christoph, Thank you for the follow up. > On Nov 19, 2019, at 5:37 PM, Langer, Christoph > wrote: > > ModulesInCustomFileSystem At line 71: — /** * Test exploded modules in a JAR file system. */ ——— I will look at the rest of the changes tomorrow Best Lance

RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-19 Thread Langer, Christoph
Hi Lance, > If you look at MultiReleaseJarTest.java, it explicitly references JAR FS in a > comment. Same for JFSTester.java in the @Summary.  These should > definitely be updated.  There are comments > in ModulesInCustomFileSystem.java as well. Ok, agreed for MultiReleaseJarTest and JFSTester.

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-19 Thread Lance Andersen
r...@oracle.com>> > Sent: Dienstag, 19. November 2019 19:57 > To: Langer, Christoph <mailto:christoph.lan...@sap.com>> > Cc: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net>; > nio-dev mailto:nio-...@openjdk.java.net>> > Subject:

RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-19 Thread Langer, Christoph
To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; nio-dev Subject: Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem Hi Christoph, Something else that should probably be done as part of this proposed change is to clean up tests such as NodeCostDumpUtil.java

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-19 Thread Lance Andersen
Hi Christoph, Something else that should probably be done as part of this proposed change is to clean up tests such as NodeCostDumpUtil.java and ModulesInCustomFileSystem.java and a few others as they have methods/fields etc... that make reference to the Jar File System. While it does not

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-18 Thread Lance Andersen
Hi Christoph, Sorry for the late reply but I was out of the office Friday > On Nov 15, 2019, at 3:40 AM, Langer, Christoph > wrote: > > Hi Lance, > > thanks for reviewing. I've addressed your points: > >> I would suggesting moving the code added to the constructor for checking >> the

RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-15 Thread Langer, Christoph
Hi Lance, thanks for reviewing. I've addressed your points: > I would suggesting moving the code added to the constructor for checking > the releaseVersion/multi-release properties to a method and grouping it > with the other methods added for supporting MR jars around line 1396. > (keeps it

Re: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-14 Thread Lance Andersen
Hi Christoph, Thank you for looking into this. Overall, I think this is OK. Not sure there is currently any downside to removing the JAR FS right now outside of keeping the multi-release code separate. I would suggesting moving the code added to the constructor for checking the

RE: RFR: 8234089: (zipfs) Remove classes JarFileSystemProvider and JarFileSystem

2019-11-14 Thread Langer, Christoph
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