Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#24703 closed defect (fixed)

Everything should be rebuilt after GCC upgrade

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-8.2
Component: packages: standard Keywords:
Cc: fbissey, embray Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 70ff832 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Due to C++ ABI incompatibilities, problems occur if code built with earlier versions of GCC is linked against code built with newer versions of GCC. This leads for example to patchbot failures mentioned below.

The simplest solution is to make every package depend on the GCC executable. This way, a GCC upgrade will force everything to be rebuilt. This may be overkill, but it's not a big problem in practice since GCC is upgraded rarely (unlikely other Sage packages, it is never automatically rebuilt except if absolutely needed, see for example #24599).


See crash_zrVXNw.log for a crash log from the arando i686 patchbot with Sage 8.2.beta5 using GCC 7.2.0. It involves brial and C++ strings.

The following doctests fail:

sage -t --warn-long 67.1 src/sage/rings/polynomial/multi_polynomial_sequence.py  # Killed due to abort
sage -t --warn-long 67.1 src/sage/rings/polynomial/pbori.pyx  # Killed due to abort
sage -t --warn-long 67.1 src/sage/rings/polynomial/multi_polynomial_libsingular.pyx  # Killed due to abort
sage -t --warn-long 67.1 src/sage/rings/polynomial/polynomial_ring_constructor.py  # Killed due to abort

Attachments (2)

crash_zrVXNw.log (135.2 KB) - added by jdemeyer 2 years ago.
Makefile (102.5 KB) - added by vbraun 2 years ago.

Download all attachments as: .zip

Change History (47)

Changed 2 years ago by jdemeyer

comment:1 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 2 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from SIGABRT on i686 involving polybori and strings to SIGABRT on i686 involving polybori and C++ strings

comment:5 Changed 2 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Cc fbissey added
  • Description modified (diff)
  • Summary changed from SIGABRT on i686 involving polybori and C++ strings to Everything should be rebuilt after GCC upgrade

comment:6 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 2 years ago by fbissey

Yup, pretty much anything that use C++ has to be rebuilt when you move to gcc-5. gcc dev have made some commitments to ABI stability for the time being. But then you also have the change of ABI for gfortran at gcc-7. The only thing that's rock solid for almost ever is C.

comment:8 Changed 2 years ago by jdemeyer

  • Branch set to u/jdemeyer/everything_should_be_rebuilt_after_gcc_upgrade

comment:9 Changed 2 years ago by git

  • Commit set to c59430df6ba5b9e7b6ba0c29aa077a12a97e748e

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

c59430dEverything should be rebuilt after GCC upgrade

comment:10 Changed 2 years ago by git

  • Commit changed from c59430df6ba5b9e7b6ba0c29aa077a12a97e748e to 057b434babb6ec6eaa7bae4afad4255a7481b1af

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

057b434Everything should be rebuilt after GCC upgrade

comment:11 Changed 2 years ago by jdemeyer

  • Status changed from new to needs_review

comment:12 Changed 2 years ago by jdemeyer

  • Cc embray added
  • Description modified (diff)

comment:13 follow-up: Changed 2 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Not sure why the patchbot is failing but the stuff looks good me, and we should include it ASAP.

comment:14 in reply to: ↑ 13 Changed 2 years ago by embray

Replying to fbissey:

Not sure why the patchbot is failing but the stuff looks good me, and we should include it ASAP.

I need to upgrade that patchbot. It's running an old version of the patchbot package that has a bug when testing "unsafe" tickets.

comment:15 Changed 2 years ago by embray

Okay, I don't really care since I'm not in the habit of building gcc anyways, but I would appreciate if someone sign off on #21524 so I don't have to keep upgrading it to incorporate changes like this.

comment:16 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/everything_should_be_rebuilt_after_gcc_upgrade to 057b434babb6ec6eaa7bae4afad4255a7481b1af
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 2 years ago by vbraun

  • Commit 057b434babb6ec6eaa7bae4afad4255a7481b1af deleted
  • Resolution fixed deleted
  • Status changed from closed to new

Breaks full build on Debian 7 (both 32 and 64 bit)

PPL:

configure: error: Cannot find GMP version 4.1.3 or higher.
GMP is the GNU Multi-Precision library:
see http://www.swox.com/gmp/ for more information.
When compiling the GMP library, do not forget to enable the C++ interface:
add --enable-cxx to the configuration options.
Error configuring the Parma Polyhedra Library.

Givaro:

checking for GMP library... yes
checking gmpxx.h usability... no
checking gmpxx.h presence... no
checking for gmpxx.h... no
your GMP does not have c++ support. Compile GMP with --enable-cxx
********************************************************************************
Error configuring givaro-4.0.4
********************************************************************************

comment:18 Changed 2 years ago by jdemeyer

Do you happen to have the install.log file?

comment:19 Changed 2 years ago by jdemeyer

Last edited 2 years ago by jdemeyer (previous) (diff)

comment:20 Changed 2 years ago by jdemeyer

  • Branch changed from 057b434babb6ec6eaa7bae4afad4255a7481b1af to u/jdemeyer/057b434babb6ec6eaa7bae4afad4255a7481b1af

comment:21 Changed 2 years ago by jdemeyer

  • Commit set to 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d
  • Status changed from new to needs_review

Reverted this:

  • build/pkgs/gcc/spkg-install

    a b $MAKE install 
    124124
    125125# Mark gfortran as not installed in case gfortran was installed earlier
    126126cd "$SAGE_SPKG_INST"
    127 rm -f gmp-* mpir-* mpfr-* mpc-* gfortran-*
     127rm -f gfortran-*
    128128
    129129# Force re-configuration: the next time that "make" is run, we need to
    130130# rebuild all packages (#24703) but we should not rebuild gcc (#19324)

New commits:

70ff832Everything should be rebuilt after GCC upgrade

comment:22 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:23 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/057b434babb6ec6eaa7bae4afad4255a7481b1af to 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:24 Changed 2 years ago by vbraun

  • Commit 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d deleted
  • Resolution fixed deleted
  • Status changed from closed to new

http://build.sagemath.org/#/builders/30/builds/6/steps/6/logs/matplotlib

Building matplotlib still fails on Debian 7 64-bit with

    gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -DPY_ARRAY_UNIQUE_SYMBOL=MPL_matplotlib__image_ARRAY_API -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -D__STDC_FORMAT_MACROS=1 -I/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/numpy/core/include -I/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/numpy/core/include -I/home/buildbot/slave/sage_git/build/local/include -I. -Iextern/agg24-svn/include -I/home/buildbot/slave/sage_git/build/local/include/python2.7 -c src/py_converters.cpp -o build/temp.linux-x86_64-2.7/src/py_converters.o
    In file included from /home/buildbot/slave/sage_git/build/local/include/python2.7/Python.h:19:0,
                     from src/py_converters.h:16,
                     from src/py_converters.cpp:3:
    /usr/include/limits.h:125:26: error: no include path in which to search for limits.h
     # include_next <limits.h>
                              ^
    In file included from src/py_converters.h:16:0,
                     from src/py_converters.cpp:3:
    /home/buildbot/slave/sage_git/build/local/include/python2.7/Python.h:22:2: error: #error "Something's broken.  UCHAR_MAX should be defined in limits.h."
     #error "Something's broken.  UCHAR_MAX should be defined in limits.h."

comment:25 Changed 2 years ago by jdemeyer

I don't see how that error is related to this ticket.

comment:26 follow-up: Changed 2 years ago by vbraun

Apparently the build was interleaved with gcc, which shouldn't happen. This also happens after backing out the ticket so its either a preexisting issue or the makefile isn't regenerated. This is the second build of gcc, it builds in parallel with everything else.

comment:27 in reply to: ↑ 26 Changed 2 years ago by jdemeyer

  • Status changed from new to needs_review

Replying to vbraun:

This also happens after backing out the ticket

I think it's a bug in the previous version of this ticket then.

This is the second build of gcc

There shouldn't be a second build of gcc.

It would be very useful to see the auto-generated build/make/Makefile from the failed build to understand what is happening.

comment:28 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Changed 2 years ago by vbraun

comment:29 Changed 2 years ago by vbraun

I attached the makefile...

comment:30 Changed 2 years ago by vbraun

I checked out just this ticket and ran make distclean && make and gcc builds twice

buildbot@sagebd07_64s02:~/slave/sage_git/build$ grep 'Found local metadata for gcc' logs/install.log 
[gcc-7.2.0] Found local metadata for gcc-7.2.0
[gcc-7.2.0] Found local metadata for gcc-7.2.0

and the second time is in parallel with lots of other stuff...

comment:31 Changed 2 years ago by jdemeyer

OK, I think I know what happened: there is indeed a serious bug that GCC is installed twice.

But that bug is not because of this ticket, it is most likely happening since #24599. I think you are just unlucky to hit this bug now and not before.

comment:32 Changed 2 years ago by jdemeyer

I would rather prefer to merge this ticket anyway and then deal with further fixes to the GCC build process in a future ticket.

comment:33 Changed 2 years ago by vbraun

Sure, did you open a followup ticket?

comment:34 Changed 2 years ago by vbraun

  • Branch changed from 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d to u/jdemeyer/everything_should_be_rebuilt_after_gcc_upgrade
  • Commit set to 057b434babb6ec6eaa7bae4afad4255a7481b1af

New commits:

057b434Everything should be rebuilt after GCC upgrade

comment:35 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/everything_should_be_rebuilt_after_gcc_upgrade to u/jdemeyer/057b434babb6ec6eaa7bae4afad4255a7481b1af
  • Commit changed from 057b434babb6ec6eaa7bae4afad4255a7481b1af to 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d

New commits:

70ff832Everything should be rebuilt after GCC upgrade

comment:36 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/057b434babb6ec6eaa7bae4afad4255a7481b1af to 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:37 follow-up: Changed 2 years ago by jhpalmieri

  • Commit 70ff832bc5ca84610c1b3b8fab34c4564ea6cc3d deleted

A question related to #24961: should we be checking that x$SAGE_INSTALL_GCC = xexists or that x$SAGE_INSTALL_GCC = xyes? On an OS X machine, I have Sage's gcc installed (from a few betas ago), but now Sage uses clang instead of gcc, so Sage's gcc will not be rebuilt as I upgraded Sage.

comment:38 in reply to: ↑ 37 ; follow-up: Changed 2 years ago by fbissey

Replying to jhpalmieri:

A question related to #24961: should we be checking that x$SAGE_INSTALL_GCC = xexists or that x$SAGE_INSTALL_GCC = xyes? On an OS X machine, I have Sage's gcc installed (from a few betas ago), but now Sage uses clang instead of gcc, so Sage's gcc will not be rebuilt as I upgraded Sage.

Really, you need to check both, with the caveat that if you installed with clang and want to switch to gcc you will break your install. So, unfortunately, a not so stupid user could break things.

comment:39 follow-up: Changed 2 years ago by jhpalmieri

In my case, I switched from gcc (the default) to clang (the new default), and upgrading to the most recent beta broke things, because this test ended up adding bad stuff to build/make/Makefile, as reported in sage-release and #21524, and now being fixed in #24961.

comment:40 in reply to: ↑ 39 ; follow-up: Changed 2 years ago by jdemeyer

Replying to jhpalmieri:

In my case, I switched from gcc (the default) to clang (the new default)

Are you sure? If the GCC spkg is installed, it should be used by Sage. If this is not the case, I consider that a bug.

comment:41 in reply to: ↑ 38 Changed 2 years ago by jdemeyer

Replying to fbissey:

Replying to jhpalmieri:

A question related to #24961: should we be checking that x$SAGE_INSTALL_GCC = xexists or that x$SAGE_INSTALL_GCC = xyes? On an OS X machine, I have Sage's gcc installed (from a few betas ago), but now Sage uses clang instead of gcc, so Sage's gcc will not be rebuilt as I upgraded Sage.

Really, you need to check both

I don't see why, but maybe I'm missing something?

comment:42 in reply to: ↑ 40 ; follow-up: Changed 2 years ago by jhpalmieri

Replying to jdemeyer:

Replying to jhpalmieri:

In my case, I switched from gcc (the default) to clang (the new default)

Are you sure? If the GCC spkg is installed, it should be used by Sage. If this is not the case, I consider that a bug.

I guess you're right, gcc is used, but configure tells me that gcc will not be installed.

comment:43 in reply to: ↑ 42 Changed 2 years ago by jdemeyer

Replying to jhpalmieri:

configure tells me that gcc will not be installed.

It won't be installed if it's already installed. Consider that a feature.

comment:44 follow-up: Changed 2 years ago by jhpalmieri

I also consider it a feature if it's not installed because clang is going to be used instead. configure does not give a reason for it not being installed, leaving me to guess as to what's going on.

comment:45 in reply to: ↑ 44 Changed 2 years ago by jdemeyer

Replying to jhpalmieri:

I also consider it a feature if it's not installed because clang is going to be used instead.

Of course. But that should work now, right?

Note: See TracTickets for help on using tickets.