Node-fs-extra: Folders become inoperable after fse.move, because fs.link isn't returning EPERM error [Mac]

Created on 22 Sep 2017  路  12Comments  路  Source: jprichardson/node-fs-extra

The Issue
fse.move currently treats files first and folders second. It depends on fs.link to return an EPERM error to check if it's a directory and therefore needs to call moveAcrossDevice() which will eventually call ncp to recursively copy:
https://github.com/jprichardson/node-fs-extra/blob/master/lib/move/index.js#L67

Unfortunately, fs.link doesn't seem to be throwing the expected EPERM error.

OS: MacOS Sierra 10.12.5
Node: 6.11.3

The Behavior
This bug is hard to catch because fs.link is still creating the hard link and unlinking the old one. Node documentation doesn't say anything about how it's doing this, it just points to the link command documentation, which specifies that it should throw an EPERM error if you try using it on folders. http://man7.org/linux/man-pages/man2/link.2.html (The unlink man-pages have a good explanation of EPERM as "The system does not allow unlinking of directories, or unlinking of directories requires privileges that the calling process doesn't have. (This is the POSIX prescribed error return; as noted above, Linux returns EISDIR for this case.)")
If I try running the link command directly from terminal, the EPERM error is returned.

The problem is that this newly moved folder is then inoperable. I get a 'Finder Wants To Make Changes' pop up asking for my password. (Permissions aren't the issue though, as all the permissions look the same as it would as if created in Finder.) After filling out the pw, the operation still does not occur.
e.g. After calling fse.move(/srcFolder, /destFolder/srcFolder), and then trying to drag /destFolder/srcFolder back to / in Finder, the pop up occurs. It also occurs if trying to delete the folder. Either way, the fs operation doesn't occur.

Note: We tested this on Windows and did not see the same issue.

Resolution
Regardless of what node is doing that causes this issue when using fs.link on a folder, fse.move should not depend only on the EPERM error to be thrown to check if it's a folder, since fs.unlink() may not return the error.

Workaround
Now using fs.rename on both files and directories on Mac/Win, which results in one fast fs operation (rename) instead of two (heavy copy + remove).
Another possibility would be to go native with https://github.com/TooTallNate/NodObjC and use NSFileManager to move directories, which would be fast as well, as there is no copy involved if the source and destination are on the same volume.

bug feature-move platform-mac

Most helpful comment

Sure

All 12 comments

@Faline10 thanks for the detailed report. I was getting the same behavior via Finder but I never spent the time to dig into what's going on. This is very helpful - we'll take a look at it!

Also filed a node bug regarding the root issue: https://github.com/nodejs/node/issues/15561

Hmm, this is strange; I can't reproduce on:

  • macOS Sierra 10.12.6
  • Node.js 6.11.3
  • fs-extra 4.0.2

fs.link fails with the correct error, and move() works fine. @Faline10 How many/how large files does your source directory contain?

I don't have any really large files in the source directory. I'm often testing this on an empty src folder.

Also just wanted to let you know that on Windows, fs.link returned EPERM correctly, and then fse.move invoked ncp.

Also, this isn't limited to my computer. Another member of my team discovered the same behavior on her mac.

@jprichardson Can you reproduce now if you try? What version of macOS are you running?

Note: I updated my workaround in the summary to state that I am now using fs.rename instead of copy and remove. In fact, I am wondering why fse.move doesn't do this? It works both on my mac and windows so far, for both files and directories, in only one fast operation.

Node bug has been closed because they believe it's an underlying issue at the kernel level. For fse, I still think the solution, and an improvement to the code, would be to use fs.rename.

@jprichardson @manidlou Thoughts here?

Off the cuff, I don't see any problems using fs.rename instead of fs.link. Are there any objections to this?

I guess not; PR welcome! @Faline10 Do you want to do the PR?

Sure

Fixed in v6-dev; will be released later this month.

Was this page helpful?
0 / 5 - 0 ratings