Skip to content

Commit 314cc5e

Browse files
committed
fix(scripts): stop wrap composition looping on a token in core content
The bash wrap strategy rewrote layer_content in place and then re-tested the string it had just modified. When the resolved core content held a literal {CORE_TEMPLATE}, every pass reintroduced the token and the loop never terminated. Consume the wrapper left to right instead, appending each segment and the core content to an accumulator. Work is bounded by the placeholders in the original wrapper and inserted content is never re-examined, matching the single-pass semantics the PowerShell (.Replace) and Python (.replace) ports already have -- so this aligns bash with the other two rather than introducing new behaviour. The regression mode is a hang rather than a wrong value, so the new parity test passes a timeout; run() grows an optional timeout parameter for that. Without it a reintroduced bug would stall the suite instead of failing it. Fixes #4385
1 parent 014a537 commit 314cc5e

3 files changed

Lines changed: 58 additions & 6 deletions

File tree

scripts/bash/common.sh

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -902,12 +902,20 @@ except Exception as exc:
902902
*'{CORE_TEMPLATE}'*) ;;
903903
*) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 2 ;;
904904
esac
905-
while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do
906-
local before="${layer_content%%\{CORE_TEMPLATE\}*}"
907-
local after="${layer_content#*\{CORE_TEMPLATE\}}"
908-
layer_content="${before}${content}${after}"
905+
# Consume the wrapper left to right instead of rewriting it in
906+
# place. Rewriting re-scanned the string just modified, so base
907+
# content holding a literal {CORE_TEMPLATE} reintroduced the
908+
# token every pass and the loop never terminated. Advancing over
909+
# ``rest`` bounds the work by the tokens in the original wrapper
910+
# and leaves inserted content untouched, matching the single-pass
911+
# semantics of .Replace()/.replace() in the PowerShell and Python
912+
# ports.
913+
local wrapped="" rest="$layer_content"
914+
while [[ "$rest" == *'{CORE_TEMPLATE}'* ]]; do
915+
wrapped="${wrapped}${rest%%\{CORE_TEMPLATE\}*}${content}"
916+
rest="${rest#*\{CORE_TEMPLATE\}}"
909917
done
910-
content="$layer_content"
918+
content="${wrapped}${rest}"
911919
;;
912920
*) echo "Error: unknown strategy '$strat'" >&2; return 2 ;;
913921
esac

tests/parity_helpers.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,25 @@ def clean_env() -> dict[str, str]:
8383

8484

8585
def run(
86-
cmd: list[str], repo: Path, env: dict[str, str] | None = None
86+
cmd: list[str],
87+
repo: Path,
88+
env: dict[str, str] | None = None,
89+
timeout: float | None = None,
8790
) -> subprocess.CompletedProcess[str]:
91+
"""Run a script variant.
92+
93+
``timeout`` guards cases whose regression mode is a hang rather than a bad
94+
value; without it such a failure would stall the suite instead of failing
95+
it. ``subprocess.TimeoutExpired`` propagates so the test reports the hang.
96+
"""
8897
return subprocess.run(
8998
cmd,
9099
cwd=repo,
91100
capture_output=True,
92101
text=True,
93102
check=False,
94103
env=env if env is not None else clean_env(),
104+
timeout=timeout,
95105
)
96106

97107

tests/test_resolve_template_python_parity.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,40 @@ def test_all_variants_preserve_composition_parity(
9191
)
9292

9393

94+
@requires_bash
95+
def test_all_variants_treat_core_token_in_core_content_as_literal(
96+
tmp_path: Path,
97+
) -> None:
98+
"""Core content holding a literal ``{CORE_TEMPLATE}`` must not be re-expanded.
99+
100+
The wrap strategy fills the placeholders present in the *wrapper*. A token
101+
that arrives as part of the composed core content is data, not a slot, so it
102+
survives into the output untouched. Rescanning the substituted string instead
103+
reintroduces a token on every pass and never terminates, so the regression
104+
mode here is a hang rather than a wrong value -- hence the timeout, without
105+
which a reintroduced bug would stall the suite instead of failing it.
106+
"""
107+
repo = make_repo(tmp_path)
108+
install_scripts(repo, SCRIPT)
109+
expected = install_composition_stack(repo, TEMPLATE, "# Core {CORE_TEMPLATE}\n")
110+
111+
results = [
112+
run(bash_cmd(repo, SCRIPT, TEMPLATE, "--json"), repo, timeout=30),
113+
run(py_cmd(repo, SCRIPT, TEMPLATE, "--json"), repo, timeout=30),
114+
]
115+
if HAS_POWERSHELL:
116+
results.append(run(ps_cmd(repo, SCRIPT, TEMPLATE, "-Json"), repo, timeout=30))
117+
118+
assert all(result.returncode == 0 for result in results)
119+
assert all(result.stderr == "" for result in results)
120+
# The wrapper contributes exactly one placeholder, so exactly one literal
121+
# token -- the one carried in by the core content -- remains in the output.
122+
assert expected.count("{CORE_TEMPLATE}") == 1
123+
assert all(
124+
json_stdout(result)["TEMPLATE_CONTENT"] == expected for result in results
125+
)
126+
127+
94128
@requires_bash
95129
def test_all_variants_read_utf8_registry_under_ascii_locale(
96130
tmp_path: Path,

0 commit comments

Comments
 (0)