fix(init): run cookiecutter template hooks from the packaged binary - #9206
fix(init): run cookiecutter template hooks from the packaged binary#9206roger-zhangg wants to merge 3 commits into
Conversation
Cookiecutter runs a Python hook as [sys.executable, script]. In a PyInstaller bundle sys.executable is sam itself, so the hook became 'sam /tmp/xxxx.py', which printed help and exited 0 -- cookiecutter read that as success and the hook was silently skipped. Supply a real interpreter instead: a system python3 when present, else this executable re-launched in hook mode. Also correct INCOMPATIBLE_PARAM_MESSAGE, which still expected the parameter order from before #9176 generated the hint from the enforced combinations.
![aws-sam-tooling-bot[bot]](https://avatars.githubusercontent.com/b/1290?s=60&v=4){kind=small}
| return | ||
|
|
||
| LOG.debug("Running template hook script %s through this executable", arguments[0]) | ||
| runpy.run_path(arguments[0], run_name="__main__") |
There was a problem hiding this comment.
[BUG] Hook mode returns before cli() is ever invoked, so it skips isolate_library_paths_for_subprocess().
In the fallback path, the hook runs inside a freshly launched PyInstaller process. The bootloader sets LD_LIBRARY_PATH / DYLD_*_PATH into the bundle for that new process — this is exactly what samcli/cli/main.py:141 calls isolate_library_paths_for_subprocess() to undo ("isolate library paths so external processes (npm, node, pip, etc.) use system libraries instead of bundled ones"). Because run_hook_script_if_requested() runs the script and calls sys.exit(0) before the click group callback executes, that cleanup never happens in the re-launched process.
Cookiecutter hooks routinely shell out (git init, npm install, pip, dotnet), and those grandchildren will inherit the bundle's library paths, which is the failure mode the isolation helper was added to prevent. Note the parent process's cleanup does not help here: the bootloader re-populates these variables when the sam binary starts.
from samcli.lib.utils.subprocess_utils import is_pyinstaller_bundle, isolate_library_paths_for_subprocess
...
LOG.debug("Running template hook script %s through this executable", arguments[0])
# The bootloader re-sets bundle library paths in this process; hooks shell out to git/npm/pip.
isolate_library_paths_for_subprocess()
runpy.run_path(arguments[0], run_name="__main__")
sys.exit(0)This path is the one taken on hosts with no system python3 — precisely the minimal environments where the native installer is used, so it is worth getting right.
There was a problem hiding this comment.
Valid, fixed in b137693.
You are right that the parent's cleanup does not carry over: the parent filters os.environ, but the bootloader re-populates the library path variables when the sam binary starts, so by the time our code runs in the re-launched process they point back into the bundle. And because this path exits before the click group callback, isolate_library_paths_for_subprocess() never ran.
isolate_library_paths_for_subprocess() is now called immediately before runpy.run_path, so the hook and anything it shells out to get filtered paths. Added a test asserting the ordering (["isolate", "run"]) rather than just that it is called, since calling it after the hook would be useless.
Your last point is the reason this mattered: this branch is only taken when there is no system python3, i.e. exactly the minimal hosts where the native installer is the reason someone has no Python at all.
| # cookiecutter returns the directory it created, which is the only reliable way to know | ||
| # where the project landed when the template chooses its own project directory name. | ||
| project_directory = cookiecutter(**params) | ||
| with patched_hook_runner(cookiecutter_hooks): |
There was a problem hiding this comment.
[GENERAL] The root cause fixed here applies to a second cookiecutter() call site that is left unpatched: Template.generate_project at samcli/lib/cookiecutter/template.py:170.
That path is reached by sam pipeline init, which lets the user point at an arbitrary template (CUSTOM_PIPELINE_TEMPLATE_SOURCE = "Custom Pipeline Template Location", samcli/commands/pipeline/init/interactive_init_flow.py:47). A custom pipeline template with a pre_gen_project.py / post_gen_project.py hook will hit the identical bundle behavior described in the PR: sys.executable is sam, click swallows the script path, exit code 0, hook silently skipped, project generated wrong. Since patched_hook_runner is already a no-op outside a bundle, wrapping the second call site is cheap and keeps the two generation paths consistent:
from cookiecutter import hooks as cookiecutter_hooks
from samcli.lib.utils.hook_script import patched_hook_runner
...
with patched_hook_runner(cookiecutter_hooks):
cookiecutter(
template=self._location,
output_dir=output_dir,
no_input=True,
extra_context=context,
overwrite_if_exists=True,
)If leaving pipeline templates out is deliberate (e.g. scoping this PR to sam init), it would help to say so, since the same silent-wrong-output failure applies there.
There was a problem hiding this comment.
Valid, fixed in b137693 — the omission was an oversight, not a scoping decision.
Verified both halves of the claim before changing anything: the second cookiecutter() call is at samcli/lib/cookiecutter/template.py:170, and sam pipeline init does reach it with a user-supplied location (CUSTOM_PIPELINE_TEMPLATE_SOURCE is offered at interactive_init_flow.py:74 and branched on at :78). So a custom pipeline template with a .py hook would hit the identical silent skip.
Template.generate_project now wraps the call the same way. As you note it costs nothing outside a bundle since patched_hook_runner returns immediately, and it keeps the two generation paths consistent — which matters here because the failure mode is silent wrong output rather than an error.
…template path The re-launched bundle never reaches the CLI callback that undoes the bootloader's library paths, so hooks shelling out to git/npm/pip inherited them. Also wrap the second cookiecutter call site, reachable via sam pipeline init with a custom template location.
| # The bootloader re-points library paths into the bundle for this process, and the CLI callback | ||
| # that normally undoes that is never reached here. Hooks routinely shell out to git, npm and pip. | ||
| isolate_library_paths_for_subprocess() | ||
| runpy.run_path(arguments[0], run_name="__main__") |
There was a problem hiding this comment.
[BUG] sys.argv is left as the sam invocation, so the hook sees an extra argument.
Cookiecutter's run_script launches [sys.executable, script_path], so a hook normally observes sys.argv == ['/tmp/tmpXXXX.py']. runpy.run_path does not touch sys.argv, so on the re-launch path the hook observes ['/path/to/sam', '/tmp/tmpXXXX.py'] instead. Any hook that branches on len(sys.argv), reads sys.argv[1], or derives its own location from sys.argv[0] then behaves differently under the packaged binary than under a pip install — the exact class of divergence this module exists to remove, and one that would surface as a silently mis-generated project rather than an error.
_replaced_attribute already provides the fix:
with replacedattribute(sys, "argv", [arguments[0]]):
runpy.run_path(arguments[0], run_name="__main__")There was a problem hiding this comment.
Valid, fixed in 7ec31c3.
One correction to the mechanism: runpy.run_path does set sys.argv[0] to the script path, so the hook does not see the sam path. Measured it rather than reasoning about it:
via real interpreter : ["/tmp/.../argv_probe.py"]
via runpy (before) : ["/tmp/.../argv_probe.py", "/tmp/.../argv_probe.py"]
So the leak is our trailing argument, not argv[0] — the hook saw two entries instead of one. Your conclusion holds exactly as stated for len(sys.argv) and sys.argv[1]; only the sys.argv[0] part does not apply.
Fixed with _replaced_attribute(sys, "argv", [arguments[0]]) as you suggested. The test asserts the hook observes ["/tmp/hook.py"] and that the caller's argv is restored afterwards.
|
|
||
| # A bundle ships no interpreter of its own, so a system one is preferred: it gives hooks the same | ||
| # environment they get from a pip install, rather than this bundle's Python and dependencies. | ||
| _INTERPRETER_CANDIDATES = ("python3", "python") |
There was a problem hiding this comment.
[GENERAL] Interpreter discovery misses the Windows py launcher, and duplicates an existing probe.
_get_python_command_name() in samcli/hook_packages/terraform/hooks/prepare/enrich.py:691 solves the same problem and tries ["python3", "py3", "python", "py"]. That extra coverage matters here: on a default python.org Windows install py.exe is placed in C:\Windows (always on PATH), python.exe is only added to PATH if the user opts into it, and python3.exe is not installed at all. On such a machine both candidates in _INTERPRETER_CANDIDATES miss, find_system_interpreter() returns None, and every .py hook falls back to in-process execution — so the preferred branch, and the rationale stated in the comment directly above (hooks get a stock environment rather than this bundle's Python and dependencies), never applies on a common Windows configuration.
Appending the launcher keeps the two probes consistent, and the -c probe below already works with it:
INTERPRETERCANDIDATES = ("python3", "python", "py")There was a problem hiding this comment.
Valid, fixed in 7ec31c3.
Confirmed _get_python_command_name at enrich.py:691 uses ["python3", "py3", "python", "py"], so I adopted that list verbatim rather than only appending py — matching the existing probe exactly is easier to justify than inventing a third ordering. Added a test covering the case you describe, where py is the only candidate on PATH.
On the duplication: I deliberately did not import the existing helper. It lives in samcli/hook_packages/terraform/... and raises PrepareHookException, so reusing it would point samcli/lib/utils at a hook package and surface a terraform-specific error from sam init. Consolidating the two probes into one shared utility is worth doing, but it touches the terraform prepare path and belongs in its own change rather than riding along with a nightly fix.
Worth noting the -c probe already guards the launcher: py can be configured to resolve to Python 2, and the probe rejects anything that is not Python 3.
…auncher runpy leaves our second argument in sys.argv, so a hook saw [script, script] instead of [script]. Also add py3/py to interpreter discovery, matching _get_python_command_name, since a default python.org Windows install puts only py.exe on PATH.
| # resolves on PATH without being an interpreter. | ||
| try: | ||
| completed = subprocess.run( | ||
| [path, "-c", "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)"], |
There was a problem hiding this comment.
[BUG] The probe accepts any Python 3.x, so a system interpreter older than the one SAM CLI supports will be preferred over the bundled one.
sys.version_info[0] == 3 is true for 3.0 through 3.6. That matters specifically on the platforms the native installer exists to serve: CentOS/RHEL 7 ships /usr/bin/python3 as 3.6.8, Amazon Linux 2 as 3.7.x. On those hosts find_system_interpreter() returns the old interpreter and the hook runs on it, even though the bundle ships 3.11 and pyproject.toml:10 declares requires-python = ">=3.10". A hook using :=, match, or f-string = specifiers then dies with a SyntaxError that a pip-installed sam would never produce.
This is also where not sharing code with the existing probe bites: _get_python_command_name() (samcli/hook_packages/terraform/hooks/prepare/enrich.py:691) borrows this candidate order but enforces a 3.7+ floor via PYTHON_VERSION_REGEX, so the two probes now disagree on what counts as usable Python. Extracting one helper would keep the floor in one place.
completed = subprocess.run(
[path, "-c", "import sys; sys.exit(0 if sys.version_info >= (3, 8) else 1)"],
...
)(3.8 is the conservative floor; (3, 10) would match what the project requires of itself.)
| # Set on the hook subprocess so a re-launched bundle runs the script instead of parsing a command name. | ||
| HOOK_SCRIPT_ENV_VAR = "SAM_CLI_RUN_HOOK_SCRIPT" | ||
|
|
||
| # A bundle ships no interpreter of its own, so a system one is preferred: it gives hooks the same |
There was a problem hiding this comment.
[GENERAL] The rationale for preferring a system interpreter is inverted, and the resulting order is the one that diverges most from a pip install.
The comment says a system python "gives hooks the same environment they get from a pip install, rather than this bundle's Python and dependencies." Under a pip install, sys.executable is the interpreter that has SAM CLI's dependencies importable — PyYAML~=6.0 is a direct dependency (pyproject.toml:41), and jinja2 comes in with cookiecutter. So a hook containing import yaml or import jinja2 succeeds today for pip users, and would also succeed on the re-launch fallback (which runs under the bundle's sys.path), but fails against a bare /usr/bin/python3 — the path this code prefers.
Net effect: the preferred branch is the only one of the three that cannot import SAM's dependencies, and the failure surfaces as a ModuleNotFoundError inside a FailedHookException for custom --location templates. Either make the re-launch path the primary and the system interpreter the fallback, or keep the current order and correct the comment to say the system interpreter is preferred for isolation, not fidelity, so the next reader does not rely on a guarantee that is not there.
1 participant
Which issue(s) does this change fix?
N/A — found via the 2026-08-28 nightly (run 33161056863), 5 failures in
tests/integration/init/test_init_command.py.Why is this change necessary?
Two unrelated causes.
1. Template hooks never run in the packaged binary (pre-existing). Cookiecutter runs a Python hook as
[sys.executable, script](hooks.py#L84). In a PyInstaller bundlesys.executableissamitself, so it becomessam /tmp/xxxx.py. Because that path starts with/, click treats it as option-like, re-entersparse_argswith no args, hitsno_args_is_helpand callsctx.exit()— printing help and exiting 0 (core.py#L1753, #L1409-L1411). Cookiecutter reads 0 as success, so the hook is silently skipped and the project is generated wrong.Reproduced on released 1.154.0 (installed Feb 2026, six months before the test existed): exit 0, hook marker never written, sam's help text in the output.
sam boguscorrectly exits 2; only paths starting with a non-alphanumeric character hit this. Official app templates contain zero hooks, so the defaultsam initis unaffected — this hits custom--locationtemplates.2. A stale expected message. #9176 generated
INCOMPATIBLE_PARAMS_HINTfromNON_INTERACTIVE_PARAM_COMBINATIONSso the hint cannot drift from the enforced check, which changed the order to--dependency-manager, --app-template. The test'sMISSING_REQUIRED_PARAM_MESSAGEalready matched;INCOMPATIBLE_PARAM_MESSAGEdid not. It escaped PR CI because that class ispr_skip.How does it address the issue?
Supply a real interpreter for
.pyhooks whenis_pyinstaller_bundle():python3/python, probed by executing it (Windows ships apythonApp Execution Alias that resolves on PATH without being an interpreter), so hooks get the same environment as a pip install;Implemented by wrapping
cookiecutter.hooks.run_script, which readssys.executableat call time, so upstream'sFailedHookExceptionhandling,make_executableand Windows shell behaviour are all reused rather than reimplemented.cookiecutter.hooksis passed in as an argument sosamcli/lib/utils/hook_script.pystays importable from__main__without paying for a cookiecutter import on everysaminvocation.Applied at both
cookiecutter()call sites:sam init(samcli/lib/init/__init__.py) andTemplate.generate_project(samcli/lib/cookiecutter/template.py), the latter reachable viasam pipeline initwith a custom template location. In hook mode the re-launched process also callsisolate_library_paths_for_subprocess()before running the hook, because the bootloader re-points library paths into the bundle and the CLI callback that normally undoes that is never reached — hooks routinely shell out to git, npm and pip. Plus the one-line message correction.What side effects does this change have?
Behaviour change worth a release note: hooks that silently no-op today will start executing, so a template whose hook fails will now fail
sam initwhere it previously appeared to succeed. That is the correct behaviour, but it is a visible change for custom-template users.Non-frozen installs are untouched — the wrapper is a no-op, verified that
run_scriptandsys.executableare both left unmodified.PYINSTALLER_RESET_ENVIRONMENTis deliberately not set: the hook child is short-lived with the parent waiting, so worker semantics are correct. The env var is popped before running the hook so a hook that shells out tosamgets the normal CLI.Verified against a simulated bundle (real cookiecutter, real
generate_project,sys.executablepointed at a stand-in for the frozen binary):No new integration test: the one #9176 added is already the regression test, and it is the only place that exercises hooks against a real binary.
Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.