fs: stop fixing EPERM on Windows

lib/internal/fs/rimraf.js

@@ -1,9 +1,4 @@
-// This file is a modified version of the rimraf module on npm. It has been
-// modified in the following ways:
-// - Use of the assert module has been replaced with core's error system.
-// - All code related to the glob dependency has been removed.
-// - Bring your own custom fs module is not currently supported.
-// - Some basic code cleanup.
+// This file is a modified version of the rimraf module on npm.
 'use strict';
 
 const {
@@ -15,16 +10,12 @@ const {
 const { Buffer } = require('buffer');
 const fs = require('fs');
 const {
-  chmod,
-  chmodSync,
   lstat,
   lstatSync,
   readdir,
   readdirSync,
   rmdir,
   rmdirSync,
-  stat,
-  statSync,
   unlink,
   unlinkSync
 } = fs;
@@ -34,9 +25,6 @@ const { sleep } = require('internal/util');
 const notEmptyErrorCodes = new SafeSet(['ENOTEMPTY', 'EEXIST', 'EPERM']);
 const retryErrorCodes = new SafeSet(
   ['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']);
-const isWindows = process.platform === 'win32';
-const epermHandler = isWindows ? fixWinEPERM : _rmdir;
-const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync;
 const readdirEncoding = 'buffer';
 const separator = Buffer.from(sep);
 
@@ -69,10 +57,6 @@ function _rimraf(path, options, callback) {
     if (err) {
       if (err.code === 'ENOENT')
         return callback(null);
-
-      // Windows can EPERM on stat.
-      if (isWindows && err.code === 'EPERM')
-        return fixWinEPERM(path, options, err, callback);
     } else if (stats.isDirectory()) {
       return _rmdir(path, options, err, callback);
     }
@@ -81,11 +65,8 @@ function _rimraf(path, options, callback) {
       if (err) {
         if (err.code === 'ENOENT')
           return callback(null);
-        if (err.code === 'EISDIR')
+        if (err.code === 'EISDIR' || err.code === 'EPERM')
           return _rmdir(path, options, err, callback);
-        if (err.code === 'EPERM') {
-          return epermHandler(path, options, err, callback);
-        }
       }
 
       return callback(err);
@@ -94,24 +75,6 @@ function _rimraf(path, options, callback) {
 }
 
 
-function fixWinEPERM(path, options, originalErr, callback) {
-  chmod(path, 0o666, (err) => {
-    if (err)
-      return callback(err.code === 'ENOENT' ? null : originalErr);
-
-    stat(path, (err, stats) => {
-      if (err)
-        return callback(err.code === 'ENOENT' ? null : originalErr);
-
-      if (stats.isDirectory())
-        _rmdir(path, options, originalErr, callback);
-      else
-        unlink(path, callback);
-    });
-  });
-}
-
-
 function _rmdir(path, options, originalErr, callback) {
   rmdir(path, (err) => {
     if (err) {
@@ -181,10 +144,6 @@ function rimrafSync(path, options) {
   } catch (err) {
     if (err.code === 'ENOENT')
       return;
-
-    // Windows can EPERM on stat.
-    if (isWindows && err.code === 'EPERM')
-      fixWinEPERMSync(path, options, err);
   }
 
   try {
@@ -194,14 +153,11 @@ function rimrafSync(path, options) {
     else
       _unlinkSync(path, options);
   } catch (err) {
-    if (err.code === 'ENOENT')
-      return;
-    if (err.code === 'EPERM')
-      return epermHandlerSync(path, options, err);
-    if (err.code !== 'EISDIR')
-      throw err;
+    if (err.code === 'EISDIR' || err.code === 'EPERM')
+      return _rmdirSync(path, options, err);
 
-    _rmdirSync(path, options, err);
+    if (err.code !== 'ENOENT')
+      throw err;
   }
 }
 
@@ -280,31 +236,4 @@ function _rmdirSync(path, options, originalErr) {
 }
 
 
-function fixWinEPERMSync(path, options, originalErr) {
-  try {
-    chmodSync(path, 0o666);
-  } catch (err) {
-    if (err.code === 'ENOENT')
-      return;
-
-    throw originalErr;
-  }
-
-  let stats;
-
-  try {
-    stats = statSync(path, { throwIfNoEntry: false });
-  } catch {
-    throw originalErr;
-  }
-
-  if (stats === undefined) return;
-
-  if (stats.isDirectory())
-    _rmdirSync(path, options, originalErr);
-  else
-    _unlinkSync(path, options);
-}
-
-
 module.exports = { rimraf, rimrafPromises, rimrafSync };

test/common/tmpdir.js

@@ -6,7 +6,29 @@ const path = require('path');
 const { isMainThread } = require('worker_threads');
 
 function rmSync(pathname) {
-  fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true });
+  let err = null;
+  const maxRetries = 10;
+
+  for (let retryNumber = 0; retryNumber < maxRetries; ++retryNumber) {
+    try {
+      fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true });
+      return;
+    } catch (_err) {
+      err = _err;
+    }
+
+    const errPath = err.path;
+    const errCode = err.code;
+
+    if (errCode === 'EACCES' || errCode === 'EPERM') {
+      const surroundingDir = path.join(errPath, '..');
+
+      try { fs.chmodSync(surroundingDir, 0o777); } catch {}
+      try { fs.chmodSync(errPath, 0o777); } catch {}
+    }
+  }
+
+  throw err;
 }
 
 const testRoot = process.env.NODE_TEST_DIR ?
@@ -38,7 +60,7 @@ function onexit() {
 
   try {
     rmSync(tmpPath);
-  } catch (e) {
+  } catch (err) {
     console.error('Can\'t clean tmpdir:', tmpPath);
 
     const files = fs.readdirSync(tmpPath);
@@ -51,8 +73,15 @@ function onexit() {
       console.error('See http://nfs.sourceforge.net/#faq_d2 for details.');
     }
 
-    console.error();
-    throw e;
+    // Manually logging err instead of throwing it, so that it doesn't get
+    // overshadowed by an error from a test failure.
+    console.error(err);
+
+    // Setting the process exit code to a non-zero exit code, so that this gets
+    // marked as `not ok` during a CI run.
+    if (!process.exitCode) {
+      process.exitCode = 1;
+    }
   }
 }