install: Update fs.access to allow io.js versions w/ fixed windows ver

This also pushes them off to their own file, so we can stop doing
the copy-pasta dance. It also makes explaining the rational of the
check cleaner I think.

PR-URL: https://github.com/npm/npm/pull/9038

4 files changed, 80 insertions, 20 deletions

diff --git a/lib/install/exists.js b/lib/install/exists.js
index 0166e406c..59100d63b 100644
--- a/lib/install/exists.js
+++ b/lib/install/exists.js
@@ -2,17 +2,9 @@
 var fs = require('fs')
 var inflight = require('inflight')
 var accessError = require('./access-error.js')
+var isFsAccessAvailable = require('./is-fs-access-available.js')
 
-// The Windows implementation of `fs.access` has a bug where it will
-// sometimes return access errors all the time for directories, even
-// when access is available. As all we actually test ARE directories, this
-// is a bit of a problem.
-// FIXME: When this is corrected in Node/iojs, update this check to be
-// a bit more specific
-var isWindows = process.platform === 'win32'
-
-// fs.access first introduced in node 0.12 / io.js
-if (fs.access && !isWindows) {
+if (isFsAccessAvailable) {
   module.exports = fsAccessImplementation
 } else {
   module.exports = fsStatImplementation
diff --git a/lib/install/is-fs-access-available.js b/lib/install/is-fs-access-available.js
new file mode 100644
index 000000000..a7d2640d7
--- /dev/null
+++ b/lib/install/is-fs-access-available.js
@@ -0,0 +1,22 @@
+'use strict'
+var fs = require('fs')
+var semver = require('semver')
+var isWindows = process.platform === 'win32'
+
+// fs.access first introduced in node 0.12 / io.js
+if (!fs.access) {
+  module.exports = false
+} else if (!isWindows) {
+  // fs.access always works on non-Windows OSes
+  module.exports = true
+} else {
+  // The Windows implementation of `fs.access` has a bug where it will
+  // sometimes return access errors all the time for directories, even
+  // when access is available. As all we actually test ARE directories, this
+  // is a bit of a problem.
+  // This was fixed in io.js version 1.5.0
+  // As of 2015-07-20, it is still unfixed in node:
+  // https://github.com/joyent/node/issues/25657
+
+  module.exports = semver.gte(process.version, '1.5.0')
+}
diff --git a/lib/install/writable.js b/lib/install/writable.js
index 3abb5eb9f..199b48f59 100644
--- a/lib/install/writable.js
+++ b/lib/install/writable.js
@@ -4,17 +4,9 @@ var fs = require('fs')
 var inflight = require('inflight')
 var accessError = require('./access-error.js')
 var andIgnoreErrors = require('./and-ignore-errors.js')
+var isFsAccessAvailable = require('./is-fs-access-available.js')
 
-// The Windows implementation of `fs.access` has a bug where it will
-// sometimes return access errors all the time for directories, even
-// when access is available. As all we actually test ARE directories, this
-// is a bit of a problem.
-// FIXME: When this is corrected in Node/iojs, update this check to be
-// a bit more specific
-var isWindows = process.platform === 'win32'
-
-// fs.access first introduced in node 0.12 / io.js
-if (fs.access && !isWindows) {
+if (isFsAccessAvailable) {
   module.exports = fsAccessImplementation
 } else {
   module.exports = fsOpenImplementation
diff --git a/test/tap/is-fs-access-available.js b/test/tap/is-fs-access-available.js
new file mode 100644
index 000000000..7374e4022
--- /dev/null
+++ b/test/tap/is-fs-access-available.js
@@ -0,0 +1,54 @@
+'use strict'
+var test = require('tap').test
+var requireInject = require('require-inject')
+var semver = require('semver')
+
+var globalProcess = global.process
+
+function loadIsFsAccessAvailable (newProcess, fs) {
+  global.process = newProcess
+  var mocks = {fs: fs}
+  var isFsAccessAvailable = requireInject('../../lib/install/is-fs-access-available.js', mocks)
+  global.process = globalProcess
+  return isFsAccessAvailable
+}
+
+var fsWithAccess = {access: function () {}}
+var fsWithoutAccess = {}
+
+if (semver.lt(process.version, '0.12.0')) {
+  test('skipping', function (t) {
+    t.pass('skipping all tests on < 0.12.0 due to process not being injectable')
+    t.end()
+  })
+} else {
+  test('mac + !fs.access', function (t) {
+    var isFsAccessAvailable = loadIsFsAccessAvailable({platform: 'darwin'}, fsWithoutAccess)
+    t.is(isFsAccessAvailable, false, 'not available')
+    t.end()
+  })
+
+  test('mac + fs.access', function (t) {
+    var isFsAccessAvailable = loadIsFsAccessAvailable({platform: 'darwin'}, fsWithAccess)
+    t.is(isFsAccessAvailable, true, 'available')
+    t.end()
+  })
+
+  test('windows + !fs.access', function (t) {
+    var isFsAccessAvailable = loadIsFsAccessAvailable({platform: 'win32'}, fsWithoutAccess)
+    t.is(isFsAccessAvailable, false, 'not available')
+    t.end()
+  })
+
+  test('windows + fs.access + node 0.12.7', function (t) {
+    var isFsAccessAvailable = loadIsFsAccessAvailable({platform: 'win32', version: '0.12.7'}, fsWithAccess)
+    t.is(isFsAccessAvailable, false, 'not available')
+    t.end()
+  })
+
+  test('windows + fs.access + node 2.4.0', function (t) {
+    var isFsAccessAvailable = loadIsFsAccessAvailable({platform: 'win32', version: '2.4.0'}, fsWithAccess)
+    t.is(isFsAccessAvailable, true, 'available')
+    t.end()
+  })
+}