This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 237882 - Provide API for detection and handling of symlinks
Summary: Provide API for detection and handling of symlinks
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Filesystems (show other bugs)
Version: 7.4
Hardware: All All
: P2 normal (vote)
Assignee: Jaroslav Havlin
URL:
Keywords: API_REVIEW_FAST
Depends on:
Blocks: 225558 238869 208392 225991
  Show dependency tree
 
Reported: 2013-11-01 10:04 UTC by Jaroslav Havlin
Modified: 2014-12-13 06:12 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Proposed Patch (25.28 KB, patch)
2013-11-01 10:07 UTC, Jaroslav Havlin
Details | Diff
Proposed Patch v2 (31.08 KB, patch)
2014-11-27 13:15 UTC, Jaroslav Havlin
Details | Diff
Proposed Patch v3 (30.15 KB, patch)
2014-12-02 14:57 UTC, Jaroslav Havlin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Havlin 2013-11-01 10:04:21 UTC
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:

isSymbolicLink()
isRecursiveSymbolicLink()
readSymbolicLink()
readSymbolicLinkPath()
getRealFileObject()
Comment 1 Jaroslav Havlin 2013-11-01 10:07:33 UTC
Created attachment 141766 [details]
Proposed Patch
Comment 2 Jaroslav Havlin 2013-11-06 14:40:00 UTC
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.

Thank you.
Comment 3 Jaroslav Tulach 2013-11-10 07:43:54 UTC
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.
Comment 4 Jaroslav Tulach 2014-11-05 09:27:12 UTC
Turning to defect to know status of this review. Close or change type...
Comment 5 Vladimir Kvashin 2014-11-26 16:37:30 UTC
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.
Comment 6 Jaroslav Havlin 2014-11-27 13:15:50 UTC
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().
Fixed.

> 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.
Comment 7 Vladimir Kvashin 2014-12-01 12:49:59 UTC
(In reply to Jaroslav Havlin from comment #6)
The patch looks right Thank you!
Comment 8 Jaroslav Tulach 2014-12-01 14:50:44 UTC
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.
Comment 9 Jaroslav Havlin 2014-12-02 14:57:55 UTC
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).
Comment 10 Jaroslav Havlin 2014-12-03 16:35:41 UTC
If there are no objections, I would like to integrate the patch next week.
Comment 11 Tomas Zezula 2014-12-03 16:47:21 UTC
No objections from my side.
Thanks Jardo!
Comment 12 Vladimir Kvashin 2014-12-04 08:50:13 UTC
No objections either. Thank you!
Comment 13 Vladimir Kvashin 2014-12-08 14:10:05 UTC
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?
Comment 14 Jaroslav Havlin 2014-12-09 08:18:50 UTC
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.
Comment 15 Jaroslav Havlin 2014-12-10 10:01:01 UTC
Integrated as http://hg.netbeans.org/core-main/rev/73dcdd1ec63b.
Comment 16 Quality Engineering 2014-12-13 06:12:36 UTC
Integrated into 'main-silver', will be available in build *201412130001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/73dcdd1ec63b
User: Jaroslav Havlin <jhavlin@netbeans.org>
Log: #237882: Provide API for detection and handling of symlinks