Skip to content

Commit

Permalink
Replace the daggy crate with petgraph
Browse files Browse the repository at this point in the history
The `daggy` crate doesn't seem maintained anymore as the last commit was
over three years ago [0] and there isn't really any activity since then.

Daggy is already based on top of `petgraph` and described as follows:
> The most prominent type is Dag - a wrapper around petgraph’s Graph
> data structure, exposing a refined API targeted towards directed
> acyclic graph related functionality.

This means that we can switch directly to `petgraph` without too much
effort. We'll loose some of the refined API, especially walking graphs
via iterators, but it doesn't really matter too much as the `Acyclic`
type, "a wrapper around graph types that enforces an acyclicity
invariant", works well enough (just a bit less "refined").

We also already used `petgraph` directly for the `tree-of` command.
This direct dependency on `petgraph` became the trigger for dropping the
dependency on `daggy` since `petgraph` yanked the release of version
`0.6.6` with the new release of version `0.7.0` [1] and the resulting CI
failure for the `petgraph` update [2] made me take a closer look at the
situation (we don't necessarily have to drop `daggy` just yet but it
seems for the best given it's unclear future and the duplicated
`petgraph` dependency that causes incompatibilities / build failures).

[0]: https://github.com/mitchmindtree/daggy
[1]: petgraph/petgraph#712
[2]: #446

Signed-off-by: Michael Weiss <michael.weiss@eviden.com>
  • Loading branch information
primeos-work committed Jan 3, 2025
1 parent f7c68cb commit 571a183
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 58 deletions.
11 changes: 0 additions & 11 deletions Cargo.lock

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
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
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
30 changes: 17 additions & 13 deletions src/job/dag.rs
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,20 +46,24 @@ 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);
let children_uuids = children
.iter(&self.dag)
.filter_map(|(_, node_idx)| self.dag.graph().node_weight(node_idx))
.map(Job::uuid)
.cloned()
.collect();
self.dag.node_indices().map(move |idx| {
let job = self.dag.node_weight(idx).unwrap(); // TODO
let mut children = self
.dag
.neighbors_directed(idx, petgraph::Outgoing)
.detach();
let mut children_uuids = Vec::<Uuid>::new();
while let Some(node_idx) = children.next_node(&self.dag) {
if let Some(job) = self.dag.node_weight(node_idx) {
children_uuids.push(*Job::uuid(job));
}
}

JobDefinition {
job,
Expand Down
58 changes: 34 additions & 24 deletions src/package/dag.rs
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

0 comments on commit 571a183

Please sign in to comment.