From 7d099280e1274195561af87948c83b73240690d2 Mon Sep 17 00:00:00 2001 From: Etienne Date: Wed, 27 Aug 2025 09:29:03 -0400 Subject: [PATCH 1/3] add assert_package_exist_locally + tests + hook it to cli update flow --- CHANGELOG.md | 6 +++++- compiler-cli/src/dependencies.rs | 22 ++++++++++++++++++++++ compiler-cli/src/dependencies/tests.rs | 23 +++++++++++++++++++++++ compiler-core/src/error.rs | 21 +++++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ede2e4b49e..8526adb35bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,7 @@ - The compiler will now raise warning for inefficient use of `list.length()` when trying to check is list empty via `0 < list.length(list)` or -`list.length(list) > 0` as well as in other cases. For example, the following + `list.length(list) > 0` as well as in other cases. For example, the following code: ```gleam @@ -343,3 +343,7 @@ - Fixed a bug where adding `echo` to the subject of a `case` expression would prevent variant inference from working correctly. ([Surya Rose](https://github.com/GearsDatapacks)) + +- Added an error message when attempting to update packages that are not dependencies + of the project, instead of failing silently. + ([Etienne Boutet](https://github.com/EtienneBoutet)) diff --git a/compiler-cli/src/dependencies.rs b/compiler-cli/src/dependencies.rs index 4c44a6b1071..901e958cb24 100644 --- a/compiler-cli/src/dependencies.rs +++ b/compiler-cli/src/dependencies.rs @@ -225,6 +225,9 @@ pub fn update(paths: &ProjectPaths, packages: Vec) -> Result<()> { UseManifest::Yes }; + let config = crate::config::read(paths.root_config())?; + assert_packages_exist_locally(&config, packages.clone())?; + // Update specific packages _ = download( paths, @@ -240,6 +243,25 @@ pub fn update(paths: &ProjectPaths, packages: Vec) -> Result<()> { Ok(()) } +fn assert_packages_exist_locally(config: &PackageConfig, packages: Vec) -> Result<()> { + let missing_packages: Vec = packages + .iter() + .filter(|package_name| { + let package_name_eco = EcoString::from(package_name.as_str()); + !config.dependencies.contains_key(&package_name_eco) + && !config.dev_dependencies.contains_key(&package_name_eco) + }) + .cloned() + .collect(); + + if !missing_packages.is_empty() { + return Err(Error::PackagesToUpdateNotExist { + packages: missing_packages, + }); + } + Ok(()) +} + /// Edit the manifest.toml file in this proejct, removing all extra requirements and packages /// that are no longer present in the gleam.toml config. pub fn cleanup(paths: &ProjectPaths, telemetry: Telem) -> Result { diff --git a/compiler-cli/src/dependencies/tests.rs b/compiler-cli/src/dependencies/tests.rs index 8ec847c6393..37c74ed23c4 100644 --- a/compiler-cli/src/dependencies/tests.rs +++ b/compiler-cli/src/dependencies/tests.rs @@ -1347,3 +1347,26 @@ fn test_pretty_print_major_versions_available() { insta::assert_snapshot!(output); } + +#[test] +fn test_assert_packages_exist_locally() { + let config = package_config( + HashMap::from([( + "existing_package".into(), + Requirement::hex("~>1.0").unwrap(), + )]), + HashMap::from([("dev_package".into(), Requirement::hex("~>2.0").unwrap())]), + ); + + assert!(assert_packages_exist_locally(&config, vec!["existing_package".to_string()]).is_ok()); + assert!(assert_packages_exist_locally(&config, vec!["dev_package".to_string()]).is_ok()); + + let result = assert_packages_exist_locally(&config, vec!["nonexistent_package".to_string()]); + assert!(result.is_err()); + match result { + Err(Error::PackagesToUpdateNotExist { packages }) => { + assert_eq!(packages, vec!["nonexistent_package".to_string()]); + } + _ => panic!("Expected PackagesToUpdateNotExist error"), + } +} diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index d2e3e7f308b..751a5fae979 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -205,6 +205,9 @@ file_names.iter().map(|x| x.as_str()).join(", "))] #[error("Packages not exist: {}", packages.iter().join(", "))] RemovedPackagesNotExist { packages: Vec }, + #[error("Packages to update not exist: {}", packages.iter().join(", "))] + PackagesToUpdateNotExist { packages: Vec }, + #[error("unable to find project root")] UnableToFindProjectRoot { path: String }, @@ -958,6 +961,24 @@ If you want to overwrite these files, delete them and run the command again. "These packages are not dependencies of your package so they could not be removed. +{} +", + packages + .iter() + .map(|p| format!(" - {}", p.as_str())) + .join("\n") + ), + level: Level::Error, + hint: None, + location: None, + }], + + Error::PackagesToUpdateNotExist { packages } => vec![Diagnostic { + title: "Packages not found".into(), + text: format!( + "These packages are not dependencies of your package so they could not +be updated. + {} ", packages From ef2148cc70c3ff7c49c9d746251db40f7517e41b Mon Sep 17 00:00:00 2001 From: Etienne Date: Wed, 27 Aug 2025 12:59:56 -0400 Subject: [PATCH 2/3] add pub --- compiler-cli/src/dependencies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-cli/src/dependencies.rs b/compiler-cli/src/dependencies.rs index 901e958cb24..ae59290a511 100644 --- a/compiler-cli/src/dependencies.rs +++ b/compiler-cli/src/dependencies.rs @@ -243,7 +243,7 @@ pub fn update(paths: &ProjectPaths, packages: Vec) -> Result<()> { Ok(()) } -fn assert_packages_exist_locally(config: &PackageConfig, packages: Vec) -> Result<()> { +pub fn assert_packages_exist_locally(config: &PackageConfig, packages: Vec) -> Result<()> { let missing_packages: Vec = packages .iter() .filter(|package_name| { From 76a31fd2248d74fcb775ce5348fe19059f40c15f Mon Sep 17 00:00:00 2001 From: Etienne Date: Fri, 29 Aug 2025 18:41:32 -0400 Subject: [PATCH 3/3] use manifest to verify packages presence --- compiler-cli/src/dependencies.rs | 9 ++++----- compiler-cli/src/dependencies/tests.rs | 28 ++++++++++++++++---------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/compiler-cli/src/dependencies.rs b/compiler-cli/src/dependencies.rs index ae59290a511..60ee48d70e0 100644 --- a/compiler-cli/src/dependencies.rs +++ b/compiler-cli/src/dependencies.rs @@ -225,8 +225,8 @@ pub fn update(paths: &ProjectPaths, packages: Vec) -> Result<()> { UseManifest::Yes }; - let config = crate::config::read(paths.root_config())?; - assert_packages_exist_locally(&config, packages.clone())?; + let manifest = read_manifest_from_disc(paths)?; + assert_packages_exist_locally(&manifest, packages.clone())?; // Update specific packages _ = download( @@ -243,13 +243,12 @@ pub fn update(paths: &ProjectPaths, packages: Vec) -> Result<()> { Ok(()) } -pub fn assert_packages_exist_locally(config: &PackageConfig, packages: Vec) -> Result<()> { +pub fn assert_packages_exist_locally(manifest: &Manifest, packages: Vec) -> Result<()> { let missing_packages: Vec = packages .iter() .filter(|package_name| { let package_name_eco = EcoString::from(package_name.as_str()); - !config.dependencies.contains_key(&package_name_eco) - && !config.dev_dependencies.contains_key(&package_name_eco) + !manifest.packages.iter().any(|p| p.name == package_name_eco) }) .cloned() .collect(); diff --git a/compiler-cli/src/dependencies/tests.rs b/compiler-cli/src/dependencies/tests.rs index 37c74ed23c4..312c840305a 100644 --- a/compiler-cli/src/dependencies/tests.rs +++ b/compiler-cli/src/dependencies/tests.rs @@ -1350,22 +1350,28 @@ fn test_pretty_print_major_versions_available() { #[test] fn test_assert_packages_exist_locally() { - let config = package_config( - HashMap::from([( - "existing_package".into(), - Requirement::hex("~>1.0").unwrap(), - )]), - HashMap::from([("dev_package".into(), Requirement::hex("~>2.0").unwrap())]), - ); + let manifest = Manifest { + requirements: HashMap::from([ + ( + "existing_package".into(), + Requirement::hex("~>1.0").unwrap(), + ), + ("dev_package".into(), Requirement::hex("~>2.0").unwrap()), + ]), + packages: vec![ + manifest_package("existing_package", "1.0.0", vec![]), + manifest_package("dev_package", "2.0.0", vec![]), + ], + }; - assert!(assert_packages_exist_locally(&config, vec!["existing_package".to_string()]).is_ok()); - assert!(assert_packages_exist_locally(&config, vec!["dev_package".to_string()]).is_ok()); + assert!(assert_packages_exist_locally(&manifest, vec!["existing_package".to_string()]).is_ok()); + assert!(assert_packages_exist_locally(&manifest, vec!["dev_package".to_string()]).is_ok()); - let result = assert_packages_exist_locally(&config, vec!["nonexistent_package".to_string()]); + let result = assert_packages_exist_locally(&manifest, vec!["nonexisting_package".to_string()]); assert!(result.is_err()); match result { Err(Error::PackagesToUpdateNotExist { packages }) => { - assert_eq!(packages, vec!["nonexistent_package".to_string()]); + assert_eq!(packages, vec!["nonexisting_package".to_string()]); } _ => panic!("Expected PackagesToUpdateNotExist error"), }