Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace the daggy crate with petgraph #448

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Replace the daggy crate with petgraph #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 0 additions & 11 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ clap_complete = "4"
colored = "2"
config = { version = "0.15", default-features = false, features = [ "toml" ] }
csv = "1"
daggy = { version = "0.8", features = [ "serde" ] }
dialoguer = "0.11"
diesel = { version = "2", features = ["postgres", "chrono", "uuid", "serde_json", "r2d2"] }
diesel_migrations = "2"
Expand Down
12 changes: 3 additions & 9 deletions src/commands/tree_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ use anyhow::Error;
use anyhow::Result;
use clap::ArgMatches;
use petgraph::dot::Dot;
use petgraph::graph::DiGraph;
use resiter::AndThen;

use crate::config::Configuration;
use crate::package::condition::ConditionData;
use crate::package::Dag;
use crate::package::DependencyType;
use crate::package::Package;
use crate::package::PackageName;
use crate::package::PackageVersionConstraint;
use crate::repository::Repository;
Expand Down Expand Up @@ -73,10 +71,8 @@ pub async fn tree_of(matches: &ArgMatches, repo: Repository, config: &Configurat
.map(|package| Dag::for_root_package(package.clone(), &repo, None, &condition_data))
.and_then_ok(|dag| {
if dot {
let petgraph: DiGraph<Package, DependencyType> = (*dag.dag()).clone().into();

let dot = Dot::with_attr_getters(
&petgraph,
dag.dag(),
&[
petgraph::dot::Config::EdgeNoLabel,
petgraph::dot::Config::NodeNoLabel,
Expand All @@ -96,13 +92,11 @@ pub async fn tree_of(matches: &ArgMatches, repo: Repository, config: &Configurat
println!("{:?}", dot);
Ok(())
} else if serial_buildorder {
let petgraph: DiGraph<Package, DependencyType> = (*dag.dag()).clone().into();

let topo_sorted = petgraph::algo::toposort(&petgraph, None)
let topo_sorted = petgraph::algo::toposort(dag.dag(), None)
.map_err(|_| Error::msg("Cyclic dependency found!"))?;

for node in topo_sorted.iter().rev() {
let package = petgraph.node_weight(*node).unwrap();
let package = dag.dag().node_weight(*node).unwrap();
println!("{}", package.display_name_version());
}
println!();
Expand Down
18 changes: 9 additions & 9 deletions src/job/dag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
// SPDX-License-Identifier: EPL-2.0
//

use daggy::Dag as DaggyDag;
use daggy::Walker;
use getset::Getters;
use petgraph::acyclic::Acyclic;
use petgraph::graph::DiGraph;
use uuid::Uuid;

use crate::job::Job;
Expand All @@ -24,7 +24,7 @@ use crate::util::docker::ImageName;
#[derive(Debug, Getters)]
pub struct Dag {
#[getset(get = "pub")]
dag: DaggyDag<Job, DependencyType>,
dag: Acyclic<DiGraph<Job, DependencyType>>,
}

impl Dag {
Expand All @@ -46,17 +46,17 @@ impl Dag {
};

Dag {
dag: dag.dag().map(build_job, |_, e| (*e).clone()),
dag: Acyclic::<_>::try_from_graph(dag.dag().map(build_job, |_, e| (*e).clone()))
.unwrap(), // The dag.dag() is already acyclic so this cannot fail
}
}

pub fn iter(&'_ self) -> impl Iterator<Item = JobDefinition> + '_ {
self.dag.graph().node_indices().map(move |idx| {
let job = self.dag.graph().node_weight(idx).unwrap(); // TODO
let children = self.dag.children(idx);
self.dag.node_indices().map(move |idx| {
let job = self.dag.node_weight(idx).unwrap(); // TODO
let children = self.dag.neighbors_directed(idx, petgraph::Outgoing);
let children_uuids = children
.iter(&self.dag)
.filter_map(|(_, node_idx)| self.dag.graph().node_weight(node_idx))
.filter_map(|node_idx| self.dag.node_weight(node_idx))
.map(Job::uuid)
.cloned()
.collect();
Expand Down
58 changes: 34 additions & 24 deletions src/package/dag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@ use std::io::Write;

use anyhow::anyhow;
use anyhow::Context;
use anyhow::Error;
use anyhow::Result;
use daggy::Walker;
use getset::Getters;
use indicatif::ProgressBar;
use itertools::Itertools;
use petgraph::acyclic::Acyclic;
use petgraph::data::Build;
use petgraph::graph::DiGraph;
use petgraph::graph::EdgeIndex;
use petgraph::graph::NodeIndex;
use ptree::Style;
use ptree::TreeItem;
use resiter::AndThen;
Expand All @@ -37,10 +40,10 @@ use crate::repository::Repository;
#[derive(Debug, Getters)]
pub struct Dag {
#[getset(get = "pub")]
dag: daggy::Dag<Package, DependencyType>,
dag: Acyclic<DiGraph<Package, DependencyType>>,

#[getset(get = "pub")]
root_idx: daggy::NodeIndex,
root_idx: NodeIndex,
}

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
Expand Down Expand Up @@ -114,8 +117,8 @@ impl Dag {
/// and adds corresponding nodes to the DAG. The edges are added later in `add_edges()`.
fn add_sub_packages<'a>(
repo: &'a Repository,
mappings: &mut HashMap<&'a Package, daggy::NodeIndex>,
dag: &mut daggy::Dag<&'a Package, DependencyType>,
mappings: &mut HashMap<&'a Package, NodeIndex>,
dag: &mut Acyclic<DiGraph<&'a Package, DependencyType>>,
p: &'a Package,
progress: Option<&ProgressBar>,
conditional_data: &ConditionData<'_>,
Expand Down Expand Up @@ -180,8 +183,8 @@ impl Dag {
// TODO: It seems easier and more efficient to do this in `add_sub_packages` as well (it
// makes that function more complex but doing it separately is weird).
fn add_edges(
mappings: &HashMap<&Package, daggy::NodeIndex>,
dag: &mut daggy::Dag<&Package, DependencyType>,
mappings: &HashMap<&Package, NodeIndex>,
dag: &mut Acyclic<DiGraph<&Package, DependencyType>>,
conditional_data: &ConditionData<'_>,
) -> Result<()> {
for (package, idx) in mappings {
Expand All @@ -193,9 +196,14 @@ impl Dag {
*pkg.name() == dep_name && dep_constr.matches(pkg.version())
})
.try_for_each(|(dep, dep_idx)| {
dag.add_edge(*idx, *dep_idx, dep_kind.clone())
dag.try_add_edge(*idx, *dep_idx, dep_kind.clone())
.map(|_| ())
.map_err(Error::from)
// Only debug formatting is available for the error and for
// cycles it is quite useless (the context below is much more
// helpful than, e.g., "Cycle(Cycle(NodeIndex(0)))") but we'll
// include it for completeness and in case of the other errors
// that could theoretically occur:
.map_err(|e| anyhow!(format!("{e:?}")))
.with_context(|| {
anyhow!(
"Failed to add package dependency DAG edge \
Expand All @@ -215,7 +223,7 @@ impl Dag {
}

// Create an empty DAG and use the above helper functions to compute the dependency graph:
let mut dag: daggy::Dag<&Package, DependencyType> = daggy::Dag::new();
let mut dag = Acyclic::<DiGraph<&Package, DependencyType>>::new();
let mut mappings = HashMap::new();

trace!("Building the package dependency DAG for package {:?}", p);
Expand All @@ -234,10 +242,11 @@ impl Dag {
trace!("Finished building the package DAG");

Ok(Dag {
dag: dag.map(
dag: Acyclic::<_>::try_from_graph(dag.map(
|_, p: &&Package| -> Package { (*p).clone() },
|_, e| (*e).clone(),
),
))
.unwrap(), // The dag is already acyclic so this cannot fail
root_idx,
})
}
Expand All @@ -249,9 +258,8 @@ impl Dag {
/// The order of the packages is _NOT_ guaranteed by the implementation
pub fn all_packages(&self) -> Vec<&Package> {
self.dag
.graph()
.node_indices()
.filter_map(|idx| self.dag.graph().node_weight(idx))
.filter_map(|idx| self.dag.node_weight(idx))
.collect()
}

Expand All @@ -261,7 +269,7 @@ impl Dag {
}

#[derive(Clone)]
pub struct DagDisplay<'a>(&'a Dag, daggy::NodeIndex, Option<daggy::EdgeIndex>);
pub struct DagDisplay<'a>(&'a Dag, NodeIndex, Option<EdgeIndex>);

impl TreeItem for DagDisplay<'_> {
type Child = Self;
Expand All @@ -270,7 +278,6 @@ impl TreeItem for DagDisplay<'_> {
let p = self
.0
.dag
.graph()
.node_weight(self.1)
.ok_or_else(|| anyhow!("Error finding node: {:?}", self.1))
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
Expand All @@ -281,7 +288,6 @@ impl TreeItem for DagDisplay<'_> {
Some(edge_idx) => self
.0
.dag
.graph()
.edge_weight(edge_idx)
.ok_or_else(|| anyhow!("Error finding edge: {:?}", self.2))
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?,
Expand All @@ -295,12 +301,16 @@ impl TreeItem for DagDisplay<'_> {
}

fn children(&self) -> Cow<[Self::Child]> {
let c = self.0.dag.children(self.1);
Cow::from(
c.iter(&self.0.dag)
.map(|(edge_idx, node_idx)| DagDisplay(self.0, node_idx, Some(edge_idx)))
.collect::<Vec<_>>(),
)
let mut children_walker = self
.0
.dag
.neighbors_directed(self.1, petgraph::Outgoing)
.detach();
let mut children = Vec::<Self::Child>::new();
while let Some((edge_idx, node_idx)) = children_walker.next(&self.0.dag) {
children.push(DagDisplay(self.0, node_idx, Some(edge_idx)));
}
Cow::from(children)
}
}

Expand Down
Loading