Skip to content

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Jan 6, 2021

Remove process.umask() call from config default

Since we now are using pacote/tar in a way that will rely on the default
process umask setting, and we set file/directory modes explicitly
anyway, there's no need to have a default umask setting that calls
process.umask()

As this method is not worker-thread safe, and is deprecated, it's best
for us to stop using it.

Fix: #1103

Depends on: isaacs/node-tar#270

@isaacs isaacs requested a review from a team as a code owner January 6, 2021 00:30
@npm-deploy-user
Copy link

npm-deploy-user commented Jan 6, 2021

angular-quickstart app-large app-medium ember-quickstart react-app
prev current status prev current status prev current status prev current status prev current status
initial install 41s 50.7s🛑 39.5s 53.2s🛑 34.5s 38.1s✅🐌 28.1s 32.6s🛑 33.5s 44.4s🛑
repeat install 9.6s 3.8s 8.6s 4.1s 8.3s 3.2s 7.7s 1.9s 9.1s 3.3s
with warm cache 32.3s 28.4s 33.8s 36.1s✅🐌 31.3s 24.3s 23.4s 20.6s 29.1s 23.5s
with node_modules 9.2s 6.7s 8.6s 5.4s 9s 4.3s 7.8s 2.4s 9.4s 6.9s
with lockfile 32.6s 35s✅🐌 31s 35.5s🛑 29s 24.7s 21.7s 21.6s 27.4s 30.7s🛑
with warm cache and node_modules 9.3s 4s 7.9s 4.5s 8.4s 3.4s 7.6s 2.3s 9.2s 3.6s
with warm cache and lockfile 24.7s 12.5s 26.3s 17.1s 24.3s 9.9s 17.8s 8.3s 21.3s 9.5s
with node_modules and lockfile 10.1s 6.4s 9.8s 5.1s 8.6s 4.1s 7.8s 2.1s 9.7s 6.6s

@isaacs isaacs force-pushed the isaacs/remove-process-umask branch from c1347b3 to 30f95fc Compare January 7, 2021 01:34
@isaacs
Copy link
Contributor Author

isaacs commented Jan 7, 2021

@isaacs isaacs force-pushed the isaacs/remove-process-umask branch from 30f95fc to b3f3987 Compare January 7, 2021 01:37
@darcyclarke darcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release semver:patch semver patch level for changes Needs Review labels Jan 7, 2021
@isaacs isaacs force-pushed the isaacs/remove-process-umask branch 2 times, most recently from 03b205c to 0acc6e4 Compare January 7, 2021 20:12
Since we now are using pacote/tar in a way that will rely on the default
process umask setting, and we set file/directory modes explicitly
anyway, there's no need to have a default umask setting that calls
process.umask()

As this method is not worker-thread safe, and is deprecated, it's best
for us to stop using it.

Fix: #1103

PR-URL: #2444
Credit: @isaacs
Close: #2444
Reviewed-by: @nlf
@isaacs isaacs changed the base branch from latest to release/v7.4.0 January 7, 2021 20:14
@isaacs isaacs force-pushed the isaacs/remove-process-umask branch from 0acc6e4 to d01746a Compare January 7, 2021 20:15
@isaacs isaacs merged commit d01746a into release/v7.4.0 Jan 7, 2021
@nlf nlf deleted the isaacs/remove-process-umask branch March 28, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: next These items should be addressed in the next release Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stop using process.umask()

4 participants