Skip to content

Commit 7c08bd3

Browse files
committed
Check names in ReqList::contains_request
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
1 parent 8aaeee2 commit 7c08bd3

File tree

2 files changed

+60
-7
lines changed

2 files changed

+60
-7
lines changed

crates/spk-schema/src/requirements_list.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// https://github.com/imageworks/spk
44

55
use std::collections::HashSet;
6+
use std::fmt::Write;
67

78
use serde::{Deserialize, Serialize};
89
use spk_schema_foundation::version::Compatibility;
@@ -81,16 +82,26 @@ impl RequirementsList {
8182
pub fn contains_request(&self, theirs: &Request) -> Compatibility {
8283
for ours in self.iter() {
8384
match (ours, theirs) {
84-
(Request::Pkg(ours), Request::Pkg(theirs)) => {
85+
(Request::Pkg(ours), Request::Pkg(theirs)) if ours.pkg.name == theirs.pkg.name => {
8586
return ours.contains(theirs);
8687
}
87-
(Request::Var(ours), Request::Var(theirs)) => {
88+
(Request::Var(ours), Request::Var(theirs))
89+
// a var request satisfy another if they have the same opt name or
90+
// if our request is package-less and has the same base name, eg:
91+
// name/value [contains] name/value
92+
// pkg.name/value [contains] pkg.name/value
93+
// name/value [contains] pkg.name/value
94+
if ours.var == theirs.var || ours.var.as_str() == theirs.var.base_name() =>
95+
{
8896
return ours.contains(theirs);
8997
}
90-
_ => continue,
98+
_ => {
99+
tracing::trace!("skip {ours}, not {theirs}");
100+
continue;
101+
}
91102
}
92103
}
93-
Compatibility::incompatible("No request exists for this")
104+
Compatibility::incompatible(format!("No request exists for {}", theirs.name()))
94105
}
95106

96107
/// Render all requests with a package pin using the given resolved packages.
@@ -136,7 +147,7 @@ impl RequirementsList {
136147
));
137148
}
138149
Some(opt) => {
139-
let rendered = request.render_pin(opt)?;
150+
let rendered = request.render_pin(opt.as_str())?;
140151
let _ = std::mem::replace(request, rendered);
141152
}
142153
}
@@ -167,6 +178,20 @@ impl RequirementsList {
167178
}
168179
}
169180

181+
impl std::fmt::Display for RequirementsList {
182+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
183+
f.write_char('[')?;
184+
let mut entries = self.0.iter().peekable();
185+
while let Some(i) = entries.next() {
186+
i.fmt(f)?;
187+
if entries.peek().is_some() {
188+
f.write_str(", ")?;
189+
}
190+
}
191+
f.write_char(']')
192+
}
193+
}
194+
170195
impl IntoIterator for RequirementsList {
171196
type Item = Request;
172197
type IntoIter = std::vec::IntoIter<Request>;
@@ -199,12 +224,11 @@ impl<'de> Deserialize<'de> for RequirementsList {
199224
let mut requirement_names = HashSet::with_capacity(size_hint);
200225
while let Some(request) = seq.next_element::<Request>()? {
201226
let name = request.name();
202-
if requirement_names.contains(name) {
227+
if !requirement_names.insert(name.to_owned()) {
203228
return Err(serde::de::Error::custom(format!(
204229
"found multiple install requirements for '{name}'"
205230
)));
206231
}
207-
requirement_names.insert(name.to_owned());
208232
requirements.push(request);
209233
}
210234
Ok(RequirementsList(requirements))

crates/spk-schema/src/requirements_list_test.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
// SPDX-License-Identifier: Apache-2.0
33
// https://github.com/imageworks/spk
44
use rstest::rstest;
5+
use serde_json::json;
6+
use spk_schema_foundation::fixtures::*;
7+
use spk_schema_foundation::version::Compatibility;
8+
use spk_schema_ident::Request;
59

610
use super::RequirementsList;
711

@@ -12,3 +16,28 @@ fn test_deserialize_no_duplicates() {
1216
serde_yaml::from_str::<RequirementsList>("[{pkg: python}, {pkg: python}]")
1317
.expect_err("should fail to deserialize with the same package twice");
1418
}
19+
20+
#[rstest]
21+
#[case(
22+
json!([
23+
{"pkg": "pkg-a"},
24+
{"pkg": "pkg-b"},
25+
]),
26+
json!({"pkg": "pkg-a"})
27+
)]
28+
#[should_panic]
29+
#[case(
30+
json!([
31+
{"pkg": "pkg-a"},
32+
{"pkg": "pkg-b"},
33+
]),
34+
json!({"pkg": "pkg-c"})
35+
)]
36+
fn test_contains_request(#[case] requests: serde_json::Value, #[case] contains: serde_json::Value) {
37+
init_logging();
38+
39+
let reqs: RequirementsList = serde_json::from_value(requests).unwrap();
40+
let contains: Request = serde_json::from_value(contains).unwrap();
41+
tracing::debug!("is {contains} contained within this? {reqs}");
42+
assert_eq!(reqs.contains_request(&contains), Compatibility::Compatible);
43+
}

0 commit comments

Comments
 (0)