Skip to content

Commit 2087873

Browse files
committed
Updates error message, help, naming, default, constants, and comments
Signed-off-by: David Gilligan-Cook <[email protected]>
1 parent a83ded8 commit 2087873

File tree

10 files changed

+121
-140
lines changed

10 files changed

+121
-140
lines changed

crates/spk-build/src/build/binary_test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ async fn test_empty_var_option_is_not_a_request() {
5353
r#"{
5454
pkg: mypackage/1.0.0,
5555
build: {
56-
host_compat: any,
56+
host_compat: Any,
5757
options: [
5858
{var: something}
5959
]
@@ -832,7 +832,7 @@ async fn test_default_build_component() {
832832
"pkg": "mypkg/1.0.0",
833833
"sources": [],
834834
"build": {
835-
"host_compat": "any",
835+
"host_compat": "Any",
836836
"options": [{"pkg": "somepkg/1.0.0"}],
837837
"script": "echo building...",
838838
},

crates/spk-cli/cmd-du/src/cmd_du_test.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ async fn test_du_trivially_works() {
5757
let spec = recipe!(
5858
{ "pkg": "my-pkg/1.0.0",
5959
"build": {
60-
"host_compat": "any",
60+
"host_compat": "Any",
6161
"script": "echo Hello World!"
6262
}
6363
}
@@ -77,7 +77,7 @@ async fn test_du_trivially_works() {
7777

7878
let mut expected_output = vec![
7979
"2local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/options.json",
80-
"186local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/spec.yaml",
80+
"179local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/spec.yaml",
8181
"0local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/build.cmpt",
8282
"17local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/build.sh",
8383
"0local/my-pkg/1.0.0/3I42H3S6/:run/spk/pkg/my-pkg/1.0.0/3I42H3S6/options.json",
@@ -211,7 +211,7 @@ async fn test_du_is_not_counting_links() {
211211
let spec = recipe!(
212212
{ "pkg": "my-pkg/1.0.0",
213213
"build": {
214-
"host_compat": "any",
214+
"host_compat": "Any",
215215
"script": "echo Hello World!"
216216
}
217217
}
@@ -254,7 +254,7 @@ async fn test_du_is_counting_links() {
254254
let spec = recipe!(
255255
{ "pkg": "my-pkg/1.0.0",
256256
"build": {
257-
"host_compat": "any",
257+
"host_compat": "Any",
258258
"script": "echo Hello World!"
259259
}
260260
}
@@ -345,7 +345,7 @@ async fn test_du_summarize_output_enabled() {
345345
let spec = recipe!(
346346
{ "pkg": "my-pkg/1.0.0",
347347
"build": {
348-
"host_compat": "any",
348+
"host_compat": "Any",
349349
"script": "echo Hello World!"
350350
}
351351
}
@@ -363,7 +363,7 @@ async fn test_du_summarize_output_enabled() {
363363
let mut opt = Opt::try_parse_from(["du", "local/my-pkg", "-s"]).unwrap();
364364
opt.du.run().await.unwrap();
365365

366-
let expected_output = format!("205local/my-pkg/{}", "".red());
366+
let expected_output = format!("198local/my-pkg/{}", "".red());
367367
let mut generated_output = opt.du.output.vec.lock().unwrap()[0].clone();
368368
generated_output.retain(|c| !c.is_whitespace());
369369

@@ -385,7 +385,7 @@ async fn test_du_summarize_output_is_not_enabled() {
385385
let spec = recipe!(
386386
{ "pkg": "my-pkg/1.0.0",
387387
"build": {
388-
"host_compat": "any",
388+
"host_compat": "Any",
389389
"script": "echo Hello World!"
390390
}
391391
}
@@ -407,7 +407,7 @@ async fn test_du_summarize_output_is_not_enabled() {
407407
"2local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/options.json",
408408
"0local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/build.cmpt",
409409
"17local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/build.sh",
410-
"186local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/spec.yaml",
410+
"179local/my-pkg/1.0.0/3I42H3S6/:build/spk/pkg/my-pkg/1.0.0/3I42H3S6/spec.yaml",
411411
"0local/my-pkg/1.0.0/3I42H3S6/:run/spk/pkg/my-pkg/1.0.0/3I42H3S6/spec.yaml",
412412
"0local/my-pkg/1.0.0/3I42H3S6/:run/spk/pkg/my-pkg/1.0.0/3I42H3S6/options.json",
413413
"0local/my-pkg/1.0.0/3I42H3S6/:run/spk/pkg/my-pkg/1.0.0/3I42H3S6/run.cmpt",
@@ -440,7 +440,7 @@ async fn test_deprecate_flag() {
440440
let spec = recipe!(
441441
{"pkg": "my-pkg/1.0.0",
442442
"build": {
443-
"host_compat": "any",
443+
"host_compat": "Any",
444444
"script": "echo Hello World!"
445445
},
446446
"deprecated": true}
@@ -470,7 +470,7 @@ async fn test_deprecate_flag() {
470470

471471
let mut opt_with_deprecate_flag = Opt::try_parse_from(["du", "local/my-pkg", "-ds"]).unwrap();
472472
opt_with_deprecate_flag.du.run().await.unwrap();
473-
let expected_output = format!("222local/my-pkg/{}", "DEPRECATED".red());
473+
let expected_output = format!("215local/my-pkg/{}", "DEPRECATED".red());
474474
let mut generated_output = opt_with_deprecate_flag.du.output.vec.lock().unwrap()[0].clone();
475475
generated_output.retain(|c| !c.is_whitespace());
476476
assert_eq!(expected_output, generated_output);
@@ -491,7 +491,7 @@ async fn test_human_readable_flag() {
491491
let spec = recipe!(
492492
{ "pkg": "my-pkg/1.0.0",
493493
"build": {
494-
"host_compat": "any",
494+
"host_compat": "Any",
495495
"script": "echo Hello World!"
496496
}
497497
}

crates/spk-cli/group2/src/cmd_new.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,12 @@ fn get_stub(name: &PkgNameBuf) -> String {
4444
4545
build:
4646
47-
build:
4847
# Set what level of host os compatibility the built package should have:
4948
# - full: package can be used on any OS
5049
# - os: package can be used anywhere that has the same OS type
5150
# - arch: package can be used anywhere that has the same OS and CPU type
5251
# - distro: package can only be used on the same OS, CPU, and OS distribution version
53-
host_compatibility: distro
52+
host_compat: distro
5453
5554
# options are all the inputs to the package build process, including
5655
# build-time dependencies

crates/spk-schema/src/build_spec.rs

+71-83
Original file line numberDiff line numberDiff line change
@@ -22,112 +22,103 @@ mod build_spec_test;
2222
// in use super::foundation::option_map
2323
/// Set what level of cross-platform compatibility the built package
2424
/// should have.
25-
#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Deserialize, Serialize, Display)]
26-
pub enum HostOsCompatibility {
25+
#[derive(
26+
Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Deserialize, Serialize, Display, Default,
27+
)]
28+
pub enum HostCompat {
2729
/// Package can only be used on the same OS distribution
28-
#[serde(alias = "distro")]
30+
#[default]
2931
Distro,
3032
/// Package can be used anywhere that has the same OS and cpu type
31-
#[serde(alias = "arch")]
3233
Arch,
3334
/// Package can be used on the same OS with any cpu or distro
34-
#[serde(alias = "os", alias = "OS")]
3535
Os,
3636
/// Package can be used on any Os
37-
#[serde(alias = "any")]
3837
Any,
3938
}
4039

41-
impl Default for HostOsCompatibility {
42-
fn default() -> Self {
43-
// TODO: possibly make the default configurable for sites, or
44-
// remove this and derive the default?
45-
Self::Distro
46-
}
47-
}
40+
// Each HostCompat value disallows certain var names when host_compat
41+
// validation is enabled in the config file.
42+
// TODO: move these to config
43+
const DISTRO_DISALLOWS: &[&OptName] = &[];
44+
const ARCH_DISALLOWS: &[&OptName] = &[OptName::distro()];
45+
const OS_DISALLOWS: &[&OptName] = &[OptName::distro(), OptName::arch()];
46+
const ANY_DISALLOWS: &[&OptName] = &[OptName::distro(), OptName::arch(), OptName::os()];
4847

49-
impl HostOsCompatibility {
48+
impl HostCompat {
5049
pub fn is_default(&self) -> bool {
5150
self == &Self::default()
5251
}
5352

53+
fn names_added(&self) -> HashSet<OptNameBuf> {
54+
// TODO: move this to constants/config
55+
let names = match self {
56+
HostCompat::Distro => vec![
57+
OptName::os().to_owned(),
58+
OptName::arch().to_owned(),
59+
OptName::distro().to_owned(),
60+
],
61+
HostCompat::Arch => vec![OptName::os().to_owned(), OptName::arch().to_owned()],
62+
HostCompat::Os => vec![OptName::os().to_owned()],
63+
HostCompat::Any => Vec::new(),
64+
};
65+
66+
names.into_iter().collect::<HashSet<OptNameBuf>>()
67+
}
68+
5469
/// Get host_options after filtering based on the cross Os
5570
/// compatibility setting.
56-
pub fn host_options(&self) -> Result<OptionMap> {
71+
pub fn host_options(&self) -> Result<Vec<Opt>> {
5772
let all_host_options = spk_schema_foundation::option_map::host_options()?;
5873

59-
// TODO: move these to constant/config
60-
let names_added = match self {
61-
HostOsCompatibility::Distro => {
62-
let mut allows = vec![
63-
OptName::os().to_owned(),
64-
OptName::arch().to_owned(),
65-
OptName::distro().to_owned(),
66-
];
67-
// Also adds the <distroname> option
68-
match all_host_options.get(OptName::distro()) {
69-
Some(distro_name) => match OptNameBuf::try_from(distro_name.clone()) {
70-
Ok(name) => allows.push(name.to_owned()),
71-
Err(_err) => {
72-
return Err(Error::HostOptionNotValidDistroNameError(
73-
distro_name.to_string(),
74-
))
75-
}
76-
},
77-
None => return Err(Error::HostOptionNoDistroName),
78-
};
79-
allows
80-
}
81-
HostOsCompatibility::Arch => {
82-
vec![OptName::os().to_owned(), OptName::arch().to_owned()]
83-
}
84-
HostOsCompatibility::Os => {
85-
vec![OptName::os().to_owned()]
74+
let mut names_added = self.names_added();
75+
if HostCompat::Distro == *self {
76+
match all_host_options.get(OptName::distro()) {
77+
Some(distro_name) => match OptNameBuf::try_from(distro_name.clone()) {
78+
Ok(name) => _ = names_added.insert(name),
79+
Err(err) => {
80+
return Err(Error::HostOptionNotValidDistroNameError(
81+
distro_name.to_string(),
82+
err,
83+
))
84+
}
85+
},
86+
None => return Err(Error::HostOptionNoDistroName),
8687
}
87-
HostOsCompatibility::Any => Vec::new(),
88-
};
88+
}
8989

90-
let mut settings = OptionMap::default();
90+
let mut settings = Vec::new();
9191
for (name, value) in all_host_options.iter() {
9292
if names_added.contains(name) {
93-
settings.insert(name.to_owned(), value.clone());
93+
let mut opt = Opt::Var(VarOpt::new(name)?);
94+
opt.set_value(value.to_string())?;
95+
settings.push(opt)
9496
}
9597
}
9698

9799
Ok(settings)
98100
}
99101

100-
/// Check the given options are compatible with the
101-
/// HostOsCompatibility setting.
102+
fn names_disallowed(&self) -> &[&OptName] {
103+
match self {
104+
HostCompat::Distro => DISTRO_DISALLOWS,
105+
HostCompat::Arch => ARCH_DISALLOWS,
106+
HostCompat::Os => OS_DISALLOWS,
107+
HostCompat::Any => ANY_DISALLOWS,
108+
}
109+
}
110+
111+
/// Check the given options are compatible with the HostCompat
112+
/// setting.
102113
pub fn validate_host_opts(&self, options: &[Opt]) -> Result<()> {
103-
let known = options
104-
.iter()
105-
.map(Opt::full_name)
106-
.map(ToOwned::to_owned)
107-
.collect::<HashSet<_>>();
114+
let known = options.iter().map(Opt::full_name).collect::<HashSet<_>>();
108115

109-
// TODO: move these to constant/config setting
110-
let disallowed_names = match self {
111-
HostOsCompatibility::Distro => Vec::new(),
112-
HostOsCompatibility::Arch => {
113-
vec![OptName::distro().to_owned()]
114-
}
115-
HostOsCompatibility::Os => {
116-
vec![OptName::distro().to_owned(), OptName::arch().to_owned()]
117-
}
118-
HostOsCompatibility::Any => {
119-
vec![
120-
OptName::distro().to_owned(),
121-
OptName::arch().to_owned(),
122-
OptName::os().to_owned(),
123-
]
124-
}
125-
};
116+
let disallowed_names = self.names_disallowed();
126117

127118
for name in disallowed_names {
128119
// If a name that this setting would add is already in the
129120
// given options then flag it as an error.
130-
if known.contains(&name) {
121+
if known.contains(name) {
131122
return Err(Error::HostOptionNotAllowedInVariantError(
132123
name.to_string(),
133124
self.to_string(),
@@ -150,7 +141,7 @@ pub struct BuildSpec {
150141
#[serde(default, skip_serializing_if = "ValidationSpec::is_default")]
151142
pub validation: ValidationSpec,
152143
#[serde(default)]
153-
pub host_compatibility: HostOsCompatibility,
144+
pub host_compat: HostCompat,
154145
}
155146

156147
impl Default for BuildSpec {
@@ -160,7 +151,7 @@ impl Default for BuildSpec {
160151
options: Vec::new(),
161152
variants: vec![v0::Variant::default()],
162153
validation: ValidationSpec::default(),
163-
host_compatibility: HostOsCompatibility::default(),
154+
host_compat: HostCompat::default(),
164155
}
165156
}
166157
}
@@ -204,16 +195,16 @@ impl BuildSpec {
204195
// controlled by the host compatibility setting.
205196
let config = spk_config::get_config()?;
206197
if config.host_compat.validate {
207-
self.host_compatibility.validate_host_opts(&opts)?;
198+
self.host_compat.validate_host_opts(&opts)?;
208199
}
209200

210201
// Add any host options that are not already present.
211-
let host_opts = self.host_compatibility.host_options()?;
212-
for (name, value) in host_opts.iter() {
213-
let mut opt = Opt::Var(VarOpt::new(name)?);
214-
opt.set_value(value.to_string())?;
202+
let host_opts = self.host_compat.host_options()?;
203+
for opt in host_opts.iter() {
204+
//let mut opt = Opt::Var(VarOpt::new(name)?);
205+
//opt.set_value(value.to_string())?;
215206
if known.insert(opt.full_name().to_owned()) {
216-
opts.push(opt);
207+
opts.push(opt.clone());
217208
}
218209
}
219210

@@ -343,10 +334,7 @@ impl<'de> Deserialize<'de> for UncheckedBuildSpec {
343334
"validation" => {
344335
unchecked.validation = map.next_value::<ValidationSpec>()?
345336
}
346-
"host_compat" | "host_compatibility" => {
347-
unchecked.host_compatibility =
348-
map.next_value::<HostOsCompatibility>()?
349-
}
337+
"host_compat" => unchecked.host_compat = map.next_value::<HostCompat>()?,
350338
_ => {
351339
// for forwards compatibility we ignore any unrecognized
352340
// field, but consume it just the same

0 commit comments

Comments
 (0)