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.

Sign up for GitHub

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

Jump to bottom

build_ext: always import _CONFIG_VARS from distutils.sysconfig instead of sysconfig #572

Merged
merged 2 commits into from
May 19, 2016
Merged

build_ext: always import _CONFIG_VARS from distutils.sysconfig instead of sysconfig #572

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f47650
build_ext: always import _CONFIG_VARS from distutils.sysconfig instea…
May 3, 2016
59b774b
build_ext: move block customizing compiler for shared lib into a func…
May 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions setuptools/command/build_ext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@
except ImportError:
_build_ext = _du_build_ext

try:
# Python 2.7 or >=3.2
from sysconfig import _CONFIG_VARS
except ImportError:
from distutils.sysconfig import get_config_var
from distutils.sysconfig import get_config_var

Copy link
Member

Choose a reason for hiding this comment

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

This change seems to be moving in the wrong direction, conflicting with the intention of the author to support the newer, preferred module. I don't know what the implications are, but what would prevent someone from making the reverse change again?

I think this behavior needs a unit test in the test suite to capture the desired behavior.

Also, is it possible to allow the newer, preferred module to still be used, but always invoke the get_config_var? What is the relationship between sysconfig._CONFIG_VARS and distutils.sysconfig._config_vars?

Copy link
Author

@anthrotype anthrotype May 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for having a look.
Yes, you're right about the need for a unit test in the test suite.

Here the intent of the "original" author (before c04abca6 broke it by importing the newer module) was to import the global distutils.sysconfig._config_vars dictionary, not to get the values, but to temporarily set them before calling distutils.sysconfig.customize_compiler. This is only for darwin, where building dynamic load libraries requires special compiler flags.

The problem is the customize_compiler function uses the _config_vars which are defined in the distutils.sysconfig module, not the one from the new sysconfig module. Therefore, importing _CONFIG_VARS from the latter, and modifying them, does not have any effect on customize_compiler.

I'm not sure about the relationship between sysconfig._CONFIG_VARS and distutils.sysconfig._config_vars.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification. After re-reading the ticket, I see you also detailed much of this there. Sorry to ask you to repeat yourself.

An alternative to a unit test would be to have a function in build_ext which documents the intention, such that a subsequent change when reviewed would be clearly in violation of the stated intention.

get_config_var("LDSHARED") # make sure _config_vars is initialized
del get_config_var
from distutils.sysconfig import _config_vars as _CONFIG_VARS
get_config_var("LDSHARED") # make sure _config_vars is initialized
del get_config_var
from distutils.sysconfig import _config_vars as _CONFIG_VARS

have_rtld = False
use_stubs = False
Expand Down