Address Manifest V2 review feedback

Signed-off-by: Lann Martin <[email protected]>

Author: lann

crates/loader/src/http.rs

@@ -4,7 +4,7 @@ use anyhow::{ensure, Context, Result};
 use sha2::Digest;
 use tokio::io::AsyncWriteExt;
 
-/// Download content from `url` while will be verified to match `digest` and
+/// Downloads content from `url` which will be verified to match `digest` and
 /// then moved to `dest`.
 pub async fn verified_download(url: &str, digest: &str, dest: &Path) -> Result<()> {
     tracing::debug!("Downloading content from {url:?}");

crates/loader/src/local.rs

@@ -124,7 +124,7 @@ impl LocalLoader {
         let source = self
             .load_component_source(component.source.clone())
             .await
-            .with_context(|| format!("Failed to load Wasm source {:?}", component.source))?;
+            .with_context(|| format!("Failed to load Wasm source {}", component.source))?;
 
         let env = component.environment.into_iter().collect();
 

crates/loader/tests/ui/invalid-digest.err

@@ -2,5 +2,5 @@ Failed to load Spin app from "<test-dir>/invalid-digest.toml"
 
 Caused by:
     0: Failed to load component `web`
-    1: Failed to load Wasm source Remote { url: "https://example.com/wasm.wasm.wasm", digest: "not-a-digest" }
+    1: Failed to load Wasm source "https://example.com/wasm.wasm.wasm" with digest "not-a-digest"
     2: invalid `digest` "not-a-digest"; must start with 'sha256:'

crates/manifest/src/schema/common.rs

@@ -1,3 +1,5 @@
+use std::fmt::Display;
+
 use serde::{Deserialize, Serialize};
 
 /// Variable definition
@@ -30,7 +32,16 @@ pub enum ComponentSource {
     },
 }
 
-/// Component source
+impl Display for ComponentSource {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ComponentSource::Local(path) => write!(f, "{path:?}"),
+            ComponentSource::Remote { url, digest } => write!(f, "{url:?} with digest {digest:?}"),
+        }
+    }
+}
+
+/// WASI files mount
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(deny_unknown_fields, untagged)]
 pub enum WasiFilesMount {

crates/oci/src/client.rs

@@ -195,6 +195,7 @@ impl Client {
         for entry in WalkDir::new(source) {
             let entry = entry?;
             if entry.file_type().is_file() {
+                // Can unwrap because we got to 'entry' from walking 'source'
                 let rel_path = entry.path().strip_prefix(source).unwrap();
                 tracing::trace!("Adding asset {rel_path:?} to component files list");
                 // Add content/path to the locked component files list
@@ -234,6 +235,7 @@ impl Client {
         for entry in WalkDir::new(source) {
             let entry = entry?;
             if entry.file_type().is_file() {
+                // Can unwrap because we got to 'entry' from walking 'source'
                 let rel_path = entry.path().strip_prefix(source).unwrap();
                 tracing::trace!("Adding new layer for asset {rel_path:?}");
                 // Construct and push layer, adding its digest to the locked component files Vec
@@ -640,7 +642,7 @@ mod test {
             let metadata = Default::default();
             let variables = Default::default();
             let locked = LockedApp {
-                spin_lock_version: spin_app::locked::FixedVersion,
+                spin_lock_version: Default::default(),
                 components,
                 triggers,
                 metadata,

crates/outbound-http/src/allowed_http_hosts.rs

@@ -76,9 +76,6 @@ impl AllowedHttpHost {
 
 // Parses a list of allowed HTTP hosts
 pub fn parse_allowed_http_hosts(list: &[impl AsRef<str>]) -> Result<AllowedHttpHosts> {
-    if list.is_empty() {
-        return Ok(AllowedHttpHosts::AllowSpecific(vec![]));
-    }
     if list.iter().any(|domain| domain.as_ref() == ALLOW_ALL_HOSTS) {
         Ok(AllowedHttpHosts::AllowAll)
     } else {

crates/ui-testing/src/lib.rs

@@ -40,7 +40,7 @@ impl UiTestsRunner {
         let tests = std::mem::take(&mut self.tests);
         let conclusion = libtest_mimic::run(&args, tests);
         if conclusion.has_failed() {
-            eprintln!("Snapshot files can be automatically updated by re-running with BLESS=1");
+            eprintln!("Snapshot files can be automatically updated by re-running with BLESS=1\n");
         }
         conclusion.exit_if_failed();
         Ok(())

examples/spin-timer/Cargo.lock

@@ -211,11 +211,10 @@ impl FromStr for ParameterValue {
 /// This function reads a file and parses it as TOML, then
 /// returns the resulting hashmap of key-value pairs.
 async fn values_from_file(path: impl AsRef<Path>) -> Result<HashMap<String, String>> {
-    // Get the absolute path of the file we're reading.
     let path = path.as_ref();
 
     // Open the file.
-    let text = tokio::fs::read_to_string(&path)
+    let text = tokio::fs::read_to_string(path)
         .await
         .with_context(|| format!("Failed to read text from values file {}", path.display()))?;
 

src/commands/new.rs

@@ -467,7 +467,7 @@ impl Display for AppSource {
             AppSource::None => write!(f, "<no source>"),
             AppSource::File(path) => write!(f, "local app {path:?}"),
             AppSource::OciRegistry(reference) => write!(f, "remote app {reference:?}"),
-            AppSource::Unresolvable(s) => write!(f, "unknown app source {s:?}"),
+            AppSource::Unresolvable(s) => write!(f, "unknown app source: {s:?}"),
         }
     }
 }