cschwan / sage-on-gentoo

(Unofficial) Gentoo Overlay for Sage- and Sage-related ebuilds
79 stars 26 forks source link

Don't patch GAP's configure.ac #736

Closed fingolfin closed 1 year ago

fingolfin commented 1 year ago

Since this ebuild does not attempt to build HPC-GAP, there is no point in patching the parts of configure.ac that are only relevant for HPC-GAP.

It also is not necessary to add AC_CONFIG_MACRO_DIR to configure.ac

With the patch remove, there is no more need to delete aclocal.m4 or to invoke eautoreconf.


Note: again I did not actually test this PR as I don't use Gentoo. So while in principle it should be sound, this should be verified.

kiwifb commented 1 year ago

I'll be honest, I'd rather keep the AC_CONFIG_MACRO_DIR bit for the sake of correctness and prevent problems if something enforces rebuild of configure scripts in the future. I probably should complain about it to the gap team. Feel free to take it if you are part of it. Read #690 for your edification. Short of it, if I run autoreconf, things should still work without me having to do something unexpected.

fingolfin commented 1 year ago

I am "the GAP team" when it comes to these matters, you won't get a more official answer than that :-).

Thank you for pointing me to issue #690. Looking at that, I suspect the core issue there is the use of eautoreconf, which presumably is based on autoreconf, which in turn used to be (and possibly still is) quite buggy in some cases (e.g. it is not really prepared for projects which use autoconf and libtool but not automake).

In this case, it look as if eautoreconf tried to "upgrade" GAP's bundled libtool, but failed?

Simple solution: Don't use eautoreconf, instead use GAP's own autogen.sh and you won't have any trouble. It doesn't do any magical either, just basically autoconf -Wall -f && autoheader -Wall -f

For a relevant discussion why we do things the way we do, see https://github.com/gap-system/gap/issues/5383. Feel free to join in.


Looking at some of the past issues here, I think quite some pain on your side could have been avoided or at least reduced if the GAP team had been involved in the discussion to ask why we do certain things and how to properly deal with them. Or maybe we would have changed things to make life easier for you and others... I've worked hard on this in the past 1-2 years, culminating in the make install support that now many downstream GAP package use, and many of them were able to substantially simplify how they build GAP (I think that was the case here, too). I am sure we can improve it further, but that requires feedback. Of course I also know how it is when you are a lone volunteer and just need to get something out, you can't always talk a lot to upstream... It's a bit of a balance.

Anyway, let's just say I'd like to encourage you to talk to us sooner the next time you run into problems. At best we may able to provide quick help, at worst at least we know about the pain point and can think about how to deal with it long term.

In closing, I'd like to thank you for maintaining this GAP ebuild, I do appreciate it!

kiwifb commented 1 year ago

With a little bit of time to reassess I was putting a fix on top of another. The way Gentoo work I would have to reproduce what autogen.sh does if I have to do it in the future.

kiwifb commented 1 year ago

Merging because you should get the credit. There is one more things to clean on top that you could not know without more knowledge of Gentoo. I'll take care of it.

fingolfin commented 1 year ago

Thank you, @kiwifb, for merging and fixing it.