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.
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
>>
>>
>
