bpo-33456: site.py: fix wrong default for key in pyvenv.cfg

[meribold]

PEP 405 says this:

By default, a virtual environment is entirely isolated from the system-level site-packages directories.

If the pyvenv.cfg file also contains a key include-system-site-packages with a value of true (not case sensitive), the site module will also add the system site directories to sys.path after the virtual environment site directories.

The documentation of the site module says (emphasis added):

If "pyvenv.cfg" […] contains the key "include-system-site-packages" set to anything other than "false" (case-insensitive), the system-level prefixes will still also be searched for site-packages; otherwise they won’t.

However, what actually happens in site.py is different: see https://github.com/python/cpython/blob/3.6/Lib/site.py#L447. The system_site variable is initialized to "true" and doesn't change unless the key include-system-site-packages exists in pyvenv.cfg.

I think system_site should be initialized to "false" so the actual behavior matches the documented behavior.

https://bugs.python.org/issue33456

[the-knights-who-say-ni]

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 our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

When your account is ready, please add a comment in this pull request
and a Python core developer will remove the CLA not signed label
to make the bot check again.

Thanks again to your contribution and we look forward to looking at it!

[vsajip]

Given that no complaints have been received about the implemented behaviour, it would seem better to update the documentation to match the implemented behaviour rather than the other way around.

You can change the patch to update the documentation, if you like.

[meribold]

Yeah, I can do that. Should I also create a PR for the PEP?

[meribold]

Just for the record, I think my proposed change doesn't affect virtual environments created with the venv module, since the created pyvenv.cfg always appears to contain the include-system-site-packages key. I only became aware of the discrepancy between the documented and actual behavior of site.py when I played around with creating a virtual environment manually.

[nathacof]

Any reason we use a string here instead of a bool?

[vsajip]

It's parsed by C code during startup, so parsing facilities are limited.

[vsajip]

@meribold I don't believe you need to update the PEP. It was the vehicle for getting the feature approved for inclusion into Python, but the documentation of the feature takes precedence IMO.

[meribold]

I reverted my commit and changed the sentence in site.rst quoted above to describe the actual behavior. Should I also:

• Squash the two commits?
• Expand the commit message?
• Rebase my branch?