◐ Shell
reader mode source ↗
Skip to content

bpo-35674: Expose posix_spawnp#11554

Merged
vstinner merged 3 commits into
python:masterfrom
nanjekyejoannah:spawnp
Jan 16, 2019
Merged

bpo-35674: Expose posix_spawnp#11554
vstinner merged 3 commits into
python:masterfrom
nanjekyejoannah:spawnp

Conversation

@nanjekyejoannah

@nanjekyejoannah nanjekyejoannah commented Jan 14, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

I have exposed posix_spawnp.

Co-Author: Victor Stinner vstinner@redhat.com

https://bugs.python.org/issue35674

@pablogsal pablogsal self-assigned this Jan 14, 2019
@pablogsal pablogsal self-requested a review January 14, 2019 14:23
@vstinner vstinner mentioned this pull request Jan 14, 2019
Closed
@vstinner

vstinner commented Jan 14, 2019

Copy link
Copy Markdown
Member

AppVeyor build failed

That's because of the #ifdef issue, see my comment.

@pablogsal

pablogsal commented Jan 14, 2019

Copy link
Copy Markdown
Member

Very good job Joannah! :)

I left some comments together with Victor's review. Ping me when you have addressed them.

@bedevere-bot

bedevere-bot commented Jan 14, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@nanjekyejoannah nanjekyejoannah force-pushed the spawnp branch 3 times, most recently from 87d6fed to e2a9603 Compare January 15, 2019 21:51
@vstinner
Fix py_posix_spawn() if HAVE_POSIX_SPAWNP is not defined …
1ad7ad8 
* Fix also indentation in posixmodule.c.
* Remove "if the *path* argument contains no directory" from
  os.posix_spawnp() doc
* Remove outdated comment from py_posix_spawn():
  /*[clinic end generated code: ...]*/
@vstinner vstinner dismissed serhiy-storchaka’s stale review January 16, 2019 13:19

Serhiy's comments have been addressed.

@vstinner

vstinner commented Jan 16, 2019

Copy link
Copy Markdown
Member

I checked Travis CI jobs. posix_spawnp is available on Linux and macOS jobs, but glibc is too old on the Linux for subprocess.

Linux:

# configure
checking for posix_spawn... yes
checking for posix_spawnp... yes

# pythoninfo
platform.libc_ver: glibc 2.19
subprocess._USE_POSIX_SPAWN: False

macOS:

# configure
checking for posix_spawn... yes
checking for posix_spawnp... yes

# pythoninfo
platform.platform: macOS-10.13.3-x86_64-i386-64bit
subprocess._USE_POSIX_SPAWN: True

@vstinner vstinner merged commit 92b8322 into python:master Jan 16, 2019
@vstinner

vstinner commented Jan 16, 2019

Copy link
Copy Markdown
Member

Well done @nanjekyejoannah :-) Thanks to Joannah for this nice enhancement and to all reviewers.

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

Reviewers

@pablogsal pablogsal pablogsal left review comments

@vstinner vstinner vstinner approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nanjekyejoannah @vstinner @pablogsal @bedevere-bot @serhiy-storchaka @the-knights-who-say-ni