From d31f2307eee6e2a0f96342a58159d265ea03c58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 27 Mar 2024 22:45:57 +0000 Subject: [PATCH] feat(install): require -g / --global flag (#23060) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation for upcoming changes to `deno install` in Deno 2. If `-g` or `--global` flag is not provided a warning will be emitted: ``` ⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use `-g` or `--global` flag. ``` The same will happen for `deno uninstall` - unless `-g`/`--global` flag is provided a warning will be emitted. Towards https://github.com/denoland/deno/issues/23062 --------- Signed-off-by: Bartek Iwańczuk Co-authored-by: David Sherret --- cli/args/flags.rs | 71 ++++++++++++++++++++++--- cli/main.rs | 2 +- cli/tools/installer.rs | 41 ++++++++++++-- tests/integration/install_tests.rs | 85 +++++++++++++++++++++++++++++- 4 files changed, 187 insertions(+), 12 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 283ebc9a31..1bd30bd683 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -181,6 +181,7 @@ pub struct InstallFlags { pub name: Option, pub root: Option, pub force: bool, + pub global: bool, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -194,6 +195,7 @@ pub struct JupyterFlags { pub struct UninstallFlags { pub name: String, pub root: Option, + pub global: bool, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -1852,12 +1854,12 @@ fn install_subcommand() -> Command { .long_about( "Installs a script as an executable in the installation root's bin directory. - deno install --allow-net --allow-read https://deno.land/std/http/file_server.ts - deno install https://examples.deno.land/color-logging.ts + deno install --global --allow-net --allow-read https://deno.land/std/http/file_server.ts + deno install -g https://examples.deno.land/color-logging.ts To change the executable name, use -n/--name: - deno install --allow-net --allow-read -n serve https://deno.land/std/http/file_server.ts + deno install -g --allow-net --allow-read -n serve https://deno.land/std/http/file_server.ts The executable name is inferred by default: - Attempt to take the file stem of the URL path. The above example would @@ -1869,7 +1871,7 @@ The executable name is inferred by default: To change the installation root, use --root: - deno install --allow-net --allow-read --root /usr/local https://deno.land/std/http/file_server.ts + deno install -g --allow-net --allow-read --root /usr/local https://deno.land/std/http/file_server.ts The installation root is determined, in order of precedence: - --root option @@ -1897,6 +1899,13 @@ These must be added to the path manually if required.") .help("Forcefully overwrite existing installation") .action(ArgAction::SetTrue)) ) + .arg( + Arg::new("global") + .long("global") + .short('g') + .help("Install a package or script as a globally available executable") + .action(ArgAction::SetTrue) + ) .arg(env_file_arg()) } @@ -1948,7 +1957,15 @@ The installation root is determined, in order of precedence: Arg::new("root") .long("root") .help("Installation root") - .value_hint(ValueHint::DirPath)) + .value_hint(ValueHint::DirPath) + ) + .arg( + Arg::new("global") + .long("global") + .short('g') + .help("Remove globally installed package or module") + .action(ArgAction::SetTrue) + ) ) } @@ -3582,6 +3599,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { let root = matches.remove_one::("root"); let force = matches.get_flag("force"); + let global = matches.get_flag("global"); let name = matches.remove_one::("name"); let mut cmd_values = matches.remove_many::("cmd").unwrap(); @@ -3594,6 +3612,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { args, root, force, + global, }); } @@ -3611,9 +3630,10 @@ fn jupyter_parse(flags: &mut Flags, matches: &mut ArgMatches) { fn uninstall_parse(flags: &mut Flags, matches: &mut ArgMatches) { let root = matches.remove_one::("root"); - + let global = matches.get_flag("global"); let name = matches.remove_one::("name").unwrap(); - flags.subcommand = DenoSubcommand::Uninstall(UninstallFlags { name, root }); + flags.subcommand = + DenoSubcommand::Uninstall(UninstallFlags { name, root, global }); } fn lsp_parse(flags: &mut Flags, _matches: &mut ArgMatches) { @@ -6525,6 +6545,28 @@ mod tests { args: vec![], root: None, force: false, + global: false, + }), + ..Flags::default() + } + ); + + let r = flags_from_vec(svec![ + "deno", + "install", + "-g", + "https://deno.land/std/http/file_server.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Install(InstallFlags { + name: None, + module_url: "https://deno.land/std/http/file_server.ts".to_string(), + args: vec![], + root: None, + force: false, + global: true, }), ..Flags::default() } @@ -6544,6 +6586,7 @@ mod tests { args: svec!["foo", "bar"], root: Some("/foo".to_string()), force: true, + global: false, }), import_map_path: Some("import_map.json".to_string()), no_remote: true, @@ -6575,6 +6618,20 @@ mod tests { subcommand: DenoSubcommand::Uninstall(UninstallFlags { name: "file_server".to_string(), root: None, + global: false, + }), + ..Flags::default() + } + ); + + let r = flags_from_vec(svec!["deno", "uninstall", "-g", "file_server"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Uninstall(UninstallFlags { + name: "file_server".to_string(), + root: None, + global: true, }), ..Flags::default() } diff --git a/cli/main.rs b/cli/main.rs index ecf171e726..0cbfeadc79 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -152,7 +152,7 @@ async fn run_subcommand(flags: Flags) -> Result { tools::jupyter::kernel(flags, jupyter_flags).await }), DenoSubcommand::Uninstall(uninstall_flags) => spawn_subcommand(async { - tools::installer::uninstall(uninstall_flags.name, uninstall_flags.root) + tools::installer::uninstall(uninstall_flags) }), DenoSubcommand::Lsp => spawn_subcommand(async { lsp::start().await }), DenoSubcommand::Lint(lint_flags) => spawn_subcommand(async { diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index ea4b5ff489..1252c2faf0 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -5,6 +5,7 @@ use crate::args::CaData; use crate::args::Flags; use crate::args::InstallFlags; use crate::args::TypeCheckMode; +use crate::args::UninstallFlags; use crate::factory::CliFactory; use crate::http_util::HttpClient; use crate::util::fs::canonicalize_path_maybe_not_exists; @@ -183,7 +184,13 @@ pub async fn infer_name_from_url(url: &Url) -> Option { Some(stem.to_string()) } -pub fn uninstall(name: String, root: Option) -> Result<(), AnyError> { +pub fn uninstall(uninstall_flags: UninstallFlags) -> Result<(), AnyError> { + let name = uninstall_flags.name; + let root = uninstall_flags.root; + + if !uninstall_flags.global { + log::warn!("⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag."); + } let cwd = std::env::current_dir().context("Unable to get CWD")?; let root = if let Some(root) = root { canonicalize_path_maybe_not_exists(&cwd.join(root))? @@ -241,6 +248,9 @@ pub async fn install_command( flags: Flags, install_flags: InstallFlags, ) -> Result<(), AnyError> { + if !install_flags.global { + log::warn!("⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag."); + } // ensure the module is cached CliFactory::from_flags(flags.clone())? .module_load_preparer() @@ -663,6 +673,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: false, + global: false, }, ) .await @@ -697,6 +708,7 @@ mod tests { name: None, root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -725,6 +737,7 @@ mod tests { name: None, root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -758,6 +771,7 @@ mod tests { name: None, root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -786,6 +800,7 @@ mod tests { name: None, root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -810,6 +825,7 @@ mod tests { name: None, root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -836,6 +852,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -864,6 +881,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -897,6 +915,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -926,6 +945,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -956,6 +976,7 @@ mod tests { name: None, root: Some(temp_dir.to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -990,6 +1011,7 @@ mod tests { name: None, root: Some(env::temp_dir().to_string_lossy().to_string()), force: false, + global: false, }, ) .await @@ -1025,6 +1047,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: false, + global: false, }, ) .await @@ -1054,6 +1077,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: false, + global: false, }, ) .await @@ -1074,6 +1098,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: false, + global: false, }, ) .await; @@ -1095,6 +1120,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: true, + global: false, }, ) .await; @@ -1125,6 +1151,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: true, + global: false, }, ) .await; @@ -1154,6 +1181,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: false, + global: false, }, ) .await @@ -1194,6 +1222,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: false, + global: false, }, ) .await @@ -1238,6 +1267,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: true, + global: false, }, ) .await; @@ -1280,6 +1310,7 @@ mod tests { name: Some("echo_test".to_string()), root: Some(temp_dir.path().to_string()), force: true, + global: false, }, ) .await; @@ -1330,8 +1361,12 @@ mod tests { File::create(file_path).unwrap(); } - uninstall("echo_test".to_string(), Some(temp_dir.path().to_string())) - .unwrap(); + uninstall(UninstallFlags { + name: "echo_test".to_string(), + root: Some(temp_dir.path().to_string()), + global: false, + }) + .unwrap(); assert!(!file_path.exists()); assert!(!file_path.with_extension("tsconfig.json").exists()); diff --git a/tests/integration/install_tests.rs b/tests/integration/install_tests.rs index 54df82549b..32c0e9080c 100644 --- a/tests/integration/install_tests.rs +++ b/tests/integration/install_tests.rs @@ -2,6 +2,7 @@ use test_util as util; use test_util::assert_contains; +use test_util::assert_not_contains; use util::TestContext; use util::TestContextBuilder; @@ -17,7 +18,7 @@ fn install_basic() { // ensure a lockfile doesn't get created or updated locally temp_dir.write("deno.json", "{}"); - context + let output = context .new_command() .args("install --check --name echo_test http://localhost:4545/echo.ts") .envs([ @@ -25,10 +26,92 @@ fn install_basic() { ("USERPROFILE", temp_dir_str.as_str()), ("DENO_INSTALL_ROOT", ""), ]) + .run(); + + output.assert_exit_code(0); + let output_text = output.combined_output(); + assert_contains!( + output_text, + "`deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag." + ); + + // no lockfile should be created locally + assert!(!temp_dir.path().join("deno.lock").exists()); + + let mut file_path = temp_dir.path().join(".deno/bin/echo_test"); + assert!(file_path.exists()); + + if cfg!(windows) { + file_path = file_path.with_extension("cmd"); + } + + let content = file_path.read_to_string(); + // ensure there's a trailing newline so the shell script can be + // more versatile. + assert_eq!(content.chars().last().unwrap(), '\n'); + + if cfg!(windows) { + assert_contains!( + content, + r#""run" "--check" "--no-config" "http://localhost:4545/echo.ts""# + ); + } else { + assert_contains!( + content, + r#"run --check --no-config 'http://localhost:4545/echo.ts'"# + ); + } + + // now uninstall + context + .new_command() + .args("uninstall echo_test") + .envs([ + ("HOME", temp_dir_str.as_str()), + ("USERPROFILE", temp_dir_str.as_str()), + ("DENO_INSTALL_ROOT", ""), + ]) .run() .skip_output_check() .assert_exit_code(0); + // ensure local lockfile still doesn't exist + assert!(!temp_dir.path().join("deno.lock").exists()); + // ensure uninstall occurred + assert!(!file_path.exists()); +} + +#[test] +fn install_basic_global() { + 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(); + + // ensure a lockfile doesn't get created or updated locally + temp_dir.write("deno.json", "{}"); + + let output = context + .new_command() + .args( + "install --global --check --name echo_test http://localhost:4545/echo.ts", + ) + .envs([ + ("HOME", temp_dir_str.as_str()), + ("USERPROFILE", temp_dir_str.as_str()), + ("DENO_INSTALL_ROOT", ""), + ]) + .run(); + + output.assert_exit_code(0); + let output_text = output.combined_output(); + assert_not_contains!( + output_text, + "`deno install` behavior will change in Deno 2. To preserve the current behavior use `-g` or `--global` flag." + ); + // no lockfile should be created locally assert!(!temp_dir.path().join("deno.lock").exists());