From 289d9aafff199dff92e17cc64317d5179fedb2fe Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Mon, 3 May 2021 13:36:53 -0700 Subject: [PATCH] move: do not create parent directory if it is root (#897) * move: do not create parent directory if it is root * fix move-sync test * move-sync test: remove dest after test is done * remove debug log * remove dest in afterEach * call done() after test is done --- lib/move-sync/__tests__/move-sync.test.js | 21 +++++++++++- lib/move-sync/move-sync.js | 8 ++++- lib/move/__tests__/move.test.js | 39 ++++++++++++++++++----- lib/move/move.js | 7 ++++ 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index dca83aaba..506b62ddb 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -10,6 +10,8 @@ const assert = require('assert') /* global afterEach, beforeEach, describe, it */ +const describeIfWindows = process.platform === 'win32' ? describe : describe.skip + function createSyncErrFn (errCode) { const fn = function () { const err = new Error() @@ -120,7 +122,7 @@ describe('moveSync()', () => { const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) }) it('should overwrite the destination directory if overwrite = true', () => { @@ -268,6 +270,23 @@ describe('moveSync()', () => { }) }) + describeIfWindows('> when dest parent is root', () => { + let dest + + afterEach(() => fse.removeSync(dest)) + + it('should not create parent directory', () => { + const src = path.join(TEST_DIR, 'a-file') + dest = path.join(path.parse(TEST_DIR).root, 'another-file') + + fse.moveSync(src, dest) + + const contents = fs.readFileSync(dest, 'utf8') + const expected = /^sonic the hedgehog\r?\n$/ + assert(contents.match(expected)) + }) + }) + describe('> when actually trying to move a folder across devices', () => { const differentDevice = '/mnt' let __skipTests = false diff --git a/lib/move-sync/move-sync.js b/lib/move-sync/move-sync.js index 7e13587af..1c36799a0 100644 --- a/lib/move-sync/move-sync.js +++ b/lib/move-sync/move-sync.js @@ -13,10 +13,16 @@ function moveSync (src, dest, opts) { const { srcStat, isChangingCase = false } = stat.checkPathsSync(src, dest, 'move', opts) stat.checkParentPathsSync(src, srcStat, dest, 'move') - mkdirpSync(path.dirname(dest)) + if (!isParentRoot(dest)) mkdirpSync(path.dirname(dest)) return doRename(src, dest, overwrite, isChangingCase) } +function isParentRoot (dest) { + const parent = path.dirname(dest) + const parsedPath = path.parse(parent) + return parsedPath.root === parent +} + function doRename (src, dest, overwrite, isChangingCase) { if (isChangingCase) return rename(src, dest, overwrite) if (overwrite) { diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index af41b93f8..d20f530d4 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -8,6 +8,8 @@ const assert = require('assert') /* global afterEach, beforeEach, describe, it */ +const describeIfWindows = process.platform === 'win32' ? describe : describe.skip + function createAsyncErrFn (errCode) { const fn = function (...args) { fn.callCount++ @@ -63,7 +65,7 @@ describe('+ move()', () => { fs.readFile(dest, 'utf8', (err, contents) => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) done() }) }) @@ -114,7 +116,7 @@ describe('+ move()', () => { fs.readFile(path.join(dest, 'another-folder', 'file3'), 'utf8', (err, contents) => { const expected = /^knuckles\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) tearDownMockFs() done() }) @@ -132,7 +134,7 @@ describe('+ move()', () => { fs.readFile(dest, 'utf8', (err, contents) => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) done() }) }) @@ -206,7 +208,7 @@ describe('+ move()', () => { fs.readFile(dest, 'utf8', (err, contents) => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) done() }) }) @@ -225,7 +227,7 @@ describe('+ move()', () => { fs.readFile(dest, 'utf8', (err, contents) => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) tearDownMockFs() done() }) @@ -244,7 +246,7 @@ describe('+ move()', () => { fs.readFile(path.join(dest, 'another-file'), 'utf8', (err, contents) => { const expected = /^tails\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) done() }) }) @@ -263,7 +265,7 @@ describe('+ move()', () => { fs.readFile(path.join(dest, 'another-folder', 'file3'), 'utf8', (err, contents) => { const expected = /^knuckles\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) tearDownMockFs() done() }) @@ -271,6 +273,27 @@ describe('+ move()', () => { }) }) + describeIfWindows('> when dest parent is root', () => { + let dest + + afterEach(done => fse.remove(dest, done)) + + it('should not create parent directory', done => { + const src = path.join(TEST_DIR, 'a-file') + dest = path.join(path.parse(TEST_DIR).root, 'another-file') + + fse.move(src, dest, err => { + assert.ifError(err) + fs.readFile(dest, 'utf8', (err, contents) => { + const expected = /^sonic the hedgehog\r?\n$/ + assert.ifError(err) + assert.ok(contents.match(expected)) + done() + }) + }) + }) + }) + describe('> clobber', () => { it('should be an alias for overwrite', done => { const src = path.join(TEST_DIR, 'a-file') @@ -284,7 +307,7 @@ describe('+ move()', () => { fs.readFile(dest, 'utf8', (err, contents) => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) - assert.ok(contents.match(expected), `${contents} match ${expected}`) + assert.ok(contents.match(expected)) done() }) }) diff --git a/lib/move/move.js b/lib/move/move.js index 23840e027..e9523e65a 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -21,6 +21,7 @@ function move (src, dest, opts, cb) { const { srcStat, isChangingCase = false } = stats stat.checkParentPaths(src, srcStat, dest, 'move', err => { if (err) return cb(err) + if (isParentRoot(dest)) return doRename(src, dest, overwrite, isChangingCase, cb) mkdirp(path.dirname(dest), err => { if (err) return cb(err) return doRename(src, dest, overwrite, isChangingCase, cb) @@ -29,6 +30,12 @@ function move (src, dest, opts, cb) { }) } +function isParentRoot (dest) { + const parent = path.dirname(dest) + const parsedPath = path.parse(parent) + return parsedPath.root === parent +} + function doRename (src, dest, overwrite, isChangingCase, cb) { if (isChangingCase) return rename(src, dest, overwrite, cb) if (overwrite) {