Skip to content

bpo-32232: by default, Setup modules are no longer built with -DPy_BUILD_CORE #6489

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

Merged
merged 2 commits into from
Apr 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ ARFLAGS= @ARFLAGS@
# Extra C flags added for building the interpreter object files.
CFLAGSFORSHARED=@CFLAGSFORSHARED@
# C flags used for building the interpreter object files
PY_CORE_CFLAGS= $(PY_CFLAGS) $(PY_CFLAGS_NODIST) $(PY_CPPFLAGS) $(CFLAGSFORSHARED) -DPy_BUILD_CORE
PY_STDMODULE_CFLAGS= $(PY_CFLAGS) $(PY_CFLAGS_NODIST) $(PY_CPPFLAGS) $(CFLAGSFORSHARED)
PY_CORE_CFLAGS= $(PY_STDMODULE_CFLAGS) -DPy_BUILD_CORE
# Strict or non-strict aliasing flags used to compile dtoa.c, see above
CFLAGS_ALIASING=@CFLAGS_ALIASING@

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
By default, modules configured in `Modules/Setup` are no longer built with
`-DPy_BUILD_CORE`. Instead, modules that specifically need that preprocessor
definition include it in their individual entries.
38 changes: 19 additions & 19 deletions Modules/Setup.dist
Original file line number Diff line number Diff line change
Expand Up @@ -104,33 +104,33 @@ PYTHONPATH=$(COREPYTHONPATH)
# This only contains the minimal set of modules required to run the
# setup.py script in the root of the Python source tree.

posix posixmodule.c # posix (UNIX) system calls
errno errnomodule.c # posix (UNIX) errno values
pwd pwdmodule.c # this is needed to find out the user's home dir
# if $HOME is not set
_sre _sre.c # Fredrik Lundh's new regular expressions
_codecs _codecsmodule.c # access to the builtin codecs and codec registry
_weakref _weakref.c # weak references
_functools _functoolsmodule.c # Tools for working with functions and callable objects
_operator _operator.c # operator.add() and similar goodies
_collections _collectionsmodule.c # Container types
_abc _abc.c # Abstract base classes
itertools itertoolsmodule.c # Functions creating iterators for efficient looping
atexit atexitmodule.c # Register functions to be run at interpreter-shutdown
_signal signalmodule.c
_stat _stat.c # stat.h interface
time timemodule.c # -lm # time operations and variables
_thread _threadmodule.c # low-level threading interface
posix -DPy_BUILD_CORE posixmodule.c # posix (UNIX) system calls
errno errnomodule.c # posix (UNIX) errno values
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously errnomodule.c does not need to be compiled with -DPy_BUILD_CORE. But is it possible that other modules in this list that build correctly now with a standard linux platform without -DPy_BUILD_CORE, may need to be build with -DPy_BUILD_CORE given some specific set of macros in pyconfig.h ? Maybe it is safer then to use -DPy_BUILD_CORE for all modules currently statically built in the interpreter (and leave the commented out modules unchanged as they are built without -DPy_BUILD_CORE in setup.py).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can treat those cases as bug reports as folks using custom pyconfig.h files start looking at building 3.7 based versions. (Of all the ones listed, the main one I'm surprised was able to build without -DPy_BUILD_CORE is _weakref).

The other case where I think it would make sense to reconsider omitting the setting is if this change turns out to be a performance regression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant that on another platform than linux, the configure stage may define a macro in pyconfig.h that causes one of the modules where -DPy_BUILD_CORE is not used in the current PR (because the build does not fail for this module on linux) to require direct access to the interpreter internal for this platform. Maybe this is far-fetched.

Copy link
Contributor

Choose a reason for hiding this comment

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

Noting the resolution of this here: https://bugs.python.org/issue32232#msg315515 (Xavier checked the modules for symbols defined in pyconfig.h.in, and this concern turns out not to apply in practice)

pwd pwdmodule.c # this is needed to find out the user's home dir
# if $HOME is not set
_sre _sre.c # Fredrik Lundh's new regular expressions
_codecs _codecsmodule.c # access to the builtin codecs and codec registry
_weakref _weakref.c # weak references
_functools -DPy_BUILD_CORE _functoolsmodule.c # Tools for working with functions and callable objects
_operator _operator.c # operator.add() and similar goodies
_collections _collectionsmodule.c # Container types
_abc _abc.c # Abstract base classes
itertools itertoolsmodule.c # Functions creating iterators for efficient looping
atexit atexitmodule.c # Register functions to be run at interpreter-shutdown
_signal -DPy_BUILD_CORE signalmodule.c
_stat _stat.c # stat.h interface
time -DPy_BUILD_CORE timemodule.c # -lm # time operations and variables
_thread -DPy_BUILD_CORE _threadmodule.c # low-level threading interface

# access to ISO C locale support
_locale _localemodule.c # -lintl

# Standard I/O baseline
_io -I$(srcdir)/Modules/_io _io/_iomodule.c _io/iobase.c _io/fileio.c _io/bytesio.c _io/bufferedio.c _io/textio.c _io/stringio.c
_io -DPy_BUILD_CORE -I$(srcdir)/Modules/_io _io/_iomodule.c _io/iobase.c _io/fileio.c _io/bytesio.c _io/bufferedio.c _io/textio.c _io/stringio.c

# The zipimport module is always imported at startup. Having it as a
# builtin module avoids some bootstrapping problems and reduces overhead.
zipimport zipimport.c
zipimport -DPy_BUILD_CORE zipimport.c

# faulthandler module
faulthandler faulthandler.c
Expand Down
2 changes: 1 addition & 1 deletion Modules/makesetup
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ sed -e 's/[ ]*#.*//' -e '/^[ ]*$/d' |
case $doconfig in
no) cc="$cc \$(CCSHARED) \$(PY_CFLAGS) \$(PY_CPPFLAGS)";;
*)
cc="$cc \$(PY_CORE_CFLAGS)";;
cc="$cc \$(PY_STDMODULE_CFLAGS)";;
esac
rule="$obj: $src; $cc $cpps -c $src -o $obj"
echo "$rule" >>$rulesf
Expand Down