fix: create Python symlink only during builds, and clean it up after

Previously in b9ddcd5 this was created
during configuration, and the symlink persisted indefinitely. This
causes problems with many tools that do not expect a codebase to include
symlinks to external absolute paths.

This PR largely reverts that commit, and instead writes the path to
link to into the config, and then creates the symlink only temporarily
during the build process, always deleting it afterwards.

lib/build.js

@@ -27,6 +27,8 @@ function build (gyp, argv, callback) {
   var arch
   var nodeDir
   var guessedSolution
+  var python
+  var buildBinsDir
 
   loadConfigGypi()
 
@@ -52,6 +54,7 @@ function build (gyp, argv, callback) {
       buildType = config.target_defaults.default_configuration
       arch = config.variables.target_arch
       nodeDir = config.variables.nodedir
+      python = config.variables.python
 
       if ('debug' in gyp.opts) {
         buildType = gyp.opts.debug ? 'Debug' : 'Release'
@@ -63,6 +66,7 @@ function build (gyp, argv, callback) {
       log.verbose('build type', buildType)
       log.verbose('architecture', arch)
       log.verbose('node dev dir', nodeDir)
+      log.verbose('python', python)
 
       if (win) {
         findSolutionFile()
@@ -187,16 +191,35 @@ function build (gyp, argv, callback) {
 
     if (!win) {
       // Add build-time dependency symlinks (such as Python) to PATH
-      const buildBinsDir = path.resolve('build', 'node_gyp_bins')
+      buildBinsDir = path.resolve('build', 'node_gyp_bins')
       process.env.PATH = `${buildBinsDir}:${process.env.PATH}`
-      log.verbose('bin symlinks', `adding symlinks (such as Python), at "${buildBinsDir}", to PATH`)
+      fs.mkdirSync(buildBinsDir, { recursive: true })
+      const symlinkDestination = path.join(buildBinsDir, 'python3')
+      try {
+        fs.unlinkSync(symlinkDestination)
+      } catch (err) {
+        if (err.code !== 'ENOENT') throw err
+      }
+      fs.symlinkSync(python, symlinkDestination)
+      log.verbose('bin symlinks', `created symlink to "${python}" in "${buildBinsDir}" and added to PATH`)
     }
 
     var proc = gyp.spawn(command, argv)
     proc.on('exit', onExit)
   }
 
   function onExit (code, signal) {
+    if (buildBinsDir) {
+      // Clean up the build-time dependency symlinks:
+      if (fs.rmSync) {
+        // fs.rm is only available in Node 14+
+        fs.rmSync(buildBinsDir, { recursive: true })
+      } else {
+        // Only used for old node, as recursive rmdir is deprecated in Node 14+
+        fs.rmdirSync(buildBinsDir, { recursive: true })
+      }
+    }
+
     if (code !== 0) {
       return callback(new Error('`' + command + '` failed with exit code: ' + code))
     }

lib/configure.js

@@ -17,7 +17,6 @@ if (win) {
 function configure (gyp, argv, callback) {
   var python
   var buildDir = path.resolve('build')
-  var buildBinsDir = path.join(buildDir, 'node_gyp_bins')
   var configNames = ['config.gypi', 'common.gypi']
   var configs = []
   var nodeDir
@@ -75,8 +74,7 @@ function configure (gyp, argv, callback) {
   function createBuildDir () {
     log.verbose('build dir', 'attempting to create "build" dir: %s', buildDir)
 
-    const deepestBuildDirSubdirectory = win ? buildDir : buildBinsDir
-    fs.mkdir(deepestBuildDirSubdirectory, { recursive: true }, function (err, isNew) {
+    fs.mkdir(buildDir, { recursive: true }, function (err, isNew) {
       if (err) {
         return callback(err)
       }
@@ -87,31 +85,11 @@ function configure (gyp, argv, callback) {
         findVisualStudio(release.semver, gyp.opts.msvs_version,
           createConfigFile)
       } else {
-        createPythonSymlink()
         createConfigFile()
       }
     })
   }
 
-  function createPythonSymlink () {
-    const symlinkDestination = path.join(buildBinsDir, 'python3')
-
-    log.verbose('python symlink', `creating symlink to "${python}" at "${symlinkDestination}"`)
-
-    fs.unlink(symlinkDestination, function (err) {
-      if (err && err.code !== 'ENOENT') {
-        log.verbose('python symlink', 'error when attempting to remove existing symlink')
-        log.verbose('python symlink', err.stack, 'errno: ' + err.errno)
-      }
-      fs.symlink(python, symlinkDestination, function (err) {
-        if (err) {
-          log.verbose('python symlink', 'error when attempting to create Python symlink')
-          log.verbose('python symlink', err.stack, 'errno: ' + err.errno)
-        }
-      })
-    })
-  }
-
   function createConfigFile (err, vsInfo) {
     if (err) {
       return callback(err)
@@ -120,7 +98,7 @@ function configure (gyp, argv, callback) {
       process.env.GYP_MSVS_VERSION = Math.min(vsInfo.versionYear, 2015)
       process.env.GYP_MSVS_OVERRIDE_PATH = vsInfo.path
     }
-    createConfigGypi({ gyp, buildDir, nodeDir, vsInfo }).then(configPath => {
+    createConfigGypi({ gyp, buildDir, nodeDir, vsInfo, python }).then(configPath => {
       configs.push(configPath)
       findConfigs()
     }).catch(err => {

lib/create-config-gypi.js

@@ -35,7 +35,7 @@ async function getBaseConfigGypi ({ gyp, nodeDir }) {
   return JSON.parse(JSON.stringify(process.config))
 }
 
-async function getCurrentConfigGypi ({ gyp, nodeDir, vsInfo }) {
+async function getCurrentConfigGypi ({ gyp, nodeDir, vsInfo, python }) {
   const config = await getBaseConfigGypi({ gyp, nodeDir })
   if (!config.target_defaults) {
     config.target_defaults = {}
@@ -75,6 +75,9 @@ async function getCurrentConfigGypi ({ gyp, nodeDir, vsInfo }) {
   // set the node development directory
   variables.nodedir = nodeDir
 
+  // set the configured Python path
+  variables.python = python
+
   // disable -T "thin" static archives by default
   variables.standalone_static_library = gyp.opts.thin ? 0 : 1
 
@@ -112,13 +115,13 @@ async function getCurrentConfigGypi ({ gyp, nodeDir, vsInfo }) {
   return config
 }
 
-async function createConfigGypi ({ gyp, buildDir, nodeDir, vsInfo }) {
+async function createConfigGypi ({ gyp, buildDir, nodeDir, vsInfo, python }) {
   const configFilename = 'config.gypi'
   const configPath = path.resolve(buildDir, configFilename)
 
   log.verbose('build/' + configFilename, 'creating config file')
 
-  const config = await getCurrentConfigGypi({ gyp, nodeDir, vsInfo })
+  const config = await getCurrentConfigGypi({ gyp, nodeDir, vsInfo, python })
 
   // ensures that any boolean values in config.gypi get stringified
   function boolsToString (k, v) {

test/test-configure-python.js

@@ -14,9 +14,7 @@ const configure = requireInject('../lib/configure', {
     mkdir: function (dir, options, cb) { cb() },
     promises: {
       writeFile: function (file, data) { return Promise.resolve(null) }
-    },
-    unlink: function (path, cb) { cb() },
-    symlink: function (target, path, cb) { cb() }
+    }
   }
 })