Upgrade pynac to support gcc-11
mkoeppe opened this issue · 79 comments
(from #31786)
pynac fails to compile with gcc-11 (headers porting) pynac/pynac#375
... and ideally to consolidate the various patches added in #31679, #31645 etc.... as also requested in https://groups.google.com/g/sage-packaging/c/_vxPV7aKNjQ/m/b2-EdWP1BgAJ
When making a new release, also care should be taken to use a recent version of libtool that supports macOS Big Sur (see #31920).
CC: @dimpase @DaveWitteMorris @kiwifb @antonio-rojas
Component: packages: standard
Author: Dima Pasechnik, Matthias Koeppe
Branch: c9c50e3
Reviewer: Matthias Koeppe, Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/31694
There are still several pynac bug tickets that may not be hard to fix, so I think I will be posting quite a few more pynac patches in the next few months, so let's wait on this. But it would be good to make an upgrade before the 9.4 release.
Aren't you preparing your patches using git? I'd recommend to create pull requests to the pynac repo to make it more transparent.
Description changed:
---
+++
@@ -1,4 +1,4 @@
-... after merging the various patches added in #31679, #31645 etc.
- upstream.
+... and ideally to consolidate the various patches added in #31679, #31645 etc.
+Description changed:
---
+++
@@ -1,3 +1,7 @@
+(from #31786)
+
+pynac fails to compile with gcc-11 (headers porting) [https://github.com/pynac/pynac/pull/375](https://github.com/pynac/pynac/pull/375)
+
... and ideally to consolidate the various patches added in #31679, #31645 etc.
Description changed:
---
+++
@@ -4,5 +4,6 @@
... and ideally to consolidate the various patches added in #31679, #31645 etc.
+When making a new release, also care should be taken to use a recent version of libtool that supports macOS Big Sur (see #31920).
Description changed:
---
+++
@@ -2,7 +2,7 @@
pynac fails to compile with gcc-11 (headers porting) [https://github.com/pynac/pynac/pull/375](https://github.com/pynac/pynac/pull/375)
-... and ideally to consolidate the various patches added in #31679, #31645 etc.
+... and ideally to consolidate the various patches added in #31679, #31645 etc.... as also requested in https://groups.google.com/g/sage-packaging/c/_vxPV7aKNjQ/m/b2-EdWP1BgAJ
When making a new release, also care should be taken to use a recent version of libtool that supports macOS Big Sur (see #31920).
I am going to cut a new release of pynac with all da patchez
Branch: u/dimpase/packages/pynac/0.7.28
Author: Dima Pasechnik
Thank you Dima! :)
yah, thanks a lot!
Dave, I've merged all your pynac patches into master, e.g.
while the log is clear they are yours, I'm still the author. If you like you can do a PR on github to change the author of these commits.
GH Actions run please
Replying to @mkoeppe:
GH Actions run please
running at https://github.com/sagemath/sagetrac-mirror/actions/runs/972993967
Replying to @dimpase:
Dave, I've merged all your pynac patches into master, e.g.
while the log is clear they are yours, I'm still the author. If you like you can do a PR on github to change the author of these commits.
Thanks, Dima, that's great! I am not concerned about the "author", so nothing needs to be done, as far as I'm concerned.
Copying these patches to the pynac repository has been on my to-do list for a long time, but I never got around to it, so thanks for doing this! In the future, I'll try to save you (and the distro managers) some trouble by making the appropriate PRs.
Replying to @DaveWitteMorris:
Copying these patches to the pynac repository has been on my to-do list for a long time, but I never got around to it, so thanks for doing this! In the future, I'll try to save you (and the distro managers) some trouble by making the appropriate PRs.
I guess you actually might have a git repo with these patches applied (at least this is how I typically do package patching), so it could have been a breeze to convert these into PRs. Anyway, it's done now.
One can set the author name and email as different from the committer's:
$ git commit --author="Author Name <email@example.com>"This can be used to amend the last commit done:
$ git commit --amend --author="Author Name <email@example.com>" --no-editReference:
- Answers to Stack Overflow question 3042437 (see more tips there)
Replying to @dimpase:
Replying to @mkoeppe:
GH Actions run please
running at https://github.com/sagemath/sagetrac-mirror/actions/runs/972993967
gentoo failed bc system eclib wanted #31443
checking whether we can link and run a program using eclib...
eclib version 20210503, using NTL bigints and NTL real and complex multiprecision floating point
yes; use eclib from the system
Replying to @dimpase:
Replying to @mkoeppe:
GH Actions run please
running at https://github.com/sagemath/sagetrac-mirror/actions/runs/972993967
looks like that everything (modulo the usual GH actions suspects, e.g. broken conda on some platforms) is working.
Review?
Broken on ubuntu-trusty
Also could you please have the GH Actions on pynac re-enabled?
see https://github.com/pynac/pynac/actions/runs/971200691
Replying to @mkoeppe:
Broken on
ubuntu-trusty
I cannot imagine that
[pynac-0.7.28] checking for flint/fmpq_poly.h... no
[pynac-0.7.28] configure: error: This package needs flint headers
is a regression. The only difference between 0.7.28 and 0.2.27 that all patches for the latter are upstreamed, and two more PRs are merged, none of them have anything to do with flint.
ubuntu-trusty is OK on 9.4.beta3 (as you can see in https://github.com/sagemath/sage/actions/runs/958667352) and broken with this branch as your GH Actions run showed. Same with linuxmint-17.
I'm sure it's just crappy CI for museum age OSs - what could have caused flint headers to suddenly become unavailable?
No, my CI does not have such problems.
As you know, you can test it locally using tox -e docker-ubuntu-trusty-standard.
Instead of arguing about the CI, could we have the config.log of pynac.
The logs are in the log artifacts of Dima's GH Actions run:
https://github.com/sagemath/sagetrac-mirror/suites/3090181081/artifacts/70720481
OK actually the config.log is not there. So a local rerun is needed to produce them
Replying to @mkoeppe:
As you know, you can test it locally using
tox -e docker-ubuntu-trusty-standard.
My office machine doesn't have docker. OK, I can do an lxc install of ubuntu trusty, maybe...
linuxmint-17 reached EOL over 2 years ago, I don't know why we still keep it.
https://forums.linuxmint.com/viewtopic.php?t=289281
ubuntu trusty is also beyond the end of unpaid support, EOL to happen in 10 months.
Let's drop them for 9.4.
We can't have this discussion on every untested package upgrade ticket.
Here's the pynac config.log:
configure:17504: checking for gmp.h
configure:17504: gcc -std=gnu11 -c -O2 -g conftest.c >&5
configure:17504: $? = 0
configure:17504: result: yes
configure:17514: checking for library containing __gmpz_get_str
configure:17544: gcc -std=gnu11 -o conftest -O2 -g -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib conftest.c >&5
/tmp/ccY7y07W.o: In function `main':
/sage/local/var/tmp/sage/build/pynac-0.7.28/src/conftest.c:34: undefined reference to `__gmpz_get_str'
collect2: error: ld returned 1 exit status
configure:17544: $? = 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "pynac"
| #define PACKAGE_TARNAME "pynac"
| #define PACKAGE_VERSION "0.7.28"
| #define PACKAGE_STRING "pynac 0.7.28"
| #define PACKAGE_BUGREPORT "<pynac-devel@googlegroups.com>"
| #define PACKAGE_URL ""
| #define PACKAGE "pynac"
| #define VERSION "0.7.28"
| #define ARCHIVE_VERSION 3
| #define ARCHIVE_AGE 0
| #define HAVE_STDIO_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_STRING_H 1
| #define HAVE_INTTYPES_H 1
| #define HAVE_STDINT_H 1
| #define HAVE_STRINGS_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_UNISTD_H 1
| #define STDC_HEADERS 1
| #define HAVE_DLFCN_H 1
| #define LT_OBJDIR ".libs/"
| #define HAVE_GMP_H 1
| /* end confdefs.h. */
|
| /* Override any GCC internal prototype to avoid an error.
| Use char because int might match the return type of a GCC
| builtin and then its argument prototype would still apply. */
| char __gmpz_get_str ();
| int
| main (void)
| {
| return __gmpz_get_str ();
| ;
| return 0;
| }
configure:17544: gcc -std=gnu11 -o conftest -O2 -g -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib conftest.c -lgmp >&5
configure:17544: $? = 0
configure:17564: result: -lgmp
configure:17578: checking for flint/fmpq_poly.h
configure:17578: gcc -std=gnu11 -c -O2 -g conftest.c >&5
In file included from /sage/local/include/flint/nmod_vec.h:29:0,
from /sage/local/include/flint/fmpz.h:31,
from /sage/local/include/flint/fmpq_poly.h:24,
from conftest.c:54:
/sage/local/include/flint/ulong_extras.h:119:1: error: unknown type name '_Thread_local'
extern FLINT_TLS_PREFIX ulong * _flint_primes[FLINT_BITS];
^
/sage/local/include/flint/ulong_extras.h:119:31: error: expected '=', ',', ';', 'asm' or '__attribute__' before '*' token
extern FLINT_TLS_PREFIX ulong * _flint_primes[FLINT_BITS];
^
/sage/local/include/flint/ulong_extras.h:120:25: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'double'
extern FLINT_TLS_PREFIX double * _flint_prime_inverses[FLINT_BITS];
^
/sage/local/include/flint/ulong_extras.h:121:25: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int'
extern FLINT_TLS_PREFIX int _flint_primes_used;
^
configure:17578: $? = 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "pynac"
| #define PACKAGE_TARNAME "pynac"
| #define PACKAGE_VERSION "0.7.28"
| #define PACKAGE_STRING "pynac 0.7.28"
| #define PACKAGE_BUGREPORT "<pynac-devel@googlegroups.com>"
| #define PACKAGE_URL ""
| #define PACKAGE "pynac"
| #define VERSION "0.7.28"
| #define ARCHIVE_VERSION 3
| #define ARCHIVE_AGE 0
| #define HAVE_STDIO_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_STRING_H 1
| #define HAVE_INTTYPES_H 1
| #define HAVE_STDINT_H 1
| #define HAVE_STRINGS_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_UNISTD_H 1
| #define STDC_HEADERS 1
| #define HAVE_DLFCN_H 1
| #define LT_OBJDIR ".libs/"
| #define HAVE_GMP_H 1
| /* end confdefs.h. */
| #include <stddef.h>
| #ifdef HAVE_STDIO_H
| # include <stdio.h>
| #endif
| #ifdef HAVE_STDLIB_H
| # include <stdlib.h>
| #endif
| #ifdef HAVE_STRING_H
| # include <string.h>
| #endif
| #ifdef HAVE_INTTYPES_H
| # include <inttypes.h>
| #endif
| #ifdef HAVE_STDINT_H
| # include <stdint.h>
| #endif
| #ifdef HAVE_STRINGS_H
| # include <strings.h>
| #endif
| #ifdef HAVE_SYS_TYPES_H
| # include <sys/types.h>
| #endif
| #ifdef HAVE_SYS_STAT_H
| # include <sys/stat.h>
| #endif
| #ifdef HAVE_UNISTD_H
| # include <unistd.h>
| #endif
| #include <flint/fmpq_poly.h>
configure:17578: result: no
configure:17584: error: This package needs flint headers
So, we have flint headers but they are broken. I'd say because glibc is too old at first glance. Glad you could dig those because they are really useful in pinpointing issues.
On this install, flint was installed from the SPKG.
And looking at the flint build log: it is using plain gcc, without -std=gnu11.
Replying to @mkoeppe:
On this install, flint was installed from the SPKG.
I could tell from the headers tested. Either something is changed at install or those particular headers are not involved in building flint. Otherwise building flint would be a problem. It is worth trying without the gnu11 bit. Good spotting.
I think the gnu11 is a setting bleeding from setting the C++ compiler to c++11. It probably shouldn't be there at all.
Replying to @kiwifb:
I think the gnu11 is a setting bleeding from setting the C++ compiler to c++11. It probably shouldn't be there at all.
Nope C++11 is supposed to be tested after. I have no idea where this one comes from. I also don't see it when I configure pynac locally (have to double check that) so I am not sure where it is introduced.
Right. There is no C code whatsoever in pynac. Everything is C++. Just the AC_LANG([C++])
is coming much too late!
Tests with a change run now at https://github.com/mkoeppe/pynac/actions/runs/977436209
Do I take it that it helped :)
Yes, this seems to do the trick.
Dima, can you please create a new release with this PR?
Replying to @mkoeppe:
Also could you please have the GH Actions on pynac re-enabled?
see https://github.com/pynac/pynac/actions/runs/971200691
no idea what "reenabled" means. They are enabled, I guess it's what we have in .github/ matters.
Like the error message says: "Actions jobs for this repository have been disabled by GitHub staff. If you are the repository owner, you can contact support via https://github.com/contact for more information."
I had to do this for one of my repositories too in the past.
Replying to @dimpase:
Replying to @DaveWitteMorris:
Copying these patches to the pynac repository has been on my to-do list for a long time, but I never got around to it, so thanks for doing this! In the future, I'll try to save you (and the distro managers) some trouble by making the appropriate PRs.
I guess you actually might have a git repo with these patches applied (at least this is how I typically do package patching), so it could have been a breeze to convert these into PRs. Anyway, it's done now.
Yes, this should have been easy, but I know almost nothing about git, so my attempts failed when I tried a few weeks ago. (I didn't realize that I needed a fork, instead of a clone.) I think I succeeded in making a PR to the pynac repository this evening, and, now that I know how, I will try to make PRs of all of my pynac patches.
Replying to @mkoeppe:
Like the error message says: "Actions jobs for this repository have been disabled by GitHub staff.
I cannot see any error messages there.
Maybe I am looking at the wrong places?
If you are the repository owner, you can contact support via https://github.com/contact for more information."
I had to do this for one of my repositories too in the past.
Replying to @dimpase:
Replying to @mkoeppe:
Like the error message says: "Actions jobs for this repository have been disabled by GitHub staff.
I cannot see any error messages there.
Maybe I am looking at the wrong places?If you are the repository owner, you can contact support via https://github.com/contact for more information."
I had to do this for one of my repositories too in the past.
OK, it's in "Annotations". Duh. OK, requested re-enabling
https://support.github.com/ticket/personal/0/1212297
Branch pushed to git repo; I updated commit sha1. New commits:
3c131f7 | wip tarball |
Branch pushed to git repo; I updated commit sha1. New commits:
11c80aa | updated tarball with PRs 377 and 378 |
tests are running on https://github.com/sagemath/sagetrac-mirror/actions/runs/978774922
(which is this branch with an extra pynac test from #31585)
Broken on debian-jessie - pynac/pynac#378 (comment)
and on ubuntu-trusty - https://github.com/sagemath/sagetrac-mirror/runs/2931188061?check_suite_focus=true
Dima, could you please make a release without PR 378, so that we can get this blocker ticket done? Just focused on what this ticket says - add support for gcc 11, consolidate existing patches; no breaking/removing of other platform support.
I volunteer to make the next pynac releases.
Replying to @mkoeppe:
I volunteer to make the next pynac releases.
let's just drop gcc 4.x. It would minimise the disruption, and it's overdue anyway.
As I said on #32074, I am +1 on this proposal to drop support for gcc 4.x. That should solve the immediate problem.
Or even better/quicker, just add AC_CHECK_FUNC(__builtin_smull_overflow,...) to spkg-configure for gcc spkg.
It's important to keep tickets focused. In particular for a blocker ticket like this one, which creates an urgency to get it in.
The urgent need to add support for gcc 11 is entirely unconnected to a decision to drop support for the gcc 4 versions used in some of our supported distributions.
It is poor practice to connect these two issues arbitrarily.
our resources are limited;
- making a new pynac release without
__builtin_smull_overflowtakes time, - re-doing the branch with
__builtin_smull_overflowtakes time - (and God help you finding a workaround not using
__builtin_smull_overflow, potentially another time waste to keep a museum compiler in...)
It's important to keep tickets focused -- that includes avoiding to give unsolicited advice to other developers regarding time management.
Changed branch from u/dimpase/packages/pynac/0.7.28 to u/mkoeppe/packages/pynac/0.7.28
Changed author from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe
Reviewer: Matthias Koeppe, ...
tests run on https://github.com/sagemath/sagetrac-mirror/actions/runs/986545913
lgtm
Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, Dima Pasechnik
Thanks!
Changed branch from u/mkoeppe/packages/pynac/0.7.28 to c9c50e3
I don't see this branch in the current develop branch, what's up?