Skip to content

Conversation

@uellue
Copy link

@uellue uellue commented Mar 20, 2019

I encountered a situation where sys.argv wasn't set when using Python as an embedded interpreter. The proposed update will avoid an AttributeError in that case. If sys.argv is set, it should not change behavior.

There's discussions related to correct behavior in this case:
https://bugs.python.org/issue32573

Is this proposed update to spawn.py the correct fix, or should the embedded case be handled differently, somehow?

https://bugs.python.org/issue32573

I encountered a situation where `sys.argv` wasn't set when using Python as an embedded interpreter. The proposed update will avoid an AttributeError in that case. If sys.argv is set, it should not change behavior.

There's discussions related to correct behavior in this case: 
https://bugs.python.org/issue32573

Is this proposed update to spawn.py the correct fix, or should the embedded case be handled differently, somehow?
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@uellue uellue changed the title Handle the case when sys.argv is not set bop-32573 Handle the case when sys.argv is not set Mar 20, 2019
uellue added a commit to uellue/LiberTEM that referenced this pull request Mar 20, 2019
sys.argv is not set in embedded interpreters https://bugs.python.org/issue32573

This workaround avoids trouble in the multiprocessing module. python/cpython#12463
sk1p pushed a commit to LiberTEM/LiberTEM that referenced this pull request Mar 20, 2019
sys.argv is not set in embedded interpreters https://bugs.python.org/issue32573

This workaround avoids trouble in the multiprocessing module. python/cpython#12463
@csabella csabella changed the title bop-32573 Handle the case when sys.argv is not set bpo-32573: Handle the case when sys.argv is not set May 11, 2019
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

In Python 3.8, sys.argv always exist: https://bugs.python.org/issue32573#msg342342

@bedevere-bot
Copy link

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.

@uellue
Copy link
Author

uellue commented May 23, 2019

Would it make sense to include the workaround and/or setting sys.argv in Python 3.7, or will it only be updated in 3.8?

@vstinner
Copy link
Member

Would it make sense to include the workaround and/or setting sys.argv in Python 3.7, or will it only be updated in 3.8?

I'm working on a whole PEP for Python 3.8: https://www.python.org/dev/peps/pep-0587/

For Python 3.7, I'm not sure that it's ok to change the behavior, but I just wrote: PR #13553.

@csabella
Copy link
Contributor

This was fixed as of 3.8 and won't be backported to earlier releases.

@csabella csabella closed this Dec 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants