8000 Optional non-atomic move for use with Docker/cross-device moves by haggholm · Pull Request #10373 · npm/npm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Aug 11, 2022. It is now read-only.

Optional non-atomic move for use with Docker/cross-device moves #10373

Closed
wants to merge 4 commits into from
Closed

Optional non-atomic move for use with Docker/cross-device moves #10373

wants to merge 4 commits into from

Conversation

haggholm
Copy link

See #9863

adalinesimonian
adalinesimonian reviewed Nov 19, 2015
View reviewed changes
package.json
@@ -61,6 +61,7 @@
"lodash.uniq": "~3.2.2",
"lodash.without": "~3.2.1",
"mkdirp": "~0.5.1",
"node-fs-extra": "~0.8.1",
Copy link

Choose a reason for hiding this comment

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

I think you meant to require fs-extra here and in the code as well - node-fs-extra points to an old fork for which the linked source repository doesn't exist anymore.

@othiym23
Copy link
Contributor

As described on #9863, this is not a good solution to the problem, because it doesn't address the underlying race condition the fs.rename() is intended to solve, where multiple installs could end up writing to the install target at the same time, leaving the installed package tree in an inconsistent or corrupt state. We need a solution for Docker users, but at this point I think that has more to do with how and where Docker mounts the filesystem used by npm, and not in npm itself.

That aside, thanks for taking the time to put this together!

@othiym23 othiym23 closed this Jan 15, 2016
@graingert
Copy link

@othiym23 do you mean #9863

@othiym23
Copy link
Contributor

Yes! Thanks for the heads up – corrected inline.

@frank-dspeed
Copy link

DOCKERFILE THAT APPLYS RENAME TO BE MOVE !!!! FOR DOCKER USERS use it if you understand this ISSUS:

FROM node:5.6
WORKDIR /usr/local/lib/node_modules/npm
RUN npm install --save fs-extra
RUN sed -i s/graceful-fs/fs-extra/g /usr/local/lib/node_modules/npm/lib/utils/rename.js
RUN sed -i s/fs.rename/fs.move/g /usr/local/lib/node_modules/npm/lib/utils/rename.js
WORKDIR /app
#...... main part of the Dockerfile greetings.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@haggholm @othiym23 @graingert @frank-dspeed @adalinesimonian
0