GeezerVison is showing github.com — Accept any mapping for os.execve env argument by youknowone · Pull Request #7141 · RustPython/RustPython
Conversation
youknowone
commented
Feb 14, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Change env parameter from PyDictRef to ArgMapping so that EnvironmentVarGuard and other mapping types are accepted.
Summary by CodeRabbit
Release Notes
- Improvements
- Enhanced
execve to accept environment variables in more flexible formats, providing improved compatibility with different input types and use cases.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Feb 14, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
📝 Walkthrough
Walkthrough
The PR modifies environment parameter handling in execve() functions across Windows and POSIX stdlib modules. It replaces PyDictRef with ArgMapping for the env parameter and introduces a helper function envobj_to_dict() to convert mapping objects to PyDictRef during execution.
Changes
| Cohort / File(s) |
Summary |
Environment parameter handling crates/vm/src/stdlib/nt.rs, crates/vm/src/stdlib/posix.rs |
Updated execve() signatures to accept env: ArgMapping instead of PyDictRef, enabling broader mapping-like environment inputs. Added envobj_to_dict() helper function to convert ArgMapping to PyDictRef, supporting both actual dicts and dict-like objects. Updated imports to include ArgMapping. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
- Update os from v3.14.2 and fix nt,posix,_os #6958: Modifies execve and spawnve environment handling in nt.rs and posix.rs, touching the same environment construction and conversion code paths.
- impl more nt #6984: Updates environment-handling codepaths in execve/environ across nt.rs and posix.rs, complementing the ArgMapping support and conversion logic.
- test_os, test_io on windows #6379: Modifies Windows environment handling in nt.rs for execve/spawnve, adjusting environment validation and processing alongside this PR's signature changes.
Suggested reviewers
Poem
🐰 A mapping so flexible, wide and free,
No longer bound to PyDictRef's decree,
With ArgMapping's grace, we expand and grow,
Environment variables now freely flow! 🌿
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
| Check name |
Status |
Explanation |
Resolution |
| Merge Conflict Detection |
⚠️ Warning |
❌ Merge conflicts detected (12 files):
⚔️ .cspell.dict/cpython.txt (content) ⚔️ Lib/test/_test_multiprocessing.py (content) ⚔️ Lib/test/test_fileio.py (content) ⚔️ Lib/test/test_httpservers.py (content) ⚔️ Lib/test/test_richcmp.py (content) ⚔️ Lib/test/test_threading.py (content) ⚔️ crates/stdlib/Cargo.toml (content) ⚔️ crates/stdlib/src/lib.rs (content) ⚔️ crates/vm/src/stdlib/io.rs (content) ⚔️ crates/vm/src/stdlib/mod.rs (content) ⚔️ crates/vm/src/stdlib/nt.rs (content) ⚔️ crates/vm/src/stdlib/posix.rs (content)
These conflicts must be resolved before merging into main. |
Resolve conflicts locally and push changes to this branch. |
✅ Passed checks (3 passed)
| Check name |
Status |
Explanation |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
✅ Passed |
The title 'Accept any mapping for os.execve env argument' directly and accurately summarizes the main change: allowing ArgMapping instead of PyDictRef for the env parameter, enabling support for various mapping types like EnvironmentVarGuard. |
| Docstring Coverage |
✅ Passed |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
⚔️ Resolve merge conflicts (beta)
- Auto-commit resolved conflicts to branch
execve
- Post resolved changes as copyable diffs in a comment
No actionable comments were generated in the recent review. 🎉
🧹 Recent nitpick comments
crates/vm/src/stdlib/nt.rs (2)
1099-1106: Consider applying the same ArgMapping treatment to spawnve.
spawnve still accepts env: PyDictRef (Line 1105), which means it won't accept EnvironmentVarGuard or other mapping types. If this is intentional for this PR, a follow-up to broaden spawnve similarly may be worthwhile.
1215-1228: Extract envobj_to_dict to a shared location to eliminate duplication.
This function is identical in both posix.rs (lines 1076–1089) and nt.rs (lines 1215–1228). Since both platform modules already import utilities from os.rs (the parent module), define envobj_to_dict once in os.rs and import it in both platform-specific modules to avoid code duplication and simplify future maintenance.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
❤️ Share
Comment @coderabbitai help to get the list of available commands and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Code has been automatically formatted
The code in this PR has been formatted using:
cargo fmt --all
Please pull the latest changes before pushing again:
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Change env parameter from PyDictRef to ArgMapping so that
EnvironmentVarGuard and other mapping types are accepted.
📦 Library Dependencies
The following Lib/ modules were modified. Here are their dependencies:
[ ] lib: cpython/Lib/http
[ ] test: cpython/Lib/test/test_httplib.py
[ ] test: cpython/Lib/test/test_http_cookiejar.py
[ ] test: cpython/Lib/test/test_http_cookies.py
[ ] test: cpython/Lib/test/test_httpservers.py
dependencies:
- http (native: collections.abc, email.message, email.parser, email.utils, errno, http.client, itertools, select, sys, time, urllib.parse)
- datetime (native: _thread, math, sys, time)
- types
- calendar, copy, enum, html, io, mimetypes, os, posixpath, re, shutil, socket, socketserver, string, threading
dependent tests: (23 tests)
- http: test_docxmlrpc test_genericalias test_hashlib test_http_cookiejar test_http_cookies test_httplib test_httpservers test_logging test_robotparser test_ssl test_ucn test_unicodedata test_urllib test_urllib2 test_urllib2_localnet test_wsgiref test_xml_dom_xmlbuilder test_xmlrpc
- urllib.request: test_pathlib test_pydoc test_site test_urllib2net test_urllibnet
[x] lib: cpython/Lib/threading.py
[x] lib: cpython/Lib/_threading_local.py
[ ] test: cpython/Lib/test/test_threading.py (TODO: 20)
[ ] test: cpython/Lib/test/test_threadedtempfile.py
[ ] test: cpython/Lib/test/test_threading_local.py (TODO: 3)
dependencies:
dependent tests: (113 tests)
- threading: test_android test_asyncio test_bz2 test_code test_concurrent_futures test_contextlib test_ctypes test_decimal test_docxmlrpc test_email test_enum test_fork1 test_ftplib test_functools test_hashlib test_httplib test_httpservers test_imaplib test_importlib test_io test_itertools test_largefile test_linecache test_logging test_opcache test_pathlib test_poll test_queue test_robotparser test_sched test_signal test_smtplib test_socket test_socketserver test_sqlite3 test_ssl test_subprocess test_super test_syslog test_termios test_threadedtempfile test_threading test_threading_local test_time test_urllib2_localnet test_weakref test_winreg test_wsgiref test_xmlrpc test_zstd
- asyncio: test_asyncio test_contextlib_async test_inspect test_os test_sys_settrace test_unittest
- concurrent.futures._base: test_concurrent_futures
- concurrent.futures.process: test_concurrent_futures
- concurrent.futures.thread: test_genericalias
- http.cookiejar: test_http_cookiejar
- logging: test_support test_unittest
- multiprocessing: test_fcntl test_multiprocessing_main_handling
- queue: test_dummy_thread
- subprocess: test_audit test_c_locale_coercion test_cmd_line test_cmd_line_script test_ctypes test_dtrace test_faulthandler test_file_eintr test_gzip test_json test_launcher test_msvcrt test_ntpath test_osx_env test_platform test_plistlib test_py_compile test_regrtest test_repl test_runpy test_script_helper test_select test_shutil test_site test_sys test_sysconfig test_tempfile test_unittest test_urllib2 test_utf8_mode test_wait3 test_webbrowser test_zipfile
- ctypes.util: test_ctypes
- ensurepip: test_ensurepip
- sysconfig: test_pyexpat test_tools test_urllib2net
- pydoc: test_pydoc
- trace: test_trace
- zipfile: test_pkgutil test_zipapp test_zipfile test_zipfile64 test_zipimport
- importlib.metadata: test_importlib test_zoneinfo
[x] test: cpython/Lib/test/test_wmi.py
dependencies:
dependent tests: (no tests depend on wmi)
Legend:
[+] path exists in CPython
[x] up-to-date, [ ] outdated
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
youknowone
added a commit
to youknowone/RustPython
that referenced
this pull request
Mar 22, 2026
Accept any mapping for os.execve env argument
youknowone
added a commit
that referenced
this pull request
Sep 16, 2026
Accept any mapping for os.execve env argument
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|