-
Notifications
You must be signed in to change notification settings - Fork 774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove*() and empty*(): remove contents as Buffers to handle non-UTF-8 filenames on Linux #612
Conversation
… non-UTF-8 filenames on Linux
@rossj the work on |
@manidlou thank you, I should have time to work on this over the weekend Edit: Working on it this week! |
@rossj ping? |
@RyanZim sorry for the silence... I kind of went down a rabbit hole and got suck in some upstream issues and scope creap. For example, #575 is about expanding Furthermore, I discovered nodejs/node#23735 which prevents fs-extra from being able to copy / delete / do anything to certain files on Windows. The main purpose of this PR is to make My suggestion is to either leave this Buffer stuff behind and continue to v8 without it, or take the Buffer improvements as they are now with updates only to |
I'm leaning towards going ahead with v8 without these changes; @manidlou? |
@RyanZim yeah I am fine with releasing v8 without these changes. |
@rossj How's it going? |
Closing this PR as abandoned. |
This is the first part of fixing issue #575 that deals with non-UTF-8 filenames. I've modified
remove()
,removeSync()
,emptyDir()
andemptyDirSync()
to use{encoding: 'buffer'}
when callingfs.readdir*
, in order to more reliably remove contents that may not be convertible to UTF-8. This situation may only be possible on Linux file systems, but the changes and additional unit tests are working on Windows NTFS and Apple APFS.Further work is required to update
copy*()
(and thereforemove*()
) to close #575, but it was becoming a bit of a rabbit hole, so I wanted to get this part squared away first.Additionally, as a side effect of these changes, it is also possible to pass a
Buffer
input path toremove()
andremoveSync()
to remove a directory directly that may have an invalid UTF-8 name. This is not yet possible with theemptyDir*
functions because that first requires Buffer support inmkdirs
.Please advise if you would be willing to review + merge this PR, or if you would like to wait for the additional changes mentioned above.