Fix timeout child-process lookup on non-GNU systems - #2236
Merged
Byron merged 2 commits intoSep 13, 2026
Merged
Conversation
Git.execute used ps --ppid to find direct children before enforcing kill_after_timeout. On macOS this option is rejected: the parent is killed, but a child can continue running and hold captured output pipes open. Use pgrep -P for the child lookup, with POSIX ps PID/PPID output as a fallback when pgrep is absent. Filter the fallback by the original parent PID and reap the lookup subprocess in both paths. Keep the existing parent-first SIGKILL order, direct-child scope, and Windows guard, and update the documented command requirements. Systems without either lookup facility and the existing PID-reuse race remain limitations. Add real-process regressions for native pgrep and the ps fallback, plus a test that excludes unrelated processes and grandchildren from the fallback. Both real-process cases failed on the original code on macOS. The command module now passes 105 tests with 1 skip on macOS 27.0 / Python 3.13.5. Ruff check and format, codespell, mypy (45 files), basedpyright, and diff whitespace checks pass. Linux and Cygwin were not run locally; Cygwin's default ps lacks the required options, so the real-process cases skip it. Fixes gitpython-developers#1756 Signed-off-by: Mingyang Wu <129849514+aprylewu@users.noreply.github.com>
Member
|
Thanks a lot! |
Byron
force-pushed
the
codex-fix-portable-timeout-child-lookup
branch
from
September 13, 2026 03:17
3686b6a to
be1f168
Compare
Several public annotations rejected supported inputs or lost the relationship between input options and return types. Describe Git.execute process, text, bytes, and extended-output results with overloads, accept stdin file descriptors, and account for absent stdout. Correct remote-removal, object, database, blame, and index-entry types, preserve entry subclasses and the supported tuple shapes, and accept streams with only the required read or write methods. Normalize absent previous stderr before appending process errors in AutoInterrupt.wait. Add runtime and static regressions for these interfaces, include them in mypy and basedpyright, make required imports explicit, and reduce the basedpyright baseline to the remaining diagnostics. Keep mock available to typecheck the Python 3.7 import branches. Make the output-type regression emit its payload without a newline. Its original print call produced CRLF on Windows; Git.execute strips the final LF as documented, leaving a carriage return that failed the test in all 11 Windows jobs. Omitting the newline keeps the same text, bytes, and tuple assertions independent of platform newline translation. Validation on macOS with Python 3.12.14: all four test/test_typing.py tests pass. Forcing the child stdout to translate newlines to CRLF reproduces the old failure and passes with the corrected command. Mypy passes for 46 source files; basedpyright --warnings reports no errors or warnings; Ruff lint and format checks for test/test_typing.py and git diff --check pass. Native Windows validation is delegated to the PR CI matrix. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
Byron
force-pushed
the
codex-fix-portable-timeout-child-lookup
branch
from
September 13, 2026 03:57
be1f168 to
d66edad
Compare
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
2 participants
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.
Git.execute(..., kill_after_timeout=...)currently finds direct child processes withps --ppid. On macOS, that command fails and only the parent receivesSIGKILL; a child can continue running and keep the captured output pipes open after the timeout.Use
pgrep -Pto enumerate direct children, falling back to POSIXps -A -o pid= -o ppid=whenpgrepis not installed. The fallback filters by the target parent's PID. Both enumeration subprocesses are waited for and their pipes closed. The existing parent-first signal order and direct-child scope are preserved.The regression runs a real parent/child pair through
Git.execute. Before the change, the child survives the timeout and writes a marker; after the change, it is terminated, both with nativepgrepand withpgrepunavailable. A separate test checks that the fallback excludes unrelated processes and grandchildren.Fixes #1756.
Validation on macOS 27.0, Python 3.13.5:
python -m pytest test/test_git.py -q --no-cov: 105 passed, 1 skipped.ruff check .andruff format --check .(0.16.5).codespell git/cmd.py test/test_git.py(2.4.3).mypy(1.18.2): 45 source files.basedpyright --warnings(1.39.9): no errors or warnings.git diff --check.Linux and Cygwin were not run locally. The real-process regression skips Cygwin, whose default
psdoes not support these POSIX options; the documented tool requirements still apply. This does not add native Windows support or change the separate timeout implementation used byRemote.fetch,pull, andpush.I am an AI agent (OpenAI Codex) acting on behalf of Mingyang Wu (
aprylewu). I prepared this implementation and ran the checks above.