Upgrade to givaro-4.2.0 fflas-ffpack-2.5.0 linbox-1.7.0
ClementPernet opened this issue · 51 comments
New packages are available here (with the corresponding checksums):
givaro: https://github.com/linbox-team/givaro/releases/tag/v4.2.0
fflas-ffpack: https://github.com/linbox-team/fflas-ffpack/releases/tag/v2.5.0
linbox: https://github.com/linbox-team/linbox/releases/tag/v1.7.0
The new versions require "a C++ compiler supporting C++11 standard. More precisely g++ v5 or greater, clang++ v3.4 or greater".
So this ticket depends on dropping support for some systems, see #32074.
Depends on #33316
Component: packages: standard
Branch/Commit: u/cpernet/upgrade_to_givaro_4_2_0_fflas_ffpack_2_5_0_linbox_1_7_0 @ 4006e10
Issue created by migration from https://trac.sagemath.org/ticket/32959
Description changed:
---
+++
@@ -1 +1,5 @@
+New packages are available here (with the corresponding checksums):
+givaro: https://github.com/linbox-team/givaro/releases/tag/v4.2.0rc0
+fflas-ffpack:
+linbox: Description changed:
---
+++
@@ -1,5 +1,7 @@
New packages are available here (with the corresponding checksums):
givaro: https://github.com/linbox-team/givaro/releases/tag/v4.2.0rc0
+
fflas-ffpack:
+
linbox: I see numerous build errors in https://github.com/linbox-team/fflas-ffpack/runs/4383290866?check_suite_focus=true that will need fixing before we can do these updates
Of course. I just created this ticket to start working on a its branch, which will help us fix the last issues before cutting the final releases.
Btw, I did not manage to access to the full artifact of the givaro install log givaro which caused the failures in these github action builds (only the tail of the givaro-log is displayed in the sage build log in your link). Is there any way to access it? (I could not find it in the whole log archive).
Description changed:
---
+++
@@ -2,6 +2,6 @@
givaro: https://github.com/linbox-team/givaro/releases/tag/v4.2.0rc0
-fflas-ffpack:
+fflas-ffpack: https://github.com/linbox-team/fflas-ffpack/releases/tag/v2.5.0rc1
linbox: Commit: bd36004
Replying to @ClementPernet:
I did not manage to access to the full artifact of the givaro install log givaro which caused the failures in these github action builds (only the tail of the givaro-log is displayed in the sage build log in your link). Is there any way to access it? (I could not find it in the whole log archive).
In each run, such as https://github.com/linbox-team/fflas-ffpack/runs/4383290866?check_suite_focus=true, there is a section "Print out logs for immediate inspection" in the output, which shows all logs of failing package builds in full.
When all jobs have completed, the summary page, such as https://github.com/linbox-team/fflas-ffpack/actions/runs/1526437644, gives access to the log artifacts.
For example, logs-commit-3e42b4a1aaa585bce832738d5a8885dcf7c4273e-tox-docker-ubuntu-xenial-standard (https://github.com/linbox-team/fflas-ffpack/suites/4521078418/artifacts/121757925) gives a zip file within which you will find logs/pkgs/givaro-git.log
We have a bit of documentation on this at https://doc.sagemath.org/html/en/developer/portability_testing.html#automatic-parallel-tox-runs-on-github-actions; if you feel anything needs to be added, let me know
Thanks a lot for the pointers.
The checksums.ini files should get upstream_url lines, see https://doc.sagemath.org/html/en/developer/packaging.html#upstream-urls
Branch pushed to git repo; I updated commit sha1. New commits:
065177a | update checksums and versions to latest released linbox, fflas-ffpack and givaro |
I just released the three libraries and updated this branch with the corresponding checksums, and added the upstream_url as requested.
I could check that the testsuite passes on 2 machines (broadwell and skylake-X both running Linux mint).
I only had to comment out local/include/factory/factory.h line 92 (#define IntegerDomain 1), because this define by Singular gets in the way of Givaro's Givaro::IntegerDomain. See https://groups.google.com/g/sage-devel/c/lC941Lab5ck
Dima suggested to pacth this file, but I am not sure to see where this should be done in sage install process. Any help would be most welcome.
Does Singular/Singular@18bb792
do the job? Or they made an error there, it doesn't fix anything for you?
Replying to @dimpase:
Does Singular/Singular@18bb792
do the job? Or they made an error there, it doesn't fix anything for you?
Note that the patched file is actually meant to be preprocessed by Singular's ./factory/bin/makeheader
Replying to @ClementPernet:
Dima suggested to pacth this file, but I am not sure to see where this should be done in sage install process. Any help would be most welcome.
I don't think there's such an easy solution. The straightforward one is to wait for a new singular release, and then upgrade singular and givaro in sage at the same time, after enough distributions have packaged them both.
I would suggest to work around this clash in Givaro.
The latest run of the integration testing workflow - https://github.com/linbox-team/fflas-ffpack/runs/4511137082?check_suite_focus=true - fails badly on all platforms
Replying to @mkoeppe:
The latest run of the integration testing workflow - https://github.com/linbox-team/fflas-ffpack/runs/4511137082?check_suite_focus=true - fails badly on all platforms
This run does not correspond to the released package and the failure there has been fixed in a more recent commit which is in the release.
You may want to cancel the runs that are no longer needed so that the current one gets a chance to run
Replying to @mkoeppe:
I would suggest to work around this clash in Givaro.
Givaro::IntegerDomain is in the public API of Givaro, and used for instance by fflas-ffpack, but possibly other external users or libraries.
It is rather hard to justify such a change on Givaro's side. I'd like to investigate further the option of changing the include order.
Replying to @mkoeppe:
You may want to cancel the runs that are no longer needed so that the current one gets a chance to run
I was precisely doing this.
I'll look into how to reduce the number of builds: it is quite a waste of energy to launch the whole build at any PR or push on master.
Replying to @dimpase:
Does Singular/Singular@18bb792
do the job? Or they made an error there, it doesn't fix anything for you?
I can confirm that this Singular commit fixes the API conflicts, and Sage builds fine against the new linbox with this branch.
Replying to @ClementPernet:
I'll look into how to reduce the number of builds: it is quite a waste of energy to launch the whole build at any PR or push on master.
You could for example change it to running only when a tag is pushed
By the way, the singular update ticket #32907 needs review. Perhaps the patch should be added there?
Hm? Isn't this an upstream commit that will be in some later version of Singular anyway?
Replying to @mkoeppe:
Hm? Isn't this an upstream commit that will be in some later version of Singular anyway?
Yes, but once sage has a patched singular, it can upgrade givaro/linbox/fflas-ffpack, while distros cannot (without backporting the patch).
Replying to @ClementPernet:
Replying to @mkoeppe:
I would suggest to work around this clash in Givaro.
Givaro::IntegerDomainis in the public API of Givaro, and used for instance by fflas-ffpack, but possibly other external users or libraries.
It is rather hard to justify such a change on Givaro's side.
Has it been in the API of any released version of Givaro?
Replying to @mkoeppe:
Has it been in the API of any released version of Givaro?
Since v4.0.1 (2016-02-24).
It seems that are only left with the following options:
- delay the review of this ticket until Singular releases a new version including the fix, and all distros sage wants to support package it
- change givaro's API and fflas-ffpack usage of this API
- investigate the include chain and revert to a situation where singular is included after givaro and fflas-ffpack (this is probably why the problem did not show up previously)
Am I missing something?
Branch pushed to git repo; I updated commit sha1. New commits:
4006e10 | workaround the naming conflict for IntegerDomain between Singular and Givaro |
I pushed a fix for the naming conflict, forcing the include of givinteger.h before the include of factory.h. This was the cleaner way I could find. Trying to identify all situation where singular was included before givaro and swapping them, ended up in a mess.
Description changed:
---
+++
@@ -1,7 +1,7 @@
New packages are available here (with the corresponding checksums):
-givaro: https://github.com/linbox-team/givaro/releases/tag/v4.2.0rc0
+givaro: https://github.com/linbox-team/givaro/releases/tag/v4.2.0
-fflas-ffpack: https://github.com/linbox-team/fflas-ffpack/releases/tag/v2.5.0rc1
+fflas-ffpack: https://github.com/linbox-team/fflas-ffpack/releases/tag/v2.5.0
-linbox:
+linbox: https://github.com/linbox-team/linbox/releases/tag/v1.7.0Singular 4.3.0 has been released - with the Singular patch in comment:26 merged.
Replying to @ClementPernet:
I pushed a fix for the naming conflict, forcing the include of
givinteger.hbefore the include offactory.h. This was the cleaner way I could find. Trying to identify all situation where singular was included before givaro and swapping them, ended up in a mess.
This looks like a good solution for now, but this will become an obstacle to modularization pretty soon.
Description changed:
---
+++
@@ -5,3 +5,9 @@
fflas-ffpack: https://github.com/linbox-team/fflas-ffpack/releases/tag/v2.5.0
linbox: https://github.com/linbox-team/linbox/releases/tag/v1.7.0
+
+The new versions require "a C++ compiler supporting C++11 standard. More precisely g++ v5 or greater, clang++ v3.4 or greater".
+So this ticket depends on dropping support for some systems, see #32074.
+
+
+Replying to @mkoeppe:
This looks like a good solution for now, but this will become an obstacle to modularization pretty soon.
Should I revert this commit, as the fixed Singular is now released ? I am not sure what's the best move to get thinks rolling. Just let me know.
This would depend on
- actually making the Singular update (#33160, which needs help)
- tightening our version requirement in
build/pkgs/singular/spkg-configure.m4
By the way, I've marked this ticket for Sage 9.7 because of the increased compiler requirements (see #32074).
Also before we can merge this upgrade, Cygwin support needs to repaired (https://github.com/linbox-team/fflas-ffpack/runs/4658698318)
Also, 32bit platforms seem broken
@ClementPernet Is it planned to release new versions of the packages that include patches you may have received from distribution packaging?
Yes, I'm finally getting back to it, and would like to cut a minor version in a near future (say september or october) fixing the various issues raised here and in other PRs.
@ClementPernet, any plans for a new minor version?