fix: Fix node global not being available during dependency installs. (

#1089)

* Add tests.

* Add more tests.

* Polish.

.yarn/versions/fdfb2523.yml

@@ -0,0 +1,9 @@
+releases:
+  "@moonrepo/cli": patch
+  "@moonrepo/core-linux-arm64-gnu": patch
+  "@moonrepo/core-linux-arm64-musl": patch
+  "@moonrepo/core-linux-x64-gnu": patch
+  "@moonrepo/core-linux-x64-musl": patch
+  "@moonrepo/core-macos-arm64": patch
+  "@moonrepo/core-macos-x64": patch
+  "@moonrepo/core-windows-x64-msvc": patch

crates/cli/tests/run_node_test.rs

@@ -366,6 +366,30 @@ fn avoids_postinstall_recursion() {
     assert.success();
 }
 
+#[test]
+fn can_exec_global_bin_as_child_process() {
+    let sandbox = node_sandbox();
+
+    let assert = sandbox.run_moon(|cmd| {
+        cmd.arg("run").arg("node:execBinSelf");
+    });
+
+    let output = assert.output();
+
+    assert!(predicate::str::contains("v18.0.0").eval(&output));
+}
+
+#[test]
+fn can_exec_global_bin_as_child_process_from_postinstall() {
+    let sandbox = node_sandbox();
+
+    let assert = sandbox.run_moon(|cmd| {
+        cmd.arg("run").arg("postinstall:noop");
+    });
+
+    assert.success();
+}
+
 mod install_deps {
     use super::*;
 

crates/core/test-utils/src/configs.rs

@@ -284,6 +284,7 @@ pub fn get_node_fixture_configs() -> (
         projects: Some(PartialWorkspaceProjects::Sources(FxHashMap::from_iter([
             ("node".into(), "base".to_owned()),
             ("lifecycles".into(), "lifecycles".to_owned()),
+            ("postinstall".into(), "postinstall".to_owned()),
             (
                 "postinstallRecursion".into(),
                 "postinstall-recursion".to_owned(),

crates/core/tool/src/lib.rs

@@ -18,9 +18,17 @@ use std::path::Path;
 /// other binaries of the same name. Otherwise, tooling like nvm will
 /// intercept execution and break our processes. We can work around this
 /// by prepending the `PATH` environment variable.
-pub fn get_path_env_var(bin_dir: &Path) -> std::ffi::OsString {
+pub fn prepend_path_env_var<I, V>(paths: I) -> std::ffi::OsString
+where
+    I: IntoIterator<Item = V>,
+    V: AsRef<Path>,
+{
     let path = env::var("PATH").unwrap_or_default();
-    let mut paths = vec![bin_dir.to_path_buf()];
+
+    let mut paths = paths
+        .into_iter()
+        .map(|p| p.as_ref().to_path_buf())
+        .collect::<Vec<_>>();
 
     paths.extend(env::split_paths(&path).collect::<Vec<_>>());
 

crates/node/platform/src/actions/run_target.rs

@@ -10,7 +10,7 @@ use moon_node_tool::NodeTool;
 use moon_process::Command;
 use moon_project::Project;
 use moon_task::Task;
-use moon_tool::{get_path_env_var, DependencyManager, Tool, ToolError};
+use moon_tool::{prepend_path_env_var, DependencyManager, Tool, ToolError};
 use moon_utils::{get_cache_dir, path, string_vec};
 use rustc_hash::FxHashMap;
 use starbase_styles::color;
@@ -163,6 +163,7 @@ pub fn create_target_command(
     let node_bin = node.get_bin_path()?;
     let node_options = create_node_options(&node.config, context, task)?;
     let mut command = Command::new(node.get_shim_path().unwrap_or(node_bin));
+    let mut is_package_manager = false;
 
     match task.command.as_str() {
         "node" | "nodejs" => {
@@ -172,12 +173,15 @@ pub fn create_target_command(
             command = Command::new(node.get_npx_path()?);
         }
         "npm" => {
+            is_package_manager = true;
             command = node.get_npm()?.create_command(node)?;
         }
         "pnpm" => {
+            is_package_manager = true;
             command = node.get_pnpm()?.create_command(node)?;
         }
         "yarn" | "yarnpkg" => {
+            is_package_manager = true;
             command = node.get_yarn()?.create_command(node)?;
         }
         bin => {
@@ -189,7 +193,12 @@ pub fn create_target_command(
         }
     };
 
-    command.env("PATH", get_path_env_var(&node.tool.get_tool_dir()));
+    if !is_package_manager {
+        command.env(
+            "PATH",
+            prepend_path_env_var([node.tool.get_bin_path()?.parent().unwrap()]),
+        );
+    }
 
     prepare_target_command(&mut command, context, task, &node.config)?;
 

crates/node/tool/src/node_tool.rs

@@ -8,7 +8,7 @@ use moon_platform_runtime::RuntimeReq;
 use moon_process::Command;
 use moon_terminal::{print_checkpoint, Checkpoint};
 use moon_tool::{
-    async_trait, get_path_env_var, load_tool_plugin, DependencyManager, Tool, ToolError,
+    async_trait, load_tool_plugin, prepend_path_env_var, DependencyManager, Tool, ToolError,
 };
 use proto_core::{Id, ProtoEnvironment, Tool as ProtoTool};
 use rustc_hash::FxHashMap;
@@ -78,7 +78,10 @@ impl NodeTool {
         let mut cmd = Command::new(self.get_npx_path()?);
 
         if !self.global {
-            cmd.env("PATH", get_path_env_var(&self.tool.get_tool_dir()));
+            cmd.env(
+                "PATH",
+                prepend_path_env_var([self.tool.get_bin_path()?.parent().unwrap()]),
+            );
         }
 
         cmd.args(exec_args)

crates/node/tool/src/npm_tool.rs

@@ -4,7 +4,7 @@ use moon_logger::debug;
 use moon_node_lang::{npm, LockfileDependencyVersions, NPM};
 use moon_process::Command;
 use moon_terminal::{print_checkpoint, Checkpoint};
-use moon_tool::{async_trait, get_path_env_var, load_tool_plugin, DependencyManager, Tool};
+use moon_tool::{async_trait, load_tool_plugin, prepend_path_env_var, DependencyManager, Tool};
 use moon_utils::is_ci;
 use proto_core::{Id, ProtoEnvironment, Tool as ProtoTool, UnresolvedVersionSpec};
 use rustc_hash::FxHashMap;
@@ -117,7 +117,13 @@ impl DependencyManager<NodeTool> for NpmTool {
         };
 
         if !self.global {
-            cmd.env("PATH", get_path_env_var(&self.tool.get_tool_dir()));
+            cmd.env(
+                "PATH",
+                prepend_path_env_var([
+                    node.get_bin_path()?.parent().unwrap(),
+                    self.tool.get_bin_path()?.parent().unwrap(),
+                ]),
+            );
         }
 
         cmd.env("PROTO_NODE_BIN", node.get_bin_path()?);

crates/node/tool/src/pnpm_tool.rs

@@ -4,7 +4,7 @@ use moon_logger::debug;
 use moon_node_lang::{pnpm, LockfileDependencyVersions, PNPM};
 use moon_process::Command;
 use moon_terminal::{print_checkpoint, Checkpoint};
-use moon_tool::{async_trait, get_path_env_var, load_tool_plugin, DependencyManager, Tool};
+use moon_tool::{async_trait, load_tool_plugin, prepend_path_env_var, DependencyManager, Tool};
 use moon_utils::is_ci;
 use proto_core::{Id, ProtoEnvironment, Tool as ProtoTool, UnresolvedVersionSpec, VersionReq};
 use rustc_hash::FxHashMap;
@@ -123,7 +123,13 @@ impl DependencyManager<NodeTool> for PnpmTool {
         };
 
         if !self.global {
-            cmd.env("PATH", get_path_env_var(&self.tool.get_tool_dir()));
+            cmd.env(
+                "PATH",
+                prepend_path_env_var([
+                    node.get_bin_path()?.parent().unwrap(),
+                    self.tool.get_bin_path()?.parent().unwrap(),
+                ]),
+            );
         }
 
         cmd.env("PROTO_NODE_BIN", node.get_bin_path()?);

crates/node/tool/src/yarn_tool.rs

@@ -5,7 +5,7 @@ use moon_node_lang::{yarn, LockfileDependencyVersions, YARN};
 use moon_process::Command;
 use moon_terminal::{print_checkpoint, Checkpoint};
 use moon_tool::{
-    async_trait, get_path_env_var, load_tool_plugin, DependencyManager, Tool, ToolError,
+    async_trait, load_tool_plugin, prepend_path_env_var, DependencyManager, Tool, ToolError,
 };
 use moon_utils::{get_workspace_root, is_ci};
 use proto_core::{Id, ProtoEnvironment, Tool as ProtoTool, UnresolvedVersionSpec};
@@ -179,7 +179,13 @@ impl DependencyManager<NodeTool> for YarnTool {
         };
 
         if !self.global {
-            cmd.env("PATH", get_path_env_var(&self.tool.get_tool_dir()));
+            cmd.env(
+                "PATH",
+                prepend_path_env_var([
+                    node.get_bin_path()?.parent().unwrap(),
+                    self.tool.get_bin_path()?.parent().unwrap(),
+                ]),
+            );
         }
 
         cmd.env("PROTO_NODE_BIN", node.get_bin_path()?);

crates/rust/tool/src/rust_tool.rs

@@ -84,8 +84,8 @@ impl Tool for RustTool {
             // When offline and the tool doesn't exist, fallback to the global binary
         } else if proto_core::is_offline() {
             debug!(
-                    "No internet connection and Rust has not been setup, falling back to global binary in PATH"
-                );
+                "No internet connection and Rust has not been setup, falling back to global binary in PATH"
+            );
 
             self.global = true;
 

packages/cli/CHANGELOG.md

@@ -10,6 +10,12 @@
   - More accurately monitors signals (ctrl+c) and shutdowns.
   - Tasks can now be configured with a timeout.
 
+## Unreleased
+
+#### 🐞 Fixes
+
+- Fixed some `PATH` inconsistencies when executing npm/pnpm/yarn binaries.
+
 ## 1.14.3
 
 #### 🚀 Updates

tests/fixtures/node/base/execBin.js

@@ -0,0 +1,3 @@
+const { spawnSync } = require('child_process');
+
+spawnSync('node', ['--version'], { stdio: 'inherit' });

tests/fixtures/node/base/moon.yml

@@ -12,6 +12,9 @@ tasks:
   mjs:
     command: node
     args: ./mjsFile.mjs
+  execBinSelf:
+    command: node
+    args: ./execBin.js
 
   # Runner cases
   exitCodeNonZero:

tests/fixtures/node/postinstall/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "test-node-postinstall",
+  "version": "1.0.0",
+  "scripts": {
+    "postinstall": "node ./postinstall.js"
+  }
+}

tests/fixtures/node/postinstall/postinstall.js

@@ -0,0 +1,3 @@
+const { spawnSync } = require('child_process');
+
+spawnSync('node', ['--version'], { stdio: 'inherit' });