From 619a24390ff15d5ea5e577a4d0391823f94e8592 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Sat, 8 Feb 2020 00:49:55 -0800 Subject: [PATCH] install: add --force flag and remove yes/no prompt (#3917) --- cli/flags.rs | 15 ++++- cli/installer.rs | 105 +++++++++++++++++++++++++-------- cli/lib.rs | 6 +- cli/tests/integration_tests.rs | 2 + 4 files changed, 101 insertions(+), 27 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 897df611cf..748a7f95e2 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -59,6 +59,7 @@ pub enum DenoSubcommand { exe_name: String, module_url: String, args: Vec, + force: bool, }, Repl, Run { @@ -320,6 +321,7 @@ fn install_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) { None }; + let force = matches.is_present("force"); let exe_name = matches.value_of("exe_name").unwrap().to_string(); let cmd_values = matches.values_of("cmd").unwrap(); let mut cmd_args = vec![]; @@ -336,6 +338,7 @@ fn install_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) { exe_name, module_url, args, + force, }; } @@ -579,6 +582,12 @@ fn install_subcommand<'a, 'b>() -> App<'a, 'b> { .help("Installation directory (defaults to $HOME/.deno/bin)") .takes_value(true) .multiple(false)) + .arg( + Arg::with_name("force") + .long("force") + .short("f") + .help("Forcefully overwrite existing installation") + .takes_value(false)) .arg( Arg::with_name("exe_name") .required(true) @@ -1791,6 +1800,7 @@ mod tests { exe_name: "deno_colors".to_string(), module_url: "https://deno.land/std/examples/colors.ts".to_string(), args: vec![], + force: false, }, ..DenoFlags::default() } @@ -1815,6 +1825,7 @@ mod tests { exe_name: "file_server".to_string(), module_url: "https://deno.land/std/http/file_server.ts".to_string(), args: vec![], + force: false, }, allow_net: true, allow_read: true, @@ -1824,12 +1835,13 @@ mod tests { } #[test] - fn install_with_args_and_dir() { + fn install_with_args_and_dir_and_force() { let r = flags_from_vec_safe(svec![ "deno", "install", "-d", "/usr/local/bin", + "-f", "--allow-net", "--allow-read", "file_server", @@ -1845,6 +1857,7 @@ mod tests { exe_name: "file_server".to_string(), module_url: "https://deno.land/std/http/file_server.ts".to_string(), args: svec!["arg1", "arg2"], + force: true, }, allow_net: true, allow_read: true, diff --git a/cli/installer.rs b/cli/installer.rs index 0cc25ff333..64fe2b4161 100644 --- a/cli/installer.rs +++ b/cli/installer.rs @@ -4,7 +4,6 @@ use regex::{Regex, RegexBuilder}; use std::env; use std::fs; use std::fs::File; -use std::io; use std::io::Error; use std::io::ErrorKind; use std::io::Write; @@ -23,15 +22,6 @@ lazy_static! { ).case_insensitive(true).build().unwrap(); } -fn yes_no_prompt(msg: &str) -> bool { - println!("{} [yN]", msg); - let mut buffer = String::new(); - io::stdin() - .read_line(&mut buffer) - .expect("Couldn't read from stdin"); - buffer.starts_with('y') | buffer.starts_with('Y') -} - fn is_remote_url(module_url: &str) -> bool { module_url.starts_with("http://") || module_url.starts_with("https://") } @@ -60,7 +50,6 @@ fn generate_executable_file( "% generated by deno install %\ndeno.exe {} %*\n", args.join(" ") ); - let file_path = file_path.with_extension(".cmd"); let mut file = File::create(&file_path)?; file.write_all(template.as_bytes())?; Ok(()) @@ -71,9 +60,6 @@ fn generate_executable_file( file_path: PathBuf, args: Vec, ) -> Result<(), Error> { - // On Windows if user is using Powershell .cmd extension is need to run the - // installed module. - // Generate batch script to satisfy that. let args: Vec = args.iter().map(|c| format!("\"{}\"", c)).collect(); let template = format!( r#"#!/bin/sh @@ -119,6 +105,7 @@ pub fn install( exec_name: &str, module_url: &str, args: Vec, + force: bool, ) -> Result<(), Error> { let installation_dir = if let Some(dir) = installation_dir { PathBuf::from(dir).canonicalize()? @@ -131,7 +118,7 @@ pub fn install( if !metadata.is_dir() { return Err(Error::new( ErrorKind::Other, - "Insallation path is not a directory", + "Installation path is not a directory", )); } } else { @@ -153,16 +140,17 @@ pub fn install( }; validate_exec_name(exec_name)?; - let file_path = installation_dir.join(exec_name); + let mut file_path = installation_dir.join(exec_name); - if file_path.exists() { - let msg = format!( - "⚠️ {} is already installed, do you want to overwrite it?", - exec_name - ); - if !yes_no_prompt(&msg) { - return Ok(()); - }; + if cfg!(windows) { + file_path = file_path.with_extension(".cmd"); + } + + if file_path.exists() && !force { + return Err(Error::new( + ErrorKind::Other, + "Existing installation found. Aborting (Use -f to overwrite)", + )); }; let mut executable_args = vec!["run".to_string()]; @@ -227,6 +215,7 @@ mod tests { "echo_test", "http://localhost:4545/cli/tests/echo_server.ts", vec![], + false, ) .expect("Install failed"); @@ -260,6 +249,7 @@ mod tests { "echo_test", "http://localhost:4545/cli/tests/echo_server.ts", vec![], + false, ) .expect("Install failed"); @@ -288,6 +278,7 @@ mod tests { "echo_test", "http://localhost:4545/cli/tests/echo_server.ts", vec!["--foobar".to_string()], + false, ) .expect("Install failed"); @@ -314,6 +305,7 @@ mod tests { "echo_test", &local_module_str, vec![], + false, ) .expect("Install failed"); @@ -326,4 +318,69 @@ mod tests { let content = fs::read_to_string(file_path).unwrap(); assert!(content.contains(&local_module_url.to_string())); } + + #[test] + fn install_force() { + let temp_dir = TempDir::new().expect("tempdir fail"); + let temp_dir_str = temp_dir.path().to_string_lossy().to_string(); + let original_home = env::var_os("HOME"); + let original_user_profile = env::var_os("HOME"); + env::set_var("HOME", &temp_dir_str); + env::set_var("USERPROFILE", &temp_dir_str); + + install( + DenoFlags::default(), + None, + "echo_test", + "http://localhost:4545/cli/tests/echo_server.ts", + vec![], + false, + ) + .expect("Install failed"); + + let mut file_path = temp_dir.path().join(".deno/bin/echo_test"); + if cfg!(windows) { + file_path = file_path.with_extension(".cmd"); + } + assert!(file_path.exists()); + + // No force. Install failed. + let no_force_result = install( + DenoFlags::default(), + None, + "echo_test", + "http://localhost:4545/cli/tests/cat.ts", // using a different URL + vec![], + false, + ); + assert!(no_force_result.is_err()); + assert!(no_force_result + .unwrap_err() + .to_string() + .contains("Existing installation found")); + // Assert not modified + let file_content = fs::read_to_string(&file_path).unwrap(); + assert!(file_content.contains("echo_server.ts")); + + // Force. Install success. + let force_result = install( + DenoFlags::default(), + None, + "echo_test", + "http://localhost:4545/cli/tests/cat.ts", // using a different URL + vec![], + true, + ); + assert!(force_result.is_ok()); + // Assert modified + let file_content_2 = fs::read_to_string(&file_path).unwrap(); + assert!(file_content_2.contains("cat.ts")); + + if let Some(home) = original_home { + env::set_var("HOME", home); + } + if let Some(user_profile) = original_user_profile { + env::set_var("USERPROFILE", user_profile); + } + } } diff --git a/cli/lib.rs b/cli/lib.rs index a8a314f32a..e752d6f2af 100644 --- a/cli/lib.rs +++ b/cli/lib.rs @@ -262,6 +262,7 @@ async fn install_command( exe_name: String, module_url: String, args: Vec, + force: bool, ) { // Firstly fetch and compile module, this // ensures the module exists. @@ -270,7 +271,7 @@ async fn install_command( fetch_command(fetch_flags, vec![module_url.to_string()]).await; let install_result = - installer::install(flags, dir, &exe_name, &module_url, args); + installer::install(flags, dir, &exe_name, &module_url, args, force); if let Err(e) = install_result { print_msg_and_exit(&e.to_string()); } @@ -447,7 +448,8 @@ pub fn main() { exe_name, module_url, args, - } => install_command(flags, dir, exe_name, module_url, args).await, + force, + } => install_command(flags, dir, exe_name, module_url, args, force).await, DenoSubcommand::Repl => run_repl(flags).await, DenoSubcommand::Run { script } => run_script(flags, script).await, DenoSubcommand::Types => types_command(), diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index c7b2a4b4ea..ac00a04c4d 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -112,6 +112,7 @@ fn installer_test_local_module_run() { "echo_test", &local_module_str, vec!["hello".to_string()], + false, ) .expect("Failed to install"); let mut file_path = temp_dir.path().join("echo_test"); @@ -159,6 +160,7 @@ fn installer_test_remote_module_run() { "echo_test", "http://localhost:4545/cli/tests/echo.ts", vec!["hello".to_string()], + false, ) .expect("Failed to install"); let mut file_path = temp_dir.path().join("echo_test");