Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,7 @@
- The compiler now correctly tracks the minimum required version for expressions
in `BitArray`s' `size` option to be `>= 1.12.0`.
([Giacomo Cavalieri](https://github.com/giacomocavalieri))

- 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)) and ([Vladislav Shakitskiy](https://github.com/vshakitskiy))
18 changes: 18 additions & 0 deletions compiler-cli/src/dependencies/dependency_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,22 @@ use super::{
remove_extra_packages, unlock_packages,
};

/// Verifies that all specified packages exist in the manifest.
pub fn ensure_packages_exist_locally(manifest: &Manifest, packages: &[EcoString]) -> Result<()> {
let missing_packages: Vec<EcoString> = packages
.iter()
.filter(|package_name| !manifest.packages.iter().any(|p| &p.name == *package_name))
.cloned()
.collect();

if !missing_packages.is_empty() {
return Err(Error::PackagesToUpdateNotExist {
packages: missing_packages,
});
}
Ok(())
}

pub struct DependencyManagerConfig {
// If `Yes` we read the manifest from disc. If not set then we ignore any
// manifest which will result in the latest versions of the dependency
Expand Down Expand Up @@ -81,10 +97,12 @@ where
if !paths.manifest().exists() {
tracing::debug!("manifest_not_present");
let manifest = self.perform_version_resolution(paths, config, None, Vec::new())?;
ensure_packages_exist_locally(&manifest, &packages_to_update)?;
return Ok(Resolved::all_added(manifest));
}

let existing_manifest = read_manifest_from_disc(paths)?;
ensure_packages_exist_locally(&existing_manifest, &packages_to_update)?;

// If we have been asked not to use the manifest then
let (requirements_changed, manifest_for_resolver) = match self.use_manifest {
Expand Down
45 changes: 45 additions & 0 deletions compiler-cli/src/dependencies/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1374,3 +1374,48 @@ fn test_pretty_print_version_updates() {

insta::assert_snapshot!(output);
}

#[test]
fn test_ensure_packages_exist_locally_all_present() {
let manifest = Manifest {
requirements: HashMap::new(),
packages: vec![
manifest_package("package_a", "1.0.0", vec![]),
manifest_package("package_b", "2.0.0", vec![]),
manifest_package("package_c", "3.0.0", vec![]),
],
};

let packages_to_check = vec!["package_a".into(), "package_b".into()];
let result = dependency_manager::ensure_packages_exist_locally(&manifest, &packages_to_check);

assert!(result.is_ok(), "All packages exist, should return Ok");
}

#[test]
fn test_ensure_packages_exist_locally_some_missing() {
let manifest = Manifest {
requirements: HashMap::new(),
packages: vec![
manifest_package("package_a", "1.0.0", vec![]),
manifest_package("package_b", "2.0.0", vec![]),
],
};

let packages_to_check = vec![
"package_a".into(),
"package_b".into(),
"missing_package".into(),
"another_missing".into(),
];
let result = dependency_manager::ensure_packages_exist_locally(&manifest, &packages_to_check);

match result {
Err(Error::PackagesToUpdateNotExist { packages }) => {
assert_eq!(packages.len(), 2);
assert!(packages.contains(&"missing_package".into()));
assert!(packages.contains(&"another_missing".into()));
}
_ => panic!("Expected PackagesToUpdateNotExist error"),
}
}
21 changes: 21 additions & 0 deletions compiler-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ file_names.iter().map(|x| x.as_str()).join(", "))]
#[error("Packages not exist: {}", packages.iter().join(", "))]
RemovedPackagesNotExist { packages: Vec<String> },

#[error("Packages to update not exist: {}", packages.iter().join(", "))]
PackagesToUpdateNotExist { packages: Vec<EcoString> },

#[error("unable to find project root")]
UnableToFindProjectRoot { path: String },

Expand Down Expand Up @@ -1012,6 +1015,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 to update not found".into(),
text: format!(
"These packages are not dependencies of your package so they could not
be updated.

{}
",
packages
Expand Down
Loading