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

distutils.sysconfig.customize_compiler() overrides CFLAGS var with OPT var if CFLAGS env var is set #80416

Closed
vstinner opened this issue Mar 8, 2019 · 19 comments
Labels
3.7 only security fixes 3.8 only security fixes build The build process and cross-build

Comments

@vstinner
Copy link
Member

vstinner commented Mar 8, 2019

BPO 36235
Nosy @vstinner, @ned-deily, @koobs, @yan12125, @miss-islington
PRs
  • bpo-36235: Fix CFLAGS in distutils customize_compiler() #12236
  • [3.7] bpo-36235: Fix CFLAGS in distutils customize_compiler() (GH-12236) #12348
  • [2.7] bpo-36235: Fix CFLAGS in distutils customize_compiler() (GH-12236) #12349
  • bpo-36235: make test_customize_compiler more robust #12380
  • bpo-36235: Enhance distutils test_customize_compiler() #12403
  • [3.7] bpo-36235: Enhance distutils test_customize_compiler() (GH-12403) #12415
  • [2.7] bpo-36235: Enhance distutils test_customize_compiler() (GH-12403) #12417
  • [2.7] bpo-36235: Fix distutils test_customize_compiler() on macOS #12751
  • bpo-36235: Fix distutils test_customize_compiler() on macOS #12764
  • [3.7] bpo-36235: Fix distutils test_customize_compiler() on macOS (GH-12764) #12768
  • 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 = None
    closed_at = <Date 2019-04-11.01:07:51.548>
    created_at = <Date 2019-03-08.12:23:45.361>
    labels = ['3.8', 'build', '3.7']
    title = 'distutils.sysconfig.customize_compiler() overrides CFLAGS var with OPT var if CFLAGS env var is set'
    updated_at = <Date 2019-04-11.01:07:51.547>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-04-11.01:07:51.547>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-04-11.01:07:51.548>
    closer = 'vstinner'
    components = ['Build']
    creation = <Date 2019-03-08.12:23:45.361>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36235
    keywords = ['patch']
    message_count = 19.0
    messages = ['337468', '337505', '337990', '337994', '337995', '338051', '338199', '338229', '338252', '338254', '338264', '338265', '338267', '338268', '339785', '339795', '339859', '339899', '339901']
    nosy_count = 5.0
    nosy_names = ['vstinner', 'ned.deily', 'koobs', 'yan12125', 'miss-islington']
    pr_nums = ['12236', '12348', '12349', '12380', '12403', '12415', '12417', '12751', '12764', '12768']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue36235'
    versions = ['Python 2.7', 'Python 3.7', 'Python 3.8']

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 8, 2019

    When a C extension is built by distutils, distutils.sysconfig.customize_compiler() is used to configure compiler flags. Extract of the code:

    (cc, cxx, opt, cflags, ccshared, ldshared, shlib_suffix, ar, ar_flags) = \
        get_config_vars('CC', 'CXX', 'OPT', 'CFLAGS',
                        'CCSHARED', 'LDSHARED', 'SHLIB_SUFFIX', 'AR', 'ARFLAGS')
    
    ...
    if 'CFLAGS' in os.environ:
        cflags = opt + ' ' + os.environ['CFLAGS']
        ldshared = ldshared + ' ' + os.environ['CFLAGS']
    if 'CPPFLAGS' in os.environ:
        cpp = cpp + ' ' + os.environ['CPPFLAGS']
        cflags = cflags + ' ' + os.environ['CPPFLAGS']
        ldshared = ldshared + ' ' + os.environ['CPPFLAGS']
    ...
    

    If the CFLAGS environment variable is set, the 'CFLAGS' configuration variable is overriden with the 'OPT' configuration variable: cflags = opt + ...

    This bug has been fixed since 2013 in Fedora and RHEL by this patch:
    https://src.fedoraproject.org/rpms/python2/blob/master/f/00168-distutils-cflags.patch

    RHEL bug report:
    https://bugzilla.redhat.com/show_bug.cgi?id=849994

    I converted that patch to a pull request.

    @vstinner vstinner added 3.7 only security fixes 3.8 only security fixes build The build process and cross-build labels Mar 8, 2019
    @ned-deily
    Copy link
    Member

    This appears to be a duplicate of bpo-969718.

    @vstinner
    Copy link
    Member Author

    New changeset 86082c2 by Victor Stinner in branch 'master':
    bpo-36235: Fix CFLAGS in distutils customize_compiler() (GH-12236)
    86082c2

    @vstinner
    Copy link
    Member Author

    New changeset 37f6971 by Victor Stinner in branch '2.7':
    bpo-36235: Fix CFLAGS in distutils customize_compiler() (GH-12236) (GH-12349)
    37f6971

    @vstinner
    Copy link
    Member Author

    New changeset 6c0e0d1 by Victor Stinner in branch '3.7':
    bpo-36235: Fix CFLAGS in distutils customize_compiler() (GH-12236) (GH-12348)
    6c0e0d1

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Mar 16, 2019

    After this patch, test_distutils failed if $CPPFLAGS is not empty when building CPython. For example:

    $ export CPPFLAGS=-DFOO=BAR
    $ ./configure
    $ make
    $ ./python -m test test_distutils   
    Run tests sequentially
    0:00:00 load avg: 0.45 [1/1] test_distutils
    test test_distutils failed -- Traceback (most recent call last):
      File "/home/yen/Projects/cpython/Lib/distutils/tests/test_sysconfig.py", line 99, in test_customize_compiler
        self.assertEqual(comp.exes['compiler'], 'my_cc --sysconfig-cflags --mycflags')
    AssertionError: 'my_cc --sysconfig-cflags --mycflags -DFOO=BAR' != 'my_cc --sysconfig-cflags --mycflags'
    - my_cc --sysconfig-cflags --mycflags -DFOO=BAR
    ?                                    

    + my_cc --sysconfig-cflags --mycflags

    test_distutils failed

    == Tests result: FAILURE ==

    1 test failed:
    test_distutils

    Total duration: 2 sec 192 ms
    Tests result: FAILURE

    Tested with commit d2fdd1f.

    This is an issue as Arch Linux uses CPPFLAGS="-D_FORTIFY_SOURCE=2" for creating packages [1].

    [1] https://git.archlinux.org/svntogit/packages.git/tree/trunk/makepkg.conf?h=packages/pacman#n39

    @vstinner
    Copy link
    Member Author

    Chih-Hsuan Yen: After this patch, test_distutils failed if $CPPFLAGS is not empty when building CPython.

    Aha, more sysconfig configuration variables and environment variables should be mocked. I wrote PR 12403 to mock all variables. Would you mind to test and review it?

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Mar 18, 2019

    STINNER Victor: Thank you very much for that much better fix! I've tested it on Arch Linux. With that patch, test_distutils passes as usual. The pull request looks good, too. Just a question: I think it should be backported to 3.7 and 2.7 branches like #12236?

    @vstinner
    Copy link
    Member Author

    New changeset 72c7b37 by Victor Stinner in branch 'master':
    bpo-36235: Enhance distutils test_customize_compiler() (GH-12403)
    72c7b37

    @vstinner
    Copy link
    Member Author

    Just a question: I think it should be backported to 3.7 and 2.7 (...)

    Right, I wrote PRs for that. I didn't use GitHub labels because the bot is currently broken.

    @vstinner
    Copy link
    Member Author

    New changeset dd1cfef by Victor Stinner in branch '3.7':
    bpo-36235: Enhance distutils test_customize_compiler() (GH-12403) (GH-12415)
    dd1cfef

    @vstinner
    Copy link
    Member Author

    New changeset 8c380e9 by Victor Stinner in branch '2.7':
    bpo-36235: Enhance distutils test_customize_compiler() (GH-12403) (GH-12417)
    8c380e9

    @vstinner
    Copy link
    Member Author

    Thanks for the review, I merged my PR and the test enhancements.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Mar 18, 2019

    Oh, I didn't know the bot is not working for now. Thank you for making the test more robust!

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 9, 2019

    Oh, the test failed on Python 2.7 on macOS:

    x86-64 High Sierra 2.7
    https://buildbot.python.org/all/#/builders/140/builds/211

    ======================================================================
    FAIL: test_customize_compiler (distutils.tests.test_sysconfig.SysconfigTestCase)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/buildbot/buildarea/2.7.billenstein-sierra/build/Lib/distutils/tests/test_sysconfig.py", line 134, in test_customize_compiler
        'sc_cc -E')
    AssertionError: '/usr/bin/clang -E' != 'sc_cc -E'

    @vstinner vstinner reopened this Apr 9, 2019
    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 9, 2019

    New changeset 22de4ce by Victor Stinner in branch '2.7':
    bpo-36235: Fix distutils test_customize_compiler() on macOS (GH-12751)
    22de4ce

    @vstinner
    Copy link
    Member Author

    Good, my change fixed x86-64 High Sierra 2.7 buildbot:
    https://buildbot.python.org/all/#/builders/140/builds/216

    I wrote PR 12764 to forward-port the fix to the master branch.

    @vstinner
    Copy link
    Member Author

    New changeset a9bd892 by Victor Stinner in branch 'master':
    bpo-36235: Fix distutils test_customize_compiler() on macOS (GH-12764)
    a9bd892

    @miss-islington
    Copy link
    Contributor

    New changeset d9b25a2 by Miss Islington (bot) in branch '3.7':
    bpo-36235: Fix distutils test_customize_compiler() on macOS (GH-12764)
    d9b25a2

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 only security fixes 3.8 only security fixes build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants