This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: EXTRA_CFLAGS get overrided by CFLAGS_NODIST
Type: behavior Stage: patch review
Components: Build Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Dormouse759, cstratak, hroncok, vstinner
Priority: normal Keywords: patch

Created on 2019-07-19 11:49 by Dormouse759, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 15020 closed hroncok, 2019-07-30 10:13
Messages (8)
msg348168 - (view) Author: Marcel Plch (Dormouse759) * Date: 2019-07-19 11:49
Problem:
If you want to override CFLAGS by setting EXTRA_CFLAGS, they may have no effect if there are contrary flags in the CFLAGS_NODIST variable.

How to reproduce:
make CFLAGS_NODIST="-O2" EXTRA_CFLAGS="-Og"

If you look at GCC arguments, there is -O2 present *after* the -Og flag. This means -Og gets ignored.
msg348183 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2019-07-19 16:46
Could you provide more info, e.g. comparison between the proper and the erroneous output, as well as what it affects (test_gdb if I recall correctly)?
msg348296 - (view) Author: Marcel Plch (Dormouse759) * Date: 2019-07-22 12:43
I believe you mean this downstream issue: https://bugzilla.redhat.com/show_bug.cgi?id=1712977

That issue is but only a consequence of a bad flag handling.
The bad flag handling affects not only test_gdb on that specific architecture, but the entire optimization level on all architectures, hence causing inconveniences in debugging in general on all architectures. It's a pure chance that one test caught this specific case.

It might also cause inconveniences with other use-cases for EXTRA_CFLAGS, as they might get overridden by CFLAGS_NODIST.

You can get the erroneous output by running the reproducer. That is:

$./configure
...
$ make CFLAGS_NODIST="-O2" EXTRA_CFLAGS="-Og"
gcc -pthread -c -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall   -Og -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -O2 -I./Include/internal  -I. -I./Include    -DPy_BUILD_CORE -o Programs/python.o ./Programs/python.c

-Og from EXTRA_CFLAGS gets overridden by -O2 from CFLAGS_NODIST.
msg348734 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-30 10:51
Python has way too many variables to control compiler flags:
---
# Compiler options
OPT=		@OPT@
BASECFLAGS=	@BASECFLAGS@
BASECPPFLAGS=	@BASECPPFLAGS@
CONFIGURE_CFLAGS=	@CFLAGS@
# CFLAGS_NODIST is used for building the interpreter and stdlib C extensions.
# Use it when a compiler flag should _not_ be part of the distutils CFLAGS
# once Python is installed (Issue #21121).
CONFIGURE_CFLAGS_NODIST=@CFLAGS_NODIST@
# LDFLAGS_NODIST is used in the same manner as CFLAGS_NODIST.
# Use it when a linker flag should _not_ be part of the distutils LDFLAGS
# once Python is installed (bpo-35257)
CONFIGURE_LDFLAGS_NODIST=@LDFLAGS_NODIST@
CONFIGURE_CPPFLAGS=	@CPPFLAGS@
CONFIGURE_LDFLAGS=	@LDFLAGS@
# Avoid assigning CFLAGS, LDFLAGS, etc. so users can use them on the
# command line to append to these values without stomping the pre-set
# values.
PY_CFLAGS=	$(BASECFLAGS) $(OPT) $(CONFIGURE_CFLAGS) $(CFLAGS) $(EXTRA_CFLAGS)
PY_CFLAGS_NODIST=$(CONFIGURE_CFLAGS_NODIST) $(CFLAGS_NODIST) -I$(srcdir)/Include/internal
# Both CPPFLAGS and LDFLAGS need to contain the shell's value for setup.py to
# be able to build extension modules using the directories specified in the
# environment variables
PY_CPPFLAGS=	$(BASECPPFLAGS) -I. -I$(srcdir)/Include $(CONFIGURE_CPPFLAGS) $(CPPFLAGS)
PY_LDFLAGS=	$(CONFIGURE_LDFLAGS) $(LDFLAGS)
PY_LDFLAGS_NODIST=$(CONFIGURE_LDFLAGS_NODIST) $(LDFLAGS_NODIST)
NO_AS_NEEDED=	@NO_AS_NEEDED@
SGI_ABI=	@SGI_ABI@
CCSHARED=	@CCSHARED@
# LINKFORSHARED are the flags passed to the $(CC) command that links
# the python executable -- this is only needed for a few systems
LINKFORSHARED=	@LINKFORSHARED@
ARFLAGS=	@ARFLAGS@
# Extra C flags added for building the interpreter object files.
CFLAGSFORSHARED=@CFLAGSFORSHARED@
# C flags used for building the interpreter object files
PY_STDMODULE_CFLAGS= $(PY_CFLAGS) $(PY_CFLAGS_NODIST) $(PY_CPPFLAGS) $(CFLAGSFORSHARED)
PY_BUILTIN_MODULE_CFLAGS= $(PY_STDMODULE_CFLAGS) -DPy_BUILD_CORE_BUILTIN
PY_CORE_CFLAGS=	$(PY_STDMODULE_CFLAGS) -DPy_BUILD_CORE
# Linker flags used for building the interpreter object files
PY_CORE_LDFLAGS=$(PY_LDFLAGS) $(PY_LDFLAGS_NODIST)
# Strict or non-strict aliasing flags used to compile dtoa.c, see above
CFLAGS_ALIASING=@CFLAGS_ALIASING@
---

I'm not sure which variables are "standard" and supposed to be overriden or not, because there is no documentation. The priority because these variables is not documented at all. For example:

* What the priority between BASECFLAGS, CFLAGS, OPT and EXTRA_FLAGS?
* Which flags are used to build Python core?
* Which flags are used to build modules of the standard library?
* Which flags are used to build third-party modules?

Linker flags are not documented neither which caused a regression for a short time (issue ith BLDSHARED if I recall correctly).

Examples of variables related to linker flags:

---
# LDFLAGS_NODIST is used in the same manner as CFLAGS_NODIST.
# Use it when a linker flag should _not_ be part of the distutils LDFLAGS
# once Python is installed (bpo-35257)
CONFIGURE_LDFLAGS_NODIST=@LDFLAGS_NODIST@
CONFIGURE_LDFLAGS=	@LDFLAGS@

PY_LDFLAGS=	$(CONFIGURE_LDFLAGS) $(LDFLAGS)
PY_LDFLAGS_NODIST=$(CONFIGURE_LDFLAGS_NODIST) $(LDFLAGS_NODIST)
PY_CORE_LDFLAGS=$(PY_LDFLAGS) $(PY_LDFLAGS_NODIST)

LDSHARED=	@LDSHARED@ $(PY_LDFLAGS)
BLDSHARED=	@BLDSHARED@ $(PY_CORE_LDFLAGS)
---
msg348737 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2019-07-30 12:55
I agree that documenting the flags is quite important, I've had a hard time trying to figure out how to implement the LDFLAGS_NODIST, and the change still broke macos builds (luckily it was fixed swiftly).

Nevertheless, this is still a bug which should be addressed.

On another note, where would be the best place to start documenting those flags? Makefile, the python docs, somewhere else?
msg348739 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-30 13:40
IMHO comments in Makefile.pre.in is a good start.
msg353781 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2019-10-02 22:24
Dug a bit further here.

The issue is that CFLAGS_NODIST will always come after normal CFLAGS (which are subsets of PY_CFLAGS and PY_CFLAGS_NODIST) [0][1].

The EXTRA_CFLAGS variable is appended at the end of PY_CFLAGS [2], hence as reported here, whatever compiler flag comes after, embedded within PY_CFLAGS_NODIST, will override the previous flags if they contradict each other.

So basically EXTRA_CFLAGS can be used only for flags that can't be overwritten by PY_CFLAGS_NODIST, which in my opinion, is not very useful in the context of just adding extra flags.

Commit adding the variable to Python 2.5: https://github.com/python/cpython/commit/08cd598c2145d00f1517c93cabf80a5d7d2a4bc0

"EXTRA_CFLAGS has been introduced as an environment variable to hold compiler flags that change binary compatibility"

Apparently it was added in order to avoid using the OPT variable for the various debug builds described in https://github.com/python/cpython/blob/master/Misc/SpecialBuilds.txt

On another note this flag will get passed down to distutils, so if it was used for building the interpreter, C extensions compiled by users will also inherit it.

Honestly I am not sure what the best solution would be here. If the various sub-debug special builds are still relevant and they stack, by doing for example $ make CFLAGS_NODIST="-DPy_TRACE_REFS" EXTRA_CFLAGS="-DPy_REF_DEBUG" then the issue can be closed, and the documentation can be more explicit that the EXTRA_CFLAGS is to be used only for the special builds.

If the EXTRA_CFLAGS can also be used for adding our own flags, then the flag handling needs to change to take this into account.

[0] https://github.com/python/cpython/blob/master/setup.py#L85
[1] https://github.com/python/cpython/blob/master/Makefile.pre.in#L115
[2] https://github.com/python/cpython/blob/master/Makefile.pre.in#L97
msg353785 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2019-10-02 22:53
Also this is due to an expected behaviour from gcc. From the documentation:

"If you use multiple -O options, with or without level numbers, the last such option is the one that is effective. "
History
Date User Action Args
2022-04-11 14:59:18adminsetgithub: 81812
2019-10-03 16:57:32brett.cannonsetnosy: - brett.cannon
2019-10-02 22:53:55cstrataksetmessages: + msg353785
2019-10-02 22:24:47cstrataksetnosy: + brett.cannon
messages: + msg353781
2019-07-30 13:40:04vstinnersetmessages: + msg348739
2019-07-30 12:55:50cstrataksetmessages: + msg348737
2019-07-30 10:51:47vstinnersetnosy: + vstinner
messages: + msg348734
2019-07-30 10:13:59hroncoksetnosy: + hroncok
2019-07-30 10:13:34hroncoksetkeywords: + patch
stage: patch review
pull_requests: + pull_request14781
2019-07-22 12:43:48Dormouse759setmessages: + msg348296
2019-07-19 16:46:40cstrataksetnosy: + cstratak
messages: + msg348183
2019-07-19 11:49:12Dormouse759create