From 6fd2a859763375a73c6ca02b3a6d30ab2780cf8b Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Wed, 16 Oct 2024 13:44:57 +0530 Subject: [PATCH 1/4] fix: consolidate pkg parse for deno install/remove --- cli/tools/registry/pm.rs | 74 ++++++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index ff900d113dcc92..5bceb08d50d42d 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -470,7 +470,7 @@ pub async fn add( let mut package_reqs = Vec::with_capacity(add_flags.packages.len()); for entry_text in add_flags.packages.iter() { - let req = AddPackageReq::parse(entry_text).with_context(|| { + let req = AddRmPackageReq::parse(entry_text).with_context(|| { format!("Failed to parse package required: {}", entry_text) })?; @@ -596,10 +596,10 @@ enum PackageAndVersion { async fn find_package_and_select_version_for_req( jsr_resolver: Arc, npm_resolver: Arc, - add_package_req: AddPackageReq, + add_package_req: AddRmPackageReq, ) -> Result { match add_package_req.value { - AddPackageReqValue::Jsr(req) => { + AddRmPackageReqValue::Jsr(req) => { let jsr_prefixed_name = format!("jsr:{}", &req.name); let Some(nv) = jsr_resolver.req_to_nv(&req).await else { if npm_resolver.req_to_nv(&req).await.is_some() { @@ -628,7 +628,7 @@ async fn find_package_and_select_version_for_req( selected_version: nv.version.to_string(), })) } - AddPackageReqValue::Npm(req) => { + AddRmPackageReqValue::Npm(req) => { let npm_prefixed_name = format!("npm:{}", &req.name); let Some(nv) = npm_resolver.req_to_nv(&req).await else { return Ok(PackageAndVersion::NotFound { @@ -653,18 +653,18 @@ async fn find_package_and_select_version_for_req( } #[derive(Debug, PartialEq, Eq)] -enum AddPackageReqValue { +enum AddRmPackageReqValue { Jsr(PackageReq), Npm(PackageReq), } #[derive(Debug, PartialEq, Eq)] -struct AddPackageReq { +struct AddRmPackageReq { alias: String, - value: AddPackageReqValue, + value: AddRmPackageReqValue, } -impl AddPackageReq { +impl AddRmPackageReq { pub fn parse(entry_text: &str) -> Result, AnyError> { enum Prefix { Jsr, @@ -719,9 +719,9 @@ impl AddPackageReq { let req_ref = JsrPackageReqReference::from_str(&format!("jsr:{}", entry_text))?; let package_req = req_ref.into_inner().req; - Ok(Ok(AddPackageReq { + Ok(Ok(AddRmPackageReq { alias: maybe_alias.unwrap_or_else(|| package_req.name.to_string()), - value: AddPackageReqValue::Jsr(package_req), + value: AddRmPackageReqValue::Jsr(package_req), })) } Prefix::Npm => { @@ -739,9 +739,9 @@ impl AddPackageReq { deno_semver::RangeSetOrTag::Tag("latest".into()), ); } - Ok(Ok(AddPackageReq { + Ok(Ok(AddRmPackageReq { alias: maybe_alias.unwrap_or_else(|| package_req.name.to_string()), - value: AddPackageReqValue::Npm(package_req), + value: AddRmPackageReqValue::Npm(package_req), })) } } @@ -781,7 +781,19 @@ pub async fn remove( for package in &remove_flags.packages { let mut removed = false; for config in configs.iter_mut().flatten() { - removed |= config.remove(package); + let req = AddRmPackageReq::parse(package).with_context(|| { + format!("Failed to parse package required: {}", package) + })?; + match req { + Ok(rm_pkg) => { + removed |= config.remove(&rm_pkg.alias); + } + Err(pkg) => { + // An alias or a package name without registry/version + // constraints. Try to remove the package anyway. + removed |= config.remove(&pkg.name); + } + } } if removed { removed_packages.push(package.clone()); @@ -913,48 +925,52 @@ mod test { #[test] fn test_parse_add_package_req() { assert_eq!( - AddPackageReq::parse("jsr:foo").unwrap().unwrap(), - AddPackageReq { + AddRmPackageReq::parse("jsr:foo").unwrap().unwrap(), + AddRmPackageReq { alias: "foo".to_string(), - value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap()) + value: AddRmPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap()) } ); assert_eq!( - AddPackageReq::parse("alias@jsr:foo").unwrap().unwrap(), - AddPackageReq { + AddRmPackageReq::parse("alias@jsr:foo").unwrap().unwrap(), + AddRmPackageReq { alias: "alias".to_string(), - value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap()) + value: AddRmPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap()) } ); assert_eq!( - AddPackageReq::parse("@alias/pkg@npm:foo").unwrap().unwrap(), - AddPackageReq { + AddRmPackageReq::parse("@alias/pkg@npm:foo") + .unwrap() + .unwrap(), + AddRmPackageReq { alias: "@alias/pkg".to_string(), - value: AddPackageReqValue::Npm( + value: AddRmPackageReqValue::Npm( PackageReq::from_str("foo@latest").unwrap() ) } ); assert_eq!( - AddPackageReq::parse("@alias/pkg@jsr:foo").unwrap().unwrap(), - AddPackageReq { + AddRmPackageReq::parse("@alias/pkg@jsr:foo") + .unwrap() + .unwrap(), + AddRmPackageReq { alias: "@alias/pkg".to_string(), - value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap()) + value: AddRmPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap()) } ); assert_eq!( - AddPackageReq::parse("alias@jsr:foo@^1.5.0") + AddRmPackageReq::parse("alias@jsr:foo@^1.5.0") .unwrap() .unwrap(), - AddPackageReq { + AddRmPackageReq { alias: "alias".to_string(), - value: AddPackageReqValue::Jsr( + value: AddRmPackageReqValue::Jsr( PackageReq::from_str("foo@^1.5.0").unwrap() ) } ); assert_eq!( - AddPackageReq::parse("@scope/pkg@tag") + AddRmPackageReq::parse("@scope/pkg@tag") .unwrap() .unwrap_err() .to_string(), From accffd394eb40fb8558abd144dc25199fe260b6a Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Wed, 16 Oct 2024 13:55:09 +0530 Subject: [PATCH 2/4] feat(test): add specs::remove::alias --- tests/specs/remove/alias/__test__.jsonc | 29 +++++++++++++++++++++++ tests/specs/remove/alias/package.json | 5 ++++ tests/specs/remove/alias/package.json.out | 2 ++ 3 files changed, 36 insertions(+) create mode 100644 tests/specs/remove/alias/__test__.jsonc create mode 100644 tests/specs/remove/alias/package.json create mode 100644 tests/specs/remove/alias/package.json.out diff --git a/tests/specs/remove/alias/__test__.jsonc b/tests/specs/remove/alias/__test__.jsonc new file mode 100644 index 00000000000000..a0c98edd9086fd --- /dev/null +++ b/tests/specs/remove/alias/__test__.jsonc @@ -0,0 +1,29 @@ +{ + "tempDir": true, + "tests": { + "alias_with_pkg": { + "steps": [{ + "args": "remove my-alias@npm:@denotest/add", + "output": "[WILDCARD]" + }, { + "args": [ + "eval", + "console.log(Deno.readTextFileSync('package.json').trim())" + ], + "output": "package.json.out" + }] + }, + "alias": { + "steps": [{ + "args": "remove my-alias", + "output": "[WILDCARD]" + }, { + "args": [ + "eval", + "console.log(Deno.readTextFileSync('package.json').trim())" + ], + "output": "package.json.out" + }] + } + } +} diff --git a/tests/specs/remove/alias/package.json b/tests/specs/remove/alias/package.json new file mode 100644 index 00000000000000..b6326e8bfbd0ba --- /dev/null +++ b/tests/specs/remove/alias/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "my-alias": "npm:@denotest/add@^1.0.0" + } +} diff --git a/tests/specs/remove/alias/package.json.out b/tests/specs/remove/alias/package.json.out new file mode 100644 index 00000000000000..2c63c0851048d8 --- /dev/null +++ b/tests/specs/remove/alias/package.json.out @@ -0,0 +1,2 @@ +{ +} From 4fefefe9b4de857793725bee8a81bcdf5d00c56a Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Wed, 16 Oct 2024 14:13:01 +0530 Subject: [PATCH 3/4] display parsed pkg name in output --- cli/tools/registry/pm.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 5bceb08d50d42d..f716dd2ca600a2 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -779,24 +779,28 @@ pub async fn remove( let mut removed_packages = vec![]; for package in &remove_flags.packages { - let mut removed = false; + let req = AddRmPackageReq::parse(package).with_context(|| { + format!("Failed to parse package required: {}", package) + })?; + let mut parsed_pkg_name = None; for config in configs.iter_mut().flatten() { - let req = AddRmPackageReq::parse(package).with_context(|| { - format!("Failed to parse package required: {}", package) - })?; - match req { + match &req { Ok(rm_pkg) => { - removed |= config.remove(&rm_pkg.alias); + if config.remove(&rm_pkg.alias) && parsed_pkg_name.is_none() { + parsed_pkg_name = Some(rm_pkg.alias.clone()); + } } Err(pkg) => { // An alias or a package name without registry/version // constraints. Try to remove the package anyway. - removed |= config.remove(&pkg.name); + if config.remove(&pkg.name) && parsed_pkg_name.is_none() { + parsed_pkg_name = Some(pkg.name.clone()); + } } } } - if removed { - removed_packages.push(package.clone()); + if let Some(pkg) = parsed_pkg_name { + removed_packages.push(pkg); } } From 3aac5f77198b1da3aeda54b9cfad82f5b37e01e1 Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Wed, 16 Oct 2024 14:13:19 +0530 Subject: [PATCH 4/4] update test --- tests/specs/remove/basic/__test__.jsonc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/specs/remove/basic/__test__.jsonc b/tests/specs/remove/basic/__test__.jsonc index fd74900b44bb51..3ca396f8ba7cbf 100644 --- a/tests/specs/remove/basic/__test__.jsonc +++ b/tests/specs/remove/basic/__test__.jsonc @@ -7,7 +7,7 @@ "args": ["eval", "console.log(Deno.readTextFileSync('deno.lock').trim())"], "output": "add_lock.out" }, { - "args": ["remove", "@std/assert", "@std/http"], + "args": ["remove", "jsr:@std/assert", "@std/http"], "output": "rm.out" }, { "args": ["eval", "console.log(Deno.readTextFileSync('deno.lock').trim())"],