From 47d19461a59cc7997f24987d8564e757683b8a5e Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Thu, 6 Jun 2024 14:21:25 -0700
Subject: [PATCH] fix(cli): Overwrite existing bin entries in `node_modules`
 (#24123)

Previously we warned on unix and didn't touch them on windows, now we
unconditionally overwrite them. This matches what npm does.
---
 .../managed/resolvers/local/bin_entries.rs    | 37 ++++++++-----------
 tests/specs/task/bin_package/__test__.jsonc   |  5 +--
 .../specs/task/bin_package/already-set-up.out |  9 -----
 3 files changed, 17 insertions(+), 34 deletions(-)
 delete mode 100644 tests/specs/task/bin_package/already-set-up.out

diff --git a/cli/npm/managed/resolvers/local/bin_entries.rs b/cli/npm/managed/resolvers/local/bin_entries.rs
index 7890177eed..4a3c5ce4f8 100644
--- a/cli/npm/managed/resolvers/local/bin_entries.rs
+++ b/cli/npm/managed/resolvers/local/bin_entries.rs
@@ -226,15 +226,6 @@ fn set_up_bin_shim(
 
   cmd_shim.set_extension("cmd");
   let shim = format!("@deno run -A npm:{}/{bin_name} %*", package.id.nv);
-  if cmd_shim.exists() {
-    if let Ok(contents) = fs::read_to_string(cmd_shim) {
-      if contents == shim {
-        // up to date
-        return Ok(());
-      }
-    }
-    return Ok(());
-  }
   fs::write(&cmd_shim, shim).with_context(|| {
     format!("Can't set up '{}' bin at {}", bin_name, cmd_shim.display())
   })?;
@@ -287,19 +278,21 @@ fn symlink_bin_entry(
 
   if let Err(err) = symlink(&original_relative, &link) {
     if err.kind() == io::ErrorKind::AlreadyExists {
-      let resolved = std::fs::read_link(&link).ok();
-      if let Some(resolved) = resolved {
-        if resolved != original_relative {
-          log::warn!(
-            "{} Trying to set up '{}' bin for \"{}\", but an entry pointing to \"{}\" already exists. Skipping...", 
-            deno_terminal::colors::yellow("Warning"), 
-            bin_name,
-            resolved.display(),
-            original_relative.display()
-          );
-        }
-        return Ok(());
-      }
+      // remove and retry
+      std::fs::remove_file(&link).with_context(|| {
+        format!(
+          "Failed to remove existing bin symlink at {}",
+          link.display()
+        )
+      })?;
+      symlink(&original_relative, &link).with_context(|| {
+        format!(
+          "Can't set up '{}' bin at {}",
+          bin_name,
+          original_relative.display()
+        )
+      })?;
+      return Ok(());
     }
     return Err(err).with_context(|| {
       format!(
diff --git a/tests/specs/task/bin_package/__test__.jsonc b/tests/specs/task/bin_package/__test__.jsonc
index 19bad633e0..d5da4b4e1e 100644
--- a/tests/specs/task/bin_package/__test__.jsonc
+++ b/tests/specs/task/bin_package/__test__.jsonc
@@ -27,17 +27,16 @@
         }
       ]
     },
-    "warns_if_already_setup": {
+    "clobbers_if_already_setup": {
       "tempDir": true,
       "steps": [{
-        "if": "unix",
         "commandName": "npm",
         "args": "install",
         "output": "\nadded 1 package in [WILDCARD]\n"
       }, {
         "if": "unix",
         "args": "task sayhi",
-        "output": "already-set-up.out"
+        "output": "task.out"
       }]
     }
   }
diff --git a/tests/specs/task/bin_package/already-set-up.out b/tests/specs/task/bin_package/already-set-up.out
deleted file mode 100644
index 336459daa2..0000000000
--- a/tests/specs/task/bin_package/already-set-up.out
+++ /dev/null
@@ -1,9 +0,0 @@
-Download http://localhost:4260/@denotest/bin
-Download http://localhost:4260/@denotest/bin/1.0.0.tgz
-Initialize @denotest/bin@1.0.0
-Warning Trying to set up [WILDCARD] bin for [WILDCARD], but an entry pointing to [WILDCARD] already exists. Skipping...
-Warning Trying to set up [WILDCARD] bin for [WILDCARD] but an entry pointing to [WILDCARD] already exists. Skipping...
-Warning Trying to set up [WILDCARD] bin for [WILDCARD], but an entry pointing to [WILDCARD] already exists. Skipping...
-Task sayhi cli-esm hi hello
-hi
-hello