From 10b5668807eaa8bcb979cd26b555ea9aec67fc0b Mon Sep 17 00:00:00 2001
From: J Robert Ray <jrray@jrray.org>
Date: Mon, 9 Oct 2023 10:45:57 -0700
Subject: [PATCH] Fix pins in components not being rendered

Packages that try to use `fromBuildEnv` in a component were not getting
the appropriate version pinned at build time, as in:

```yaml
pkg: my-pkg/1.0.0

build:
  options:
    - pkg: test

install:
  components:
    - name: run
      files: ['*']
      requirements:
        - pkg: test
          fromBuildEnv: Binary
```

Signed-off-by: J Robert Ray <jrray@jrray.org>
---
 crates/spk-schema/src/install_spec.rs      | 10 +++-
 crates/spk-schema/src/install_spec_test.rs | 61 ++++++++++++++++++++++
 crates/spk-schema/src/requirements_list.rs | 11 ++--
 3 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/crates/spk-schema/src/install_spec.rs b/crates/spk-schema/src/install_spec.rs
index 35b227a55..516f69b3d 100644
--- a/crates/spk-schema/src/install_spec.rs
+++ b/crates/spk-schema/src/install_spec.rs
@@ -40,7 +40,15 @@ impl InstallSpec {
         options: &OptionMap,
         resolved: impl Iterator<Item = &'a BuildIdent>,
     ) -> Result<()> {
-        self.requirements.render_all_pins(options, resolved)
+        let resolved_by_name = resolved.map(|x| (x.name(), x)).collect();
+        self.requirements
+            .render_all_pins(options, &resolved_by_name)?;
+        for component in self.components.iter_mut() {
+            component
+                .requirements
+                .render_all_pins(options, &resolved_by_name)?;
+        }
+        Ok(())
     }
 }
 
diff --git a/crates/spk-schema/src/install_spec_test.rs b/crates/spk-schema/src/install_spec_test.rs
index 5f19313c9..1f41d227c 100644
--- a/crates/spk-schema/src/install_spec_test.rs
+++ b/crates/spk-schema/src/install_spec_test.rs
@@ -1,3 +1,64 @@
 // Copyright (c) Sony Pictures Imageworks, et al.
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/imageworks/spk
+
+use rstest::rstest;
+use spk_schema_foundation::ident_component::Component;
+use spk_schema_foundation::option_map::OptionMap;
+use spk_schema_foundation::version::BINARY_STR;
+use spk_schema_ident::{parse_ident_range, PkgRequest, Request, RequestedBy};
+
+use crate::{InstallSpec, RequirementsList};
+
+#[rstest]
+fn test_render_all_pins_renders_requirements_in_components() {
+    let mut install_spec = InstallSpec::default();
+    let mut requirements = RequirementsList::default();
+    requirements.insert_or_replace({
+        Request::Pkg(
+            PkgRequest::new(
+                parse_ident_range("test").unwrap(),
+                RequestedBy::SpkInternalTest,
+            )
+            .with_pin(Some(BINARY_STR.to_string())),
+        )
+    });
+    install_spec
+        .components
+        .iter_mut()
+        .find(|c| c.name == Component::Run)
+        .unwrap()
+        .requirements = requirements;
+
+    // Expected value before pinning.
+    let Request::Pkg(req) = &install_spec
+        .components
+        .iter()
+        .find(|c| c.name == Component::Run)
+        .unwrap()
+        .requirements[0]
+    else {
+        panic!("Expected a Pkg request");
+    };
+    assert_eq!(req.to_string(), "test");
+
+    install_spec
+        .render_all_pins(
+            &OptionMap::default(),
+            ["test/1.2.3/GMTG3CXY".parse().unwrap()].iter(),
+        )
+        .unwrap();
+
+    // Now the install requirement inside the run component should be pinned to
+    // version 1.2.3.
+    let Request::Pkg(req) = &install_spec
+        .components
+        .iter()
+        .find(|c| c.name == Component::Run)
+        .unwrap()
+        .requirements[0]
+    else {
+        panic!("Expected a Pkg request");
+    };
+    assert_eq!(req.to_string(), "test/Binary:1.2.3");
+}
diff --git a/crates/spk-schema/src/requirements_list.rs b/crates/spk-schema/src/requirements_list.rs
index 53a78d683..2ebc790e7 100644
--- a/crates/spk-schema/src/requirements_list.rs
+++ b/crates/spk-schema/src/requirements_list.rs
@@ -6,6 +6,7 @@ use std::collections::HashSet;
 use std::fmt::Write;
 
 use serde::{Deserialize, Serialize};
+use spk_schema_foundation::name::PkgName;
 use spk_schema_foundation::version::Compatibility;
 use spk_schema_ident::{BuildIdent, PinPolicy};
 
@@ -116,19 +117,15 @@ impl RequirementsList {
     }
 
     /// Render all requests with a package pin using the given resolved packages.
-    pub fn render_all_pins<'a>(
+    pub fn render_all_pins(
         &mut self,
         options: &OptionMap,
-        resolved: impl Iterator<Item = &'a BuildIdent>,
+        resolved_by_name: &std::collections::HashMap<&PkgName, &BuildIdent>,
     ) -> Result<()> {
-        let mut by_name = std::collections::HashMap::new();
-        for pkg in resolved {
-            by_name.insert(pkg.name(), pkg);
-        }
         self.0 = std::mem::take(&mut self.0).into_iter().filter_map(|request| {
             match &request {
                 Request::Pkg(pkg_request) => {
-                    match by_name.get(pkg_request.pkg.name()) {
+                    match resolved_by_name.get(pkg_request.pkg.name()) {
                         None if pkg_request.pin.is_none() && pkg_request.pin_policy == PinPolicy::IfPresentInBuildEnv => {
                             None
                         }