From c40d6037271a8d6e05a7cf7eb03036ee432610c5 Mon Sep 17 00:00:00 2001 From: Erik Kemperman Date: Fri, 29 Sep 2017 16:09:36 +0200 Subject: [PATCH] Update: Correct symlink/junction behaviour --- README.md | 14 ++--- lib/constants.js | 1 + lib/dest/options.js | 6 +- lib/dest/prepare.js | 13 +++- .../write-contents/write-symbolic-link.js | 42 ++++++++----- lib/symlink/link-file.js | 41 +++++++----- lib/symlink/options.js | 12 +--- lib/symlink/prepare.js | 9 ++- test/dest-symlinks.js | 4 ++ test/integration.js | 63 +++++++++++++++++++ test/symlink.js | 41 +++--------- 11 files changed, 153 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index 78055317..4b3f289f 100644 --- a/README.md +++ b/README.md @@ -211,10 +211,11 @@ Default: `false` ##### `options.useJunctions` When creating a symlink, whether or not a directory symlink should be created as a `junction`. +This option is only relevant on Windows and ignored elsewhere. Type: `Boolean` -Default: `true` on Windows, `false` on all other platforms +Default: `true` ### `symlink(folder[, options])` @@ -239,14 +240,6 @@ Type: `String` Default: `process.cwd()` -##### `options.mode` - -The mode the symlinks should be created with. - -Type: `Number` - -Default: The `mode` of the input file (`file.stat.mode`) if any, or the process mode if the input file has no `mode` property. - ##### `options.dirMode` The mode the directory should be created with. @@ -274,10 +267,11 @@ Default: `false` ##### `options.useJunctions` Whether or not a directory symlink should be created as a `junction`. +This option is only relevant on Windows and ignored elsewhere. Type: `Boolean` -Default: `true` on Windows, `false` on all other platforms +Default: `true` [glob-stream]: https://github.com/gulpjs/glob-stream [node-glob]: https://github.com/isaacs/node-glob diff --git a/lib/constants.js b/lib/constants.js index f0c28256..ba6b8e4e 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -2,5 +2,6 @@ module.exports = { MASK_MODE: parseInt('7777', 8), + LINK_MODE: parseInt('120777', 8), DEFAULT_FILE_MODE: parseInt('0666', 8), }; diff --git a/lib/dest/options.js b/lib/dest/options.js index d89d7281..ec558315 100644 --- a/lib/dest/options.js +++ b/lib/dest/options.js @@ -1,9 +1,5 @@ 'use strict'; -var os = require('os'); - -var isWindows = (os.platform() === 'win32'); - var config = { cwd: { type: 'string', @@ -36,7 +32,7 @@ var config = { // Symlink options useJunctions: { type: 'boolean', - default: isWindows, + default: true, }, relativeSymlinks: { type: 'boolean', diff --git a/lib/dest/prepare.js b/lib/dest/prepare.js index afa09c10..d40c40dd 100644 --- a/lib/dest/prepare.js +++ b/lib/dest/prepare.js @@ -5,13 +5,14 @@ var path = require('path'); var fs = require('graceful-fs'); var through = require('through2'); +var constants = require('../constants'); + function prepareWrite(folderResolver, optResolver) { if (!folderResolver) { throw new Error('Invalid output folder'); } function normalize(file, enc, cb) { - var mode = optResolver.resolve('mode', file); var cwd = path.resolve(optResolver.resolve('cwd', file)); var outFolderPath = folderResolver.resolve('outFolder', file); @@ -22,8 +23,14 @@ function prepareWrite(folderResolver, optResolver) { var writePath = path.resolve(basePath, file.relative); // Wire up new properties - file.stat = (file.stat || new fs.Stats()); - file.stat.mode = mode; + if (file.symlink) { + file.stat = new fs.Stats(); + file.stat.mode = constants.LINK_MODE; + } else { + file.stat = (file.stat || new fs.Stats()); + file.stat.mode = optResolver.resolve('mode', file);; + + } file.cwd = cwd; file.base = basePath; file.path = writePath; diff --git a/lib/dest/write-contents/write-symbolic-link.js b/lib/dest/write-contents/write-symbolic-link.js index ddd4c92d..35a14215 100644 --- a/lib/dest/write-contents/write-symbolic-link.js +++ b/lib/dest/write-contents/write-symbolic-link.js @@ -1,12 +1,15 @@ 'use strict'; +var os = require('os'); var path = require('path'); +var fs = require('graceful-fs'); + var fo = require('../../file-operations'); -function writeSymbolicLink(file, optResolver, onWritten) { - var isDirectory = file.isDirectory(); +var isWindows = (os.platform() === 'win32'); +function writeSymbolicLink(file, optResolver, onWritten) { // This option provides a way to create a Junction instead of a // Directory symlink on Windows. This comes with the following caveats: // * NTFS Junctions cannot be relative. @@ -18,25 +21,36 @@ function writeSymbolicLink(file, optResolver, onWritten) { // For example, JetBrains product lines will delete the entire // contents of the TARGET directory because the product does not // realize it's a symlink as the JVM and Node return false for isSymlink. - var useJunctions = optResolver.resolve('useJunctions', file); + var useJunctions = isWindows && optResolver.resolve('useJunctions', file); - var symDirType = useJunctions ? 'junction' : 'dir'; - var symType = isDirectory ? symDirType : 'file'; var isRelative = optResolver.resolve('relativeSymlinks', file); + var flag = optResolver.resolve('flag', file); - // This is done after prepare() to use the adjusted file.base property - if (isRelative && symType !== 'junction') { - file.symlink = path.relative(file.base, file.symlink); + if (useJunctions) { + fs.stat(file.symlink, onStat); + } else { + onStat(); } - var flag = optResolver.resolve('flag', file); + function onStat(statErr, stat) { + if (statErr) { + return onWritten(statErr); + } + + var symDirType = useJunctions ? 'junction' : 'dir'; + var symType = stat && stat.isDirectory() ? symDirType : 'file'; - var opts = { - flag: flag, - type: symType, - }; + // This is done after prepare() to use the adjusted file.base property + if (isRelative && symType !== 'junction') { + file.symlink = path.relative(file.base, file.symlink); + } - fo.symlink(file.symlink, file.path, opts, onWritten); + var opts = { + flag: flag, + type: symType, + }; + fo.symlink(file.symlink, file.path, opts, onWritten); + } } module.exports = writeSymbolicLink; diff --git a/lib/symlink/link-file.js b/lib/symlink/link-file.js index 13ea2f58..d5b70cfd 100644 --- a/lib/symlink/link-file.js +++ b/lib/symlink/link-file.js @@ -1,16 +1,18 @@ 'use strict'; +var os = require('os'); var path = require('path'); +var fs = require('graceful-fs'); var through = require('through2'); var fo = require('../file-operations'); +var isWindows = (os.platform() === 'win32'); + function linkStream(optResolver) { function linkFile(file, enc, callback) { - var isDirectory = file.isDirectory(); - // This option provides a way to create a Junction instead of a // Directory symlink on Windows. This comes with the following caveats: // * NTFS Junctions cannot be relative. @@ -22,25 +24,36 @@ function linkStream(optResolver) { // For example, JetBrains product lines will delete the entire // contents of the TARGET directory because the product does not // realize it's a symlink as the JVM and Node return false for isSymlink. - var useJunctions = optResolver.resolve('useJunctions', file); + var useJunctions = isWindows && optResolver.resolve('useJunctions', file); - var symDirType = useJunctions ? 'junction' : 'dir'; - var symType = isDirectory ? symDirType : 'file'; var isRelative = optResolver.resolve('relativeSymlinks', file); + var flag = optResolver.resolve('flag', file); - // This is done after prepare() to use the adjusted file.base property - if (isRelative && symType !== 'junction') { - file.symlink = path.relative(file.base, file.symlink); + if (useJunctions) { + fs.stat(file.symlink, onStat); + } else { + onStat(); } - var flag = optResolver.resolve('flag', file); + function onStat(statErr, stat) { + if (statErr) { + return callback(statErr); + } + + var symDirType = useJunctions ? 'junction' : 'dir'; + var symType = stat && stat.isDirectory() ? symDirType : 'file'; - var opts = { - flag: flag, - type: symType, - }; + // This is done after prepare() to use the adjusted file.base property + if (isRelative && symType !== 'junction') { + file.symlink = path.relative(file.base, file.symlink); + } - fo.symlink(file.symlink, file.path, opts, onSymlink); + var opts = { + flag: flag, + type: symType, + }; + fo.symlink(file.symlink, file.path, opts, onSymlink); + } function onSymlink(symlinkErr) { if (symlinkErr) { diff --git a/lib/symlink/options.js b/lib/symlink/options.js index eaa49982..5e68bd62 100644 --- a/lib/symlink/options.js +++ b/lib/symlink/options.js @@ -1,13 +1,9 @@ 'use strict'; -var os = require('os'); - -var isWindows = (os.platform() === 'win32'); - var config = { useJunctions: { type: 'boolean', - default: isWindows, + default: true, }, relativeSymlinks: { type: 'boolean', @@ -17,12 +13,6 @@ var config = { type: 'string', default: process.cwd, }, - mode: { - type: 'number', - default: function(file) { - return file.stat ? file.stat.mode : null; - }, - }, dirMode: { type: 'number', }, diff --git a/lib/symlink/prepare.js b/lib/symlink/prepare.js index 8c1e5e4e..0c38af9a 100644 --- a/lib/symlink/prepare.js +++ b/lib/symlink/prepare.js @@ -1,19 +1,18 @@ 'use strict'; -// TODO: currently a copy-paste of prepareWrite but should be customized - var path = require('path'); var fs = require('graceful-fs'); var through = require('through2'); +var constants = require('../constants'); + function prepareSymlink(folderResolver, optResolver) { if (!folderResolver) { throw new Error('Invalid output folder'); } function normalize(file, enc, cb) { - var mode = optResolver.resolve('mode', file); var cwd = path.resolve(optResolver.resolve('cwd', file)); var outFolderPath = folderResolver.resolve('outFolder', file); @@ -24,8 +23,8 @@ function prepareSymlink(folderResolver, optResolver) { var writePath = path.resolve(basePath, file.relative); // Wire up new properties - file.stat = (file.stat || new fs.Stats()); - file.stat.mode = mode; + file.stat = new fs.Stats(); + file.stat.mode = constants.LINK_MODE; file.cwd = cwd; file.base = basePath; file.symlink = file.path; diff --git a/test/dest-symlinks.js b/test/dest-symlinks.js index 29d02804..ff5cd256 100644 --- a/test/dest-symlinks.js +++ b/test/dest-symlinks.js @@ -49,6 +49,8 @@ describe('.dest() with symlinks', function() { expect(files.length).toEqual(1); expect(file.symlink).toEqual(symlink); expect(files[0].symlink).toEqual(symlink); + expect(files[0].stat).toExist(); + expect(files[0].stat.isSymbolicLink()).toBe(true); expect(files[0].path).toEqual(outputPath); } @@ -74,6 +76,8 @@ describe('.dest() with symlinks', function() { expect(files.length).toEqual(1); expect(outputLink).toEqual(path.normalize('../fixtures/test.txt')); + expect(files[0].stat).toExist(); + expect(files[0].stat.isSymbolicLink()).toBe(true); } pipe([ diff --git a/test/integration.js b/test/integration.js index 44405d32..b8f0cc20 100644 --- a/test/integration.js +++ b/test/integration.js @@ -1,9 +1,11 @@ 'use strict'; +var os = require('os'); var path = require('path'); var fs = require('graceful-fs'); var miss = require('mississippi'); +var expect = require('expect'); var vfs = require('../'); @@ -12,17 +14,25 @@ var testStreams = require('./utils/test-streams'); var testConstants = require('./utils/test-constants'); var pipe = miss.pipe; +var concat = miss.concat; var count = testStreams.count; var base = testConstants.outputBase; var inputBase = path.join(base, './in/'); +var inputDirpath = testConstants.inputDirpath; +var outputDirpath = testConstants.outputDirpath; +var symlinkDirpath = testConstants.symlinkDirpath; var inputGlob = path.join(inputBase, './*.txt'); var outputBase = path.join(base, './out/'); +var outputSymlink = path.join(symlinkDirpath, './foo'); +var outputDirpathSymlink = path.join(outputDirpath, './foo'); var content = testConstants.content; var clean = cleanup(base); +var isWindows = (os.platform() === 'win32'); + describe('integrations', function() { beforeEach(clean); @@ -49,4 +59,57 @@ describe('integrations', function() { vfs.dest(outputBase), ], done); }); + + it('(*nix) sources a directory, creates a symlink and copies it', function(done) { + if (isWindows) { + this.skip(); + return; + } + + function assert(files) { + var symlinkResult = fs.readlinkSync(outputSymlink); + var destResult = fs.readlinkSync(outputDirpathSymlink); + + expect(symlinkResult).toEqual(inputDirpath); + expect(destResult).toEqual(inputDirpath); + expect(files[0].stat).toExist(); + expect(files[0].stat.isSymbolicLink()).toBe(true); + expect(files[0].symlink).toEqual(inputDirpath); + } + + pipe([ + vfs.src(inputDirpath), + vfs.symlink(symlinkDirpath), + vfs.dest(outputDirpath), + concat(assert), + ], done); + }); + + it('(windows) sources a directory, creates a junction and copies it', function(done) { + if (!isWindows) { + this.skip(); + return; + } + + function assert(files) { + // Junctions add an ending separator + var expected = inputDirpath + path.sep; + var symlinkResult = fs.readlinkSync(outputSymlink); + var destResult = fs.readlinkSync(outputDirpathSymlink); + + expect(symlinkResult).toEqual(expected); + expect(destResult).toEqual(expected); + expect(files[0].stat).toExist(); + expect(files[0].stat.isSymbolicLink()).toBe(true); + expect(files[0].symlink).toEqual(inputDirpath); + } + + pipe([ + vfs.src(inputDirpath), + vfs.symlink(symlinkDirpath), + vfs.dest(outputDirpath), + concat(assert), + ], done); + }); + }); diff --git a/test/symlink.js b/test/symlink.js index 6dea8456..016c1dbb 100644 --- a/test/symlink.js +++ b/test/symlink.js @@ -10,9 +10,7 @@ var miss = require('mississippi'); var vfs = require('../'); var cleanup = require('./utils/cleanup'); -var statMode = require('./utils/stat-mode'); var isWindows = require('./utils/is-windows'); -var applyUmask = require('./utils/apply-umask'); var testStreams = require('./utils/test-streams'); var isDirectory = require('./utils/is-directory-mock'); var testConstants = require('./utils/test-constants'); @@ -111,6 +109,8 @@ describe('symlink stream', function() { expect(files[0].base).toEqual(outputBase, 'base should have changed'); expect(files[0].path).toEqual(outputPath, 'path should have changed'); expect(files[0].symlink).toEqual(outputLink, 'symlink should be set'); + expect(files[0].stat).toExist('stat should be set'); + expect(files[0].stat.isSymbolicLink()).toBe(true, 'stat should be symbolic'); expect(outputLink).toEqual(inputPath); } @@ -144,6 +144,8 @@ describe('symlink stream', function() { expect(files[0].base).toEqual(outputBase, 'base should have changed'); expect(files[0].path).toEqual(outputPath, 'path should have changed'); expect(files[0].symlink).toEqual(outputLink, 'symlink should be set'); + expect(files[0].stat).toExist('stat should be set'); + expect(files[0].stat.isSymbolicLink()).toBe(true, 'stat should be symbolic'); expect(outputLink).toEqual(inputPath); } @@ -170,6 +172,8 @@ describe('symlink stream', function() { expect(files[0].base).toEqual(outputBase, 'base should have changed'); expect(files[0].path).toEqual(outputPath, 'path should have changed'); expect(files[0].symlink).toEqual(outputLink, 'symlink should be set'); + expect(files[0].stat).toExist('stat should be set'); + expect(files[0].stat.isSymbolicLink()).toBe(true, 'stat should be symbolic'); expect(outputLink).toEqual(inputPath); } @@ -195,6 +199,8 @@ describe('symlink stream', function() { expect(files[0].base).toEqual(outputBase, 'base should have changed'); expect(files[0].path).toEqual(outputPath, 'path should have changed'); expect(files[0].symlink).toEqual(outputLink, 'symlink should be set'); + expect(files[0].stat).toExist('stat should be set'); + expect(files[0].stat.isSymbolicLink()).toBe(true, 'stat should be symbolic'); expect(outputLink).toEqual(path.normalize('../fixtures/test.txt')); } @@ -220,6 +226,8 @@ describe('symlink stream', function() { expect(files[0].base).toEqual(outputBase, 'base should have changed'); expect(files[0].path).toEqual(outputPath, 'path should have changed'); expect(files[0].symlink).toEqual(outputLink, 'symlink should be set'); + expect(files[0].stat).toExist('stat should be set'); + expect(files[0].stat.isSymbolicLink()).toBe(true, 'stat should be symbolic'); expect(outputLink).toEqual(inputPath); } @@ -527,35 +535,6 @@ describe('symlink stream', function() { ], done); }); - it('uses different modes for files and directories', function(done) { - // Changing the mode of a file is not supported by node.js in Windows. - if (isWindows) { - this.skip(); - return; - } - - var dirMode = applyUmask('722'); - var fileMode = applyUmask('700'); - - var file = new File({ - base: inputBase, - path: inputPath, - contents: null, - }); - - function assert(files) { - expect(statMode(outputDirpath)).toEqual(dirMode); - // TODO: the file doesn't actually get the mode updated - expect(files[0].stat.mode).toEqual(fileMode); - } - - pipe([ - from.obj([file]), - vfs.symlink(outputDirpath, { mode: fileMode, dirMode: dirMode }), - concat(assert), - ], done); - }); - it('reports IO errors', function(done) { // Changing the mode of a file is not supported by node.js in Windows. // This test is skipped on Windows because we have to chmod the file to 0.