Initial webrev with changes for JDK 9 jrtfs

[Xueming Shen]

On 3/14/2016 7:48 PM, Sundararajan Athijegannathan wrote:
> Hi,
>
> Thanks for the review. I've filed a bug to track the cleanups
> suggested:  https://bugs.openjdk.java.net/browse/JDK-8151860
>
> Quick comments:
>
> 1) jrt fs is read-only file system as you noted. Copying content into
> jrt fs does not make sense. Eventually read-only file system exception
> is thrown.
> But, yes that cast can be avoided.

Hi Sundar,

What I meant is without that "check&cast-ing", the copyToTarget might
actually work if the "dst/target" is a non-jrtfs-path", for example, copy
a "class" file out of the jrtfs and store into the local file system 
(those code
originally is designed/implemented to work that way, whether or not it
works depends on if the src is readable and if the dst is writable). The
question here is if this is something we want to do for the jrtfs. That is
a design decision to make.

-Sherman



>
> 2) jrt-fs.jar is generated from jimage and jrt-fs sources and expected
> to work on previous JDK (jdk 8 in this case). This is to support
> cross-JDK reference to platform classes (from IDEs).
> So, it is better to avoid internal JDK classes such as sun.nio.fs.Globs
> in jrt-fs code.
>
> Thanks,
> -Sundar
>
> On 3/15/2016 4:51 AM, Xueming Shen wrote:
>> (5) JrtFileSystemProvider.copy(src, dst, ...options)
>>
>> The "dst/target" should not be cast to "AbstractJrtPath"? the jrtfs
>> itself is
>> a readonly, I'm not sure if we want to support the operation of copying
>> a "file" output jrtfs. The copy/paste "copyToTarget" appears to be
>> functional,
>> if it's not a "AbstractJrtPath".
>>
>>
>> On 03/14/2016 04:08 PM, Xueming Shen wrote:
>>> (1) JrtFilePath: it does not seem to help performance to use the
>>> byte[] as the
>>>                   internal storage.
>>>
>>> (2) AbstractJrtFilesystem.java
>>>
>>>        getPathMatcher:
>>>         ....
>>>         if (syntax.equalsIgnoreCase(GLOB_SYNTAX)) {
>>>              expr = JrtUtils.toRegexPattern(input);
>>>          } else if (syntax.equalsIgnoreCase(REGEX_SYNTAX)) {
>>>               expr = input;
>>>          } else {
>>>                  throw new UnsupportedOperationException("Syntax '" +
>>> syntax
>>>                          + "' not recognized");
>>>         }
>>>
>>> (3) can't JrtFileSystem use sun.nio.fs.Globs ? instead of its own copy?
>>>
>>> (4) JrtFilesystem.nodesToIterator()
>>>
>>>      Stream has a "iterator() method. why need a collect() here?
>>> different performance?
>>>      for example
>>>
>>>        private Iterator<Path> nodesToIterator(AbstractJrtPath dir,
>>> List<Node> childNodes) {
>>>          return childNodes.stream()
>>>                           .map(child ->
>>> (Path)dir.resolve(toJrtPath(child.getNameString()).getFileName()))
>>>                           .iterator();
>>>        }
>>>
>>>
>>> sherman
>>>
>>>