Skip to content

gh-82616: Add process_group support to subprocess.Popen#23930

Merged
gpshead merged 18 commits into
python:mainfrom
gpshead:subprocess-issue38435
May 5, 2022
Merged

gh-82616: Add process_group support to subprocess.Popen#23930
gpshead merged 18 commits into
python:mainfrom
gpshead:subprocess-issue38435

Conversation

@gpshead

@gpshead gpshead commented Dec 25, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Adds a process_group= parameter to subprocess APIs to help us deprecate
uses of preexec_fn.

https://bugs.python.org/issue38435

@gpshead
Add setpgid support to subprocess.POpen
1454088 
Adds a setpgid parameter to subprocess APIs to help us deprecate
uses of preexec_fn.
@gpshead gpshead added the type-feature A feature request or enhancement label Dec 25, 2020
@gpshead gpshead changed the title Add setpgid support to subprocess.POpen bpo-38435: Add setpgid support to subprocess.POpen Dec 25, 2020
@gpshead gpshead self-assigned this Dec 25, 2020
@gpshead gpshead marked this pull request as draft December 25, 2020 04:42
@gpshead gpshead marked this pull request as ready for review December 25, 2020 04:51
@gpshead gpshead requested a review from vstinner December 25, 2020 05:23
ZackerySpytz
ZackerySpytz reviewed Dec 25, 2020
View reviewed changes

@ZackerySpytz ZackerySpytz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is subprocess.Popen, not subprocess.POpen. Please fix the title and the news entry.

Comment thread Misc/NEWS.d/next/Library/2020-12-24-19-11-53.bpo-38435.rEHTAR.rst Outdated
Comment thread Doc/library/subprocess.rst Outdated
Comment thread Modules/_posixsubprocess.c
@serhiy-storchaka serhiy-storchaka changed the title bpo-38435: Add setpgid support to subprocess.POpen bpo-38435: Add setpgid support to subprocess.Popen Dec 29, 2020
serhiy-storchaka
serhiy-storchaka reviewed Dec 29, 2020
View reviewed changes
Comment thread Doc/library/subprocess.rst Outdated
Comment thread Doc/library/subprocess.rst Outdated
Comment thread Doc/library/subprocess.rst Outdated
@github-actions

github-actions Bot commented Jan 29, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jan 29, 2021
gpshead and others added 7 commits May 5, 2022 10:59
@gpshead
Merge branch 'main' into subprocess-issue38435
645256e
@gpshead @ZackerySpytz
Add missing \ in Doc/library/subprocess.rst
b58758b 
Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@gpshead @serhiy-storchaka
better ReST in Doc/library/subprocess.rst
acba394 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@gpshead @serhiy-storchaka
improve formatting in Doc/library/subprocess.rst
cef3bbe 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@gpshead @serhiy-storchaka
improve formatting in Doc/library/subprocess.rst
e594a01 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@gpshead
better formatting
ab9f0a5
@gpshead
versionadded 3.11
3b3a5d0
@gpshead gpshead added stdlib Standard Library Python modules in the Lib/ directory extension-modules C modules in the Modules dir and removed stale Stale PR or inactive for long period of time. labels May 5, 2022
@gpshead gpshead changed the title bpo-38435: Add setpgid support to subprocess.Popen gh-82616: Add setpgid support to subprocess.Popen May 5, 2022
@gpshead gpshead changed the title gh-82616: Add setpgid support to subprocess.Popen gh-82616: Add process_group support to subprocess.Popen May 5, 2022
serhiy-storchaka
serhiy-storchaka reviewed May 5, 2022
View reviewed changes
Comment thread Doc/library/subprocess.rst Outdated
@gpshead

gpshead commented May 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

The Address sanitizer failure blocking this makes no logical sense. I cannot reproduce it on my systems because an address sanitizer build doesn't even work at all.

Update: Zach Ware pointed out that I also needed to export ASAN_OPTIONS=detect_leaks=0:allocator_may_return_null=1:handle_segv=0 for that run. With that I can get a functioning ASAN build at least, but am still unable to reproduce the issue.
Update 2: You need an ubuntu 20.04 gcc 9.4 based system to reproduce it. anything more modern does not. regardless, the asan failure didn't make sense. I'm having those tests skipped under asan.

@gpshead gpshead requested review from 1st1 and asvetlov as code owners May 5, 2022 22:15
@gpshead gpshead merged commit f6dd14c into python:main May 5, 2022
@gpshead gpshead deleted the subprocess-issue38435 branch May 5, 2022 23:22
@gpshead gpshead mentioned this pull request May 5, 2022
Open
@vstinner

vstinner commented May 5, 2022

Copy link
Copy Markdown
Member

(post-merge review ;-)) LGTM.

It might help to add a build check (C11 static_assert()) that the pid_t type is unsigned. A macro like that can be used:

#define IS_TYPE_UNSIGNED(type) (((type)0 - 1) > 0)

@JelleZijlstra JelleZijlstra mentioned this pull request May 7, 2022
Merged
JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request May 7, 2022
@JelleZijlstra
Add new arguments to subprocess.Poopen
07c8716 
Most recent one in python/cpython#23930. This makes me wish we
could have had type evaluation functions.
@mattip mattip mentioned this pull request Oct 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@vstinner vstinner Awaiting requested review from vstinner
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
+1 more reviewer
@ZackerySpytz ZackerySpytz ZackerySpytz left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@gpshead gpshead

Labels

extension-modules C modules in the Modules dir stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@gpshead @vstinner @serhiy-storchaka @ZackerySpytz @the-knights-who-say-ni @bedevere-bot