Skip to content

Accept any mapping for os.execve env argument#7141

Merged
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:execve
Feb 15, 2026
Merged

Accept any mapping for os.execve env argument#7141
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:execve

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 14, 2026 •
edited by coderabbitai bot
Loading

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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 14, 2026 •
edited
Loading

📝 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

  • arihant2math

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 docstrings
🧪 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.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2026

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:
git pull origin execve

@youknowone youknowone force-pushed the execve branch 3 times, most recently from 501e74e to 034f9ae Compare February 15, 2026 13:25
@youknowone
Accept any mapping for os.execve env argument
72b4ef3 
Change env parameter from PyDictRef to ArgMapping so that
EnvironmentVarGuard and other mapping types are accepted.
@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2026

📦 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:

  • threading

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
      • venv: test_venv
    • 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

@youknowone youknowone marked this pull request as ready for review February 15, 2026 17:51
@youknowone youknowone merged commit 79453c3 into RustPython:main Feb 15, 2026
14 checks passed
@youknowone youknowone deleted the execve branch February 15, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone