Hide Forgot
Created attachment 516223 [details] Partial side-by-side diff of /usr/lib/python-2.x/distutils/sysconfig.py Description of problem: (Build-system regression between RHEL5 -> RHEL6) When building a custom python C or C++ module we use the python distutils module, part of the python rpm. The user creates a setup.py config file. To append flags to the compile line the user adds lines like this: from os import environ environ['CFLAGS']='-Wno-write-strings' # your options may vary This is supposed to get *merged* with all the flags that were originally used to build the python baseline. This works in the python 2.4 supplied with RHEL5. There is a regression in RHEL6. When the user specifies CFLAGS like this, some critical build flags are inhibited. Looking at the python source, this appears to be an obvious coding error on line 186 of /usr/lib/python2.6/distutils/sysconfig.py. Version-Release number of selected component (if applicable): python-2.6.6-20.el6.i686 (and all prior python-2.6 packages) How reproducible: Always Steps to Reproduce: 1. Use a distutils "setup.py" file to build a python module 2. Add a build flag using environ['CFLAGS'] = '-yourflaghere' 3. Build using: python setup.py build Actual results: Build errors, due to the lack of -fno-strict-aliasing flag. The CFLAGS should be *added* to the existing CFLAGS from the original build, but they result in a subset of the original flags. Expected results: No build errors, since CFLAGS is appended to the original python build string. Additional info: It looks like this error was introduced in python 2.5, according to this: http://bugs.python.org/issue969718 .. and it's propagated up into python 3.x Comparing /usr/lib/python2.6/distutils/sysconfig.py to the RHEL 5 version (python 2.4) the error in customize_compiler() is obvious (see attached side-by-side diff). In 2.4, the original 'OPT' flags gets 'BASECFLAGS' merged with it, then any user-provided optional CFLAGS and CPPFLAGS are merged with it to form the final cc_cmd. In 2.6, upstream mucked it up. Looking at what gets recorded in get_config_vars() 'OPT' and 'CFLAGS', it appears that CFLAGS is a super-set of OPT (all the OPT flags are duplicated at the end of CFLAGS). The problem happens at lines: 185 if 'CFLAGS' in os.environ: 186 cflags = opt + ' ' + os.environ['CFLAGS'] .. snip .. 193 cc_cmd = cc + ' ' + cflags Now if the user specifies CFLAGS, the original build's CFLAGS gets replaced with OPT. Which is a subset, most notably missing '-fno-strict-aliasing' which is fairly critical to ensure correct code compilation. This looks like a simple coding mistake; When modifying this section to use cflags instead of opt as the container for compile options, line 186 *should* have been: 186 cflags = cflags + ' ' + os.environ['CFLAGS'] .. or .. 186 cflags = cflags + ' ' + opt + ' ' + os.environ['CFLAGS'] At this point, python upstream doesn't care about the 2.6.x tree. I would be great if Red Hat would patch this bug in the RHEL 6 python.
Created attachment 516224 [details] Partial side-by-side diff of /usr/lib/python-2.x/distutils/sysconfig.py
Comment on attachment 516224 [details] Partial side-by-side diff of /usr/lib/python-2.x/distutils/sysconfig.py Python 2.4 on the left, 2.6 source on the right.
Thanks for filing this bug report. The usage of BASECFLAGS was originally added to distutils.sysconfig back in 2003 in: http://svn.python.org/view?view=revision&revision=31317 It was removed in 2005 with: http://svn.python.org/view?view=revision&revision=38848 So it looks like this change affects 2.5 tarballs onwards One possible workaround might be something like this in each setup.py, to get the desired append behavior: from distutils.sysconfig import get_config_var cv = get_config_var('CFLAGS')this contains environ['CFLAGS'] = cv + ' ' + '-yourflaghere' In particular, this does pull in the "-fno-strict-aliasing" flag. However, am I right in thinking that you have a number of such setup.py files? I'm looking at possible patches to distutils.sysconfig for this.
Created attachment 517911 [details] Possible candidate patch The attached candidate patch changes the handling of when CFLAGS is specified in the environment With this patch, if CFLAGS is in the environment, it regenerates the cflags based on: (BASECFLAGS from Makefile)[$CFLAGS from env](OPT from Makefile)(EXTRA_CFLAGS from Makefile) rather than appending them to OPT from the Makefile. Given that CFLAGS in the Makefile is currently: $(BASECFLAGS) [copy of OPT without the -DNDEBUG] $(OPT) $(EXTRA_CFLAGS) this would allow code that overrides CFLAGS to get the full build flags. Unlike the Debian patch referred to in http://bugs.python.org/issue969718 it doesn't directly try to add support for additional Makefile variables being overridden by environment variables (OPT, BASECFLAGS); this is an attempt to minimize the risk of this change. (Though parse_makefile() can already fall back to reading vars from the environment, if they aren't defined). I'm in two minds about this versus the more simple: - cflags = opt + ' ' + os.environ['CFLAGS'] + cflags = cflags + ' ' + os.environ['CFLAGS'] but need to think through the possible risks of either candidate change, in the light of possible pre-existing code that may set arbitrary combinations of BASECFLAGS, OPT, CFLAGS, EXTRA_CFLAGS in the build environment.
Great detective work looking up the change history! Actually, I have a single very simple setup.py: ---------------- environ["CFLAGS"] = "-Wno-write-string" module1 = Extension('psm', include_dirs = [src+'/inc'], library_dirs = [bin+'/lib'], libraries = ['psm','stdc++'], sources = ['psm_module.cc']) setup (name = 'psm', version = '1.0', description = 'PSM support', ext_modules = [module1]) ---------------- But our customers execute this build on RHEL3 .. RHEL6 and Fedora. The additional flag was just to suppress a compiler warning, in lieu of (char*) casting a bunch of static initializer strings (tuple KW lists). I came across CFLAGS here: http://docs.python.org/release/2.6.7/install/index.html?highlight=cflags Compiler flags can also be supplied through setting the CFLAGS environment variable. If set, the contents of CFLAGS will be added to the compiler flags specified in the Setup file. The problem with the proposed work-around is that it looks like it's not backward compatible; I'd have to check python versions then set CFLAGS accordingly. Awkward, but it should work. But there could be other folks out there that also used CFLAGS prior to RHEL6 that would hit this bug (showing some altruism :) ). It would be really nice to be able to span RHEL versions without version checking. The thing that I found confusing is the in python 2.6 sysconfig.py is that 'opt' is *only* used if CFLAGS is specified. Otherwise it's ignored. That's what led me to think that this was search&replace error in the patch - they missed changing this instance of 'opt' to 'cflags'. In the revised build process, it appears that CFLAGS is a super-set of OPT. The real question (that I don't have the background to answer) is what is the real intent of CFLAGS with respect to setup.py & distutils? Should it augment or replace the original CFLAGS? The python-docs suggest that it should be additive to the base set of flags. If get_config_vars('CFLAGS') now contains the full set of build flags, it seems that just appending os.environ['CFLAGS'] would be best. By re-creating CFLAGS, we're vulnerable to future changes in the python Makefile composition. If the way the Makefile assembles CFLAGS changes, then distutils might break again. Thanks for looking into this.
Created attachment 518113 [details] Alternate, simpler patch As far as patching RHEL, the simple solution seems best: - cflags = opt + ' ' + os.environ['CFLAGS'] + cflags = cflags + ' ' + os.environ['CFLAGS'] Case (i): a build that doesn't have CFLAGS set in the environment isn't affected. Case (ii): a build that does, changes: the compiler flags are changed from OPT + user-cflags to CFLAGS + cflags: Given that CFLAGS in the Makefile is currently: $(BASECFLAGS) [copy of OPT without the -DNDEBUG] $(OPT) $(EXTRA_CFLAGS) then for case (ii), the compilation flags change from: $(OPT) + $(user CFLAGS) to: $(BASECFLAGS) [copy of OPT without the -DNDEBUG] $(OPT) $(EXTRA_CFLAGS) + $(user CFLAGS) This means that the compilation flags gain: -fno-strict-aliasing along with various duplicates of other options. Both with and without the proposed change, a build that has OPT or BASECFLAGS in the environment isn't affected by them. As is, a build that has EXTRA_CFLAGS set in the env is affected by it until CFLAGS is set, which stops it taking effect. With this proposed change, EXTRA_CFLAGS takes effect regardless of whether CFLAGS is set. Arguably this is more correct.
(In reply to comment #6) > casting a bunch of static initializer strings (tuple KW lists). I came across > CFLAGS here: > http://docs.python.org/release/2.6.7/install/index.html?highlight=cflags > > Compiler flags can also be supplied through setting the CFLAGS > environment variable. If set, the contents of CFLAGS will be > added to the compiler flags specified in the Setup file. [...snip...] > The real question (that I don't have the background to answer) is what is the > real intent of CFLAGS with respect to setup.py & distutils? Should it augment > or replace the original CFLAGS? The python-docs suggest that it should be > additive to the base set of flags. If get_config_vars('CFLAGS') now contains Thanks for the documentation link! I'm inclined to regard those docs as describing the intent: that it should augment, rather than replacing.
Development Management has reviewed and declined this request. You may appeal this decision by reopening this request.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHBA-2011-1564.html