From c88612d6d3bd964e53e4923bce1cdf7c6d6a8f55 Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc@chromium.org>
Date: Tue, 27 Jun 2023 16:37:22 -0700
Subject: [PATCH] Fix `activated_path_skip` on repeated running of
 `emsdk_env.sh` (#1250)

The first time around `node` was being correctly added to the PATH, but
the second time around this code was observing the emsdk copy of node
in the PATH and assuming it could be skipped.

Fixes: #1240
---
 .circleci/config.yml   |  1 +
 emsdk.py               | 13 +++++++++++--
 test/test_node_path.sh | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100755 test/test_node_path.sh

diff --git a/.circleci/config.yml b/.circleci/config.yml
index dc40d23..c4d2734 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -80,6 +80,7 @@ jobs:
       - run:
           name: Install debian packages
           command: apt-get update -q && apt-get install -q -y cmake build-essential openjdk-8-jre-headless ksh zsh
+      - run: test/test_node_path.sh
       - run: test/test.sh
       - run: test/test_source_env.sh
       - run:
diff --git a/emsdk.py b/emsdk.py
index 11f8fdb..fb3a3a2 100644
--- a/emsdk.py
+++ b/emsdk.py
@@ -1438,9 +1438,18 @@ def get_required_path(active_tools):
   path_add = [to_native_path(EMSDK_PATH)]
   for tool in active_tools:
     if hasattr(tool, 'activated_path'):
-      if hasattr(tool, 'activated_path_skip') and which(tool.activated_path_skip):
-        continue
       path = to_native_path(tool.expand_vars(tool.activated_path))
+      # If the tool has an activated_path_skip attribute then we don't add
+      # the tools path to the users path if a program by that name is found
+      # in the existing PATH.  This allows us to, for example, add our version
+      # node to the users PATH if, and only if, they don't already have a
+      # another version of node in thier PATH.
+      if hasattr(tool, 'activated_path_skip'):
+        current_path = which(tool.activated_path_skip)
+        # We found an executable by this name in the current PATH, but we
+        # ignore our own version for this purpose.
+        if current_path and os.path.dirname(current_path) != path:
+          continue
       path_add.append(path)
   return path_add
 
diff --git a/test/test_node_path.sh b/test/test_node_path.sh
new file mode 100755
index 0000000..582152b
--- /dev/null
+++ b/test/test_node_path.sh
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+
+echo "Test that node is added to that PATH if, and only if, it is not one already present".
+
+if [ -n "$EMSDK" ]; then
+    echo "EMSDK is already defined in this shell. Run tests in a shell without sourcing emsdk_env.sh first"
+    exit 1
+fi
+
+DIR=$(dirname "$BASH_SOURCE")
+cd $DIR/..
+
+./emsdk install latest
+./emsdk activate latest
+
+if which node; then
+  echo "Test should be run without node in the path"
+  exit 1
+fi
+
+# Run emsdk_env.sh and confirm that node was added to the PATH
+. emsdk_env.sh
+
+if ! which node; then
+  echo "node not found in path after emsdk_env.sh"
+  exit 1
+fi
+
+# Run emsdk_env.sh again and confirm that node is still in the PATH
+. emsdk_env.sh
+
+if ! which node; then
+  echo "node not found in path after emsdk_env.sh"
+  exit 1
+fi