interim commit

Author: mkatychev

crates/wasm-pkg-client/src/caching/mod.rs

@@ -46,7 +46,7 @@ pub trait Cache {
 /// A client that caches response data using the given cache implementation. Can be used without an
 /// underlying client to be used as a read-only cache.
 pub struct CachingClient<T> {
-    client: Option<Client>,
+    pub client: Option<Client>,
     cache: Arc<T>,
 }
 

crates/wasm-pkg-client/src/local.rs

@@ -10,8 +10,9 @@ use std::{
 use anyhow::anyhow;
 use async_trait::async_trait;
 use futures_util::{StreamExt, TryStreamExt};
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
+use tempfile::TempDir;
 use tokio_util::io::ReaderStream;
 use wasm_pkg_common::{
     config::RegistryConfig,
@@ -27,12 +28,13 @@ use crate::{
     ContentStream, PublishingSource,
 };
 
-#[derive(Clone, Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct LocalConfig {
     pub root: PathBuf,
 }
 
-pub(crate) struct LocalBackend {
+#[derive(Clone)]
+pub struct LocalBackend {
     pub(crate) root: PathBuf,
 }
 
@@ -60,6 +62,13 @@ impl LocalBackend {
     fn version_path(&self, package: &PackageRef, version: &Version) -> PathBuf {
         self.package_dir(package).join(format!("{version}.wasm"))
     }
+
+    pub fn temp_dir() -> Result<(Self, TempDir), Error> {
+        let handle = TempDir::new()?;
+        let root = handle.path().to_owned();
+        tracing::debug!(registry_dir=%root.display(), "created temporary directory");
+        Ok((Self { root }, handle))
+    }
 }
 
 #[async_trait]

crates/wasm-pkg-client/src/overlay/mod.rs

@@ -20,9 +20,7 @@ pub(crate) struct OverlayBackend {
 
 impl OverlayBackend {
     fn new(remotes: HashMap<PackageRef, InnerClient>) -> Result<Self, Error> {
-        let handle = TempDir::new()?;
-        let root = handle.path().to_owned();
-        let local = LocalBackend { root };
+        let (local, handle) = LocalBackend::temp_dir()?;
         Ok(Self {
             local,
             remotes,
@@ -41,10 +39,14 @@ impl OverlayBackend {
 impl PackageLoader for OverlayBackend {
     async fn list_all_versions(&self, package: &PackageRef) -> Result<Vec<VersionInfo>, Error> {
         let mut versions = self.local.list_all_versions(package).await?;
-        let mut remote_versions = self.remote(package)?.list_all_versions(package).await?;
-        versions.append(&mut remote_versions);
-        versions.sort();
-        versions.dedup();
+
+        if let Some(remote) = self.remotes.get(package) {
+            let mut remote_versions = remote.list_all_versions(package).await?;
+            versions.append(&mut remote_versions);
+            versions.sort();
+            versions.dedup();
+        }
+
         Ok(versions)
     }
 

crates/wasm-pkg-common/src/package.rs

@@ -79,7 +79,7 @@ impl std::fmt::Display for PackageSpec {
 }
 
 /// A package spec combines a [`PackageRef`] with an optional version.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 pub struct PackageSpec {
     pub package: PackageRef,
     pub version: Option<Version>,

crates/wasm-pkg-common/src/registry.rs

@@ -57,8 +57,4 @@ impl TryFrom<String> for Registry {
     }
 }
 
-/// Represents a directed edge in a package dependency graph.
-#[derive(Clone, Debug)]
-pub struct DependencyOf;
-
-pub type DependencyGraph<N> = Acyclic<DiGraph<N, DependencyOf>>;
+pub type DependencyGraph<N> = Acyclic<DiGraph<N, petgraph::Direction>>;

crates/wasm-pkg-core/src/resolver.rs

@@ -15,14 +15,16 @@ use indexmap::{IndexMap, IndexSet};
 use petgraph::{
     acyclic::Acyclic,
     graph::{DiGraph, NodeIndex},
+    Direction,
 };
 use semver::{Comparator, Op, Version, VersionReq};
 use tokio::io::{AsyncRead, AsyncReadExt};
+use tracing::Level;
 use wasm_pkg_client::{
     caching::{CachingClient, FileCache},
     Client, Config, ContentDigest, Error as WasmPkgError, PackageRef, Release, VersionInfo,
 };
-use wasm_pkg_common::registry::{DependencyGraph, DependencyOf};
+use wasm_pkg_common::{package::PackageSpec, registry::DependencyGraph};
 use wit_component::DecodedWasm;
 use wit_parser::{PackageId, PackageName, Resolve, UnresolvedPackageGroup, WorldId};
 
@@ -171,32 +173,6 @@ pub struct LocalResolution {
     pub path: PathBuf,
 }
 
-pub struct LocalDependencies {
-    pub packages: HashMap<PackageRef, LocalResolution>,
-    pub graph: DiGraph<PackageRef, DependencyOf>,
-}
-
-impl LocalDependencies {
-    pub fn sort(&self) -> Result<Vec<PackageRef>> {
-        // sort our packages topologically
-        let acyclic_graph = Acyclic::try_from(self.graph.clone()).map_err(|e| {
-            anyhow::anyhow!(
-                "detected cyclical dependencies with package: {}",
-                self.graph[e.node_id()]
-            )
-        })?;
-
-        Ok(acyclic_graph
-            .nodes_iter()
-            .map(|id| self.graph[id].clone())
-            .collect())
-    }
-
-    pub fn has_no_dependencies(&self) -> bool {
-        self.graph.raw_edges().is_empty()
-    }
-}
-
 /// Represents a resolution of a dependency.
 #[derive(Debug, Clone)]
 #[allow(clippy::large_enum_variant)]
@@ -872,11 +848,11 @@ fn visit<'a>(
 /// State for tracking dependencies during upload.
 pub struct PublishPlan {
     /// Graph of publishable packages where the edges are `(dependency -DependencyOf->) dependent)`
-    dependents: DependencyGraph<PackageRef>,
+    dependents: DependencyGraph<PackageSpec>,
     /// Mapping [`PackageRef`]s to the respective index inside the dependency graph.
     // TODO look at using cargo's `InternedString` type for `PackageRef`:
     // https://docs.rs/cargo/latest/cargo/util/interning/struct.InternedString.html
-    indices: HashMap<PackageRef, NodeIndex>,
+    indices: HashMap<PackageRef, (NodeIndex, PathBuf)>,
 }
 
 impl PublishPlan {
@@ -894,8 +870,32 @@ impl PublishPlan {
         })
     }
 
-    pub fn iter<'a>(&'a self) -> impl Iterator<Item = &'a PackageRef> + 'a {
-        self.indices.iter().map(|(pkg, _)| pkg)
+    pub fn iter<'a>(&'a self) -> impl Iterator<Item = &'a PackageSpec> + 'a {
+        self.dependents.nodes_iter().map(|id| &self.dependents[id])
+    }
+
+    pub fn iter_edges<'a>(&'a self) -> impl Iterator<Item = &'a PackageSpec> + 'a {
+        use petgraph::visit::IntoNeighborsDirected;
+        self.dependents.nodes_iter().map(|id| {
+            let dep = &self.dependents[id];
+            tracing::warn!("ITER {dep}");
+            let mut neighbors = self
+                .dependents
+                .neighbors_directed(id, Direction::Outgoing)
+                .peekable();
+            if neighbors.peek().is_none() {
+                tracing::debug!("{dep} has no dependents");
+            }
+
+            if tracing::enabled!(Level::DEBUG) {
+                while let Some(id) = neighbors.next() {
+                    let pkg = &self.dependents[id];
+                    tracing::debug!("{dep} -(DependencyOF)-> {pkg}");
+                }
+            }
+
+            dep
+        })
     }
 
     pub fn is_empty(&self) -> bool {
@@ -909,30 +909,35 @@ impl PublishPlan {
     /// Returns the set of packages that are ready for publishing (i.e. have no outstanding dependencies).
     ///
     /// These will not be returned in future calls.
-    pub fn take_ready(&mut self) -> BTreeSet<PackageRef> {
+    pub fn take_ready(&mut self) -> BTreeSet<PackageSpec> {
         self.dependents
             .nodes_iter()
             // there are no dependents on `self.dendents[id]`
             .filter(|id| self.dependents.neighbors(*id).count() == 0)
             .map(|id| {
                 let pkg = &self.dependents[id];
-                self.indices.remove(&pkg);
+                self.indices.remove(&pkg.package);
                 pkg.clone()
             })
             .collect()
     }
 
+    ///
+    pub fn get_path(&self, pkg: &PackageRef) -> Option<&Path> {
+        self.indices.get(pkg).map(|(_, p)| p.as_ref())
+    }
+
     /// Packages confirmed to be available in the registry, potentially allowing additional
     /// packages to be "ready".
     pub fn mark_confirmed(&mut self, published: impl IntoIterator<Item = PackageRef>) {
         for pkg in published {
-            let id = self
+            let (id, _) = self
                 .indices
                 .remove(&pkg)
-                .expect("PackageRef has no associated index");
+                .expect("PackageSpec has no associated index");
             self.dependents
                 .remove_node(id)
-                .expect("index has no associated PackageRef");
+                .expect("index has no associated PackageSpec");
         }
     }
 }
@@ -942,7 +947,7 @@ impl PublishPlan {
 /// e.g. "foo:a@0.1.0, bar:b@0.2.0, and baz:c@0.3.0".
 ///
 /// Note: the final separator (e.g. "and" in the previous example) can be chosen.
-pub fn package_list(pkgs: impl IntoIterator<Item = PackageRef>, final_sep: &str) -> String {
+pub fn package_list(pkgs: impl IntoIterator<Item = PackageSpec>, final_sep: &str) -> String {
     let mut names: Vec<_> = pkgs.into_iter().map(|pkg| pkg.to_string()).collect();
     names.sort();
 

crates/wasm-pkg-core/src/wit.rs

@@ -2,22 +2,19 @@
 
 use std::{
     collections::{HashMap, HashSet},
-    path::Path,
+    path::{Path, PathBuf},
     str::FromStr,
 };
 
 use anyhow::{Context, Result};
-use petgraph::{data::Build, graph::NodeIndex};
+use petgraph::{data::Build, graph::NodeIndex, Direction};
 use semver::{Version, VersionReq};
 use wasm_metadata::{AddMetadata, AddMetadataField};
 use wasm_pkg_client::{
     caching::{CachingClient, FileCache},
     PackageRef,
 };
-use wasm_pkg_common::{
-    package::PackageSpec,
-    registry::{DependencyGraph, DependencyOf},
-};
+use wasm_pkg_common::{package::PackageSpec, registry::DependencyGraph};
 use wit_component::WitPrinter;
 use wit_parser::{PackageId, PackageName, Resolve};
 
@@ -182,25 +179,45 @@ pub fn get_packages(
 
 pub fn get_local_dependencies(
     paths: &[impl AsRef<Path>],
-) -> Result<(DependencyGraph<PackageRef>, HashMap<PackageRef, NodeIndex>)> {
+) -> Result<(
+    DependencyGraph<PackageSpec>,
+    HashMap<PackageRef, (NodeIndex, PathBuf)>,
+)> {
     let pkg_trees = paths
         .into_iter()
-        .map(|p| get_packages(p))
+        .map(|path| get_packages(path).map(|(pkg, deps)| ((pkg, path), deps)))
         .collect::<Result<Vec<_>, _>>()?;
     let mut graph = DependencyGraph::new();
     let mut indices = HashMap::new();
     // establish all nodes
-    for (spec, _) in &pkg_trees {
-        let id = graph.add_node(spec.package());
-        if indices.insert(spec.package(), id).is_some() {
+    for ((spec, path), _) in &pkg_trees {
+        let id = graph.add_node(spec.clone());
+        if indices
+            .insert(spec.package(), (id, path.as_ref().to_owned()))
+            .is_some()
+        {
             anyhow::bail!("duplicate references to package detected: {spec}");
         }
     }
-    for (spec, deps) in pkg_trees {
+    for ((spec, _), deps) in pkg_trees {
         // TODO handle version matching for dependencies
         for (dep, _version) in deps {
-            if let Some(&dep_id) = indices.get(&dep) {
-                graph.add_edge(dep_id, indices[&spec.package], DependencyOf);
+            if let Some(&(dep, _)) = indices.get(&dep) {
+                let (pkg, _) = indices[&spec.package];
+                // // dep -(DependencyOF)-> pkg
+                // graph.try_update_edge(dep, pkg, Direction::Outgoing);
+                // // pkg -(DependsOn)-> dep
+                graph
+                    .try_update_edge(pkg, dep, Direction::Outgoing)
+                    .map_err(|e| match e {
+                        petgraph::acyclic::AcyclicEdgeError::Cycle(cycle) => {
+                            anyhow::anyhow!("cyclical depndency detected")
+                                .context(format!("package {}", graph[cycle.node_id()]))
+                                .context(format!("package {}", graph[pkg]))
+                        }
+                        petgraph::acyclic::AcyclicEdgeError::SelfLoop => todo!(),
+                        petgraph::acyclic::AcyclicEdgeError::InvalidEdge => todo!(),
+                    })?;
             }
         }
     }