Repository navigation
fix(template): don't treat ${VAR} as an unresolved token - #10
Merged
Merged
Conversation
`find_unresolved_token` matches any `{word}`, which includes the inside of
a shell parameter expansion. That was harmless while `command` and `[env]`
went unrendered, but they are rendered now, so the two syntaxes share a
document — and a recipe whose command says `echo ${LABCTL_RUN_ID}` fails at
submit with:
Error: unresolved template token "{LABCTL_RUN_ID}" in "..."
`${…}` is shell syntax, never a labctl token, so skip a `{` preceded by `$`.
The other expansion forms (`${VAR:-default}`, `${#arr[@]}`) already fall out
of the scan on their non-identifier bytes; this covers the plain `${NAME}`
case that doesn't.
The exemption is positional, not a blanket escape: a genuine unresolved
token elsewhere in the same string is still caught, and a labctl token
sitting next to a shell expansion still renders. Both are tested.
Found submitting a real recipe against the berlin registry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's wrong
find_unresolved_tokenmatches any{word}shape — including the inside of a shell parameter expansion. That was harmless whilecommandand[env]went unrendered, but #8 started rendering them, so shell syntax and labctl token syntax now share a document.The result is that a recipe whose command contains
${LABCTL_RUN_ID}— or${HOME}, or${out}— is rejected at submit:${…}is shell parameter expansion and is never a labctl token, so this is a false positive, and it hits the common case: any non-trivialbash -ccommand. This is a regression from #8 (mine), found submitting a real recipe against the berlin registry.The fix
Skip a
{that is immediately preceded by$. The other expansion forms (${VAR:-default},${#arr[@]}) already fall out of the token scan on their non-identifier bytes — it's the plain${NAME}case that collides.The exemption is deliberately positional, not a blanket escape:
echo ${HOME} {outputs.typo.path}still fails at submit;${HOME}/{run.id}→${HOME}/run_abc.Both are covered by new tests, alongside a table of expansion forms that must round-trip untouched.
cargo test --all-features: 39 passed, 0 failed (8 PG-gated tests ignored as usual).