From 97fb39c6dc19b7b28a4e1781efcfcf7556408bb1 Mon Sep 17 00:00:00 2001 From: Ryan Bottriell Date: Fri, 7 Oct 2022 17:18:44 -0700 Subject: [PATCH] Update default tag to work for any level of object Signed-off-by: Ryan Bottriell --- .../crates/liquid/src/error_test.rs | 2 +- .../crates/liquid/src/tag_default.rs | 71 ++++++++++++++++--- .../crates/liquid/src/tag_default_test.rs | 30 ++++++++ crates/spk-schema/src/lib.rs | 2 +- crates/spk-schema/src/template.rs | 5 +- docs/use/spec.md | 18 ++--- packages/cmake/cmake.spk.yaml | 8 +-- packages/gnu/gcc/gcc48.spk.yaml | 12 ++-- packages/gnu/gcc/gcc63.spk.yaml | 10 +-- packages/python/python2.spk.yaml | 7 +- packages/python/python3.spk.yaml | 2 +- 11 files changed, 124 insertions(+), 43 deletions(-) diff --git a/crates/spk-schema/crates/liquid/src/error_test.rs b/crates/spk-schema/crates/liquid/src/error_test.rs index 9be829c9ba..7f02eb296f 100644 --- a/crates/spk-schema/crates/liquid/src/error_test.rs +++ b/crates/spk-schema/crates/liquid/src/error_test.rs @@ -12,7 +12,7 @@ fn test_error_position_extraction() { crate::render_template(TPL, &json!({})).expect_err("expected template render to fail"); let expected = r#" 1 | {% default = data | replace ''%} - | ^ unexpected "="; expected Identifier + | ^ unexpected "="; expected Variable "#; let message = err.to_string(); assert_eq!(message, expected); diff --git a/crates/spk-schema/crates/liquid/src/tag_default.rs b/crates/spk-schema/crates/liquid/src/tag_default.rs index 887d578239..80d3ae305b 100644 --- a/crates/spk-schema/crates/liquid/src/tag_default.rs +++ b/crates/spk-schema/crates/liquid/src/tag_default.rs @@ -2,9 +2,20 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/imageworks/spk +use liquid::{Object, ValueView}; use liquid_core::error::ResultLiquidExt; use liquid_core::parser::FilterChain; -use liquid_core::{Language, ParseTag, Renderable, Result, Runtime, TagReflection, TagTokenIter}; +use liquid_core::runtime::Variable; +use liquid_core::{ + Language, + ParseTag, + Renderable, + Result, + Runtime, + TagReflection, + TagTokenIter, + Value, +}; #[cfg(test)] #[path = "./tag_default_test.rs"] @@ -37,10 +48,8 @@ impl ParseTag for DefaultTag { ) -> Result> { let dst = arguments .expect_next("Identifier expected.")? - .expect_identifier() - .into_result()? - .to_string() - .into(); + .expect_variable() + .into_result()?; arguments .expect_next("Assignment operator \"=\" expected.")? @@ -65,7 +74,7 @@ impl ParseTag for DefaultTag { #[derive(Debug)] struct Default { - dst: liquid_core::model::KString, + dst: Variable, src: FilterChain, } @@ -83,10 +92,54 @@ impl Renderable for Default { .trace_with(|| self.trace().into())? .into_owned(); - let name = self.dst.as_str().into(); - if runtime.try_get(&[name]).is_none() { - runtime.set_global(self.dst.clone(), value); + let dst = self.dst.evaluate(runtime)?; + let (base_name, sub_path) = dst + .split_first() + .expect("Path guarantees at least one entry"); + let mut existing = runtime + .try_get(&[base_name.clone()]) + .map(|v| v.to_value()) + .unwrap_or(Value::Nil); + let mut current = &mut existing; + for key in sub_path.iter() { + match current { + Value::Nil => { + *current = Value::Object(Object::new()); + current = current + .as_object_mut() + .expect("current was just assigned as an object") + .entry(key.to_kstr()) + .or_insert(Value::Nil); + } + Value::Scalar(_) => { + return Err(liquid::Error::with_msg( + "cannot index into scalar for default assignment", + ) + .trace(format!("trying to traverse into {key:?}"))) + } + Value::Array(_) => { + return Err(liquid::Error::with_msg( + "cannot index into array for default assignment", + ) + .trace(format!("trying to traverse into {key:?}"))) + } + Value::Object(obj) => { + current = obj.entry(key.to_kstr()).or_insert(Value::Nil); + } + Value::State(_) => { + return Err(liquid::Error::with_msg( + "cannot index into state for default assignment", + ) + .trace(format!("trying to traverse into {key:?}"))) + } + } + } + if current.is_nil() { + tracing::debug!("setting template default for: {dst} = {value:?}"); + *current = value; } + // put back the extracted value that has now been modified + runtime.set_global(base_name.to_kstr().into(), existing); Ok(()) } } diff --git a/crates/spk-schema/crates/liquid/src/tag_default_test.rs b/crates/spk-schema/crates/liquid/src/tag_default_test.rs index 3d6eedfc7b..f947ef3185 100644 --- a/crates/spk-schema/crates/liquid/src/tag_default_test.rs +++ b/crates/spk-schema/crates/liquid/src/tag_default_test.rs @@ -55,3 +55,33 @@ sources: crate::render_template(TPL, &options).expect("template should not fail to render"); assert_eq!(rendered, EXPECTED); } + +#[rstest] +fn test_template_rendering_defaults_depth() { + // ensure that defaults can be set at any + // level without interfering + + let options = json!({ + "one": { + "a": 10, + } + }); + static TPL: &str = r#" +{% default two = 2 -%} +{% default one.b = 20 -%} +{% default other.a.b = 50 -%} +{{ two }} +{{ one.a }} +{{ one.b }} +{{ other.a.b }} +"#; + static EXPECTED: &str = r#" +2 +10 +20 +50 +"#; + let rendered = + crate::render_template(TPL, &options).expect("template should not fail to render"); + assert_eq!(rendered, EXPECTED); +} diff --git a/crates/spk-schema/src/lib.rs b/crates/spk-schema/src/lib.rs index 233fc031f6..e720965366 100644 --- a/crates/spk-schema/src/lib.rs +++ b/crates/spk-schema/src/lib.rs @@ -52,7 +52,7 @@ pub use spk_schema_foundation::{ FromYaml, }; pub use spk_schema_ident::{self as ident, Ident}; -pub use template::{Template, TemplateExt, TemplateData}; +pub use template::{Template, TemplateData, TemplateExt}; pub use test_spec::TestStage; pub use validation::{default_validators, ValidationSpec, Validator}; pub use {serde_json, spk_schema_validators as validators}; diff --git a/crates/spk-schema/src/template.rs b/crates/spk-schema/src/template.rs index a44c5446e2..a22e9e8360 100644 --- a/crates/spk-schema/src/template.rs +++ b/crates/spk-schema/src/template.rs @@ -26,7 +26,6 @@ pub trait TemplateExt: Template { fn from_file(path: &Path) -> Result; } - /// The structured data that should be made available /// when rendering spk templates into recipes #[derive(serde::Serialize, Debug, Clone)] @@ -52,7 +51,7 @@ struct SpkInfo { impl Default for SpkInfo { fn default() -> Self { Self { - version: env!("CARGO_PKG_VERSION") + version: env!("CARGO_PKG_VERSION"), } } } @@ -63,7 +62,7 @@ impl TemplateData { TemplateData { spk: SpkInfo::default(), opt: options.to_yaml_value_expanded(), - env: std::env::vars().into_iter().collect() + env: std::env::vars().into_iter().collect(), } } } diff --git a/docs/use/spec.md b/docs/use/spec.md index 435b303ce9..6d10c6677a 100644 --- a/docs/use/spec.md +++ b/docs/use/spec.md @@ -435,7 +435,7 @@ One common templating use case is to allow your package spec to be reused to bui ```yaml # {% default opt.version = "2.3.4" %} -pkg: my-package/{{ version }} +pkg: my-package/{{ opt.version }} ``` Which could then be invoked for different versions at the command line: @@ -453,11 +453,11 @@ In addition to the default tags and filters within the liquid language, spk prov **default** -The `default` tag can be used to more easily declare the default value for a variable. The following two statements are equivalent: +The `default` tag can be used to more easily declare the default value for a variable at any level of the template data structure. Most commonly, this can be used to declare default option or environment variable values so that they are not required to be given by the developer. ```liquid -{% assign version = version | default: "2.3.4" %} -{% default version = "2.3.4" %} +{% default opt.version = "2.3.4" %} +{% default env.PWD = "/tmp" %} ``` ##### Filters @@ -467,10 +467,10 @@ The `default` tag can be used to more easily declare the default value for a var The `compare_version` allows for comparing spk versions using any of the [version comparison operators](/use/versioning). It takes one or two parameters, depending on the data that you have to give. In all cases, the parameters are concatenated together and parsed as a version range. For example, the following assignments to py_3 all end up checking the same statement. ```liquid -{% assign is_py3 = python.version | compare_version: ">=3" %} -{% assign is_py3 = python.version | compare_version: ">=", 3 %} +{% assign is_py3 = opt.python.version | compare_version: ">=3" %} +{% assign is_py3 = opt.python.version | compare_version: ">=", 3 %} {% assign three = 3 %} -{% assign is_py3 = python.version | compare_version: ">=", three %} +{% assign is_py3 = opt.python.version | compare_version: ">=", three %} ``` **parse_version** @@ -494,7 +494,7 @@ The `parse_version` filter breaks down an spk version into it's components, eith The `replace_re` filter works like the built-in `replace` filter, except that it matches using a perl-style regular expression and allows group replacement in the output. These regular expressions do not support look-arounds or back-references. For example: ```liquid -{% default version = "2.3.4" %} -{% assign major_minor = version | replace_re: "(\d+)\.(\d+).*", "$1.$2" %} +{% default opt.version = "2.3.4" %} +{% assign major_minor = opt.version | replace_re: "(\d+)\.(\d+).*", "$1.$2" %} {{ major_minor }} # 2.3 ``` diff --git a/packages/cmake/cmake.spk.yaml b/packages/cmake/cmake.spk.yaml index 7b9bad3799..99a48fe2cb 100644 --- a/packages/cmake/cmake.spk.yaml +++ b/packages/cmake/cmake.spk.yaml @@ -1,5 +1,5 @@ -# {% assign version = opt.version | default: "3.16.4" %} -pkg: cmake/{{ version }} +# {% default opt.version = "3.16.4" %} +pkg: cmake/{{ opt.version }} api: v0/package sources: @@ -8,8 +8,8 @@ sources: # the tar file. - path: ./ - script: - - export TARFILE=cmake-{{ version }}-Linux-x86_64.tar.gz - - if [ ! -e ./$TARFILE ] ; then wget https://github.com/Kitware/CMake/releases/download/v{{ version }}/$TARFILE ; fi + - export TARFILE=cmake-{{ opt.version }}-Linux-x86_64.tar.gz + - if [ ! -e ./$TARFILE ] ; then wget https://github.com/Kitware/CMake/releases/download/v{{ opt.version }}/$TARFILE ; fi build: options: diff --git a/packages/gnu/gcc/gcc48.spk.yaml b/packages/gnu/gcc/gcc48.spk.yaml index 9423cd5da0..b9ef9500e2 100644 --- a/packages/gnu/gcc/gcc48.spk.yaml +++ b/packages/gnu/gcc/gcc48.spk.yaml @@ -1,9 +1,9 @@ -# {{ assign version = opt.version | default: "4.8.5" }} -pkg: gcc/{{ version }} +# {{ default opt.version = "4.8.5" }} +pkg: gcc/{{ opt.version }} api: v0/package sources: - - tar: http://ftpmirror.gnu.org/gnu/gcc/gcc-{{ version }}/gcc-{{ version }}.tar.gz + - tar: http://ftpmirror.gnu.org/gnu/gcc/gcc-{{ opt.version }}/gcc-{{ opt.version }}.tar.gz - path: patch-gcc46-texi.diff build: @@ -25,10 +25,10 @@ build: - pkg: coreutils - pkg: binutils - pkg: make - - pkg: gcc/<={{ version }} + - pkg: gcc/<={{ opt.version }} script: - - patch -d gcc-{{ version }} -p0