bpo-31904: disable os.popen and impacted test cases on VxWorks by pxinwr · Pull Request #21687 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits9 Checks0 Files changed

Conversation

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

[ Show hidden characters]({{ revealButtonHref }})

pxinwr

ZackerySpytz

@pxinwr pxinwr changed the titlebpo-41442: add unix_shell requirement checking for test_posix.PosixTester.test_getgroups bpo-31904: disable os.popen and impacted test cases on VxWorks

Nov 24, 2020

@pxinwr

@pxinwr

vstinner

Choose a reason for hiding this comment

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

Is this change a temporary workaround until subprocess is fixed on VxWorks, or is this skip supposed to be a temporary workaround?

If it's a temporary workaround, I am not sure if it's a good idea to document the skip.

"popen", "extsep"]
"extsep"]
VXWORKS = (sys.platform == 'vxworks')

Choose a reason for hiding this comment

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

This variable is only at a single place: you can remove it.

Choose a reason for hiding this comment

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

removed.

return None
if name == 'nt':
return returncode
if not VXWORKS:

Choose a reason for hiding this comment

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

I suggest: if sys.platform != 'vxworks':.

Choose a reason for hiding this comment

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

changed accordingly.

@pxinwr

@pxinwr

Is this change a temporary workaround until subprocess is fixed on VxWorks, or is this skip supposed to be a temporary workaround?

If it's a temporary workaround, I am not sure if it's a good idea to document the skip.

os.popen() runs cmd in shell, i.e. shell=True was set when calling subprocess.Popen(). VxWorks has no user space shell provided so that we can't create a new shell and run commands inside. For the subprocess module, VxWorks can't support features needing shell to run. So when I created this PR, I don't intend to be a temporary workaround.

vstinner

@pxinwr @vstinner

Co-authored-by: Victor Stinner vstinner@python.org

@pxinwr

@vstinner

adorilson pushed a commit to adorilson/cpython that referenced this pull request

Mar 13, 2021

@pxinwr @adorilson

@pxinwr pxinwr deleted the fix-issue-31904-shell branch

May 7, 2021 07:42