Skip to content

Commit fb5a7ec

Browse files
BYKBurak Yigit Kaya
authored andcommitted
Fix symlinking on Windows
Fixes symlink creation on Windows systems and adds a test for symlinking directories since it needs special treatment on Windows. Fixes #301.
1 parent 6bdee43 commit fb5a7ec

File tree

2 files changed

+90
-59
lines changed

2 files changed

+90
-59
lines changed

src/ln.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
var fs = require('fs');
22
var path = require('path');
33
var common = require('./common');
4-
var os = require('os');
54

65
//@
76
//@ ### ln([options,] source, dest)
@@ -42,15 +41,29 @@ function _ln(options, source, dest) {
4241
}
4342

4443
if (options.symlink) {
45-
if ((isAbsolute && !fs.existsSync(sourcePath)) || !fs.existsSync(path.resolve(process.cwd(), path.dirname(dest), source))) {
44+
var isWindows = common.platform === 'win';
45+
var linkType = isWindows ? 'file' : null;
46+
var resolvedSourcePath = isAbsolute ? sourcePath : path.resolve(process.cwd(), path.dirname(dest), source);
47+
if (!fs.existsSync(resolvedSourcePath)) {
4648
common.error('Source file does not exist', true);
49+
} else if (isWindows && fs.statSync(resolvedSourcePath).isDirectory()) {
50+
linkType = 'junction';
51+
}
52+
53+
try {
54+
fs.symlinkSync(linkType === 'junction' ? resolvedSourcePath: source, dest, linkType);
55+
} catch (err) {
56+
common.error(err.message);
4757
}
48-
fs.symlinkSync(source, dest, os.platform() === "win32" ? "junction" : null);
4958
} else {
5059
if (!fs.existsSync(source)) {
5160
common.error('Source file does not exist', true);
5261
}
53-
fs.linkSync(source, dest);
62+
try {
63+
fs.linkSync(source, dest);
64+
} catch (err) {
65+
common.error(err.message);
66+
}
5467
}
5568
}
5669
module.exports = _ln;

test/ln.js

Lines changed: 73 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,26 @@
11
var shell = require('..');
22
var common = require('../src/common');
3+
var isWindows = common.platform === 'win';
34

45
var assert = require('assert'),
56
fs = require('fs'),
67
path = require('path');
78

89
shell.config.silent = true;
910

11+
// On Windows, symlinks for files need admin permissions. This helper
12+
// skips certain tests if we are on Windows and got an EPERM error
13+
function skipOnWinForEPERM (action, test) {
14+
action();
15+
var error = shell.error();
16+
17+
if (isWindows && error && /EPERM:/.test(error)) {
18+
console.log("Got EPERM when testing symlinks on Windows. Assuming non-admin environment and skipping test.");
19+
} else {
20+
test();
21+
}
22+
}
23+
1024
shell.rm('-rf', 'tmp');
1125
shell.mkdir('tmp');
1226

@@ -45,13 +59,6 @@ assert.ok(shell.error());
4559
// Valids
4660
//
4761

48-
// On Windows, symlinks for files need admin permissions.
49-
// It is also broken now since current implementation simply uses `'junction'` type which is only
50-
// valid for directories.
51-
// TODO: Fix this for Windows and also add symlink tests for directories
52-
if (common.platform === 'win')
53-
shell.exit(123);
54-
5562
shell.ln('tmp/file1', 'tmp/linkfile1');
5663
assert(fs.existsSync('tmp/linkfile1'));
5764
assert.equal(
@@ -64,17 +71,25 @@ assert.equal(
6471
'new content 1'
6572
);
6673

67-
shell.ln('-s', 'file2', 'tmp/linkfile2');
68-
assert(fs.existsSync('tmp/linkfile2'));
69-
assert.equal(
70-
fs.readFileSync('tmp/file2').toString(),
71-
fs.readFileSync('tmp/linkfile2').toString()
72-
);
73-
fs.writeFileSync('tmp/file2', 'new content 2');
74-
assert.equal(
75-
fs.readFileSync('tmp/linkfile2').toString(),
76-
'new content 2'
77-
);
74+
skipOnWinForEPERM(shell.ln.bind(shell, '-s', 'file2', 'tmp/linkfile2'), function () {
75+
assert(fs.existsSync('tmp/linkfile2'));
76+
assert.equal(
77+
fs.readFileSync('tmp/file2').toString(),
78+
fs.readFileSync('tmp/linkfile2').toString()
79+
);
80+
fs.writeFileSync('tmp/file2', 'new content 2');
81+
assert.equal(
82+
fs.readFileSync('tmp/linkfile2').toString(),
83+
'new content 2'
84+
);
85+
});
86+
87+
// Symbolic link directory test
88+
shell.mkdir('tmp/ln');
89+
shell.touch('tmp/ln/hello');
90+
shell.ln('-s', 'ln', 'tmp/dir1');
91+
assert(fs.existsSync('tmp/ln/hello'));
92+
assert(fs.existsSync('tmp/dir1/hello'));
7893

7994
shell.ln('-f', 'tmp/file1.js', 'tmp/file2.js');
8095
assert(fs.existsSync('tmp/file2.js'));
@@ -88,46 +103,49 @@ assert.equal(
88103
'new content js'
89104
);
90105

91-
shell.ln('-sf', 'file1.txt', 'tmp/file2.txt');
92-
assert(fs.existsSync('tmp/file2.txt'));
93-
assert.equal(
94-
fs.readFileSync('tmp/file1.txt').toString(),
95-
fs.readFileSync('tmp/file2.txt').toString()
96-
);
97-
fs.writeFileSync('tmp/file1.txt', 'new content txt');
98-
assert.equal(
99-
fs.readFileSync('tmp/file2.txt').toString(),
100-
'new content txt'
101-
);
106+
skipOnWinForEPERM(shell.ln.bind(shell, '-sf', 'file1.txt', 'tmp/file2.txt'), function () {
107+
assert(fs.existsSync('tmp/file2.txt'));
108+
assert.equal(
109+
fs.readFileSync('tmp/file1.txt').toString(),
110+
fs.readFileSync('tmp/file2.txt').toString()
111+
);
112+
fs.writeFileSync('tmp/file1.txt', 'new content txt');
113+
assert.equal(
114+
fs.readFileSync('tmp/file2.txt').toString(),
115+
'new content txt'
116+
);
117+
});
102118

103119
// Abspath regression
104-
shell.ln('-sf', 'file1', path.resolve('tmp/abspath'));
105-
assert(fs.existsSync('tmp/abspath'));
106-
assert.equal(
107-
fs.readFileSync('tmp/file1').toString(),
108-
fs.readFileSync('tmp/abspath').toString()
109-
);
110-
fs.writeFileSync('tmp/file1', 'new content 3');
111-
assert.equal(
112-
fs.readFileSync('tmp/abspath').toString(),
113-
'new content 3'
114-
);
120+
skipOnWinForEPERM(shell.ln.bind(shell, '-sf', 'file1', path.resolve('tmp/abspath')), function () {
121+
assert(fs.existsSync('tmp/abspath'));
122+
assert.equal(
123+
fs.readFileSync('tmp/file1').toString(),
124+
fs.readFileSync('tmp/abspath').toString()
125+
);
126+
fs.writeFileSync('tmp/file1', 'new content 3');
127+
assert.equal(
128+
fs.readFileSync('tmp/abspath').toString(),
129+
'new content 3'
130+
);
131+
});
115132

116133
// Relative regression
117-
shell.ln('-sf', 'file1.txt', 'tmp/file2.txt');
118-
shell.mkdir('-p', 'tmp/new');
119-
// Move the symlink first, as the reverse confuses `mv`.
120-
shell.mv('tmp/file2.txt', 'tmp/new/file2.txt');
121-
shell.mv('tmp/file1.txt', 'tmp/new/file1.txt');
122-
assert(fs.existsSync('tmp/new/file2.txt'));
123-
assert.equal(
124-
fs.readFileSync('tmp/new/file1.txt').toString(),
125-
fs.readFileSync('tmp/new/file2.txt').toString()
126-
);
127-
fs.writeFileSync('tmp/new/file1.txt', 'new content txt');
128-
assert.equal(
129-
fs.readFileSync('tmp/new/file2.txt').toString(),
130-
'new content txt'
131-
);
134+
skipOnWinForEPERM(shell.ln.bind(shell, '-sf', 'file1.txt', 'tmp/file2.txt'), function () {
135+
shell.mkdir('-p', 'tmp/new');
136+
// Move the symlink first, as the reverse confuses `mv`.
137+
shell.mv('tmp/file2.txt', 'tmp/new/file2.txt');
138+
shell.mv('tmp/file1.txt', 'tmp/new/file1.txt');
139+
assert(fs.existsSync('tmp/new/file2.txt'));
140+
assert.equal(
141+
fs.readFileSync('tmp/new/file1.txt').toString(),
142+
fs.readFileSync('tmp/new/file2.txt').toString()
143+
);
144+
fs.writeFileSync('tmp/new/file1.txt', 'new content txt');
145+
assert.equal(
146+
fs.readFileSync('tmp/new/file2.txt').toString(),
147+
'new content txt'
148+
);
149+
});
132150

133151
shell.exit(123);

0 commit comments

Comments
 (0)