Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-29585: Fix sysconfig.get_config_var("PYTHONFRAMEWORK") #2483

Merged
merged 2 commits into from
Jun 29, 2017

Conversation

methane
Copy link
Member

@methane methane commented Jun 29, 2017

After GH-136 merged, PYTHONFRAMEWORK config variable are in Makefile and pyconfig.h.

When reading string variable from Makefile, quote is removed.
But when reading them from pyconfig.h, quote is not removed. And config variables in
pyconfig.h override config variables in Makefile.

So sysconfig.get_config_var("PYTHONFRAMEWORK") was changed '' to '""' in
non framework build, and 'PYTHON' to '"PYTHON"' in framework build.

I thought about unquoting string config variables in pyconfig.h, but there is one
string variables already and it may cause unintentional backward incompatibility.
(sysconfig.get_config_var('PY_FORMAT_SIZE_T') is '"z"').

So this pull request just rename PYTHONFRAMEWORK to _PYTHONFRAMEWORK
in pyconfig.h. Variables which name starts with _ is not exported to sysconfig.

methane added 2 commits June 29, 2017 14:44
`PYTHONFRAMEWORK` is defined in `Makefile` and it shoulnd't be used
in `pyconfig.h`.

`sysconfig.py --generate-posix-vars` reads config vars from Makefile
and `pyconfig.h`.  Conflicting variables should be avoided.

Especially, string config variables in Makefile are unquoted, but
in `pyconfig.h` are keep quoted.  So it should be private (starts with
underscore).
@methane
Copy link
Member Author

methane commented Jun 29, 2017

https://travis-ci.org/python/cpython/jobs/248237175

macOS test now passed.

@methane methane merged commit 6b42eb1 into python:master Jun 29, 2017
@methane methane deleted the fix-framework branch June 29, 2017 06:31
@vstinner
Copy link
Member

Thanks for fixing maxOS :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants