Skip to content

Commit

Permalink
moss/registry: Restrict removal lookups to installed candidates
Browse files Browse the repository at this point in the history
If we don't do this we could end up (in future) look at next-candidate
for dependency info and break the graph entirely, removing packages
that don't exist.

Signed-off-by: Ikey Doherty <ikey@serpentos.com>
  • Loading branch information
ikeycode committed Oct 14, 2023
1 parent 88b97e2 commit 034445f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
7 changes: 3 additions & 4 deletions moss/src/cli/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ pub async fn handle(args: &ArgMatches, root: &Path) -> Result<(), Error> {
return Err(Error::NotImplemented);
}

let mut transaction = client.registry.transaction()?;

// Add all installed packages to transaction
transaction
.add(installed_ids.iter().cloned().cloned().collect())
let mut transaction = client
.registry
.transaction_with_packages(installed_ids.iter().cloned().cloned().collect())
.await?;

// Remove all pkgs for removal
Expand Down
8 changes: 8 additions & 0 deletions moss/src/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ impl Registry {
pub fn transaction(&self) -> Result<Transaction<'_>, transaction::Error> {
transaction::new(self)
}

/// Return a new transaction for this registry initialised with the incoming package set as installed
pub async fn transaction_with_packages(
&self,
incoming: Vec<package::Id>,
) -> Result<Transaction<'_>, transaction::Error> {
transaction::new_with_packages(self, incoming).await
}
}

#[cfg(test)]
Expand Down
27 changes: 24 additions & 3 deletions moss/src/registry/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ enum ProviderFilter {
All(Provider),
}

enum Lookup {
InstalledOnly,
Global,
}

/// A Transaction is used to modify one system state to another
#[derive(Clone, Debug)]
pub struct Transaction<'a> {
Expand All @@ -47,10 +52,20 @@ pub(super) fn new(registry: &Registry) -> Result<Transaction<'_>, Error> {
})
}

/// Populate the transaction on initialisation
pub(super) async fn new_with_packages(
registry: &Registry,
incoming: Vec<package::Id>,
) -> Result<Transaction<'_>, Error> {
let mut tx = new(registry)?;
tx.update(incoming, Lookup::InstalledOnly).await?;
Ok(tx)
}

impl<'a> Transaction<'a> {
/// Add a package to this transaction
pub async fn add(&mut self, incoming: Vec<package::Id>) -> Result<(), Error> {
self.update(incoming).await
self.update(incoming, Lookup::Global).await
}

/// Remove a set of packages and their reverse dependencies
Expand All @@ -74,7 +89,7 @@ impl<'a> Transaction<'a> {
}

/// Update internal package graph with all incoming packages & their deps
async fn update(&mut self, incoming: Vec<package::Id>) -> Result<(), Error> {
async fn update(&mut self, incoming: Vec<package::Id>, lookup: Lookup) -> Result<(), Error> {
let mut items = incoming;

loop {
Expand All @@ -98,7 +113,13 @@ impl<'a> Transaction<'a> {
};

// Now get it resolved
let search = self.resolve_installation_provider(provider).await?;
let search = match lookup {
Lookup::Global => self.resolve_installation_provider(provider).await?,
Lookup::InstalledOnly => {
self.resolve_provider(ProviderFilter::InstalledOnly(provider))
.await?
}
};

// Add dependency node
let need_search = !self.packages.node_exists(&search);
Expand Down

0 comments on commit 034445f

Please sign in to comment.