Chore: consolidate writeable directory finding logic #4323
Assignees
Comments
BYK
added a commit
that referenced
this issue
Sep 6, 2017
**Summary** Fixes #4320 and fixes #4323. We were using `fs.access` when selecting the global prefix folder automatically which only checks for permissions but not actual writeability. This caused issues on Heroku where one of our first tries had the correct permissions but was on a read-only file system. **Test plan** Existing cache folder fallback tests should be enough for now. We should move the core of those tests for the newly added `fs.getFirstWriteableFolder` method. lol typo Actually test writeability of global prefix folder
BYK
added a commit
that referenced
this issue
Sep 6, 2017
**Summary** Fixes #4320 and fixes #4323. We were using `fs.access` when selecting the global prefix folder automatically which only checks for permissions but not actual writeability. This caused issues on Heroku where one of our first tries had the correct permissions but was on a read-only file system. **Test plan** Existing cache folder fallback tests should be enough for now. We should move the core of those tests for the newly added `fs.getFirstWriteableFolder` method.
arcanis
pushed a commit
that referenced
this issue
Sep 7, 2017
* Fix: Make sure global prefix folder is writeable when selecting it **Summary** Fixes #4320 and fixes #4323. We were using `fs.access` when selecting the global prefix folder automatically which only checks for permissions but not actual writeability. This caused issues on Heroku where one of our first tries had the correct permissions but was on a read-only file system. **Test plan** Existing cache folder fallback tests should be enough for now. We should move the core of those tests for the newly added `fs.getFirstWriteableFolder` method. * Fix wrong error message template used from getGlobalPrefix * Better error message * Add process.execPath as a last resort * Add back $DESTDIR support removed from #3721 * Fix DESTDIR typo * Fix skippedFolder error * don't use rimraf to remove a file * Don't use process.execPath * Defer write checks for global prefix * flow type * Just warn when a proper global folder cannot be found, instead of failing * Add TODO about inconsistent npm-registry code * Keep the old behavior * Update fs.js
joaolucasl
pushed a commit
to joaolucasl/yarn
that referenced
this issue
Oct 27, 2017
) * Fix: Make sure global prefix folder is writeable when selecting it **Summary** Fixes yarnpkg#4320 and fixes yarnpkg#4323. We were using `fs.access` when selecting the global prefix folder automatically which only checks for permissions but not actual writeability. This caused issues on Heroku where one of our first tries had the correct permissions but was on a read-only file system. **Test plan** Existing cache folder fallback tests should be enough for now. We should move the core of those tests for the newly added `fs.getFirstWriteableFolder` method. * Fix wrong error message template used from getGlobalPrefix * Better error message * Add process.execPath as a last resort * Add back $DESTDIR support removed from yarnpkg#3721 * Fix DESTDIR typo * Fix skippedFolder error * don't use rimraf to remove a file * Don't use process.execPath * Defer write checks for global prefix * flow type * Just warn when a proper global folder cannot be found, instead of failing * Add TODO about inconsistent npm-registry code * Keep the old behavior * Update fs.js
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Brought up at #4320 (comment). We currently have two (or more?) different code paths for finding a writeable directory: one in cache, one in finding the global prefix.
Ideally, we'd have a single method that takes a list of directories and returns the first one that is writable.
The text was updated successfully, but these errors were encountered: