Bug 408630 - org.eclipse.jgit.util.FileUtil.createSymLink() normalizes link target
Summary: org.eclipse.jgit.util.FileUtil.createSymLink() normalizes link target
Status: NEW
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-21 16:03 EDT by Andrey Loskutov CLA
Modified: 2013-08-12 18:06 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Loskutov CLA 2013-05-21 16:03:11 EDT
See original review comment in Gerrit [1].

Unfortunately if target of a link is not normalized path, new File(target) will normalize it, so that physical link will differ on file system from the original one. E.g. "/tmp/" target will be created as "/tmp" etc. This is limitation of Java 7 implementation, native git does everything right. As a result, working directory of the repository containing not normalized link targets will be *always* dirty after checkout.

It would be nice if we could report here some kind of warning:
if(!nioTarget.toString().equals(target)){ 
   Sys.err.println("Failed to create expected link (target is not normalized). Link: '" + path + "', original target: '" + target + "', created target: '" + nioTarget + "'.") 
}

Additionally it would be nice to have some extra javadoc for the method in FileUtil and in FS class.

[1] https://git.eclipse.org/r/#/c/9378/25/org.eclipse.jgit.java7/src/org/eclipse/jgit/util/FileUtil.java
Comment 1 Robin Rosenberg CLA 2013-05-21 18:57:38 EDT
I think we should check for the normalized case and report link as matching if that is the case
Comment 2 Robin Rosenberg CLA 2013-07-10 04:44:02 EDT
Is there any other normalization beside stripping extra slashes?
Comment 3 Andrey Loskutov CLA 2013-07-10 05:02:48 EDT
(In reply to comment #2)
> Is there any other normalization beside stripping extra slashes?

Not that we've seen this. BTW it strips all "extra" slashes, not just trailing:

..//com/make/Makefile will be changed to ../com/make/Makefile
Comment 4 Robin Rosenberg CLA 2013-07-10 05:51:37 EDT
It seems new File(string) does this, so I think we'll use that to normalize whatever we read from disk.
Comment 5 Andrey Loskutov CLA 2013-07-10 07:22:10 EDT
(In reply to comment #4)
> It seems new File(string) does this

It *does* it and so all File/Path based API's.
Comment 6 Robin Rosenberg CLA 2013-08-12 18:06:39 EDT
Maybe https://git.eclipse.org/r/#/c/15393/ fixes this