Allow symlinked output directories #1322
Conversation
| @@ -588,6 +588,23 @@ describe('cli', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should not error if output directory is a symlink', function(done) { | |||
| if (process.platform === 'win32') { | |||
| return done(); // no symlink support on win32 | |||
There was a problem hiding this comment.
@am11 is this true? I thought Windows had a symlink equivalent.
There was a problem hiding this comment.
@xzyfer, Windows has two kinds of symlinks: soft, junction. Both are supported by node.js. The crude difference between Windows and Unix symlinks is instead of a flat file with a simple link, Windows has it as some encrypted hash in registry (a special folder like control panel etc.). Therefore they are not portable (can't zip them, cant git-package them and so on). git add will result in adding the entire directory. If it is a junction (even the linked dir is within the repo it will read everything with new dir prefix), otherwise the softlink can't be added at all.
|
Thanks @nibblebot! Are you able to squash your commits? I'm not sure bailing out early for Windows is correct. I've asked @am11 (our resident Windows expert) to confirm. |
|
docs on node's symlink creation API: certainly windows 'junctions', 'shortcuts' are not equivalent to symlinks; that is they are not interchangeable. The use case I have is an existing project with symlinks (assumes UNIX). I'm not sure how common junctions are used in windows projects but if capability was needed another few lines in |
|
and squashed |
| assert.deepEqual(files, ['.gitkeep', 'one.css', 'two.css', 'nested'].sort()); | ||
| var nestedFiles = fs.readdirSync(path.join(dest, 'nested')); | ||
| assert.deepEqual(nestedFiles, ['three.css']); | ||
| rimraf.sync(dest+'/*'); |
There was a problem hiding this comment.
removes everything except .gitkeep
|
@nibblebot, thanks. Your fix in For the test, we do not store symlink within our git repos, because they are non-portable file system artifacts. Instead in the test case, create a symlink on the fly, work with it and then dispose it. You can delete all the fixtures you added in this PR. This would be a portable solution. |
|
@am11 sounds good. will take a stab at that |
nibblebot
force-pushed
the
master
branch
2 times, most recently
from
73913ac to
7c6bc8e
Compare
|
squashed, setup/teardown for directory and symlink, all tests pass |
|
Thanks @nibblebot. We can reuse an existing fixture sub-directory instead of |
|
fixed |
|
LGTM. 🎉 @xzyfer. PTAL. |
|
Interesting: symbolic links might also to cause the issue also in #1323, where we call |
|
Great work everyone! |
Allow symlinked output directories
Should fix the concern the user has in #1212 where using a symlinked output directory incorrectly errors out saying
An output directory must be specified when compiling a directory/cc @ysimonson