Skip to content

Commit

Permalink
feat(install): require -g / --global flag (#23060)
Browse files Browse the repository at this point in the history 
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 #23062

---------

Signed-off-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Co-authored-by: David Sherret <dsherret@users.noreply.github.com>
  • Loading branch information
@bartlomieju @dsherret
bartlomieju and dsherret committed Mar 27, 2024
1 parent 2dc37f4 commit d31f230
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 12 deletions.
71 changes: 64 additions & 7 deletions cli/args/flags.rs
View file
Expand Up @@ -181,6 +181,7 @@ pub struct InstallFlags {
pub name: Option<String>,
pub root: Option<String>,
pub force: bool,
pub global: bool,
}

#[derive(Clone, Debug, Eq, PartialEq)]
Expand All @@ -194,6 +195,7 @@ pub struct JupyterFlags {
pub struct UninstallFlags {
pub name: String,
pub root: Option<String>,
pub global: bool,
}

#[derive(Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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)
)
)
}

Expand Down Expand Up @@ -3582,6 +3599,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) {
let root = matches.remove_one::<String>("root");

let force = matches.get_flag("force");
let global = matches.get_flag("global");
let name = matches.remove_one::<String>("name");
let mut cmd_values = matches.remove_many::<String>("cmd").unwrap();

Expand All @@ -3594,6 +3612,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) {
args,
root,
force,
global,
});
}

Expand All @@ -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::<String>("root");

let global = matches.get_flag("global");
let name = matches.remove_one::<String>("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) {
Expand Down Expand Up @@ -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()
}
Expand All @@ -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,
Expand Down Expand Up @@ -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()
}
Expand Down
2 changes: 1 addition & 1 deletion cli/main.rs
View file
Expand Up @@ -152,7 +152,7 @@ async fn run_subcommand(flags: Flags) -> Result<i32, AnyError> {
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 {
Expand Down
41 changes: 38 additions & 3 deletions cli/tools/installer.rs
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -183,7 +184,13 @@ pub async fn infer_name_from_url(url: &Url) -> Option<String> {
Some(stem.to_string())
}

pub fn uninstall(name: String, root: Option<String>) -> 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))?
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -663,6 +673,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -697,6 +708,7 @@ mod tests {
name: None,
root: Some(env::temp_dir().to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -725,6 +737,7 @@ mod tests {
name: None,
root: Some(env::temp_dir().to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -758,6 +771,7 @@ mod tests {
name: None,
root: Some(env::temp_dir().to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -786,6 +800,7 @@ mod tests {
name: None,
root: Some(env::temp_dir().to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand All @@ -810,6 +825,7 @@ mod tests {
name: None,
root: Some(env::temp_dir().to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -956,6 +976,7 @@ mod tests {
name: None,
root: Some(temp_dir.to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -990,6 +1011,7 @@ mod tests {
name: None,
root: Some(env::temp_dir().to_string_lossy().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -1025,6 +1047,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -1054,6 +1077,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: false,
global: false,
},
)
.await
Expand All @@ -1074,6 +1098,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: false,
global: false,
},
)
.await;
Expand All @@ -1095,6 +1120,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: true,
global: false,
},
)
.await;
Expand Down Expand Up @@ -1125,6 +1151,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: true,
global: false,
},
)
.await;
Expand Down Expand Up @@ -1154,6 +1181,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -1194,6 +1222,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: false,
global: false,
},
)
.await
Expand Down Expand Up @@ -1238,6 +1267,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: true,
global: false,
},
)
.await;
Expand Down Expand Up @@ -1280,6 +1310,7 @@ mod tests {
name: Some("echo_test".to_string()),
root: Some(temp_dir.path().to_string()),
force: true,
global: false,
},
)
.await;
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit d31f230

Please sign in to comment.