fix: Move temp folder instead of renaming #83
Conversation
mxdvl
force-pushed
the
patch-1
branch
5 times, most recently
from
ebee61e
to
de7a96f
Compare
As nodeModulesDirForPackageInner is asynchronous and contains several `await` calls, the file can be created after the first check for existence. The error thrown by `Deno.rename` is not an instance of `Deno.errors.AlreadyExists`, but `move` does work as expected. Tested on macOS (OS error 66) and linux (OS error 39)
ensure behaviour is kept as-is, to make review process easier
|
Any chance you could get a look @lucacasonato? |
marvinhagemeister
changed the title
|
Thanks @marvinhagemeister, wasn’t sure if you were a maintainer as well! I think someone will need to approve the actions for them to run on this PR. Alternatively, recreating my branch in this repo–as opposed to the fork–should also trigger them. |
|
@lucacasonato Do you have an idea why the CI is not getting triggered? |
If there’s no button to “approve workflows” as per the docs, I think the second-best approach is to own the changes yourselves. You could achieve this by copying the Feel free to recreate this PR or cherry-pick those commits yourselves, by the way, authorhip of the change is less important that the fix actually going out! 🙇 |
|
All green now. I don't have the power to merge so either of you will have to do it. |
|
I'm currently investigating this. We found a reproduction case, but the changes in this PR don't seem to fully fix it. |
|
Fixed by #107 |
What does this change?
Use
movemethod fromstd/fs, which returns the correct errors for existing files.Why?
Fixes #82
There may be a deeper issue, which is that we do not get the correct underlying error from the filesystem:
https://github.com/denoland/deno_core/blob/69891d0685d8e0ab5d80d6e3204c241796046a30/core/error_codes.rs#L50C52-L50C52