From f60640267776638d4699fbf05095dc6d1a9016f0 Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Thu, 22 Aug 2024 14:55:17 -0700
Subject: [PATCH] fix(install): Use relative symlinks in deno install (#25164)

Fixes https://github.com/denoland/deno/issues/25161
---
 cli/npm/managed/resolvers/local.rs            | 56 +++++++++++--------
 cli/util/fs.rs                                |  2 +-
 .../install/move_after_install/__test__.jsonc | 28 ++++++++++
 .../move_after_install/test-project/main.mjs  |  4 ++
 .../test-project/package.json                 |  5 ++
 5 files changed, 70 insertions(+), 25 deletions(-)
 create mode 100644 tests/specs/install/move_after_install/__test__.jsonc
 create mode 100644 tests/specs/install/move_after_install/test-project/main.mjs
 create mode 100644 tests/specs/install/move_after_install/test-project/package.json

diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 61e50ba8d3..8f729d79c7 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -1048,42 +1048,50 @@ fn symlink_package_dir(
   // need to delete the previous symlink before creating a new one
   let _ignore = fs::remove_dir_all(new_path);
 
+  let old_path_relative =
+    crate::util::path::relative_path(new_parent, old_path)
+      .unwrap_or_else(|| old_path.to_path_buf());
+
   #[cfg(windows)]
-  return junction_or_symlink_dir(old_path, new_path);
+  {
+    junction_or_symlink_dir(&old_path_relative, old_path, new_path)
+  }
   #[cfg(not(windows))]
-  symlink_dir(old_path, new_path)
+  {
+    symlink_dir(&old_path_relative, new_path).map_err(Into::into)
+  }
 }
 
 #[cfg(windows)]
 fn junction_or_symlink_dir(
+  old_path_relative: &Path,
   old_path: &Path,
   new_path: &Path,
 ) -> Result<(), AnyError> {
-  use deno_core::anyhow::bail;
-  // Use junctions because they're supported on ntfs file systems without
-  // needing to elevate privileges on Windows
+  static USE_JUNCTIONS: std::sync::atomic::AtomicBool =
+    std::sync::atomic::AtomicBool::new(false);
 
-  match junction::create(old_path, new_path) {
+  if USE_JUNCTIONS.load(std::sync::atomic::Ordering::Relaxed) {
+    // Use junctions because they're supported on ntfs file systems without
+    // needing to elevate privileges on Windows.
+    // Note: junctions don't support relative paths, so we need to use the
+    // absolute path here.
+    return junction::create(old_path, new_path)
+      .context("Failed creating junction in node_modules folder");
+  }
+
+  match symlink_dir(old_path_relative, new_path) {
     Ok(()) => Ok(()),
-    Err(junction_err) => {
-      if cfg!(debug_assertions) {
-        // When running the tests, junctions should be created, but if not then
-        // surface this error.
-        log::warn!("Error creating junction. {:#}", junction_err);
-      }
-
-      match symlink_dir(old_path, new_path) {
-        Ok(()) => Ok(()),
-        Err(symlink_err) => bail!(
-          concat!(
-            "Failed creating junction and fallback symlink in node_modules folder.\n\n",
-            "{:#}\n\n{:#}",
-          ),
-          junction_err,
-          symlink_err,
-        ),
-      }
+    Err(symlink_err)
+      if symlink_err.kind() == std::io::ErrorKind::PermissionDenied =>
+    {
+      USE_JUNCTIONS.store(true, std::sync::atomic::Ordering::Relaxed);
+      junction::create(old_path, new_path).map_err(Into::into)
     }
+    Err(symlink_err) => Err(
+      AnyError::from(symlink_err)
+        .context("Failed creating symlink in node_modules folder"),
+    ),
   }
 }
 
diff --git a/cli/util/fs.rs b/cli/util/fs.rs
index c414abd591..145f9c83b8 100644
--- a/cli/util/fs.rs
+++ b/cli/util/fs.rs
@@ -509,7 +509,7 @@ pub fn hard_link_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> {
   Ok(())
 }
 
-pub fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), AnyError> {
+pub fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), Error> {
   let err_mapper = |err: Error| {
     Error::new(
       err.kind(),
diff --git a/tests/specs/install/move_after_install/__test__.jsonc b/tests/specs/install/move_after_install/__test__.jsonc
new file mode 100644
index 0000000000..1699e9f6f9
--- /dev/null
+++ b/tests/specs/install/move_after_install/__test__.jsonc
@@ -0,0 +1,28 @@
+{
+  "tempDir": true,
+  "envs": {
+    "DENO_FUTURE": "1"
+  },
+  "steps": [
+    {
+      "cwd": "./test-project",
+      "args": "install",
+      "output": "[WILDCARD]"
+    },
+    {
+      "cwd": "./test-project",
+      "args": "run -A main.mjs",
+      "output": "5\n"
+    },
+    {
+      "commandName": "mv",
+      "args": "test-project test-project-moved",
+      "output": ""
+    },
+    {
+      "cwd": "./test-project-moved",
+      "args": "run -A main.mjs",
+      "output": "5\n"
+    }
+  ]
+}
diff --git a/tests/specs/install/move_after_install/test-project/main.mjs b/tests/specs/install/move_after_install/test-project/main.mjs
new file mode 100644
index 0000000000..1beed6ef4e
--- /dev/null
+++ b/tests/specs/install/move_after_install/test-project/main.mjs
@@ -0,0 +1,4 @@
+import { getValue, setValue } from "@denotest/esm-basic";
+
+setValue(5);
+console.log(getValue());
diff --git a/tests/specs/install/move_after_install/test-project/package.json b/tests/specs/install/move_after_install/test-project/package.json
new file mode 100644
index 0000000000..54ca824d64
--- /dev/null
+++ b/tests/specs/install/move_after_install/test-project/package.json
@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@denotest/esm-basic": "*"
+  }
+}