Many native filesystems have support for symbolic links.
It might be useful if NetBeans Filesystems API provided some methods for handling them.
The proposed patch introduces these new methods in org.openide.filesystems.FileObject:
Created attachment 141766 [details]
Please review new Filesystems API for symbolic links.
If there is anyone who would like to use the new methods, please let me know.
If you are interested in symlinks in NetBeans, please review also
bug 225558 (native notifiers and symlinks) and bug 225991 (copy and symlinks).
I would like to know your opinion on whether the IDE should fully support
symlinks (special handling when copying, moving, deleting), only prevent failures
caused by recursive symlinks, or do not have any special support for symlinks at all.
Y01 There is also LocalFileSystem which can have symlinks as well. There would need to be new methods like boolean isSymbolicLink(String path) in a new AbstractFileSystem.Nio2Info interface... not the highest priority, I guess.
Turning to defect to know status of this review. Close or change type...
VK01 I agree with the patch; I would only propose to rename getRealFileObject() to getCanonicalFileObject().
We also discussed this with Tomáš Z who proposed to remove isRecursiveSymbolicLink() method from FileObject and leave it as static method in FileUtil only. I agree with both options.
Created attachment 150743 [details]
Proposed Patch v2
> Y01 There is also LocalFileSystem which can have symlinks as well.
- Added nested interface Nio2Info into AbstractFileSystem with these methods:
- boolean isSymbolicLink(String name)
- String readSymbolicLink(String name)
- String getCanonicalName(String name)
- LocalFileSystem has three new method with the same names as above
- LocalFileSystem.Impl now implements AbstractFileSystem.Nio2Info,
the newly implemented methods delegate to new methods in LocalFileSystem
Is it what you mean?
> not the highest priority, I guess.
So it doesn't need to be part of this patch and it can be added as soon as we have some use for the new methods?
Note: Test cases and documentation for Nio2Info are pending.
> VK01 I agree with the patch; I would only propose to rename
> getRealFileObject() to getCanonicalFileObject().
> We also discussed this with Tomáš Z who proposed to remove
> isRecursiveSymbolicLink() method from FileObject and leave it as static
> method in FileUtil only. I agree with both options.
Fixed. The reason for having this method in FileObject was:
> * Filesystem implementations can override this method for better
> * performance.
I suppose it is not too probable that someone would make use of it, so it can be moved to FileUtil.
Thank you for your comments.
(In reply to Jaroslav Havlin from comment #6)
The patch looks right Thank you!
Re. Y01 - Maybe I was wrong in using Nio2Info name. Possibly Symlink or Symlinks or SymlinkInfo is better description of what the interface does.
Y02 - The new protected methods in LocalFileSystem are not used, right? In such case remove them or make them package private. The other protected methods are there only because of my early API design mistake and we don't need to continue making it. Keep them just in LocalFileSystem.Impl
Y02a - should the methods remain, they should be documented.
Created attachment 150843 [details]
Proposed Patch v3
> Re. Y01 - Maybe I was wrong in using Nio2Info name. Possibly Symlink or
> Symlinks or SymlinkInfo is better description of what the interface does.
Renamed to SymlinkInfo.
> Y02 - The new protected methods in LocalFileSystem are not used, right? In such
> case remove them or make them package private.
I removed the new protected methods in LocalFileSystem.
Thank you for your comments.
Also corrected JavaDoc for SymlinkInfo.getCanonicalName(String). It now says that it returns path to a file, not a file name (which would mean that it returns path relative to filesystem root, but the symbolic link can reference a file outside of this LocalFileSystem).
If there are no objections, I would like to integrate the patch next week.
No objections from my side.
No objections either. Thank you!
Jarda H, for you to not refactor remote FS, we can do as follows: you make changes in file system API and standard NetBeans file systems and attach the patch. I then attach the correspondent patch for remote FS, then you apply both patches and push Will this work?
I will integrate the patch tomorrow. If there are any objections or additional comments, please let me know as soon as possible.
Thank you very much for reviewing.
Integrated as http://hg.netbeans.org/core-main/rev/73dcdd1ec63b.
Integrated into 'main-silver', will be available in build *201412130001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
User: Jaroslav Havlin <email@example.com>
Log: #237882: Provide API for detection and handling of symlinks