[Merge] ~mitchdz/ubuntu/+source/python2.7:mitch/python2.7_flag_opt into ubuntu/+source/python2.7:ubuntu/jammy-devel

Lena Voytek mp+458533 at code.launchpad.net
Tue Jan 30 18:28:44 UTC 2024


This patch looks great, and so do the tests. I just have 1 suggestion and a diff comment. Since this is a full patch and not just a simple diff from upstream, I recommend naming it lp2002043-add-optimization-flags-to-cflags.patch rather than lp2002043-add-optimization-flags-to-cflags.diff. That's just a nitpick though and not a necessary change. Overall LGTM, I can sponsor whenever you'd like.

Diff comments:

> diff --git a/debian/patches/lp2002043-add-optimization-flags-to-cflags.diff b/debian/patches/lp2002043-add-optimization-flags-to-cflags.diff
> new file mode 100644
> index 0000000..0d14998
> --- /dev/null
> +++ b/debian/patches/lp2002043-add-optimization-flags-to-cflags.diff
> @@ -0,0 +1,59 @@
> +Subject: Add optimization to cflags in sysconfig.py when building C modules.
> +
> +When compiling C code using python, the resulting binary is not being optimized
> +properly. Currently this is caused by the 'OPT' variable never being imported or
> +used in distutils/sysconfig.py and not being appended to the cflags variable.
> +This patch fixes the issue by importing 'OPT' and appending it to 'cflags'.
> +
> +Author: Mitchell Dzurick <mitchell.dzurick at canonical.com>
> +Date: Sun, 15 Oct 2023 17:50:49 +0300
> +Bug-Ubuntu: https://bugs.launchpad.net/bugs/2002043
> +--- a/Lib/distutils/sysconfig.py
> ++++ b/Lib/distutils/sysconfig.py
> +@@ -190,12 +190,22 @@
> +                 _osx_support.customize_compiler(_config_vars)
> +                 _config_vars['CUSTOMIZED_OSX_COMPILER'] = 'True'
> + 
> +-        (cc, cxx, cflags, extra_cflags, basecflags,
> +-         ccshared, ldshared, so_ext, ar, ar_flags,
> +-         configure_cppflags, configure_cflags, configure_ldflags) = \
> +-            get_config_vars('CC', 'CXX', 'CFLAGS', 'EXTRA_CFLAGS', 'BASECFLAGS',
> +-                            'CCSHARED', 'LDSHARED', 'SO', 'AR', 'ARFLAGS',
> +-                            'CONFIGURE_CPPFLAGS', 'CONFIGURE_CFLAGS', 'CONFIGURE_LDFLAGS')
> ++        lp2002043_ubuntu_cflags_workaround = 'APPLY_LP2002043_UBUNTU_CFLAGS_WORKAROUND' in os.environ
> ++
> ++        if (lp2002043_ubuntu_cflags_workaround):
> ++            (cc, cxx, cflags, extra_cflags, basecflags, opt,
> ++            ccshared, ldshared, so_ext, ar, ar_flags,
> ++            configure_cppflags, configure_cflags, configure_ldflags) = \
> ++                get_config_vars('CC', 'CXX', 'CFLAGS', 'EXTRA_CFLAGS', 'BASECFLAGS', 'OPT',
> ++                                'CCSHARED', 'LDSHARED', 'SO', 'AR', 'ARFLAGS',
> ++                                'CONFIGURE_CPPFLAGS', 'CONFIGURE_CFLAGS', 'CONFIGURE_LDFLAGS')
> ++        else:
> ++            (cc, cxx, cflags, extra_cflags, basecflags,
> ++            ccshared, ldshared, so_ext, ar, ar_flags,
> ++            configure_cppflags, configure_cflags, configure_ldflags) = \
> ++                get_config_vars('CC', 'CXX', 'CFLAGS', 'EXTRA_CFLAGS', 'BASECFLAGS',
> ++                                'CCSHARED', 'LDSHARED', 'SO', 'AR', 'ARFLAGS',
> ++                                'CONFIGURE_CPPFLAGS', 'CONFIGURE_CFLAGS', 'CONFIGURE_LDFLAGS')
> + 
> +         if 'CC' in os.environ:
> +             newcc = os.environ['CC']
> +@@ -228,7 +238,16 @@
> +             cflags = ' '.join(str(x) for x in (basecflags, os.environ['CFLAGS'], extra_cflags) if x)
> +             ldshared = ldshared + ' ' + os.environ['CFLAGS']
> +         elif configure_cflags:
> +-            cflags = ' '.join(str(x) for x in (basecflags, configure_cflags, extra_cflags) if x)
> ++            print("There exists a bug where optimization flags from the environment are not used.")
> ++            print("There is an opt-in workaround to inherit optimization flags. To use this")
> ++            print("workaround, make sure APPLY_LP2002043_UBUNTU_CFLAGS_WORKAROUND is set in your")
> ++            print("environment. See LP: #2002043 for further context.")

It would be good to also provide the short URL to the bug here - https://launchpad.net/bugs/2002043

> ++            if (lp2002043_ubuntu_cflags_workaround):
> ++                print("APPLY_LP2002043_UBUNTU_CFLAGS_WORKAROUND detected, using workaround for compiler flag optimizations.")
> ++                cflags = ' '.join(str(x) for x in (basecflags, opt, configure_cflags, extra_cflags) if x)
> ++            else:
> ++                print("APPLY_LP2002043_UBUNTU_CFLAGS_WORKAROUND not detected.")
> ++                cflags = ' '.join(str(x) for x in (basecflags, configure_cflags, extra_cflags) if x)
> +             ldshared = ldshared + ' ' + configure_cflags
> +         if 'CPPFLAGS' in os.environ:
> +             cpp = cpp + ' ' + os.environ['CPPFLAGS']


-- 
https://code.launchpad.net/~mitchdz/ubuntu/+source/python2.7/+git/python2.7/+merge/458533
Your team Ubuntu Sponsors is requested to review the proposed merge of ~mitchdz/ubuntu/+source/python2.7:mitch/python2.7_flag_opt into ubuntu/+source/python2.7:ubuntu/jammy-devel.




More information about the Ubuntu-sponsors mailing list