Relative junction should behave like dangling symlink

[vweevers]

A junction path must be absolute, which is why fs.symlink resolves paths on Windows if the type is junction. E.g. fs.symlink("../foo", "my/link", "junction"). From the docs:

fs.symlink(srcpath, dstpath, [type], callback)

Note that Windows junction points require the destination path to be absolute. When using 'junction', the destination argument will automatically be normalized to absolute path.

Sidenote: Reading that, you'd think the dstpath argument is normalized. What the docs mean to say is that the srcpath (the target) is normalized. The function signature in the source is fs.symlink(destination, path); these inconsistencies are confusing.

However, the target seems to be normalized relative to the working directory - not relative to the link's parent directory like it should:

man ln

Symbolic links can hold arbitrary text; if later resolved, a relative link is interpreted in relation to its parent directory.

Of course, a junction is not the same as a symlink, but the behavior should be consistent, given that node on unix ignores the junction argument and will create a dangling symlink. It should do the same on Windows, or at least simulate it. This PR normalizes the target, relative to the link's parent directory. Meaning:

fs.symlink("..\foo", "C:\my\link", "junction")

becomes:

fs.symlink("C:\foo", "C:\my\link", "junction")

The included test is copied from test-fs-symlink-dir-junction.js. These tests could probably be merged, I didn't (yet) for the sake of clarity - and because the other (regular symlink) tests fail anyway on Windows XP.

[piscisaureus]

Completely right. I am suprised this didn't get caught earlier.
The patch LGTM too.

[piscisaureus]

And thanks :)

[jasnell]

@piscisaureus ... looks like this never landed... any further thoughts on it?

[piscisaureus]

@jasnell - it landed in io.js: nodejs/node@764c5c7
I consider it a proper bug so from my perspective it could land in node 0.10/0.12 as well, but at the time I wasn't landing patches in joyent/node.

[jasnell]

@vweevers @piscisaureus ... I know it's been a while on this one, but after apply this fix to the current v0.10, the included test case fails with...

=== release test-fs-symlink-dir-junction-relative ===                          
Path: simple/test-fs-symlink-dir-junction-relative
linkData: /Users/james/Node/main/node-joyent/test/fixtures/cycles/
linkPath: /Users/james/Node/main/node-joyent/test/tmp/cycles_link
relative target: ../fixtures/cycles
assert.js:93
  throw new assert.AssertionError({
        ^
AssertionError: "../fixtures/cycles" == "/Users/james/Node/main/node-joyent/test/fixtures/cycles/"
    at /Users/james/Node/main/node-joyent/test/simple/test-fs-symlink-dir-junction-relative.js:54:14
    at Object.oncomplete (fs.js:108:15)
Command: out/Release/node /Users/james/Node/main/node-joyent/test/simple/test-fs-symlink-dir-junction-relative.js

[vweevers]

The tests were later improved, see this and this.