Skip to content

Conversation

@emmanuel-ferdman
Copy link
Contributor

PR Summary

The env parameter in pexpect was typed as os._Environ[str], which only accepts os.environ directly. This broke common patterns like passing {"PATH": "/usr/bin"} or os.environ.copy(). The fix changes the type to Mapping[str, str] (or subprocess._ENV for PopenSpawn) to match what the code actually accepts. PopenSpawn passes env to subprocess.Popen, while spawn/pxssh/run pass it to both utils.which() (which does env.get('PATH') and requires string keys/values) and ptyprocess. All of these work fine with any dict-like mapping, not just the special os.environ object.

Closes #14915

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@emmanuel-ferdman
Copy link
Contributor Author

Hi @srittau @donbarbos, sorry for bumping, but could you please review this PR when you get the chance?

@srittau
Copy link
Collaborator

srittau commented Jan 23, 2026

It seems that ultimately, os.execvpe is called (via ptyprocess):

https://github.com/pexpect/ptyprocess/blob/97f068c80952c702873d801d2c8b262b79685d38/ptyprocess/ptyprocess.py#L298C24-L298C31

In that case it's probably best to just use os._ExecEnv as the annotation. We might tighten that annotation in the future, since it seems overly broad. (Mapping is quite problematic in that regard.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pexpect: PopenSpawn() env arg hint could be improved

2 participants