Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

install: Use EXDEV aware move instead of rename #15901

Merged
merged 1 commit into from Mar 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/install/action/extract.js
Expand Up @@ -3,7 +3,7 @@ var path = require('path')
var iferr = require('iferr')
var asyncMap = require('slide').asyncMap
var fs = require('graceful-fs')
var rename = require('../../utils/rename.js')
var move = require('../../utils/move.js')
var gentlyRm = require('../../utils/gently-rm.js')
var updatePackageJson = require('../update-package-json')
var npm = require('../../npm.js')
Expand Down Expand Up @@ -68,7 +68,7 @@ function stageBundledModule (bundler, child, staging, parentPath, next) {

function moveModule () {
if (child.fromBundle) {
return rename(stageFrom, stageTo, iferr(next, updateMovedPackageJson))
return move(stageFrom, stageTo, iferr(next, updateMovedPackageJson))
} else {
return fs.stat(stageFrom, function (notExists, exists) {
if (exists) {
Expand Down
24 changes: 12 additions & 12 deletions lib/install/action/finalize.js
Expand Up @@ -4,7 +4,7 @@ var rimraf = require('rimraf')
var fs = require('graceful-fs')
var mkdirp = require('mkdirp')
var asyncMap = require('slide').asyncMap
var rename = require('../../utils/rename.js')
var move = require('../../utils/move.js')
var gentlyRm = require('../../utils/gently-rm')
var moduleStagingPath = require('../module-staging-path.js')

Expand All @@ -26,31 +26,31 @@ module.exports = function (staging, pkg, log, next) {

function destStatted (doesNotExist) {
if (doesNotExist) {
rename(extractedTo, pkg.path, whenMoved)
move(extractedTo, pkg.path, whenMoved)
} else {
moveAway()
}
}

function whenMoved (renameEr) {
if (!renameEr) return next()
if (renameEr.code !== 'ENOTEMPTY') return next(renameEr)
function whenMoved (moveEr) {
if (!moveEr) return next()
if (moveEr.code !== 'ENOTEMPTY' && moveEr.code !== 'EEXIST') return next(moveEr)
moveAway()
}

function moveAway () {
rename(pkg.path, delpath, whenOldMovedAway)
move(pkg.path, delpath, whenOldMovedAway)
}

function whenOldMovedAway (renameEr) {
if (renameEr) return next(renameEr)
rename(extractedTo, pkg.path, whenConflictMoved)
function whenOldMovedAway (moveEr) {
if (moveEr) return next(moveEr)
move(extractedTo, pkg.path, whenConflictMoved)
}

function whenConflictMoved (renameEr) {
function whenConflictMoved (moveEr) {
// if we got an error we'll try to put back the original module back,
// succeed or fail though we want the original error that caused this
if (renameEr) return rename(delpath, pkg.path, function () { next(renameEr) })
if (moveEr) return move(delpath, pkg.path, function () { next(moveEr) })
fs.readdir(path.join(delpath, 'node_modules'), makeTarget)
}

Expand All @@ -65,7 +65,7 @@ module.exports = function (staging, pkg, log, next) {
asyncMap(files, function (file, done) {
var from = path.join(delpath, 'node_modules', file)
var to = path.join(pkg.path, 'node_modules', file)
rename(from, to, done)
move(from, to, done)
}, cleanup)
}

Expand Down
12 changes: 6 additions & 6 deletions lib/install/action/move.js
Expand Up @@ -8,7 +8,7 @@ var mkdirp = require('mkdirp')
var rmStuff = require('../../unbuild.js').rmStuff
var lifecycle = require('../../utils/lifecycle.js')
var updatePackageJson = require('../update-package-json.js')
var rename = require('../../utils/rename.js')
var move = require('../../utils/move.js')

/*
Move a module from one point in the node_modules tree to another.
Expand Down Expand Up @@ -46,7 +46,7 @@ function moveModuleOnly (from, to, log, done) {

log.silly('move', 'move existing destination node_modules away', toModules)

rename(toModules, tempToModules, removeDestination(done))
move(toModules, tempToModules, removeDestination(done))

function removeDestination (next) {
return function (er) {
Expand All @@ -62,7 +62,7 @@ function moveModuleOnly (from, to, log, done) {
function moveToModulesBack (next) {
return function () {
log.silly('move', 'move existing destination node_modules back', toModules)
rename(tempToModules, toModules, iferr(done, next))
move(tempToModules, toModules, iferr(done, next))
}
}

Expand All @@ -76,22 +76,22 @@ function moveModuleOnly (from, to, log, done) {
function moveNodeModules (next) {
return function () {
log.silly('move', 'move source node_modules away', fromModules)
rename(fromModules, tempFromModules, iferr(doMove(next), doMove(moveNodeModulesBack(next))))
move(fromModules, tempFromModules, iferr(doMove(next), doMove(moveNodeModulesBack(next))))
}
}

function doMove (next) {
return function () {
log.silly('move', 'move module dir to final dest', from, to)
rename(from, to, iferr(done, next))
move(from, to, iferr(done, next))
}
}

function moveNodeModulesBack (next) {
return function () {
mkdirp(from, iferr(done, function () {
log.silly('move', 'put source node_modules back', fromModules)
rename(tempFromModules, fromModules, iferr(done, next))
move(tempFromModules, fromModules, iferr(done, next))
}))
}
}
Expand Down
10 changes: 5 additions & 5 deletions lib/install/action/remove.js
Expand Up @@ -6,7 +6,7 @@ var asyncMap = require('slide').asyncMap
var mkdirp = require('mkdirp')
var npm = require('../../npm.js')
var andIgnoreErrors = require('../and-ignore-errors.js')
var rename = require('../../utils/rename.js')
var move = require('../../utils/move.js')

// This is weird because we want to remove the module but not it's node_modules folder
// allowing for this allows us to not worry about the order of operations
Expand All @@ -26,11 +26,11 @@ function removeLink (pkg, next) {
function removeDir (pkg, log, next) {
var modpath = path.join(path.dirname(pkg.path), '.' + path.basename(pkg.path) + '.MODULES')

rename(path.join(pkg.path, 'node_modules'), modpath, unbuildPackage)
move(path.join(pkg.path, 'node_modules'), modpath, unbuildPackage)

function unbuildPackage (renameEr) {
function unbuildPackage (moveEr) {
npm.commands.unbuild(pkg.path, true, function () {
rimraf(pkg.path, renameEr ? andRemoveEmptyParents(pkg.path) : moveModulesBack)
rimraf(pkg.path, moveEr ? andRemoveEmptyParents(pkg.path) : moveModulesBack)
})
}

Expand Down Expand Up @@ -58,7 +58,7 @@ function removeDir (pkg, log, next) {
var to = path.join(pkg.path, 'node_modules', file)
// we ignore errors here, because they can legitimately happen, for instance,
// bundled modules will be in both node_modules folders
rename(from, to, andIgnoreErrors(done))
move(from, to, andIgnoreErrors(done))
}, cleanup)
}

Expand Down
19 changes: 19 additions & 0 deletions lib/utils/move.js
@@ -0,0 +1,19 @@
'use strict'
module.exports = wrappedMove

var fs = require('graceful-fs')
var move = require('@npmcorp/move')
var Bluebird = require('bluebird')

function wrappedMove (from, to, cb) {
var movePromise = move(from, to, {fs: fs, Promise: Bluebird, maxConcurrency: 4})
if (cb) {
return movePromise.then(function (value) {
cb(value)
}, function (err) {
cb(err)
})
} else {
return movePromise
}
}
21 changes: 7 additions & 14 deletions lib/utils/rename.js
@@ -1,16 +1,9 @@
'use strict'
var fs = require('graceful-fs')
var SaveStack = require('./save-stack.js')
/*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth leaving this as an empty file so it doesn't break anyone using:

# Fix bug https://github.com/npm/npm/issues/9863
RUN cd $(npm root -g)/npm \
  && npm install fs-extra \
  && sed -i -e s/graceful-fs/fs-extra/ -e s/fs\.rename/fs.move/ ./lib/utils/rename.js

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm grumpy about it but yeah, since this is fixing things for exactly the folks who would have had that patch, we probably shouldn't break the work around. =D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll remove the hack w/ npm@5, since it's a breaking bump anyway.


module.exports = rename
This is a stub file to ensure that the following hack doesn't break. This can be removed w/ npm@5.

function rename (from, to, cb) {
var saved = new SaveStack(rename)
fs.rename(from, to, function (er) {
if (er) {
return cb(saved.completeWith(er))
} else {
return cb()
}
})
}
# Fix bug https://github.com/npm/npm/issues/9863
RUN cd $(npm root -g)/npm \
&& npm install fs-extra \
&& sed -i -e s/graceful-fs/fs-extra/ -e s/fs\.rename/fs.move/ ./lib/utils/rename.js
*/
13 changes: 13 additions & 0 deletions node_modules/@npmcorp/move/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 52 additions & 0 deletions node_modules/@npmcorp/move/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 84 additions & 0 deletions node_modules/@npmcorp/move/move.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions node_modules/@npmcorp/move/node_modules/@npmcorp/copy/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.