Skip to content

allow gcc-15 from the system #39977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Apr 19, 2025

Fedora 42 comes with gcc-15 by default, so it should be allowed, as asked on sage-release

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

@tobiasdiez - CI still does useless Fedora 30, I don't know why. Can we replace it with Fedora 42 - this would test this PR, too

@dimpase dimpase requested a review from tobiasdiez April 19, 2025 16:14
@enriqueartal
Copy link
Contributor

The packages gap, singular and planarity have problems which can be solved using system package. I attarch the log for ecl.
ecl-24.5.10.log

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

does Fedora have a usable ecl package?

I'll report this to upstream ecl, perhaps they already have a fix

@enriqueartal
Copy link
Contributor

Not now. I have used a former src.rpm and I could produce a package using gcc-14. Then maxima produces also an error, I am trying to make a package maxima-runtime-ecl (again with gcc-14) which has been disabled in Fedora.

@enriqueartal
Copy link
Contributor

With ecl and maxima system packages I get errors in sagelib,
sagelib-10.7.beta1.log
If I am not wrong they are related with linbox and ecl.

By the way, patch system package (2.8.1) is not accepted, no clue why in config.log

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

Note that gcc-15 has default c23 C standard. This is a bump from c17. Adding -std=c17 to CFLAGS might help with ecl and maxima.

That is,

--- a/build/pkgs/ecl/spkg-install.in
+++ b/build/pkgs/ecl/spkg-install.in
@@ -1,6 +1,6 @@
 cd src
 
-export CFLAGS
+export CFLAGS="-std=c17 $CFLAGS"
 export CXXFLAGS
 export LDFLAGS

(and maxima gets its flags from ecl).

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

By the way, patch system package (2.8.1) is not accepted, no clue why in config.log
most probably it reports its version as 2.8, that's why. Anyway, this is fixed by #39943
(testing appreciated)

Copy link

github-actions bot commented Apr 19, 2025

Documentation preview for this PR (built with commit 64e347f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@enriqueartal
Copy link
Contributor

No luck with sagelib with or without system-packages. On the other side, patch is no longer a package using #39943

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

By the way, patch system package (2.8.1) is not accepted, no clue why in config.log
most probably it reports its version as 2.8, that's why. Anyway, this is fixed by #39943
(testing appreciated)

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

ECL problem reported here:
https://gitlab.com/embeddable-common-lisp/ecl/-/issues/775

@dimpase
Copy link
Member Author

dimpase commented Apr 19, 2025

No luck with sagelib with or without system-packages.

please post errors. Are they again due to c23 used for C, or there are also C++ issues?

@enriqueartal
Copy link
Contributor

Thanks. I am not sure how to proceed. First I modified spkg-install.in as in #39977 (comment). I attach sagelib-10.7.beta1.log using ecl and system packages. Then I remove ecl system package, and this is the log of failure: ecl-24.5.10.log. Finally, I tried to compile maxima with ecl system package: maxima-5.47.0.log.
I hope it may help.

@tobiasdiez
Copy link
Contributor

@tobiasdiez - CI still does useless Fedora 30, I don't know why. Can we replace it with Fedora 42 - this would test this PR, too

Sure, just edit

and the ci-linux.

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

@enriqueartal : in regard of ecl, can you replace c17 in CFLAGS with gnu17

i.e. in the patch above it should be

-export CFLAGS
+export CFLAGS="-std=gnu17 $CFLAGS" 

@enriqueartal
Copy link
Contributor

One step achieved. Now ecl and maxima are OK; still issues with sagelib regarding ecl and linbox.

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

@tobiasdiez - CI still does useless Fedora 30, I don't know why. Can we replace it with Fedora 42 - this would test this PR, too

Sure, just edit


and the ci-linux.

How about doing this on top of your recent PR updating CI? (I meant #39942)

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

One step achieved. Now ecl and maxima are OK; still issues with sagelib regarding ecl and linbox.

I see issues with linbox (more patches needed)

With ecl, running plane C (gcc) while building sage.matrix.matrix_numpy_integer_dense extension, and anything else which includes ecl.h, should be done with -std=gnu17.
Can you try something like

CFLAGS="-std=gnu17" make -j8 build

@tobiasdiez - what to do here in meson.build? Does it have something line extension_data_c, analogous to extension_data_cpp ? (I am still unable to use meson docs - e.g. I cannot find anything in extension_data_cpp)

@enriqueartal
Copy link
Contributor

I think ecl's issues are gone, still issues with linbox with sagelib
sagelib-10.7.beta1.log

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

Do you use system's givaro? Or a built one? Anyway, please try #39936 (for givaro - but it's important, as linbox depends on givaro)

@enriqueartal
Copy link
Contributor

I use system's givaro (4.2.1). I can try with spkg.

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

I use system's givaro (4.2.1). I can try with spkg.

It's fine, should make no difference. For linbox, I think, 2 more patches are needed (incidentally they are needed for the new Apple's clang 17), as I added here: https://github.com/Macaulay2/homebrew-tap/blob/main/Formula/linbox.rb

Specifically, add to build/pkgs/linbox/patches "https://github.com/linbox-team/linbox/commit/d1f618fb0ee4a84be3ccddcfc28b257f34e1cbf7.patch?full_index=1"
and "https://github.com/linbox-team/linbox/commit/4a1e1395804d4630ec556c61ba3f2cb67e140248.patch?full_index=1"
(save them as 41.patch and 42.patch, so that they apply in the order following the rest of the patches)

@enriqueartal
Copy link
Contributor

Ok for linbox spkg but sagelib not OK

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

I gather linbox needs

--- a/linbox/blackbox/block-hankel.h
+++ b/linbox/blackbox/block-hankel.h
@@ -345,8 +345,8 @@ namespace LinBox
 		template<class Vector1, class Vector2>
 		Vector1& apply(Vector1 &x, const Vector2 &y) const
 		{
-			linbox_check(this->_coldim == y.size());
-			linbox_check(this->_rowdim == x.size());
+			linbox_check(this->coldim() == y.size());
+			linbox_check(this->rowdim() == x.size());
 			BlasMatrixDomain<Field> BMD(field());
 #ifdef BHANKEL_TIMER
 			_chrono.clear();

as well

@dimpase
Copy link
Member Author

dimpase commented Apr 20, 2025

and probably more, from linbox PRs 320, 322

@enriqueartal
Copy link
Contributor

Thanks! I saved as 43.patch and now it works. If my memory is OK gap and singular had also issues with gcc-15; for gap, even outside sage.

@enriqueartal
Copy link
Contributor

Last thing for today: gap spkg is OK, not for singular. Thanks.

@enriqueartal
Copy link
Contributor

and probably more, from linbox PRs 320, 322

I used nothing from these PRs

@dimpase
Copy link
Member Author

dimpase commented Apr 21, 2025

I've put the linbox patches on #39985
Please update your branch accodingly

@enriqueartal
Copy link
Contributor

Another test in a virtual machine failed with contourpy. In the former installation, it is ok for contourpy.

@dimpase
Copy link
Member Author

dimpase commented Apr 22, 2025

somewhere the sin of passing CFLAGS to a C++ compiler is committed

@enriqueartal
Copy link
Contributor

somewhere the sin of passing CFLAGS to a C++ compiler is committed

It seems that for some packages

CFLAGS="-std=gnu17" make -jn build

is needed and for others it is just make -jn build. Somehow I has to combine them, including maybe some intermediate cleaning for sagelib.

@erentar
Copy link

erentar commented Apr 29, 2025

i ran

git reset --hard && git clean -dfX
git pull https://github.com/dimpase/sage.git
make configure
./configure
make -j $(nprocs) build

maxima and planarity fails.
logs are attached
maxima-5.47.0.log
planarity-3.0.1.0.log

@dimpase
Copy link
Member Author

dimpase commented Apr 29, 2025

this is not a surprise. Planarity is probably easy to fix, less sure about maxima.

ecl, the Lisp compiler, used to build Maxima, is being fixed upstream, not sure about Maxima itself

@enriqueartal
Copy link
Contributor

I tested again with the new beta and it was basically fine. I applied the patches of this PR and also of #39985 and #40033 (because of previous singular failures). The only issue with standard packages was with gap, solved using

CFLAGS="-std=gnu17" make -jn build

Is there any other way? The test gave problems with src/sage/schemes/curves/point.py (signal errors which do not appear when running directly) and src/sage/groups/finitely_presented_named.py (time out, also disappeared when tested alone).
On the other side, I was unable to compile gap_packages.

@dimpase
Copy link
Member Author

dimpase commented May 1, 2025

gap_packages might need gcc-15 fixes. please post errors here, or on a new issue

@dimpase
Copy link
Member Author

dimpase commented May 1, 2025

we can place CFLAGS="-std=gnu17 into spkg-install.m4 files, something like

export CFLAGS = "-std=gnu17 $CFLAGS"

(check the precise syntax in similar places)

@enriqueartal
Copy link
Contributor

Adding this line to spkg-install.m4 file of sagelib and export CFLAGS="-std=gnu17 $CFLAGS_NON_NATIVE" to the one of gap, it works. For gap_packages, only guava needs this line, but semigroups does not work, see
semigroups.log
In fedora there is a small issue with the location of some planarity files, creating some symbolic links the problems are gone.

@dimpase dimpase requested a review from nbruin May 2, 2025 00:00
@dimpase
Copy link
Member Author

dimpase commented May 2, 2025

the tests pass, please review

@enriqueartal
Copy link
Contributor

Is there any test machine running gcc 15?

@enriqueartal
Copy link
Contributor

I cannot find anymore the message below. Did you got it? For me, maxima worked using the spkg-install.in file for ecl in this PR. I used planarity system package. Forcing spkg gave an error but it goes away adding export CFLAGS = "-std=gnu17 $CFLAGS" in the spkg-install.in file for planarity.
Still no way to get semigroups built

@enriqueartal how can i reproduce your successful build?

here's what i've tried:

git reset --hard && git clean -dfX
git pull --rebase [https://github.com/dimpase/sage.git](https://urldefense.com/v3/__https://github.com/dimpase/sage.git__;!!D9dNQwwGXtA!UiqZqOz6c0X6rWZL2kGi5EkWlEd0LS_mXb9uzmcj_h2vWOjKpWCjIjGj3Id9gj8tGg66XL3XkXJLGbTuoJRakiY$)
git pull --rebase [https://github.com/dimpase/sage.git](https://urldefense.com/v3/__https://github.com/dimpase/sage.git__;!!D9dNQwwGXtA!UiqZqOz6c0X6rWZL2kGi5EkWlEd0LS_mXb9uzmcj_h2vWOjKpWCjIjGj3Id9gj8tGg66XL3XkXJLGbTuoJRakiY$) linbox_fixes_gcc15_clang17
git pull --rebase [https://github.com/dimpase/sage.git](https://urldefense.com/v3/__https://github.com/dimpase/sage.git__;!!D9dNQwwGXtA!UiqZqOz6c0X6rWZL2kGi5EkWlEd0LS_mXb9uzmcj_h2vWOjKpWCjIjGj3Id9gj8tGg66XL3XkXJLGbTuoJRakiY$) test442332
make configure
./configure
make -j $(nproc) build

didnt work sadly, still the same problem with maxima and planarity```

@dimpase
Copy link
Member Author

dimpase commented May 2, 2025

Does the latest Semigroups build for you stand-alone? https://semigroups.github.io/Semigroups/

@enriqueartal
Copy link
Contributor

Unfortunately, not:

En el fichero incluido desde ./bin/include/libsemigroups/sims1.hpp:1269,
                 desde src/pkg.cpp:59:
./bin/include/libsemigroups/sims1.tpp: In member function ‘libsemigroups::Sims1<T>::iterator_base& libsemigroups::Sims1<T>::iterator_base::operator=(libsemigroups::Sims1<T>::iterator_base&&)’:
./bin/include/libsemigroups/sims1.tpp:622:39: error: ‘class libsemigroups::Sims1<T>::iterator_base’ has no member named ‘long_rules’ [-Wtemplate-body]
  622 |     _longs           = std::move(that.long_rules());
      |                                       ^~~~~~~~~~
make: *** [Makefile.gappkg:130: gen/src/pkg.o] Error 1

I think it is the same error.

@enriqueartal
Copy link
Contributor

In the rpm of fedora there is this patch

@dimpase
Copy link
Member Author

dimpase commented May 2, 2025

Let us do optional packages on another PR, depending on this one. Can we get this one positively reviewed and move on?

@enriqueartal
Copy link
Contributor

Agree with gap_packages. There are a couple of things:

  • Singular built only after Singular 4.4.1 + Flint 3.3.2 #40033. It is unclear to me which is the direction for dependence.
  • There are some more spkg-install.in modified files: gap, planarity, and sagelib.
    With these changes, for me it is OK.

@dimpase
Copy link
Member Author

dimpase commented May 2, 2025

#40033 is not a dependency here

@dimpase dimpase mentioned this pull request May 2, 2025
5 tasks
@nbruin
Copy link
Contributor

nbruin commented May 2, 2025

I tried a build on dimpase/develop on F42 and things failed in planarity, in the same way as in the log that enriqueartal already attached.

restarting with

CFLAGS="-std=gnu17" make -j20 build

got me a little further, but ultimately things crashed in linbox (which I wasn't able to install via system packages due to some version conflict) which complained with "cc1plus: warning: command-line option '-std=gnu17' is valid for C/ObjC but not for C++" and then produced:

[spkg-install] ../../linbox/vector/blas-subvector.h:121:20: error: 'LinBox::BlasSubvector<_Vector>::Self_t' has no member named 'data'; did you mean 'at'? [-Wtemplate-body]

@dimpase
Copy link
Member Author

dimpase commented May 3, 2025

this one is needed for mere allowing the use of gcc-15. It's not promising you that everything works with it as well.

can we please do things incrementally.
This PR does not break anything

@dimpase
Copy link
Member Author

dimpase commented May 3, 2025

Please open a separate issue for planarity, put logs there, and link it to this PR. Do not spam this PR with things which are beyond its scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants