Report all target validation fails not just the first

Signed-off-by: itowlson <[email protected]>

Author: itowlson

crates/build/src/deployment.rs

@@ -2,7 +2,6 @@
 
 //! A library for building Spin components.
 
-mod deployment;
 mod manifest;
 
 use anyhow::{anyhow, bail, Context, Result};
@@ -17,7 +16,11 @@ use subprocess::{Exec, Redirection};
 use crate::manifest::component_build_configs;
 
 /// If present, run the build command of each component.
-pub async fn build(manifest_file: &Path, component_ids: &[String], skip_target_checks: bool) -> Result<()> {
+pub async fn build(
+    manifest_file: &Path,
+    component_ids: &[String],
+    skip_target_checks: bool,
+) -> Result<()> {
     let (components, deployment_targets, manifest) = component_build_configs(manifest_file)
         .await
         .with_context(|| {
@@ -37,16 +40,23 @@ pub async fn build(manifest_file: &Path, component_ids: &[String], skip_target_c
     build_result?;
 
     if let Ok(manifest) = &manifest {
-        let should_check_targets = !skip_target_checks && !deployment_targets.is_empty();
-        if should_check_targets {
+        if !skip_target_checks {
             let resolution_context =
                 spin_environments::ResolutionContext::new(manifest_file.parent().unwrap()).await?;
-            spin_environments::validate_application_against_environment_ids(
-                deployment_targets.iter(),
+            let errors = spin_environments::validate_application_against_environment_ids(
+                &deployment_targets,
                 manifest,
                 &resolution_context,
             )
             .await?;
+
+            for error in &errors {
+                terminal::error!("{error}");
+            }
+
+            if !errors.is_empty() {
+                anyhow::bail!("All components built successfully, but one or more was incompatible with one or more of the deployment targets.");
+            }
         }
     }
 

crates/build/src/lib.rs

@@ -4,7 +4,8 @@ use std::{collections::BTreeMap, path::Path};
 
 use spin_manifest::{schema::v2, ManifestVersion};
 
-use crate::deployment::DeploymentTargets;
+pub type DeploymentTarget = String;
+pub type DeploymentTargets = Vec<DeploymentTarget>;
 
 /// Returns a map of component IDs to [`v2::ComponentBuildConfig`]s for the
 /// given (v1 or v2) manifest path. If the manifest cannot be loaded, the
@@ -49,13 +50,7 @@ fn build_configs_from_manifest(
 fn deployment_targets_from_manifest(
     manifest: &spin_manifest::schema::v2::AppManifest,
 ) -> DeploymentTargets {
-    let target_environments = manifest.application.targets.clone();
-    // let components = manifest
-    //     .components
-    //     .iter()
-    //     .map(|(id, c)| (id.to_string(), c.source.clone()))
-    //     .collect();
-    DeploymentTargets::new(target_environments)
+    manifest.application.targets.clone()
 }
 
 async fn fallback_load_build_configs(
@@ -83,13 +78,6 @@ async fn fallback_load_build_configs(
 async fn fallback_load_deployment_targets(
     manifest_file: impl AsRef<Path>,
 ) -> Result<DeploymentTargets> {
-    // fn try_parse_component_source(c: (&String, &toml::Value)) -> Option<(String, spin_manifest::schema::v2::ComponentSource)> {
-    //     let (id, ctab) = c;
-    //     let cs = ctab.as_table()
-    //         .and_then(|c| c.get("source"))
-    //         .and_then(|cs| spin_manifest::schema::v2::ComponentSource::deserialize(cs.clone()).ok());
-    //     cs.map(|cs| (id.to_string(), cs))
-    // }
     let manifest_text = tokio::fs::read_to_string(manifest_file).await?;
     Ok(match ManifestVersion::detect(&manifest_text)? {
         ManifestVersion::V1 => Default::default(),
@@ -106,12 +94,7 @@ async fn fallback_load_deployment_targets(
                 .filter_map(|t| t.as_str())
                 .map(|s| s.to_owned())
                 .collect();
-            // let components = table
-            //     .get("component")
-            //     .and_then(|cs| cs.as_table())
-            //     .map(|table| table.iter().filter_map(try_parse_component_source).collect())
-            //     .unwrap_or_default();
-            DeploymentTargets::new(target_environments)
+            target_environments
         }
     })
 }

crates/build/src/manifest.rs

@@ -1,8 +1,10 @@
 use anyhow::Context;
 
-pub async fn load_environment(env_id: &str) -> anyhow::Result<TargetEnvironment> {
+pub async fn load_environment(env_id: impl AsRef<str>) -> anyhow::Result<TargetEnvironment> {
     use futures_util::TryStreamExt;
 
+    let env_id = env_id.as_ref();
+
     let (pkg_name, pkg_ver) = env_id.split_once('@').with_context(|| format!("Failed to parse target environment {env_id} as package reference - is the target correct?"))?;
 
     // TODO: this requires wkg configuration which shouldn't be on users:

crates/environments/src/environment_definition.rs

@@ -8,19 +8,23 @@ pub use loader::ResolutionContext;
 use loader::{load_and_resolve_all, ComponentToValidate};
 
 pub async fn validate_application_against_environment_ids(
-    env_ids: impl Iterator<Item = &str>,
+    env_ids: &[impl AsRef<str>],
     app: &spin_manifest::schema::v2::AppManifest,
     resolution_context: &ResolutionContext,
-) -> anyhow::Result<()> {
-    let envs = join_all_result(env_ids.map(load_environment)).await?;
+) -> anyhow::Result<Vec<anyhow::Error>> {
+    if env_ids.is_empty() {
+        return Ok(Default::default());
+    }
+
+    let envs = join_all_result(env_ids.iter().map(load_environment)).await?;
     validate_application_against_environments(&envs, app, resolution_context).await
 }
 
 async fn validate_application_against_environments(
     envs: &[TargetEnvironment],
     app: &spin_manifest::schema::v2::AppManifest,
     resolution_context: &ResolutionContext,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<Vec<anyhow::Error>> {
     use futures::FutureExt;
 
     for trigger_type in app.triggers.keys() {
@@ -40,31 +44,45 @@ async fn validate_application_against_environments(
         .await
         .context("Failed to prepare components for target environment checking")?;
 
+    let mut errs = vec![];
+
     for (trigger_type, component) in components_by_trigger_type {
         for component in &component {
-            validate_component_against_environments(envs, &trigger_type, component).await?;
+            errs.extend(
+                validate_component_against_environments(envs, &trigger_type, component).await?,
+            );
         }
     }
 
-    Ok(())
+    Ok(errs)
 }
 
 async fn validate_component_against_environments(
     envs: &[TargetEnvironment],
     trigger_type: &TriggerType,
     component: &ComponentToValidate<'_>,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<Vec<anyhow::Error>> {
+    let mut errs = vec![];
+
     for env in envs {
         let worlds = env.worlds(trigger_type);
-        validate_wasm_against_any_world(env, &worlds, component).await?;
+        if let Some(e) = validate_wasm_against_any_world(env, &worlds, component)
+            .await
+            .err()
+        {
+            errs.push(e);
+        }
     }
 
-    tracing::info!(
-        "Validated component {} {} against all target worlds",
-        component.id(),
-        component.source_description()
-    );
-    Ok(())
+    if errs.is_empty() {
+        tracing::info!(
+            "Validated component {} {} against all target worlds",
+            component.id(),
+            component.source_description()
+        );
+    }
+
+    Ok(errs)
 }
 
 async fn validate_wasm_against_any_world(

crates/environments/src/lib.rs

@@ -32,7 +32,11 @@ pub struct BuildCommand {
     /// By default, if the application manifest specifies one or more deployment targets, Spin
     /// checks that all components are compatible with those deployment targets. Specify
     /// this option to bypass those target checks.
-    #[clap(long = "skip-target-checks", alias = "skip-target-check", takes_value = false)]
+    #[clap(
+        long = "skip-target-checks",
+        alias = "skip-target-check",
+        takes_value = false
+    )]
     skip_target_checks: bool,
 
     /// Run the application after building.