Fix: Add $NODE when running scripts and make yarn env reflect these

__tests__/commands/run.js

@@ -1,6 +1,14 @@
 /* @flow */
 
-jest.mock('../../src/util/execute-lifecycle-script');
+jest.mock('../../src/util/execute-lifecycle-script', () => {
+  return {
+    // $FlowFixMe
+    ...require.requireActual('../../src/util/execute-lifecycle-script'),
+    execCommand: jest.fn(),
+  };
+});
+
+import path from 'path';
 
 import {run as buildRun} from './_helpers.js';
 import {BufferReporter} from '../../src/reporters/index.js';
@@ -10,9 +18,7 @@ import * as reporters from '../../src/reporters/index.js';
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 90000;
 
-const execCommand: $FlowFixMe = require('../../src/util/execute-lifecycle-script').execCommand;
-
-const path = require('path');
+const {execCommand}: $FlowFixMe = require('../../src/util/execute-lifecycle-script');
 
 beforeEach(() => execCommand.mockClear());
 
@@ -77,11 +83,34 @@ test('properly handle bin scripts', (): Promise<void> => {
 
 test('properly handle env command', (): Promise<void> => {
   return runRun(['env'], {}, 'no-args', (config, reporter): ?Promise<void> => {
-    const rprtr = new reporters.BufferReporter({stdout: null, stdin: null});
-
-    rprtr.info(`${JSON.stringify(process.env, null, 2)}`);
-
-    expect(reporter.getBuffer()).toEqual(rprtr.getBuffer());
+    // $FlowFixMe
+    const result = JSON.parse(reporter.getBuffer()[0].data);
+
+    const env = {};
+    let pathVarName = 'PATH';
+    for (const key of Object.keys(process.env)) {
+      // Filter out yarn-added `npm_` variables since we run tests through yarn already
+      if (key.startsWith('npm_')) {
+        continue;
+      }
+      // We need this below for Windows which has case-insensitive env vars
+      // If we used `process.env` directly, node takes care of this for us,
+      // but since we use a subset of it, we need to get the "real" path key
+      // name for Jest's case-sensitive object comparison below.
+      if (key.toUpperCase() === 'PATH') {
+        pathVarName = key;
+      }
+      env[key] = process.env[key];
+    }
+
+    result[pathVarName] = result[pathVarName] ? result[pathVarName].split(path.delimiter) : [];
+    // $FlowFixMe
+    env[pathVarName] = env[pathVarName] ? expect.arrayContaining(env[pathVarName].split(path.delimiter)) : [];
+
+    expect(result).toMatchObject(env);
+    expect(result).toHaveProperty('npm_lifecycle_event');
+    expect(result).toHaveProperty('npm_execpath');
+    expect(result).toHaveProperty('npm_node_execpath');
   });
 });
 

src/cli/commands/run.js

@@ -2,7 +2,7 @@
 
 import type {Reporter} from '../../reporters/index.js';
 import type Config from '../../config.js';
-import {execCommand} from '../../util/execute-lifecycle-script.js';
+import {execCommand, makeEnv} from '../../util/execute-lifecycle-script.js';
 import {MessageError} from '../../errors.js';
 import {registries} from '../../resolvers/index.js';
 import * as fs from '../../util/fs.js';
@@ -87,7 +87,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
         await execCommand(stage, config, cmdWithArgs, config.cwd);
       }
     } else if (action === 'env') {
-      reporter.info(`${JSON.stringify(process.env, null, 2)}`);
+      reporter.log(JSON.stringify(await makeEnv('env', config.cwd, config), null, 2), {force: true});
     } else {
       let suggestion;
 

src/util/execute-lifecycle-script.js

@@ -33,7 +33,13 @@ export async function makeEnv(
 ): {
   [key: string]: string,
 } {
-  const env = Object.assign({}, process.env);
+  const env = {
+    NODE: process.execPath,
+    // This lets `process.env.NODE` to override our `process.execPath`.
+    // This is a bit confusing but it is how `npm` was designed so we
+    // try to be compatible with that.
+    ...process.env,
+  };
 
   // Merge in the `env` object specified in .yarnrc
   const customEnv = config.getOption('env');
@@ -42,7 +48,7 @@ export async function makeEnv(
   }
 
   env.npm_lifecycle_event = stage;
-  env.npm_node_execpath = env.NODE || process.execPath;
+  env.npm_node_execpath = env.NODE;
   env.npm_execpath = env.npm_execpath || process.mainModule.filename;
 
   // Set the env to production for npm compat if production mode.
@@ -117,23 +123,9 @@ export async function makeEnv(
     env[envKey] = val;
   }
 
-  return env;
-}
-
-export async function executeLifecycleScript(
-  stage: string,
-  config: Config,
-  cwd: string,
-  cmd: string,
-  spinner?: ReporterSpinner,
-): LifecycleReturn {
-  // if we don't have a spinner then pipe everything to the terminal
-  const stdio = spinner ? undefined : 'inherit';
-
-  const env = await makeEnv(stage, cwd, config);
-
   // split up the path
-  const pathParts = (env[constants.ENV_PATH_KEY] || '').split(path.delimiter);
+  const envPath = env[constants.ENV_PATH_KEY];
+  const pathParts = envPath ? envPath.split(path.delimiter) : [];
 
   // Include node-gyp version that was bundled with the current Node.js version,
   // if available.
@@ -156,15 +148,30 @@ export async function executeLifecycleScript(
     pathParts.unshift(path.join(cwd, binFolder));
   }
 
-  await checkForGypIfNeeded(config, cmd, pathParts);
-
   if (config.scriptsPrependNodePath) {
     pathParts.unshift(path.join(path.dirname(process.execPath)));
   }
 
   // join path back together
   env[constants.ENV_PATH_KEY] = pathParts.join(path.delimiter);
 
+  return env;
+}
+
+export async function executeLifecycleScript(
+  stage: string,
+  config: Config,
+  cwd: string,
+  cmd: string,
+  spinner?: ReporterSpinner,
+): LifecycleReturn {
+  // if we don't have a spinner then pipe everything to the terminal
+  const stdio = spinner ? undefined : 'inherit';
+
+  const env = await makeEnv(stage, cwd, config);
+
+  await checkForGypIfNeeded(config, cmd, env[constants.ENV_PATH_KEY].split(path.delimiter));
+
   // get shell
   if (process.platform === 'win32') {
     // handle windows run scripts starting with a relative path