set-envs: include booleans, skip already-set envs

Fix #1650

PR-URL: #1652
Credit: @isaacs
Close: #1652
Reviewed-by: @MylesBorins

lib/config/set-envs.js

@@ -26,21 +26,25 @@ const sameArrayValue = (def, val) => {
     return false
   }
   for (let i = 0; i < def.length; i++) {
+    /* istanbul ignore next - there are no array configs where the default
+     * is not an empty array, so this loop is a no-op, but it's the correct
+     * thing to do if we ever DO add a config like that. */
     if (def[i] !== val[i]) {
       return false
     }
   }
   return true
 }
 
-const setEnv = (rawKey, rawVal) => {
+const setEnv = (env, rawKey, rawVal) => {
   const val = envVal(rawVal)
   const key = envKey(rawKey, val)
-  if (key)
-    process.env[key] = val
+  if (key && val !== null) {
+    env[key] = val
+  }
 }
 
-const setEnvs = npm => {
+const setEnvs = (npm, env = process.env) => {
   // This ensures that all npm config values that are not the defaults are
   // shared appropriately with child processes, without false positives.
   //
@@ -51,38 +55,37 @@ const setEnvs = npm => {
   // if the key is NOT the default value,
   //   if the env is setting it, then leave it (already set)
   //   otherwise, set the env
-  console.error(npm.config.list)
-  const { config: { list: [cli, env] } } = npm
-  const cliSet = new Set(Object.keys(cli))
-  const envSet = new Set(Object.keys(env))
+  const { config: { list: [cliConf, envConf] } } = npm
+  const cliSet = new Set(Object.keys(cliConf))
+  const envSet = new Set(Object.keys(envConf))
   const { defaults } = require('./defaults.js')
   // the configs form a prototype chain, so we can for/in over cli to
   // see all the current values, and hasOwnProperty to see if it's
   // set therre.
-  for (const key in cli) {
-    if (sameConfigValue(defaults[key], cli[key])) {
-      if (!sameConfigValue(defaults[key], env[key])) {
+  for (const key in cliConf) {
+    if (sameConfigValue(defaults[key], cliConf[key])) {
+      if (!sameConfigValue(defaults[key], envConf[key])) {
         // getting set back to the default in the cli config
-        setEnv(key, cli[key])
+        setEnv(env, key, cliConf[key])
       }
     } else {
       // config is not the default
       if (!(envSet.has(key) && !cliSet.has(key))) {
         // was not set in the env, so we have to put it there
-        setEnv(key, cli[key])
+        setEnv(env, key, cliConf[key])
       }
     }
   }
 
   // also set some other common ones.
-  process.env.npm_execpath = require.main.filename
-  process.env.npm_node_execpath = process.execPath
-  process.env.npm_command = npm.command
+  env.npm_execpath = require.main.filename
+  env.npm_node_execpath = process.execPath
+  env.npm_command = npm.command
 
   // note: this doesn't afect the *current* node process, of course, since
   // it's already started, but it does affect the options passed to scripts.
-  if (configs['node-options']) {
-    process.env.NODE_OPTIONS = configs['node-options']
+  if (cliConf['node-options']) {
+    env.NODE_OPTIONS = cliConf['node-options']
   }
 }
 

test/lib/config/set-envs.js

@@ -0,0 +1,165 @@
+const t = require('tap')
+const setEnvs = require('../../../lib/config/set-envs.js')
+const { defaults } = require('../../../lib/config/defaults.js')
+
+t.test('set envs that are not defaults and not already in env', t => {
+  const envConf = Object.create(defaults)
+  const cliConf = Object.create(envConf)
+  const npm = {
+    config: {
+      list: [cliConf, envConf]
+    }
+  }
+  const extras = {
+    npm_execpath: require.main.filename,
+    npm_node_execpath: process.execPath,
+    npm_command: undefined
+  }
+  const env = {}
+  setEnvs(npm, env)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create')
+  envConf.call = 'me, maybe'
+  setEnvs(npm, env)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create, already in env')
+  delete envConf.call
+  cliConf.call = 'me, maybe'
+  setEnvs(npm, env)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_call: 'me, maybe'
+  }, 'set in env, because changed from default in cli')
+  envConf.call = 'me, maybe'
+  cliConf.call = ''
+  cliConf['node-options'] = 'some options for node'
+  setEnvs(npm, env)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_call: '',
+    npm_config_node_options: 'some options for node',
+    NODE_OPTIONS: 'some options for node'
+  }, 'set in env, because changed from default in env, back to default in cli')
+  t.end()
+})
+
+t.test('set envs that are not defaults and not already in env, array style', t => {
+  const envConf = Object.create(defaults)
+  const cliConf = Object.create(envConf)
+  const npm = {
+    config: {
+      list: [cliConf, envConf]
+    }
+  }
+  const extras = {
+    npm_execpath: require.main.filename,
+    npm_node_execpath: process.execPath,
+    npm_command: undefined
+  }
+  const env = {}
+  setEnvs(npm, env)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create')
+  envConf.omit = ['dev']
+  setEnvs(npm, env)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create, already in env')
+  delete envConf.omit
+  cliConf.omit = ['dev', 'optional']
+  setEnvs(npm, env)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_omit: 'dev\n\noptional'
+  }, 'set in env, because changed from default in cli')
+  envConf.omit = ['optional', 'peer']
+  cliConf.omit = []
+  setEnvs(npm, env)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_omit: ''
+  }, 'set in env, because changed from default in env, back to default in cli')
+  t.end()
+})
+
+t.test('set envs that are not defaults and not already in env, boolean edition', t => {
+  const envConf = Object.create(defaults)
+  const cliConf = Object.create(envConf)
+  const npm = {
+    config: {
+      list: [cliConf, envConf]
+    }
+  }
+  const extras = {
+    npm_execpath: require.main.filename,
+    npm_node_execpath: process.execPath,
+    npm_command: undefined
+  }
+  const env = {}
+  setEnvs(npm, env)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create')
+  envConf.audit = false
+  setEnvs(npm, env)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create, already in env')
+  delete envConf.audit
+  cliConf.audit = false
+  cliConf.ignoreObjects = {
+    some: { object: 12345 }
+  }
+  setEnvs(npm, env)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_audit: ''
+  }, 'set in env, because changed from default in cli')
+  envConf.audit = false
+  cliConf.audit = true
+  setEnvs(npm, env)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_audit: 'true'
+  }, 'set in env, because changed from default in env, back to default in cli')
+  t.end()
+})
+
+t.test('default to process.env', t => {
+  const envConf = Object.create(defaults)
+  const cliConf = Object.create(envConf)
+  const npm = {
+    config: {
+      list: [cliConf, envConf]
+    }
+  }
+  const extras = {
+    npm_execpath: require.main.filename,
+    npm_node_execpath: process.execPath,
+    npm_command: undefined
+  }
+  const env = {}
+  const envDescriptor = Object.getOwnPropertyDescriptor(process, 'env')
+  Object.defineProperty(process, 'env', {
+    value: env,
+    configurable: true,
+    enumerable: true,
+    writable: true
+  })
+  t.teardown(() => Object.defineProperty(process, env, envDescriptor))
+
+  setEnvs(npm)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create')
+  envConf.audit = false
+  setEnvs(npm)
+  t.strictSame(env, { ...extras }, 'no new environment vars to create, already in env')
+  delete envConf.audit
+  cliConf.audit = false
+  cliConf.ignoreObjects = {
+    some: { object: 12345 }
+  }
+  setEnvs(npm)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_audit: ''
+  }, 'set in env, because changed from default in cli')
+  envConf.audit = false
+  cliConf.audit = true
+  setEnvs(npm)
+  t.strictSame(env, {
+    ...extras,
+    npm_config_audit: 'true'
+  }, 'set in env, because changed from default in env, back to default in cli')
+  t.end()
+})