Bug 727364

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