testing this out on a windows machine lead to crash:

"error": "TypeError: Cannot read property 'size' of undefined", "errorStackTrace": "TypeError: Cannot read property 'size' of undefined\n at node_modules\graceful-fs\polyfills.js:112:45\n at node_modules\graceful-fs\polyfills.js:331:29\n at FSReqWrap.oncomplete (fs.js:114:15)"

taking a quick look -- perhaps there needs to be error handling on the first call to fs.stat?

Node v7.9.0
Windows Server 2008

@conceptualspace I've added some additional sanity checks that should solve your crash. Thanks for testing and reporting the issue!

that would be fine if node actually reported an EEXIST error, but it does not on windows -- we just get EACCS/EPERM if the destination already exists

> fs.mkdirSync('test1');
> fs.mkdirSync('test2');
> fs.rename('test2', 'test1', function(err) {console.log(err);});
Error: EPERM: operation not permitted, rename 'D:\Dev\test2' -> 'D:\Dev\test1'

win 10x64
node v8.4.0

This only happens for directories and if you do the same on Linux/OSX you'll see it works just fine since the native rename in Linux will overwrite an existing target no matter what. So if we want to normalize this behavior, we should probably check for an existing target and unlink it if it exists.

fs.mkdir will throw Error: EEXIST: file already exists on all platforms if you try and create a dir that already existsw.

The current polyfill will get stuck on Windows at renaming a directory when the target already exists. I think that we should make sure it works the same on both platform (i.e. remove the target directory if it exists).

I tested using the following code: https://gist.github.com/mekwall/b6c44c6855aa9adbd97af1bba9386e50

The last commit normalizes the behavior. This is what I get with the test code from my gist linked above.

Windows:

FS
Trying to rename dir
Failed!
GFS
Trying to rename dir
Success!

Linux:

FS
Trying to rename dir
Success!
GFS
Trying to rename dir
Success!

I was playing around the EPERM issue during the day and it's now clear that the issue's cause is indeed in the resource locking as was written above by @mekwall. This is the way I've got it proven for myself:

• I open the file for reading and close it (release the locking) only after the 20 seconds:

        const fd = await promisify(fs.open)(this.file, "r+");

        setTimeout(async () => {
            await promisify(fs.close)(fd);    
        }, 20 * 1000);

• Then in another workflow I run fs.rename with the target set to the same opened file applying backoff retry for 60 seconds max and I get file renaming successfully completed after the 20 seconds - exactly after releasing the file locking doing await promisify(fs.close)(fd);.

So backoff retry scenario should be really helpful, following the @mekwall 's idea. Antiviruses, Dropbox syncing and such kind of software apparently open files for a long time and very frequently, that's why there are so many EPERM issues posted on Github (which usually go to the close state without the actual resolution). So if there is no retry scenario, but just a single renaming attempt (or other actions that might cause EPERM, not only the renaming), then we can likely get EPERM error on Windows happening, that's it.

This is the test case I mentioned above https://github.com/vladimiry/fs-no-eperm-anymore/tree/master/src/test/locking - locked fs.rename call.

@isaacs do you think you could please take a look?

What's the latest on this PR? It would be great to get this in to fix the issues we are seeing with the latest versions of Jest on Windows

I encounter the EPERM issues very frequently still as do my colleagues. A colleague recently got a new workstation with windows 10, and could not do npm install with any version of npm he tried. He got the closest with npm 4.2.0. We tried blowing out the graceful-fs dependency used by npm in his installation with the files from this pull request, which fixed his issues. I hope this fix becomes a part of npm soon. Thank you for fixing it

Thanks @mekwall for writing this patch! I work on TypeScript, Visual Studio, and Visual Studio Code. The underlying problem described here is coming up a lot for VS developers because the TypeScript language service puts a file watcher on the project folder (including when the user is just writing JavaScript). We typically see errors like this:

npm ERR!   stack: 'Error: EPERM: operation not permitted, rename \'C:\\foo\\node_modules\\pug-error\\package.json.731450482\' -> \'C:\\foo\\node_modules\\pug-error\\package.json\''

As the PR describes, this is definitely related to file watchers - in my testing, if we disable the file watching from the TSLS side, the error stops happening.

I've done some testing and this PR definitely fixes the issue. The particular scenario we're trying to make work has gone from failing 1 in 4 times to succeeding the last 35 times in a row.

We really need this to be merged - it's impacting a lot of developers and there's nothing that can be done on our end. Developers running npm install while VS or VS Code are open with TS or JS files in the editor end up seeing this error all the time and it's going to result in a lot of support noise on the NPM and VS side.

@mekwall

Thank you, thank you, thank you, thank you, thank yo-----

It may take longer to run a big npm install pass, but unlike any other solution including all the stuff the NPM team has been throwing up against the wall in a bid to see what sticks, this actually WORKS.

NPM team really needs to merge this PR ASAP, could any guys in their team give a response?

Hi. It seems like there's a semantic change here. If the target directory is not empty, then it should fail with ENOTEMPTY. But, the fs.rmdir call (which should error in that case) is ignoring the error.

Also, I'm not sure it's a good idea to spin on CPU for potentially 60 seconds in this case. Is it possible to make this time shorter, or allow it to be configured?

Sadly I don't have time to fix this at the moment. Any takers?

@mekwall @isaacs Addressed feedback in #131.

Closing this in favor of #131.