From 86fd717e464a81476f2e5ff91a5e4f4f6c55abbe Mon Sep 17 00:00:00 2001 From: Yash-Chindam Date: Wed, 2 Sep 2026 04:02:14 +0530 Subject: [PATCH] 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 --- scripts/bash/common.sh | 18 ++++++++--- tests/parity_helpers.py | 12 ++++++- tests/test_resolve_template_python_parity.py | 34 ++++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 33f90b8dbb..9efcfad5e6 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -902,12 +902,20 @@ except Exception as exc: *'{CORE_TEMPLATE}'*) ;; *) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 2 ;; esac - while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do - local before="${layer_content%%\{CORE_TEMPLATE\}*}" - local after="${layer_content#*\{CORE_TEMPLATE\}}" - layer_content="${before}${content}${after}" + # Consume the wrapper left to right instead of rewriting it in + # place. Rewriting re-scanned the string just modified, so base + # content holding a literal {CORE_TEMPLATE} reintroduced the + # token every pass and the loop never terminated. Advancing over + # ``rest`` bounds the work by the tokens in the original wrapper + # and leaves inserted content untouched, matching the single-pass + # semantics of .Replace()/.replace() in the PowerShell and Python + # ports. + local wrapped="" rest="$layer_content" + while [[ "$rest" == *'{CORE_TEMPLATE}'* ]]; do + wrapped="${wrapped}${rest%%\{CORE_TEMPLATE\}*}${content}" + rest="${rest#*\{CORE_TEMPLATE\}}" done - content="$layer_content" + content="${wrapped}${rest}" ;; *) echo "Error: unknown strategy '$strat'" >&2; return 2 ;; esac diff --git a/tests/parity_helpers.py b/tests/parity_helpers.py index 27627dab5b..3a3878de8d 100644 --- a/tests/parity_helpers.py +++ b/tests/parity_helpers.py @@ -83,8 +83,17 @@ def clean_env() -> dict[str, str]: def run( - cmd: list[str], repo: Path, env: dict[str, str] | None = None + cmd: list[str], + repo: Path, + env: dict[str, str] | None = None, + timeout: float | None = None, ) -> subprocess.CompletedProcess[str]: + """Run a script variant. + + ``timeout`` guards cases whose regression mode is a hang rather than a bad + value; without it such a failure would stall the suite instead of failing + it. ``subprocess.TimeoutExpired`` propagates so the test reports the hang. + """ return subprocess.run( cmd, cwd=repo, @@ -92,6 +101,7 @@ def run( text=True, check=False, env=env if env is not None else clean_env(), + timeout=timeout, ) diff --git a/tests/test_resolve_template_python_parity.py b/tests/test_resolve_template_python_parity.py index 9af5554b44..2bf9977e14 100644 --- a/tests/test_resolve_template_python_parity.py +++ b/tests/test_resolve_template_python_parity.py @@ -91,6 +91,40 @@ def test_all_variants_preserve_composition_parity( ) +@requires_bash +def test_all_variants_treat_core_token_in_core_content_as_literal( + tmp_path: Path, +) -> None: + """Core content holding a literal ``{CORE_TEMPLATE}`` must not be re-expanded. + + The wrap strategy fills the placeholders present in the *wrapper*. A token + that arrives as part of the composed core content is data, not a slot, so it + survives into the output untouched. Rescanning the substituted string instead + reintroduces a token on every pass and never terminates, so the regression + mode here is a hang rather than a wrong value -- hence the timeout, without + which a reintroduced bug would stall the suite instead of failing it. + """ + repo = make_repo(tmp_path) + install_scripts(repo, SCRIPT) + expected = install_composition_stack(repo, TEMPLATE, "# Core {CORE_TEMPLATE}\n") + + results = [ + run(bash_cmd(repo, SCRIPT, TEMPLATE, "--json"), repo, timeout=30), + run(py_cmd(repo, SCRIPT, TEMPLATE, "--json"), repo, timeout=30), + ] + if HAS_POWERSHELL: + results.append(run(ps_cmd(repo, SCRIPT, TEMPLATE, "-Json"), repo, timeout=30)) + + assert all(result.returncode == 0 for result in results) + assert all(result.stderr == "" for result in results) + # The wrapper contributes exactly one placeholder, so exactly one literal + # token -- the one carried in by the core content -- remains in the output. + assert expected.count("{CORE_TEMPLATE}") == 1 + assert all( + json_stdout(result)["TEMPLATE_CONTENT"] == expected for result in results + ) + + @requires_bash def test_all_variants_read_utf8_registry_under_ascii_locale( tmp_path: Path,