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.

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

BASECFLAGS are not passed to module build line #40367

Closed
vaxhacker mannequin opened this issue Jun 9, 2004 · 21 comments
Closed

BASECFLAGS are not passed to module build line #40367

vaxhacker mannequin opened this issue Jun 9, 2004 · 21 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@vaxhacker
Copy link
Mannequin

vaxhacker mannequin commented Jun 9, 2004

BPO 969718
Nosy @malemburg, @loewis, @birkenfeld, @ronaldoussoren, @mdickinson, @marienz, @pitrou, @vstinner, @tarekziade, @jwilk, @ned-deily, @merwok, @ambv, @koobs
Superseder
  • bpo-36235: distutils.sysconfig.customize_compiler() overrides CFLAGS var with OPT var if CFLAGS env var is set
  • Files
  • distutils_opt_flag.patch: Preliminary Patch
  • debian-sysconfig-flags.patch
  • sysconfig-flags-minimal.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tarekziade'
    closed_at = <Date 2019-03-18.17:34:44.770>
    created_at = <Date 2004-06-09.15:56:47.000>
    labels = ['type-bug', 'library']
    title = 'BASECFLAGS are not passed to module build line'
    updated_at = <Date 2019-03-18.17:34:44.769>
    user = 'https://bugs.python.org/vaxhacker'

    bugs.python.org fields:

    activity = <Date 2019-03-18.17:34:44.769>
    actor = 'vstinner'
    assignee = 'tarek'
    closed = True
    closed_date = <Date 2019-03-18.17:34:44.770>
    closer = 'vstinner'
    components = ['Distutils']
    creation = <Date 2004-06-09.15:56:47.000>
    creator = 'vaxhacker'
    dependencies = []
    files = ['19750', '20689', '20698']
    hgrepos = []
    issue_num = 969718
    keywords = ['patch']
    message_count = 21.0
    messages = ['21099', '21100', '21101', '21102', '93701', '121944', '127850', '127851', '128031', '128032', '128034', '128051', '128060', '128065', '128155', '128158', '128159', '200379', '200383', '200385', '338266']
    nosy_count = 20.0
    nosy_names = ['lemburg', 'loewis', 'georg.brandl', 'ronaldoussoren', 'mark.dickinson', 'vaxhacker', 'nyogtha', 'marienz', 'pitrou', 'vstinner', 'jyasskin', 'tarek', 'jwilk', 'ned.deily', 'eric.araujo', 'rpetrov', 'Arfrever', 'lambacck', 'lukasz.langa', 'koobs']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '36235'
    type = 'behavior'
    url = 'https://bugs.python.org/issue969718'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    @vaxhacker
    Copy link
    Mannequin Author

    vaxhacker mannequin commented Jun 9, 2004

    The value of BASECFLAGS from
    /prefix/lib/pythonver/config/Makefile is not present on
    the compile command for modules being built by
    distutils ("python setup.py build"). It seems that
    only the value of OPT is passed along.

    This is insufficient when BASECFLAGS contains
    "-fno-static-aliasing", since recent versions of gcc
    will emit incorrect (crashing) code if this flag is not
    provided, when compiling certain modules (the mx
    products from egenix, for example).

    I did try to set CFLAGS in my environment, as directed
    by documentation, but this also had zero effect on the
    final build command.

    @vaxhacker vaxhacker mannequin added stdlib Python modules in the Lib dir labels Jun 9, 2004
    @nyogtha
    Copy link
    Mannequin

    nyogtha mannequin commented Jan 13, 2006

    Logged In: YES
    user_id=1426882

    This is still a bug in Python 2.4.2.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 12, 2006

    Logged In: YES
    user_id=21627

    I don't think I will do anything about this anytime soon, so
    unassigning myself.

    @marienz
    Copy link
    Mannequin

    marienz mannequin commented Jan 26, 2007

    I'm seeing a variation of this bug in python 2.5.

    As far as I can tell in python 2.4.3 on linux it passes BASECFLAGS and OPT, appending CFLAGS from the environment to that if set. In python 2.5 it passes CFLAGS from the Makefile (which is defined as $(BASECFLAGS) $(OPT) $(EXTRA_CFLAGS)), or OPT and the CFLAGS from the environment if CFLAGS is set there (this change was made in revision 45232). That means that if you run setup.py with CFLAGS set they must include -fno-strict-aliasing if using python 2.5.

    I think it would be preferable to prepend BASECFLAGS instead of OPT if CFLAGS is set in the environment. On my linux machine after building python 2.5 with CFLAGS set to "-O2 -march=athlon-xp" the Makefile has:

    OPT= -DNDEBUG -g -O3 -Wall -Wstrict-prototypes
    BASECFLAGS= -fno-strict-aliasing
    CFLAGS= $(BASECFLAGS) $(OPT) $(EXTRA_CFLAGS)

    If I run a setup.py with CFLAGS unset it runs:

    gcc -pthread -fno-strict-aliasing -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC ...

    Which is reasonable. If I run it with CFLAGS="-O2 -march=athlon-xp":

    gcc -pthread -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -O2 -march=athlon-xp -fPIC ...

    Which misses -fno-strict-aliasing and still includes all the general flags that I'm trying to set through CFLAGS.

    If it used BASECFLAGS from the Makefile instead of OPT it would be:

    gcc -pthread -fno-strict-aliasing -O2 -march=athlon-xp -fPIC ...

    Which is what I think is the desired result here.

    @akitada akitada mannequin added type-bug An unexpected behavior, bug, or error labels Feb 11, 2009
    @lambacck
    Copy link
    Mannequin

    lambacck mannequin commented Oct 7, 2009

    I am running into a problem related to this. I am attempting to cross
    compile extensions but Fedora includes -march in the OPT variable. Since
    there is no way to exclude the OPT values the build fails.

    It seems that forcing OPT to stay the same is a disservice generally
    speaking since as Marien said you potentially get extra conflicting
    flags (-O seems like a stand-out since that could mess with gdb).

    2.6 and 3.1 say:
    if 'CFLAGS' in os.environ:
    cflags = opt + ' ' + os.environ['CFLAGS']
    ldshared = ldshared + ' ' + os.environ['CFLAGS']

    There is no ability to override opt. I'd be willing to put together a
    patch to allow opt to be overridden by an environment variable if I
    could get some support for this from a core maintainer.

    I am also open to other suggestions about how to get around this. I
    potentially have time to put into fixing this (or the more general cross
    compile issue).

    @lambacck
    Copy link
    Mannequin

    lambacck mannequin commented Nov 21, 2010

    I am attaching a preliminary patch to allow override of $(OPT). I am not sure this is sufficient, but am wary of breaking packages that depend on the existing behaviour.

    The logic indeed seems wrong, but maybe this is something that has to go in distutils2 rather than distutils.

    @merwok
    Copy link
    Member

    merwok commented Feb 4, 2011

    Pythons in Debian seem to be immune to this problem, thanks to this patch: http://deb.li/3ku1g (via http://lists.debian.org/debian-python/2010/12/msg00005.html).

    I haven’t had time to learn the intricacies of make variables yet, so I can’t approve a patch. I’m adding people from the nosy lists of other make variables bugs, hopefully someone will be able to review.

    @merwok
    Copy link
    Member

    merwok commented Feb 4, 2011

    Duplicate report from Stephan Krah:

    When CFLAGS are set, distutils drops -fno-strict-aliasing (among other
    flags):

    $ python2.7 setup.py build
    ...
    gcc -pthread -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I./src -I/usr/local/include -I/usr/local/include/python2.7 -c src/gmpy.c -o build/temp.linux-x86_64-2.7/src/gmpy.o
    ...
    
    $ CFLAGS="-fomit-frame-pointer" python2.7 setup.py build
    ...
    gcc -pthread -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fomit-frame-pointer -fPIC -I./src -I/usr/local/include -I/usr/local/include/python2.7 -c src/gmpy.c -o build/temp.linux-x86_64-2.7/src/gmpy.o
    src/gmpy.c: In function ‘_cmp_to_object’:
    src/gmpy.c:4692: warning: dereferencing type-punned pointer will break strict-aliasing rules
    ...

    I'm not sure if this is intentional. The documentation says:

    "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."

    To me, this sounds as if they should be appended to the regular flags.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Feb 5, 2011

    Éric, the Debian patch looks good to me and it solves my build problem.

    The only question I have is why EXTRA_CFLAGS still go behind CFLAGS
    and cannot be overridden via the environment.

    But as it is, the patch is an improvement. I'm attaching the version
    for 2.7.

    @pitrou
    Copy link
    Member

    pitrou commented Feb 5, 2011

    Why is OPT duplicated in get_config_vars(...)?
    Why do OPT and BASECFLAGS environ vars override their Makefile values instead of accumulating with them?
    Why is EXTRA_CFLAGS not configurable through environ?

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Feb 5, 2011

    Antoine Pitrou <report@bugs.python.org> wrote:

    Why is OPT duplicated in get_config_vars(...)?

    I missed that, thanks.

    Why do OPT and BASECFLAGS environ vars override their Makefile values
    instead of accumulating with them?

    I think it would go too far to append in three places. If the environment
    CFLAGS go to the end, everything can be overridden with a single variable.

    Why is EXTRA_CFLAGS not configurable through environ?

    I don't know. Ideally the Debian people would comment if they had any
    reasons for that. For me it would be sufficient if CFLAGS were configurable
    without deleting OPT, BASECFLAGS or EXTRA_CFLAGS.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Feb 6, 2011

    EXTRA_CFLAGS were removed in r38848. Is it necessary to configure
    OPT, BASECFLAGS and EXTRA_CFLAGS via the environment? I think it
    might be sufficient to take CFLAGS from the Makefile and append
    the environment CFLAGS.

    Chris, would the new minimal patch solve your problem, too?

    @lambacck
    Copy link
    Mannequin

    lambacck mannequin commented Feb 6, 2011

    I am not convinced that the minimal patch would work for my original issue. I wanted to be able to override the -march option which shows up in OPT on Fedora. I was cross-compiling to a target architecture that does not support the -march option so I would not be able to provide a different one as override.

    The proposed minimal patch would leave the OPT value and make it unchangeable because CFLAGS would pull out a value for OPT from the Makefile which shows as in my current Ubuntu system:

    OPT= -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes
    BASECFLAGS= -fno-strict-aliasing
    CFLAGS= $(BASECFLAGS) $(OPT) $(EXTRA_CFLAGS)

    The debian-sysconfig-flags looks the most correct because it allows override at any point by emulating the CFLAGS = $(BASECFLAGS) $(OPT) $(EXTRA_CFLAGS) logic.

    It still looks like it needs an override for EXTRA_CFLAGS and the logic for LD_SHARED looks wrong. In the Makefile LDSHARED is just:
    LDSHARED= $(CC) -shared -Wl,-O1 -Wl,-Bsymbolic-functions

    Why does LDSHARED need CFLAGS and CPPFLAGS? When provided as an override but not in the Makefile? Do we need to allow for this override in the case of overriding OPT, BASEFLAGS or EXTRA_CFLAGS?

    @pitrou
    Copy link
    Member

    pitrou commented Feb 6, 2011

    I am not convinced that the minimal patch would work for my original
    issue. I wanted to be able to override the -march option which shows
    up in OPT on Fedora. I was cross-compiling to a target architecture
    that does not support the -march option so I would not be able to
    provide a different one as override.

    I don't understand how you can cross-compile using the host Python
    Makefile. Could you explain?

    The proposed minimal patch would leave the OPT value and make it
    unchangeable because CFLAGS would pull out a value for OPT from the
    Makefile which shows as in my current Ubuntu system:

    OPT= -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes
    BASECFLAGS= -fno-strict-aliasing
    CFLAGS= $(BASECFLAGS) $(OPT) $(EXTRA_CFLAGS)

    The debian-sysconfig-flags looks the most correct because it allows
    override at any point by emulating the CFLAGS = $(BASECFLAGS) $(OPT)
    $(EXTRA_CFLAGS) logic.

    It still looks like it needs an override for EXTRA_CFLAGS

    EXTRA_CFLAGS is not defined in the Makefile so it would probably be
    taken from the environment anyway (if my understand of Makefiles is
    right). That seems to explain the name "EXTRA_CFLAGS", but I'm no
    specialist of the jungle that have building systems become under Unix.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Feb 7, 2011

    Chris Lambacher <report@bugs.python.org> wrote:

    I was cross-compiling to a target architecture that does not support
    the -march option so I would not be able to provide a different one
    as override.

    Just out of curiosity: You are cross-compiling a C-extension? How can
    you be sure that all values in pyconfig.h are correct?

    The debian-sysconfig-flags looks the most correct because it allows
    override at any point by emulating the CFLAGS = $(BASECFLAGS) $(OPT)
    $(EXTRA_CFLAGS) logic.

    It still looks like it needs an override for EXTRA_CFLAGS and the
    logic for LD_SHARED looks wrong. In the Makefile LDSHARED is just:
    LDSHARED= $(CC) -shared -Wl,-O1 -Wl,-Bsymbolic-functions

    I think Antoine is correct: EXTRA_CFLAGS show up as an empty string in
    sysconfig (even if passed to configure and make), so there is no need
    to override them.

    As an aside, why do EXTRA_CFLAGS still exist in py3k when CFLAGS
    can already be passed to configure and make?

    I agree that the ldshared logic looks wrong. It is possible to specify
    LDFLAGS via the environment, so why append CFLAGS. Changing this could
    break existing build scripts though.

    However, I don't see how the Debian patch could break anything, unless
    someone relies on the fact that BASECFLAGS are suppressed in case CFLAGS
    are given. I think the odds of this are virtually zero.

    Any interest in a cleaned up version of the Debian patch (remove
    double opt, line length, add documentation for overriding BASECFLAGS
    and OPT)?

    I'm asking, since I'm unsure about the degree of frozenness of distutils.

    @lambacck
    Copy link
    Mannequin

    lambacck mannequin commented Feb 7, 2011

    Antoine said:

    I don't understand how you can cross-compile using the host Python
    Makefile. Could you explain?

    The get_config_vars() function parses the host Makefile to get the values that we are discussing overriding.

    EXTRA_CFLAGS is not defined in the Makefile so it would probably be
    taken from the environment anyway (if my understand of Makefiles is
    right). That seems to explain the name "EXTRA_CFLAGS", but I'm no
    specialist of the jungle that have building systems become under Unix.

    You are correct, EXTRA_CFLAGS is not in the Makefile, but does provide us with an expected order of evaluation. i.e. Distutils is generating the equivalent command lines from the pieces and we should be respecting the Makefile order for consistency. Also, I don't know if there is a way to insert EXTRA_CFLAGS as an option to the ./configure script when Python is built such that it shows up in the resulting Makefile. If there is no way to get EXTRA_CFLAGS into the Makefile, then we should take it entirely out of the equation in the construction of the cflags var.

    Stephan Said:

    Just out of curiosity: You are cross-compiling a C-extension? How can
    you be sure that all values in pyconfig.h are correct?
    That is the whole point of this discussion, if you can sub out the build parameters, you can insert the path to the .h files for the embedded target before any host specific .h files.

    In order to build a C extension, you pretty much have to do it through Distutils and therefore the host python interpreter or else do a lot of extra work to get the right settings for each individual extension.
    See the following for some basic how-to info:
    http://whatschrisdoing.com/blog/2006/10/06/howto-cross-compile-python-25/
    http://whatschrisdoing.com/blog/2009/10/16/cross-compiling-python-extensions/

    @pitrou
    Copy link
    Member

    pitrou commented Feb 7, 2011

    Any interest in a cleaned up version of the Debian patch (remove
    double opt, line length, add documentation for overriding BASECFLAGS
    and OPT)?

    I think it would be nice, but it should have a test if possible.
    Otherwise there'll keep being regressions.

    @ned-deily
    Copy link
    Member

    Since I just noticed this and haven't seen it mentioned already: for the record, the Python Makefile for current versions is affected by this issue. The "sharedmods" target, which calls setup.py to build the standard library shared modules, explicitly passes into Distutils via shell variables values for CC, LDSHARED, and OPT. Unfortunately, as noted in this issue, Distutils does not look for an OPT variable. So, while CC and LDSHARED can be overridden from the make command with either macro arguments or env vars, OPT cannot: only the value determined at configure time will be used.

    I think that Chris's original distutils_opt_flag.patch should be applied to allow OPT to be overridden, without changing any other current behavior. AFAICT, the only compatibility issue would be if a script happened to already have an OPT env variable defined which would now get used by Distutils. I think the risks of that are pretty small and, in case, much smaller than the more extensive tweaking of Distutils behavior as is done in the Debian patches.

    My interest in this comes from discovering that the OS X installer build script has been overriding OPT for its own purposes, thereby inadvertently dropping compiler options determined by configure (things like -fwrapv) which can result in a broken interpreter. I've changed the installer build to no longer do that. But that does mean that users of the OS X installer will now see those missing compiler options during extension module builds and it is conceivable that could cause problems for some ext modules and there wouldn't be a simple way to work around them (e.g. by setting an OPT env value). If no one has any strong objections, I'll plan to commit that patch soon.

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Oct 19, 2013

    OPT should not be used in Distutils at all.

    Lib/distutils/sysconfig.py should have:
    if 'CFLAGS' in os.environ:
    cflags = os.environ['CFLAGS']

    Makefile.pre.in should have:
    $(RUNSHARED) CC='$(CC)' LDSHARED='$(BLDSHARED)' CFLAGS='$(PY_CFLAGS)' \
    _TCLTK_INCLUDES='$(TCLTK_INCLUDES)' _TCLTK_LIBS='$(TCLTK_LIBS)' \
    $(PYTHON_FOR_BUILD) $(srcdir)/setup.py $$quiet build

    PY_CFLAGS is defined as:
    PY_CFLAGS= $(BASECFLAGS) $(OPT) $(CONFIGURE_CFLAGS) $(CFLAGS) $(EXTRA_CFLAGS)

    So then OPT could be overriden when calling make.

    See my patch for Distutils in bug bpo-1222585. That patch also cleans handling of flags.

    @ned-deily
    Copy link
    Member

    Arfrever, your proposal is certainly one of many possible solutions and one that would be appropriate if we were designing the Python configure, Makefile, and Distutils components from scratch or looking at major changes to Distutils. But we're not at this point. We all know that Distutils is brittle and I think at this point in its life it is best to change as little as possible without really good reason. For better or worse, most distributors and package maintainers have figured out how to make Distutils do what they need to do until the next generation of package building starts being addressed post-3.4. The proposed patch solves a very specific problem with very little risk (IMO) to breaking anyone's current solutions.

    @vstinner
    Copy link
    Member

    This issue has been fixed by bpo-36235.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants