refactor: Pass module paths and sys.argv to CPython as objects - #18
Open
ndonkoHenri wants to merge 3 commits into
Open
refactor: Pass module paths and sys.argv to CPython as objects#18ndonkoHenri wants to merge 3 commits into
sys.argv to CPython as objects#18ndonkoHenri wants to merge 3 commits into
Conversation
sp_apply_module_paths rendered the path list into Python source text — escaping each path into a quoted literal inside a hand-sized buffer — and evaluated the result against __main__'s globals. Data traveling through the code channel needs quoting, and the quoting layer is where the bugs lived: an apostrophe-dense path could overflow the original buffer estimate, a path ending in a backslash produced a SyntaxError (a raw string cannot end in one), and a path containing a newline broke the generated source. Build the list with the C API instead (PyList_New / PyUnicode_FromString / PyList_SetItem, all abi3), expose it as `_sp_paths` in a private globals dict wired to the interpreter's builtins, and run the fixed bootstrap script there (the former SP_MODULE_PATHS_EPILOGUE, now SP_MODULE_PATHS_SCRIPT). Paths are objects end to end, so nothing is escaped and the whole estimate/escape/guard machinery is deleted. Private globals mean the script's imports and temporaries cannot collide with the user program's namespace, so the script's cleanup `del` goes away too — and __main__'s globals are left exactly as before this change. sp_pyrun_string's compile+eval core is factored out as sp_pyrun_string_in_globals (the __main__ lookup gains the missing NULL check on PyModule_GetDict). Failures surface an explicit error at each step — including a RuntimeError if builtins are unavailable — before the shared sp_paths_failed tail reports on stderr.
Same treatment as the module paths: sp_apply_program_name formatted the
program name into "import sys; sys.argv = [r'''name''']". Any name
containing ''' broke the generated statement, and names that pushed it
past the 1024-byte snprintf buffer broke startup — the truncation cut
the closing quotes off the statement, producing a SyntaxError at boot
rather than a truncated value. Build the one-element list with the C
API and install it with PySys_SetObject("argv", ...) — identical end
state, no quoting, no length limit. The default "python" fallback is
unchanged, and `sys` in __main__ is untouched (the stdio redirect
already provides that binding, as before).
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
1 participant
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.
Based/stacked on #17
Why rewrite working code
sp_apply_module_pathsandsp_apply_program_namepassed data (module paths, the program name) into the interpreter by rendering it into Python source text — escaping each value into a quoted literal inside a hand-sized buffer, then evaluating the result. Anything traveling through the code channel needs quoting, and the quoting layer is where the bugs were:SyntaxError(a raw string cannot end in one) — app fails to start;'''broke ther'''…'''literal — same;SyntaxErrorat boot, not a truncated value;What this does instead
Module paths (commit 1): the list is built with the C API (
PyList_New/PyUnicode_FromString/PyList_SetItem, all abi3) and exposed as_sp_pathsin a private globals dict wired to the interpreter's builtins; the fixed bootstrap script (dedupe +site.addsitedir, unchanged logic from the base branch) runs against those globals. Data is objects end to end — nothing is escaped, and the estimate/escape/guard machinery is deleted. Private globals also mean the script's imports and temporaries cannot touch the user program's__main__, so its cleanupdelis gone and__main__is left exactly as before this change (no observable behavior difference).sp_pyrun_string's compile+eval core is factored out assp_pyrun_string_in_globals.sys.argv (commit 2): same treatment — one-element list via the C API, installed with
PySys_SetObject("argv", …). Identical end state, no quoting, no length limit; the"python"default is unchanged.Verification
0x030c0000three ways: guard-compile, the vendored 3.14 headers' guards, and symbol presence in the Windows abi3 stub (python3.lib).PyEval_GetBuiltinsis the only builtins accessor at this level (PyEval_GetFrameBuiltinsis 3.13+).serious_python_run()end to end against an embedded interpreter (3.12 and 3.14), mirroring the real plugins' env (PYTHONPATHduplication,LC_CTYPE=UTF-8). Scenarios, all passing:.pthpath-line +import-line processing, PYTHONPATH dedupe, precedence order, apostrophe / trailing-backslash / newline / non-ASCII dirs, a non-directorymodules.zipentry (Android shape), and__main__-globals cleanliness;'''-containing name, 3000-byte name;rc=1with the[serious_python_run]stderr marker, no crash..pthfixture.