[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