Merge pull request #179 from mkatychev/feat/display-dependency-impl

feat(core,logging): attach location/dependency where `wasm_pkg_core::wit::resolve_dependencies` failed

Author: lann

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

@@ -20,6 +20,12 @@ pub struct FileCache {
     root: PathBuf,
 }
 
+impl std::fmt::Display for FileCache {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.root.display())
+    }
+}
+
 impl FileCache {
     /// Creates a new file cache that stores data in the given directory.
     pub async fn new(root: impl AsRef<Path>) -> anyhow::Result<Self> {

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

@@ -225,7 +225,7 @@ impl Client {
                     self.config
                         .namespace_registry(package.namespace())
                         .and_then(|meta| {
-                            // If the overriden registry matches the registry we are trying to resolve, we
+                            // If the overridden registry matches the registry we are trying to resolve, we
                             // should use the metadata, otherwise we'll need to fetch the metadata from the
                             // registry
                             match (meta, is_override) {

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

@@ -42,6 +42,16 @@ pub enum RegistryMapping {
     Custom(CustomConfig),
 }
 
+impl RegistryMapping {
+    /// returns the inner [`Registry`] object for both mapping variants
+    fn registry(&self) -> &Registry {
+        match self {
+            RegistryMapping::Registry(reg) => reg,
+            RegistryMapping::Custom(custom) => &custom.registry,
+        }
+    }
+}
+
 /// Custom registry configuration
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct CustomConfig {
@@ -181,27 +191,21 @@ impl Config {
     /// - The default registry
     /// - Hard-coded fallbacks for certain well-known namespaces
     pub fn resolve_registry(&self, package: &PackageRef) -> Option<&Registry> {
-        if let Some(RegistryMapping::Registry(reg)) = self.package_registry_overrides.get(package) {
-            Some(reg)
-        } else if let Some(RegistryMapping::Custom(custom)) =
-            self.package_registry_overrides.get(package)
+        let namespace = package.namespace();
+        // look in `self.package_registry_overrides `
+        // then in `self.namespace_registries`
+        if let Some(reg) = self
+            .package_registry_overrides
+            .get(package)
+            .or_else(|| self.namespace_registries.get(namespace))
+            .map(|pkg| pkg.registry())
         {
-            Some(&custom.registry)
-        } else if let Some(RegistryMapping::Registry(reg)) =
-            self.namespace_registries.get(package.namespace())
-        {
-            Some(reg)
-        } else if let Some(RegistryMapping::Custom(custom)) =
-            self.namespace_registries.get(package.namespace())
-        {
-            Some(&custom.registry)
+            return Some(reg);
         } else if let Some(reg) = self.default_registry.as_ref() {
-            Some(reg)
-        } else if let Some(reg) = self.fallback_namespace_registries.get(package.namespace()) {
-            Some(reg)
-        } else {
-            None
+            return Some(reg);
         }
+
+        self.fallback_namespace_registries.get(namespace)
     }
 
     /// Returns the default registry.

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

@@ -364,7 +364,7 @@ impl AsRef<File> for Locker {
     }
 }
 
-// NOTE(thomastaylor312): These lock file primitives from here on down are mostly copyed wholesale
+// NOTE(thomastaylor312): These lock file primitives from here on down are mostly copied wholesale
 // from the lock file implementation of cargo-component with some minor modifications to make them
 // work with tokio
 

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

@@ -38,6 +38,28 @@ pub enum Dependency {
     Local(PathBuf),
 }
 
+impl std::fmt::Display for Dependency {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Dependency::Package(RegistryPackage {
+                name,
+                version,
+                registry,
+            }) => {
+                let registry = registry.as_deref().unwrap_or("_");
+                let name = name.as_ref().map(|n| n.to_string());
+
+                write!(
+                    f,
+                    "{{registry=\"{registry}\" package=\"{}@{version}\"}}",
+                    name.as_deref().unwrap_or("_:_"),
+                )
+            }
+            Dependency::Local(path_buf) => write!(f, "{}", path_buf.display()),
+        }
+    }
+}
+
 impl FromStr for Dependency {
     type Err = anyhow::Error;
 
@@ -413,7 +435,7 @@ impl<'a> DependencyResolver<'a> {
                 if !force_override
                     && (self.resolutions.contains_key(name) || self.dependencies.contains_key(name))
                 {
-                    tracing::debug!(%name, "dependency already exists and override is not set, ignoring");
+                    tracing::debug!(%name, %dependency, "dependency already exists and override is not set, ignoring");
                     return Ok(());
                 }
                 self.dependencies.insert(
@@ -432,7 +454,7 @@ impl<'a> DependencyResolver<'a> {
                 });
 
                 // This is a bit of a hack, but if there are multiple local dependencies that are
-                // nested and overriden, getting the packages from the local package treats _all_
+                // nested and overridden, getting the packages from the local package treats _all_
                 // deps as registry deps. So if we're handling a local path and the dependencies
                 // have a registry package already, override it. Otherwise follow normal overrides.
                 // We should definitely fix this and change where we resolve these things

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

@@ -179,16 +179,16 @@ pub async fn resolve_dependencies(
     let mut resolver = DependencyResolver::new_with_client(client, lock_file)?;
     // add deps from config first in case they're local deps and then add deps from the directory
     if let Some(overrides) = config.overrides.as_ref() {
-        for (pkg, ovride) in overrides.iter() {
+        for (pkg, ovr) in overrides.iter() {
             let pkg: PackageRef = pkg.parse().context("Unable to parse as a package ref")?;
-            let dep = match (ovride.path.as_ref(), ovride.version.as_ref()) {
-                (Some(path), None) => {
-                    let path = tokio::fs::canonicalize(path).await?;
-                    Dependency::Local(path)
-                }
-                (Some(path), Some(_)) => {
-                    tracing::warn!("Ignoring version override for local package");
-                    let path = tokio::fs::canonicalize(path).await?;
+            let dep = match (ovr.path.as_ref(), ovr.version.as_ref()) {
+                (Some(path), v) => {
+                    if v.is_some() {
+                        tracing::warn!("Ignoring version override for local package");
+                    }
+                    let path = tokio::fs::canonicalize(path)
+                        .await
+                        .with_context(|| format!("resolving local dependency {}", path.display()))?;
                     Dependency::Local(path)
                 }
                 (None, Some(version)) => Dependency::Package(RegistryPackage {
@@ -201,10 +201,12 @@ pub async fn resolve_dependencies(
                     continue;
                 }
             };
+
+            tracing::debug!(dependency = %dep);
             resolver
                 .add_dependency(&pkg, &dep)
                 .await
-                .context("Unable to add dependency")?;
+                .with_context(|| format!("unable to add dependency {dep}"))?;
         }
     }
     let (_name, packages) = get_packages(path)?;

crates/wkg/src/main.rs

@@ -66,6 +66,7 @@ impl Common {
         } else {
             FileCache::global_cache_path().context("unable to find cache directory")?
         };
+
         FileCache::new(dir).await
     }
 
@@ -76,6 +77,8 @@ impl Common {
         let config = self.load_config().await?;
         let cache = self.load_cache().await?;
         let client = Client::new(config);
+
+        tracing::debug!(filecache_dir = %cache);
         Ok(CachingClient::new(Some(client), cache))
     }
 }

crates/wkg/tests/fixtures/wasi-http/wit/types.wit

@@ -205,7 +205,7 @@ interface types {
     /// list with the same key.
     entries: func() -> list<tuple<field-key,field-value>>;
 
-    /// Make a deep copy of the Fields. Equivelant in behavior to calling the
+    /// Make a deep copy of the Fields. Equivalent in behavior to calling the
     /// `fields` constructor on the return value of `entries`. The resulting
     /// `fields` is mutable.
     clone: func() -> fields;
@@ -310,7 +310,7 @@ interface types {
     /// `delete` operations will fail with `header-error.immutable`.
     ///
     /// This headers resource is a child: it must be dropped before the parent
-    /// `outgoing-request` is dropped, or its ownership is transfered to
+    /// `outgoing-request` is dropped, or its ownership is transferred to
     /// another component by e.g. `outgoing-handler.handle`.
     headers: func() -> headers;
   }
@@ -426,19 +426,19 @@ interface types {
     finish: static func(this: incoming-body) -> future-trailers;
   }
 
-  /// Represents a future which may eventaully return trailers, or an error.
+  /// Represents a future which may eventually return trailers, or an error.
   ///
   /// In the case that the incoming HTTP Request or Response did not have any
   /// trailers, this future will resolve to the empty set of trailers once the
   /// complete Request or Response body has been received.
   resource future-trailers {
 
     /// Returns a pollable which becomes ready when either the trailers have
-    /// been received, or an error has occured. When this pollable is ready,
+    /// been received, or an error has occurred. When this pollable is ready,
     /// the `get` method will return `some`.
     subscribe: func() -> pollable;
 
-    /// Returns the contents of the trailers, or an error which occured,
+    /// Returns the contents of the trailers, or an error which occurred,
     /// once the future is ready.
     ///
     /// The outer `option` represents future readiness. Users can wait on this
@@ -450,7 +450,7 @@ interface types {
     ///
     /// The inner `result` represents that either the HTTP Request or Response
     /// body, as well as any trailers, were received successfully, or that an
-    /// error occured receiving them. The optional `trailers` indicates whether
+    /// error occurred receiving them. The optional `trailers` indicates whether
     /// or not trailers were present in the body.
     ///
     /// When some `trailers` are returned by this method, the `trailers`
@@ -483,7 +483,7 @@ interface types {
     /// `delete` operations will fail with `header-error.immutable`.
     ///
     /// This headers resource is a child: it must be dropped before the parent
-    /// `outgoing-request` is dropped, or its ownership is transfered to
+    /// `outgoing-request` is dropped, or its ownership is transferred to
     /// another component by e.g. `outgoing-handler.handle`.
     headers: func() -> headers;
 
@@ -507,7 +507,7 @@ interface types {
   ///
   /// If the user code drops this resource, as opposed to calling the static
   /// method `finish`, the implementation should treat the body as incomplete,
-  /// and that an error has occured. The implementation should propogate this
+  /// and that an error has occurred. The implementation should propagate this
   /// error to the HTTP protocol by whatever means it has available,
   /// including: corrupting the body on the wire, aborting the associated
   /// Request, or sending a late status code for the Response.
@@ -539,14 +539,14 @@ interface types {
     ) -> result<_, error-code>;
   }
 
-  /// Represents a future which may eventaully return an incoming HTTP
+  /// Represents a future which may eventually return an incoming HTTP
   /// Response, or an error.
   ///
   /// This resource is returned by the `wasi:http/outgoing-handler` interface to
   /// provide the HTTP Response corresponding to the sent Request.
   resource future-incoming-response {
     /// Returns a pollable which becomes ready when either the Response has
-    /// been received, or an error has occured. When this pollable is ready,
+    /// been received, or an error has occurred. When this pollable is ready,
     /// the `get` method will return `some`.
     subscribe: func() -> pollable;
 
@@ -560,8 +560,8 @@ interface types {
     /// is `some`, and error on subsequent calls.
     ///
     /// The inner `result` represents that either the incoming HTTP Response
-    /// status and headers have recieved successfully, or that an error
-    /// occured. Errors may also occur while consuming the response body,
+    /// status and headers have received successfully, or that an error
+    /// occurred. Errors may also occur while consuming the response body,
     /// but those will be reported by the `incoming-body` and its
     /// `output-stream` child.
     get: func() -> option<result<result<incoming-response, error-code>>>;