sage_build_cython: Do not rely on CC environment variable being set

[mkoeppe]

(from #30580):

src/sage_setup/command/sage_build_cython.py contains this code:

# Work around GCC-4.8 bug which miscompiles some sig_on() statements:
# * https://github.com/sagemath/sage-prod/issues/14460
# * https://github.com/sagemath/sage-prod/issues/20226
# * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56982
if subprocess.call("""$CC --version | grep -i 'gcc.* 4[.]8' >/dev/null """, shell=True) == 0:
    extra_compile_args.append('-fno-tree-copyrename')

This gives an (ignored) error when CC is not set -- which can happen if invoked outside of sage-env, for example when building an sdist.

$ python3 -u setup.py --no-user-cfg sdist >/tmp/res
/bin/sh: 1: --version: not found

We remove this code, which is obsolete after #33316 (Drop support for GCC < 6.3 in Sage 9.7)

CC: @dimpase @orlitzky @kiwifb

Component: build

Author: Matthias Koeppe

Branch/Commit: 805bf82

Reviewer: François Bissey

Issue created by migration from https://trac.sagemath.org/ticket/30876

[dimpase]

comment:1

What does happen if CC is not set, how is setup.py getting the compilers to use? Via distutils?
Can't CC be set in setup.py if needed?

[mkoeppe]

comment:2

Yes, distutils. So distutils should also be used to construct the command line for this version test.

[dimpase]

comment:3

it looks doable, but ugly as hell:

from distutils.ccompiler import new_compiler, get_default_compiler
new_compiler(get_default_compiler()).compile(["foo.c"])

and then

$ strings hello.o | grep GCC
GCC: (Gentoo 8.3.1-r2 p4) 8.3.1 20190518

allows version extraction.

Wouldn't it be easier to, e.g., parse the banner of Python?

$ python
Python 3.7.9 (default, Sep 15 2020, 15:01:07) 
[GCC 8.3.1 20190518] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> 

[mkoeppe]

comment:4

I think the call to gcc --version can be implemented in a simple and clean way in sage_build_ext, which has access to self.compiler.compiler

[dimpase]

comment:5

If I do

>>> cc=new_compiler(get_default_compiler())
>>> _=cc.compile(["foo.c"], extra_preargs=['-dumpversion'])
8.3.1

then I see the version value. So one can wrap this in a script.
(one can also use --version instead of -dumpversion for a longer output.)

[dimpase]

comment:6

I've also asked on stackoverflow, nobody knows...
https://stackoverflow.com/questions/64768024/finding-distutils-c-compiler-version

[mkoeppe]

comment:7

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

[orlitzky]

comment:9

We already reject gcc-4.7 and older in spkg-configure.m4. Would anyone notice if we rejected 4.8 (May, 2014) as well?

And if we do want to continue supporting gcc-4.8 from the system, couldn't we append -fno-tree-copyrename to CFLAGS to fix this? The optimization is buggy in 4.8.x so I don't think it's unjustified.

[mkoeppe]

comment:10

Replying to @orlitzky:

Would anyone notice if we rejected 4.8 (May, 2014) as well?

Yes, this is the compiler on ubuntu-trusty and centos-7, which are still widely used.

We are dropping support for a platform with gcc 4.9 (debian-jessie) in Sage 9.5 (for #25009) because it turns to be more broken than 4.8.

[mkoeppe]

comment:11

A trivial fix is to just replace $CC by ${CC-gcc}

[dimpase]

comment:12

nobody nowadays is using gcc 4 on Centos 7. There are system packages to get you gcc 9. cf. e.g. https://stackoverflow.com/a/67212990/557937

Same story with Ubuntu, more or less.
https://askubuntu.com/a/1149383/309919

We can and should drop gcc 4, without dropping OSs.

[mkoeppe]

comment:13

We can and should add variants of fedora/centos with devtoolset to our tox/GH Actions. So far this is neither tested nor documented anywhere.

[mkoeppe]

comment:14

I've opened #32965/#32966 for this.

[dimpase]

comment:15

probably even more relevant than these semi-vanilla cases would be the Azure docker container used in cibuildwheel to build manylinux wheels.

[mkoeppe]

Branch: u/mkoeppe/sage_build_cython__do_not_rely_on_cc_environment_variable_being_set

[mkoeppe]

Commit: 53c55d2

[mkoeppe]

New commits:

fd0a71f	src/sage_setup/command/sage_build_cython.py: Use gcc if CC unset
53c55d2	src/sage_setup/command/sage_build_cython.py: Remove workaround for GCC 4.8

[mkoeppe]

Author: Matthias Koeppe

[kiwifb]

comment:21

Is the branch really in the state you want? The last commit basically remove the content of the previous one. Otherwise I am all for removing useless bits.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

805bf82

src/sage_setup/command/sage_build_cython.py: Remove workaround for GCC 4.8

[sagetrac-git]

Changed commit from 53c55d2 to 805bf82

[mkoeppe]

comment:23

Thanks. I've squashed/rebased it. Yes, all of it can go away

[kiwifb]

Reviewer: François Bissey

[kiwifb]

comment:24

We should really get that in quick. It has been waiting long enough.

[mkoeppe]

comment:25

Thanks!

[vbraun]

Changed branch from u/mkoeppe/sage_build_cython__do_not_rely_on_cc_environment_variable_being_set to 805bf82