fix(install): should work with non-existent relative root (#21161)

Closes #21160

cli/args/flags.rs

@@ -163,7 +163,7 @@ pub struct InstallFlags {
   pub module_url: String,
   pub args: Vec<String>,
   pub name: Option<String>,
-  pub root: Option<PathBuf>,
+  pub root: Option<String>,
   pub force: bool,
 }
 
@@ -177,7 +177,7 @@ pub struct JupyterFlags {
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct UninstallFlags {
   pub name: String,
-  pub root: Option<PathBuf>,
+  pub root: Option<String>,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -1762,7 +1762,6 @@ These must be added to the path manually if required.")
         Arg::new("root")
           .long("root")
           .help("Installation root")
-          .value_parser(value_parser!(PathBuf))
           .value_hint(ValueHint::DirPath))
       .arg(
         Arg::new("force")
@@ -1822,7 +1821,6 @@ The installation root is determined, in order of precedence:
         Arg::new("root")
           .long("root")
           .help("Installation root")
-          .value_parser(value_parser!(PathBuf))
           .value_hint(ValueHint::DirPath))
 )
 }
@@ -3407,7 +3405,7 @@ fn info_parse(flags: &mut Flags, matches: &mut ArgMatches) {
 fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) {
   runtime_args_parse(flags, matches, true, true);
 
-  let root = matches.remove_one::<PathBuf>("root");
+  let root = matches.remove_one::<String>("root");
 
   let force = matches.get_flag("force");
   let name = matches.remove_one::<String>("name");
@@ -3438,7 +3436,7 @@ fn jupyter_parse(flags: &mut Flags, matches: &mut ArgMatches) {
 }
 
 fn uninstall_parse(flags: &mut Flags, matches: &mut ArgMatches) {
-  let root = matches.remove_one::<PathBuf>("root");
+  let root = matches.remove_one::<String>("root");
 
   let name = matches.remove_one::<String>("name").unwrap();
   flags.subcommand = DenoSubcommand::Uninstall(UninstallFlags { name, root });
@@ -6361,7 +6359,7 @@ mod tests {
           name: Some("file_server".to_string()),
           module_url: "https://deno.land/std/http/file_server.ts".to_string(),
           args: svec!["foo", "bar"],
-          root: Some(PathBuf::from("/foo")),
+          root: Some("/foo".to_string()),
           force: true,
         }),
         import_map_path: Some("import_map.json".to_string()),

cli/tests/integration/install_tests.rs

@@ -1,38 +1,33 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-use std::fs;
-use std::process::Command;
 use test_util as util;
 use test_util::assert_contains;
-use test_util::assert_ends_with;
+use util::TestContext;
-use test_util::TempDir;
+use util::TestContextBuilder;
 
 #[test]
 fn install_basic() {
-  let _guard = util::http_server();
+  let context = TestContextBuilder::new()
-  let temp_dir = TempDir::new();
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
   let temp_dir_str = temp_dir.path().to_string();
 
   // ensure a lockfile doesn't get created or updated locally
   temp_dir.write("deno.json", "{}");
 
-  let status = util::deno_cmd()
+  context
-    .current_dir(temp_dir.path())
+    .new_command()
-    .arg("install")
+    .args("install --check --name echo_test http://localhost:4545/echo.ts")
-    .arg("--check")
-    .arg("--name")
-    .arg("echo_test")
-    .arg("http://localhost:4545/echo.ts")
     .envs([
       ("HOME", temp_dir_str.as_str()),
       ("USERPROFILE", temp_dir_str.as_str()),
       ("DENO_INSTALL_ROOT", ""),
     ])
-    .spawn()
+    .run()
-    .unwrap()
+    .skip_output_check()
-    .wait()
+    .assert_exit_code(0);
-    .unwrap();
-  assert!(status.success());
 
   // no lockfile should be created locally
   assert!(!temp_dir.path().join("deno.lock").exists());
@@ -62,20 +57,17 @@ fn install_basic() {
   }
 
   // now uninstall
-  let status = util::deno_cmd()
+  context
-    .current_dir(temp_dir.path())
+    .new_command()
-    .arg("uninstall")
+    .args("uninstall echo_test")
-    .arg("echo_test")
     .envs([
       ("HOME", temp_dir_str.as_str()),
       ("USERPROFILE", temp_dir_str.as_str()),
       ("DENO_INSTALL_ROOT", ""),
     ])
-    .spawn()
+    .run()
-    .unwrap()
+    .skip_output_check()
-    .wait()
+    .assert_exit_code(0);
-    .unwrap();
-  assert!(status.success());
 
   // ensure local lockfile still doesn't exist
   assert!(!temp_dir.path().join("deno.lock").exists());
@@ -85,27 +77,22 @@ fn install_basic() {
 
 #[test]
 fn install_custom_dir_env_var() {
-  let _guard = util::http_server();
+  let context = TestContext::with_http_server();
-  let temp_dir = TempDir::new();
+  let temp_dir = context.temp_dir();
   let temp_dir_str = temp_dir.path().to_string();
 
-  let status = util::deno_cmd()
+  context
-    .current_dir(util::root_path()) // different cwd
+    .new_command()
-    .arg("install")
+    .cwd(util::root_path()) // different cwd
-    .arg("--check")
+    .args("install --check --name echo_test http://localhost:4545/echo.ts")
-    .arg("--name")
-    .arg("echo_test")
-    .arg("http://localhost:4545/echo.ts")
     .envs([
       ("HOME", temp_dir_str.as_str()),
       ("USERPROFILE", temp_dir_str.as_str()),
       ("DENO_INSTALL_ROOT", temp_dir_str.as_str()),
     ])
-    .spawn()
+    .run()
-    .unwrap()
+    .skip_output_check()
-    .wait()
+    .assert_exit_code(0);
-    .unwrap();
-  assert!(status.success());
 
   let mut file_path = temp_dir.path().join("bin/echo_test");
   assert!(file_path.exists());
@@ -114,7 +101,7 @@ fn install_custom_dir_env_var() {
     file_path = file_path.with_extension("cmd");
   }
 
-  let content = fs::read_to_string(file_path).unwrap();
+  let content = file_path.read_to_string();
   if cfg!(windows) {
     assert_contains!(
       content,
@@ -130,114 +117,134 @@ fn install_custom_dir_env_var() {
 
 #[test]
 fn installer_test_local_module_run() {
-  let temp_dir = TempDir::new();
+  let context = TestContext::with_http_server();
-  let bin_dir = temp_dir.path().join("bin");
+  let temp_dir = context.temp_dir();
-  std::fs::create_dir(&bin_dir).unwrap();
-  let status = util::deno_cmd()
-    .current_dir(util::root_path())
-    .arg("install")
-    .arg("--name")
-    .arg("echo_test")
-    .arg("--root")
-    .arg(temp_dir.path())
-    .arg(util::testdata_path().join("echo.ts"))
-    .arg("hello")
-    .spawn()
-    .unwrap()
-    .wait()
-    .unwrap();
-  assert!(status.success());
-  let mut file_path = bin_dir.join("echo_test");
-  if cfg!(windows) {
-    file_path = file_path.with_extension("cmd");
-  }
-  assert!(file_path.exists());
-  // NOTE: using file_path here instead of exec_name, because tests
-  // shouldn't mess with user's PATH env variable
-  let output = Command::new(file_path)
-    .current_dir(temp_dir.path())
-    .arg("foo")
-    .env("PATH", util::target_dir())
-    .output()
-    .unwrap();
-  let stdout_str = std::str::from_utf8(&output.stdout).unwrap().trim();
-  assert_ends_with!(stdout_str, "hello, foo");
-}
-
-#[test]
-fn installer_test_remote_module_run() {
-  let _g = util::http_server();
-  let temp_dir = TempDir::new();
-  let bin_dir = temp_dir.path().join("bin");
-  std::fs::create_dir(&bin_dir).unwrap();
-  let status = util::deno_cmd()
-    .current_dir(util::testdata_path())
-    .arg("install")
-    .arg("--name")
-    .arg("echo_test")
-    .arg("--root")
-    .arg(temp_dir.path())
-    .arg("http://localhost:4545/echo.ts")
-    .arg("hello")
-    .spawn()
-    .unwrap()
-    .wait()
-    .unwrap();
-  assert!(status.success());
-  let mut file_path = bin_dir.join("echo_test");
-  if cfg!(windows) {
-    file_path = file_path.with_extension("cmd");
-  }
-  assert!(file_path.exists());
-  let output = Command::new(file_path)
-    .current_dir(temp_dir.path())
-    .arg("foo")
-    .env("PATH", util::target_dir())
-    .output()
-    .unwrap();
-  assert_ends_with!(
-    std::str::from_utf8(&output.stdout).unwrap().trim(),
-    "hello, foo",
-  );
-}
-
-#[test]
-fn check_local_by_default() {
-  let _guard = util::http_server();
-  let temp_dir = TempDir::new();
   let temp_dir_str = temp_dir.path().to_string();
+  let echo_ts_str = util::testdata_path().join("echo.ts").to_string();
 
-  let status = util::deno_cmd()
+  context
-    .current_dir(temp_dir.path())
+    .new_command()
-    .arg("install")
+    .cwd(util::root_path())
-    .arg(util::testdata_path().join("./install/check_local_by_default.ts"))
+    .args_vec([
+      "install",
+      "--name",
+      "echo_test",
+      "--root",
+      temp_dir_str.as_str(),
+      echo_ts_str.as_str(),
+      "hello",
+    ])
     .envs([
       ("HOME", temp_dir_str.as_str()),
       ("USERPROFILE", temp_dir_str.as_str()),
       ("DENO_INSTALL_ROOT", ""),
     ])
-    .status()
+    .run()
-    .unwrap();
+    .skip_output_check()
-  assert!(status.success());
+    .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");
+  }
+  assert!(file_path.exists());
+  let output = context
+    .new_command()
+    .name(&file_path)
+    .cwd(temp_dir.path())
+    .args("foo")
+    .env("PATH", util::target_dir())
+    .run();
+  output.assert_matches_text("hello, foo");
+  output.assert_exit_code(0);
+}
+
+#[test]
+fn installer_test_remote_module_run() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  let root_dir = temp_dir.path().join("root");
+  let bin_dir = root_dir.join("bin");
+  context
+    .new_command()
+    .args("install --name echo_test --root ./root http://localhost:4545/echo.ts hello")
+    .run()
+    .skip_output_check()
+    .assert_exit_code(0);
+  let mut bin_file_path = bin_dir.join("echo_test");
+  if cfg!(windows) {
+    bin_file_path = bin_file_path.with_extension("cmd");
+  }
+  assert!(bin_file_path.exists());
+  let output = context
+    .new_command()
+    .name(&bin_file_path)
+    .cwd(root_dir)
+    .args("foo")
+    .env("PATH", util::target_dir())
+    .run();
+  output.assert_matches_text("hello, foo");
+  output.assert_exit_code(0);
+
+  // now uninstall with the relative path
+  context
+    .new_command()
+    .args("uninstall --root ./root echo_test")
+    .run()
+    .skip_output_check()
+    .assert_exit_code(0);
+  assert!(!bin_file_path.exists());
+}
+
+#[test]
+fn check_local_by_default() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  let temp_dir_str = temp_dir.path().to_string();
+  let script_path =
+    util::testdata_path().join("./install/check_local_by_default.ts");
+  let script_path_str = script_path.to_string_lossy().to_string();
+  context
+    .new_command()
+    .args_vec(["install", script_path_str.as_str()])
+    .envs([
+      ("HOME", temp_dir_str.as_str()),
+      ("USERPROFILE", temp_dir_str.as_str()),
+      ("DENO_INSTALL_ROOT", ""),
+    ])
+    .run()
+    .skip_output_check()
+    .assert_exit_code(0);
 }
 
 #[test]
 fn check_local_by_default2() {
-  let _guard = util::http_server();
+  let context = TestContextBuilder::new()
-  let temp_dir = TempDir::new();
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
   let temp_dir_str = temp_dir.path().to_string();
-
+  let script_path =
-  let status = util::deno_cmd()
+    util::testdata_path().join("./install/check_local_by_default2.ts");
-    .current_dir(temp_dir.path())
+  let script_path_str = script_path.to_string_lossy().to_string();
-    .arg("install")
+  context
-    .arg(util::testdata_path().join("./install/check_local_by_default2.ts"))
+    .new_command()
+    .args_vec(["install", script_path_str.as_str()])
     .envs([
       ("HOME", temp_dir_str.as_str()),
       ("NO_COLOR", "1"),
       ("USERPROFILE", temp_dir_str.as_str()),
       ("DENO_INSTALL_ROOT", ""),
     ])
-    .status()
+    .run()
-    .unwrap();
+    .skip_output_check()
-  assert!(status.success());
+    .assert_exit_code(0);
 }

cli/tools/installer.rs

@@ -176,9 +176,10 @@ pub async fn infer_name_from_url(url: &Url) -> Option<String> {
   Some(stem)
 }
 
-pub fn uninstall(name: String, root: Option<PathBuf>) -> Result<(), AnyError> {
+pub fn uninstall(name: String, root: Option<String>) -> Result<(), AnyError> {
+  let cwd = std::env::current_dir().context("Unable to get CWD")?;
   let root = if let Some(root) = root {
-    canonicalize_path_maybe_not_exists(&root)?
+    canonicalize_path_maybe_not_exists(&cwd.join(root))?
   } else {
     get_installer_root()?
   };
@@ -303,15 +304,15 @@ async fn resolve_shim_data(
   flags: &Flags,
   install_flags: &InstallFlags,
 ) -> Result<ShimData, AnyError> {
+  let cwd = std::env::current_dir().context("Unable to get CWD")?;
   let root = if let Some(root) = &install_flags.root {
-    canonicalize_path_maybe_not_exists(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 cwd = std::env::current_dir().context("Unable to get CWD")?;
   let module_url = resolve_url_or_path(&install_flags.module_url, &cwd)?;
 
   let name = if install_flags.name.is_some() {
@@ -635,7 +636,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: false,
       },
     )
@@ -669,7 +670,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec![],
         name: None,
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -691,7 +692,7 @@ mod tests {
         module_url: "http://localhost:4545/subdir/main.ts".to_string(),
         args: vec![],
         name: None,
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -715,7 +716,7 @@ mod tests {
           .to_string(),
         args: vec![],
         name: None,
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -741,7 +742,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -769,7 +770,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec!["--foobar".to_string()],
         name: Some("echo_test".to_string()),
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -802,7 +803,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -831,7 +832,7 @@ mod tests {
         module_url: "http://localhost:4545/echo_server.ts".to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -861,7 +862,7 @@ mod tests {
         module_url: "npm:cowsay".to_string(),
         args: vec![],
         name: None,
-        root: Some(temp_dir.clone()),
+        root: Some(temp_dir.to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -895,7 +896,7 @@ mod tests {
         module_url: "npm:cowsay".to_string(),
         args: vec![],
         name: None,
-        root: Some(env::temp_dir()),
+        root: Some(env::temp_dir().to_string_lossy().to_string()),
         force: false,
       },
     )
@@ -930,7 +931,7 @@ mod tests {
         module_url: local_module_str.to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: false,
       },
     )
@@ -959,7 +960,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: false,
       },
     )
@@ -979,7 +980,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: false,
       },
     )
@@ -1000,7 +1001,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: true,
       },
     )
@@ -1030,7 +1031,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: true,
       },
     )
@@ -1059,7 +1060,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: false,
       },
     )
@@ -1099,7 +1100,7 @@ mod tests {
         module_url: local_module_str.to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: false,
       },
     )
@@ -1143,7 +1144,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_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: true,
       },
     )
@@ -1185,7 +1186,7 @@ mod tests {
         module_url: file_module_string.to_string(),
         args: vec![],
         name: Some("echo_test".to_string()),
-        root: Some(temp_dir.path().to_path_buf()),
+        root: Some(temp_dir.path().to_string()),
         force: true,
       },
     )
@@ -1237,7 +1238,7 @@ mod tests {
       File::create(file_path).unwrap();
     }
 
-    uninstall("echo_test".to_string(), Some(temp_dir.path().to_path_buf()))
+    uninstall("echo_test".to_string(), Some(temp_dir.path().to_string()))
       .unwrap();
 
     assert!(!file_path.exists());

cli/util/fs.rs

@@ -206,8 +206,14 @@ pub fn canonicalize_path_maybe_not_exists_with_fs(
         return Ok(canonicalized_path);
       }
       Err(err) if err.kind() == ErrorKind::NotFound => {
-        names_stack.push(path.file_name().unwrap());
+        names_stack.push(match path.file_name() {
-        path = path.parent().unwrap();
+          Some(name) => name.to_owned(),
+          None => return Err(err),
+        });
+        path = match path.parent() {
+          Some(parent) => parent,
+          None => return Err(err),
+        };
       }
       Err(err) => return Err(err),
     }