Skip to content

Commit

Permalink
Merge pull request #740 from byllyfish/close_fds
Browse files Browse the repository at this point in the history
Change default for close_fds back to True.
  • Loading branch information
byllyfish authored Jul 1, 2024
2 parents 996724b + 910fc7c commit fcb5e0c
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
10 changes: 4 additions & 6 deletions shellous/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class Options: # pylint: disable=too-many-instance-attributes
pty: PtyAdapterOrBool = False
"True if child process should be controlled using a pseudo-terminal (pty)."

close_fds: bool = False
close_fds: bool = True
"True if child process should close all file descriptors."

audit_callback: _AuditFnT = None
Expand Down Expand Up @@ -595,12 +595,10 @@ def set( # pylint: disable=unused-argument, too-many-locals, too-many-arguments
`shellous.raw` and `shellous.cbreak` that can be used as arguments to
the `pty` option.
**close_fds** (bool) default=False<br>
**close_fds** (bool) default=True<br>
Close all unnecessary file descriptors in the child process. This
defaults to False to align with `posix_spawn` requirements. Please refer
to the documentation on [inheritance of file descriptors](
https://docs.python.org/3/library/os.html#inheritance-of-file-descriptors)
for behavior on Unix and Windows.
defaults to True to align with the default behavior of the subprocess
module.
**audit_callback** (Callable(phase, info) | None) default=None<br>
Specify a function to call as the command execution goes through its
Expand Down
7 changes: 4 additions & 3 deletions tests/test_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ def _hook(*info):
# uvloop doesn't implement audit hooks.
assert any(event.startswith("('subprocess.Popen',") for event in events)

if _has_posix_spawn():
assert any(event.startswith("('os.posix_spawn',") for event in events)
if _has_posix_spawn() and sys.version_info < (3, 13):
# Do not expect posix_spawn to be used on Python 3.12 or earlier.
assert not any(event.startswith("('os.posix_spawn',") for event in events)


@pytest.mark.skipif(not _has_posix_spawn(), reason="posix_spawn")
Expand All @@ -84,7 +85,7 @@ def _hook(*info):

# This command does not include a directory path, so it is resolved
# through PATH.
result = await sh("ls", "README.md")
result = await sh("ls", "README.md").set(close_fds=False)

finally:
_HOOK = None
Expand Down

0 comments on commit fcb5e0c

Please sign in to comment.