💥 Correct the behaviour of DENO_INSTALL_ROOT within (un)install com…

…mands

As can be seen in the documentation, `DENO_INSTALL_ROOT` is meant to be the folder wherein Deno will install executables, with the default being `$HOME/.deno/bin`.

However, the current behaviour of the `install` and `uninstall` commands, if `DENO_INSTALL_ROOT` is set, is to still append "bin" to the path. This leads to executables being installed in a folder like `$HOME/.deno/bin/bin`, which is obviously not the intended behaviour.

This commit removes this appending. Do note that this is a breaking change in the behaviour of the `--root` flag, as previously it referred to a folder wherein the `bin` folder was located, but now it refers to the `bin` folder itself. For example: `deno install --root /foo/bar test.ts` would previously install the executable in `/foo/bar/bin/test`, but now it will install it in `/foo/bar/test`.

cli/args/flags.rs

@@ -2579,12 +2579,12 @@ The executable name is inferred by default:
   - If the resulting name has an <p(245)>@...</> suffix, strip it.
 
 To change the installation root, use <c>--root</>:
-  <p(245)>deno install -g --allow-net --allow-read --root /usr/local jsr:@std/http/file-server</>
+  <p(245)>deno install -g --allow-net --allow-read --root /usr/local/bin jsr:@std/http/file-server</>
 
 The installation root is determined, in order of precedence:
   - <p(245)>--root</> option
   - <p(245)>DENO_INSTALL_ROOT</> environment variable
-  - <p(245)>$HOME/.deno</>
+  - <p(245)>$HOME/.deno/bin</>
 
 These must be added to the path manually if required."), UnstableArgsConfig::ResolutionAndRuntime)
     .visible_alias("i")
@@ -2755,12 +2755,12 @@ fn uninstall_subcommand() -> Command {
   <p(245)>deno uninstall --global file_server</>
 
 To change the installation root, use <c>--root</> flag:
-  <p(245)>deno uninstall --global --root /usr/local serve</>
+  <p(245)>deno uninstall --global --root /usr/local/bin serve</>
 
 The installation root is determined, in order of precedence:
   - <p(245)>--root</> option
   - <p(245)>DENO_INSTALL_ROOT</> environment variable
-  - <p(245)>$HOME/.deno</>"),
+  - <p(245)>$HOME/.deno/bin</>"),
     UnstableArgsConfig::None,
   )
   .defer(|cmd| {

cli/tools/installer.rs

@@ -136,6 +136,7 @@ fn get_installer_root() -> Result<PathBuf, io::Error> {
         )
       })?;
   home_path.push(".deno");
+  home_path.push("bin");
   Ok(home_path)
 }
 
@@ -213,12 +214,11 @@ pub async fn uninstall(
   };
 
   let cwd = std::env::current_dir().context("Unable to get CWD")?;
-  let root = if let Some(root) = uninstall_flags.root {
+  let installation_dir = if let Some(root) = uninstall_flags.root {
     canonicalize_path_maybe_not_exists(&cwd.join(root))?
   } else {
     get_installer_root()?
   };
-  let installation_dir = root.join("bin");
 
   // ensure directory exists
   if let Ok(metadata) = fs::metadata(&installation_dir) {
@@ -473,12 +473,11 @@ async fn resolve_shim_data(
   install_flags_global: &InstallFlagsGlobal,
 ) -> Result<ShimData, AnyError> {
   let cwd = std::env::current_dir().context("Unable to get CWD")?;
-  let root = if let Some(root) = &install_flags_global.root {
+  let installation_dir = if let Some(root) = &install_flags_global.root {
     canonicalize_path_maybe_not_exists(&cwd.join(root))?
   } else {
     get_installer_root()?
   };
-  let installation_dir = root.join("bin");
 
   // Check if module_url is remote
   let module_url = resolve_url_or_path(&install_flags_global.module_url, &cwd)?;
@@ -872,7 +871,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
@@ -1161,7 +1160,8 @@ mod tests {
 
   #[tokio::test]
   async fn install_npm_lockfile_default() {
-    let temp_dir = canonicalize_path(&env::temp_dir()).unwrap();
+    let temp_dir = TempDir::new();
+    let bin_dir = temp_dir.path().join("bin");
     let shim_data = resolve_shim_data(
       &HttpClientProvider::new(None, None),
       &Flags {
@@ -1175,14 +1175,14 @@ mod tests {
         module_url: "npm:cowsay".to_string(),
         args: vec![],
         name: None,
-        root: Some(temp_dir.to_string_lossy().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
     .await
     .unwrap();
 
-    let lock_path = temp_dir.join("bin").join(".cowsay.lock.json");
+    let lock_path = bin_dir.join(".cowsay.lock.json");
     assert_eq!(
       shim_data.args,
       vec![
@@ -1249,7 +1249,7 @@ mod tests {
         module_url: local_module_str.to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
@@ -1279,7 +1279,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
@@ -1300,7 +1300,7 @@ mod tests {
         module_url: "http://localhost:4545/cat.ts".to_string(), // using a different URL
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
@@ -1322,7 +1322,7 @@ mod tests {
         module_url: "http://localhost:4545/cat.ts".to_string(), // using a different URL
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: true,
       },
     )
@@ -1353,7 +1353,7 @@ mod tests {
         module_url: "http://localhost:4545/cat.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: true,
       },
     )
@@ -1383,7 +1383,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec!["\"".to_string()],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
@@ -1424,7 +1424,7 @@ mod tests {
         module_url: local_module_str.to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: false,
       },
     )
@@ -1470,7 +1470,7 @@ mod tests {
         module_url: "http://localhost:4545/cat.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: true,
       },
     )
@@ -1513,7 +1513,7 @@ mod tests {
         module_url: file_module_string.to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_string()),
+        root: Some(bin_dir.to_string()),
         force: true,
       },
     )
@@ -1570,7 +1570,7 @@ mod tests {
       UninstallFlags {
         kind: UninstallKind::Global(UninstallFlagsGlobal {
           name: "echo_test".to_string(),
-          root: Some(temp_dir.path().to_string()),
+          root: Some(bin_dir.to_string()),
         }),
       },
     )

tests/integration/install_tests.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
+use std::fs;
 use test_util as util;
 use test_util::assert_contains;
 use test_util::assert_not_contains;
@@ -200,6 +201,9 @@ fn installer_test_local_module_run() {
   let context = TestContext::with_http_server();
   let temp_dir = context.temp_dir();
   let temp_dir_str = temp_dir.path().to_string();
+  let bin_dir = temp_dir.path().join("bin");
+  std::fs::create_dir(&bin_dir).unwrap();
+  let bin_dir_str = bin_dir.path().to_string();
   let echo_ts_str = util::testdata_path().join("echo.ts").to_string();
 
   context
@@ -211,7 +215,7 @@ fn installer_test_local_module_run() {
       "--name",
       "echo_test",
       "--root",
-      temp_dir_str.as_str(),
+      bin_dir_str.as_str(),
       echo_ts_str.as_str(),
       "hello",
     ])
@@ -224,7 +228,6 @@ fn installer_test_local_module_run() {
     .skip_output_check()
     .assert_exit_code(0);
 
-  let bin_dir = temp_dir.path().join("bin");
   let mut file_path = bin_dir.join("echo_test");
   if cfg!(windows) {
     file_path = file_path.with_extension("cmd");
@@ -252,7 +255,7 @@ fn installer_test_remote_module_run() {
   let bin_dir = root_dir.join("bin");
   context
     .new_command()
-    .args("install --name echo_test --root ./root -g http://localhost:4545/echo.ts hello")
+    .args("install --name echo_test --root ./root/bin -g http://localhost:4545/echo.ts hello")
     .run()
     .skip_output_check()
     .assert_exit_code(0);
@@ -274,7 +277,7 @@ fn installer_test_remote_module_run() {
   // now uninstall with the relative path
   context
     .new_command()
-    .args("uninstall -g --root ./root echo_test")
+    .args("uninstall -g --root ./root/bin echo_test")
     .run()
     .skip_output_check()
     .assert_exit_code(0);

tests/specs/cert/cafile_install/__test__.jsonc

@@ -8,7 +8,7 @@
       "-n",
       "echo_test",
       "--root",
-      "$PWD",
+      "$PWD/bin",
       "-g",
       "https://localhost:5545/echo.ts"
     ],

tests/specs/install/future_install_global/assert.js

@@ -1,4 +1,4 @@
-const dirs = Deno.readDir("./bins/bin");
+const dirs = Deno.readDir("./bins");
 
 let found = false;
 for await (const entry of dirs) {