Support system installations of ECL and fix the kenzo SPKG build
thierry-FreeBSD opened this issue · 110 comments
I tried to use ECL and Maxima from system packages as suggested by #27330, but without success for the moment.
The spkg-configure.m4 are trivial (see attachments), but the problems occur during build:
In [dochtml] [manifolds] this error is produced:
RuntimeError: ECL says: The function SET-LOCALE-SUBDIR is undefined
Actually this error is reproducible out of the Sage's build mechanism:
$ ecl
> (require 'maxima)
> (in-package :maxima)
MAXIMA> (set-locale-subdir)
Condition of type: UNDEFINED-FUNCTION
The function MAXIMA::SET-LOCALE-SUBDIR is undefined.
According to Maxima's code, set-locale-subdir is defined in src/init-cl.lisp and:
;; If a file is declared as private-file it is loaded
;; from the build tree (which makes an out-of-tree
;; build possible), but not compiled (which means
;; that with ECL or ABCL it won't end up in the final image)
;; => no out-of-tree build for ECL.
Maybe someone with Lisp-fu could patch it, if possible?
Depends on #31593
CC: @orlitzky @kiwifb @antonio-rojas @spaghettisalat @tobiasdiez @isuruf @mkoeppe @miguelmarco @jhpalmieri @jcuevas-rozo @slel @vbraun
Component: build: configure
Keywords: ECL, Maxima
Author: Thierry Thomas, Dima Pasechnik, Michael Orlitzky
Branch/Commit: 5cbe423
Reviewer: Matthias Koeppe
Issue created by migration from https://trac.sagemath.org/ticket/29617
To be copied as build/pkgs/ecl/spkg-configure.m4
Attachment: ecl_spkg-configure.m4.gz
Attachment: maxima_spkg-configure.m4.gz
To be copied as build/pkgs/maxima/spkg-configure.m4
Note: I also tried to use only ECL from the system packages, and build maxima from Sage, but in this case Maxima fails
- with the default ECLDIR in sage-env the cmp lisp files are not found:
ecl -norc -eval '(progn (load "../lisp-utils/defsystem.lisp") (load "../lisp-utils/make-depends.lisp") (funcall (intern "CREATE-DEPENDENCY-FILE" :mk) "binary-ecl/maxima" "ecl-depends.mk") (quit))'
;;; Loading "/usr/ports/math/sage/work/stage/usr/local/var/tmp/sage/build/maxima-5.42.2/src/src/../lisp-utils/defsystem.lisp"
An error occurred during initialization:
Filesystem error with pathname #P"SYS:CMP.NEWEST".
- when ECLDIR is patched to the right value, that works, but it fails when it tries to install the maxima library under this directory (unless you are root!).
I would wait for #22191 before attempting this if I were you. We have a mountain of patches for ecl-16.1.2 that won't be present in any system copy, and ecl-16.1.3 is incompatible with sage (that's why we haven't upgraded to it after so many years). The latest v20.4.24 release is from just a few days ago, though, and with any luck it will work (unpatched) with sage and we can look for v20.4.24 on the system.
OK, in FreeBSD the ECL system package is at 16.1.3!
I did not know that it was incompatible with sage, maybe this is the source of the problem, thanks.
Dependencies: #22191
Note: applying patches from build/pkgs/ecl/patches and build/pkgs/maxima/patches to the FreeBSD ports, I have been able to use them as system packages. For ECL 16.1.3, the patches from #22191 are required.
perhaps one can get FreeBSD's ECL bumped to v20.4.24 ? Well, we do ship a lot of patches for it in #22191 - but they are all upstream patches.
Changed author from gh-thierry-FreeBSD to Thierry Thomas
Changed author from Thierry Thomas to Thierry Thomas, Dima Pasechnik
Branch: u/dimpase/packages/eclconfig
It appears that perhaps Gentoo's ecl misses a number of important patches - in particular it can't build maxima.fas (some path mixup).
I'm not an ecl maintainer, so we will probably have to wait for the next point release of ecl to address all of the bugs that sage fixes with patches. Afterwards the spkg-configure.m4 should check for a version newer than that patch release.
If we add this spkg-configure now, we're going to be in a position similar to the one we were in with pari a few months ago. The upstream release is unsuitable, so we need to test for all of the bugs fixed by the custom patches so that we don't accept a distro version that's missing them.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3b7c2f6 | adding spkg-configure for ecl |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
301800f | adding spkg-configure for ecl |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
83b9f9d | adding spkg-configure for ecl |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d5a4fc0 | adding spkg-configure for ecl |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
aea5768 | adding spkg-configure for ecl |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
341e99c | adding spkg-configure for ecl |
I've narrowed the ticket to ECL. Dealing with a system maxima can be done on a follow-up ticket.
Following up on the discussion in #30770, when using a (sufficiently patched) system ECL, our maxima install script should not attempt to install the FASL in the (typically read only) ECL system directory but rather set the configuration variable MAXIMA_FAS to the correct path.
Changed branch from u/dimpase/packages/eclconfig to u/mkoeppe/packages/eclconfig
Branch pushed to git repo; I updated commit sha1. New commits:
271f030 | adding spkg-configure for ecl |
How can we detect a sufficiently patched ECL, as is presumably shipped by conda, arch, gentoo, ...?
Branch pushed to git repo; I updated commit sha1. New commits:
c6062b2 | build/pkgs/ecl/distros/arch.txt: New |
Now that #31336 is in, I don't think we need to detect anything other than the ECL version. If they stick to the new versioning scheme, then the following constant (defined in /usr/include/ecl/config.h, here) can probably be checked with a simple integer comparison:
#define ECL_VERSION_NUMBER 210201
+1
Changed branch from u/mkoeppe/packages/eclconfig to u/mjo/ticket/29617
Changed author from Thierry Thomas, Dima Pasechnik to Thierry Thomas, Dima Pasechnik, Michael Orlitzky
maxima install script needs adjustment, see comment 19
In sage-on-gentoo it works fine but the following doctest failure appeared
sage -t --long --random-seed=0 /usr/lib/python3.9/site-packages/sage/interfaces/tests.py
**********************************************************************
File "/usr/lib/python3.9/site-packages/sage/interfaces/tests.py", line 34, in sage.interfaces.tests
Failed example:
subprocess.call("echo syntax error | ecl", **kwds) in (0, 255)
Expected:
True
Got:
False
Not sure if it is because of any other aspect of sage-on-gentoo/Gentoo or the lone patch in vanilla sage.
Replying to @kiwifb:
In sage-on-gentoo it works fine but the following doctest failure appeared
...
Not sure if it is because of any other aspect of sage-on-gentoo/Gentoo or the lone patch in vanilla sage.
That's from the missing patch. When stderr is /dev/full, ecl used to go into an infinite loop. That particular problem is fixed, but the test in question actually goes a bit further and checks what happens if both stdout and stderr are full. In short: it segfaults. I reported this upstream a few hours ago not realizing we had a test for it: https://gitlab.com/embeddable-common-lisp/ecl/-/issues/634
I'd rather not add 139 (segault) to the list of allowed return codes there... so maybe we should make the ECL test weaker for the time being? The patch that sage carries is no longer being considered upstream anywhere.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7e259fc | build/pkgs/ecl: Add more to distros/ |
88d1ea5 | adding spkg-configure for ecl |
04c4412 | build/pkgs/ecl/distros/arch.txt: New |
3824393 | Trac #29617: check for ecl-config instead of ecl. |
30f5f15 | Trac #29617: compile a check for ECL_VERSION_NUMBER. |
dde89cf | Trac #29617: substitute ecl-config path into sage_conf. |
1dcda9d | Trac #29617: remove failing ECL test. |
What's the correct way to substitute a $SAGE_LOCAL path into sage_conf.py.in at build time? The "substitute ecl-config path into sage_conf" commit is currently broken with the SPKG because I don't know what to put in there. It works with the system copy though.
Replying to @orlitzky:
What's the correct way to substitute a
$SAGE_LOCALpath intosage_conf.py.inat build time?
OK to just use ecl-config without path, as we may assume that SAGE_LOCAL/bin is near the front of the PATH.
Ok, that worked.
We still need the maxima fix from comment:19. And I guess we should check the other packages (kenzo, fricas) that depend on ecl as well.
Replying to @mkoeppe:
Replying to @orlitzky:
What's the correct way to substitute a
$SAGE_LOCALpath intosage_conf.py.inat build time?OK to just use
ecl-configwithout path, as we may assume thatSAGE_LOCAL/binis near the front of the PATH.
I ran into the same problem with MAXIMA_FAS, which should default to the SAGE_LOCAL path where the Maxima SPKG installs it.
I've worked around this in the latest commit but it's pretty ugly. I made the SPKG location the default again, undoing part of #26100. That works because SAGE_LOCAL is available in env.py.
But there are two other things you might want to set MAXIMA_FAS to: an unusual but correct path, and "no path." Setting it to the correct path is easy, but now "no path" is achieved by setting MAXIMA_FAS = <junk> in sage_conf, and counting on the fallback in maxima_lib.py.
I'm certainly open to other ideas. If we decide to keep the workaround, @antonio-rojas in particular should make sure I didn't mess anything up too badly for distro packages.
This passes tests with my system ECL and SPKG maxima on Gentoo.
Replying to @orlitzky:
I'm certainly open to other ideas. If we decide to keep the workaround, @antonio-rojas in particular should make sure I didn't mess anything up too badly for distro packages.
Hi,
Looks good as far as the Arch distro package is concerned. ecl_eval("(require 'maxima \"{}\")".format(MAXIMA_FAS)) fails because the ecl modules are in the default upstream path /usr/lib/ecl-${version}, but then ecl_eval("(require 'maxima)") succeeds.
this works on my Gentoo box, with system's ECL, too.
Matthias, as a Lisp guru, what's your opinion on this? Maybe it all may be fixed on Lisp level?
Work Issues: fix kenzo SPKG with system ECL
I would think (hope) that the current solution is more complicated than needed.
Distributions are able to install maxima in a way that a simple (REQUIRE :MAXIMA) works in ecl.
(Note that this is not just a search for a maxima.fasl but rather goes through EXT:*MODULE-PROVIDER-FUNCTIONS* into which system definition facilities such as MK:DEFSYSTEM or ASDF hook.)
If the Sage distribution has to install maxima, then we install the FASL in a custom directory and set the variable MAXIMA_FAS to the correct value in sage_conf. No defaults should be set in env.py.
Instead of the try/except in maxima_lib.py, just dispatch on empty/nonempty value of MAXIMA_FAS.
Replying to @mkoeppe:
If the Sage distribution has to install maxima, then we install the FASL in a custom directory and set the variable
MAXIMA_FASto the correct value insage_conf. No defaults should be set inenv.py.Instead of the
try/exceptinmaxima_lib.py, just dispatch on empty/nonempty value ofMAXIMA_FAS.
This was the first thing I tried, but I ran into the same problem as in comment:34. With no (?) way to substitute $SAGE_LOCAL at ./configure-time, I made the $SAGE_LOCAL location the default at runtime instead. That way you can effectively point MAXIMA_FAS at SAGE_LOCAL by leaving it undefined.
Pretty much all of the extra complexity comes from that. If there's a way to get $SAGE_LOCAL into MAXIMA_FAS at ./configure-time, I can make this branch a lot simpler.
OK, thanks for the explanation, I had forgotten about this issue.
I'll work on a general solution for this problem.
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
dfad5c9 | adding spkg-configure for ecl |
d2561f3 | build/pkgs/ecl/distros/arch.txt: New |
94e6941 | Trac #29617: check for ecl-config instead of ecl. |
d69775a | Trac #29617: compile a check for ECL_VERSION_NUMBER. |
fddcc0c | Trac #29617: substitute ecl-config path into sage_conf. |
e562adc | Trac #29617: remove failing ECL test. |
fb1b8ca | Trac #29617: don't set ECLDIR in sage-env. |
835cfc0 | Trac #29617: move MAXIMA_FAS to sage_conf. |
7e3a721 | Trac #29617: modify Kenzo interface to support system ECL. |
9f1afba | Trac #29617: fix kenzo SPKG build. |
Changed work issues from fix kenzo SPKG with system ECL to none
Looks a lot better now, just needs testing.
I would suggest to change the lines
if KENZO_FAS is not None:
if MAXIMA_FAS is not None:
to if not KENZO_FAS etc. so that it makes no difference whether these variables are undefined in sage_conf.py or defined as the empty string.
Branch pushed to git repo; I updated commit sha1. New commits:
4aeb535 | Trac #29617: treat empty MAXIMA_FAS and KENZO_FAS as None. |
+ # assumes that $SAGE_LOCAL/bin is somewhere near
+ # the beginning of the user's PATH
+ AC_SUBST(SAGE_ECL_CONFIG, ['${prefix}'/bin/ecl-config])
this comment is no longer true and should be removed
+ from sage.env import KENZO_FAS
+ if not KENZO_FAS:
+ ecl_eval("(require :kenzo \"{}\")".format(KENZO_FAS))
+ else:
+ ecl_eval("(require :kenzo)")
I think this one is backwards
ecl_eval("(setf *load-verbose* NIL)")
-if MAXIMA_FAS is not None:
+if not MAXIMA_FAS:
ecl_eval("(require 'maxima \"{}\")".format(MAXIMA_FAS))
this one too
Could you add some comments regarding the "fasb" business for Kenzo?
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
677cd22 | Trac #29617: substitute ecl-config path into sage_conf. |
fe77e5e | Trac #29617: remove failing ECL test. |
db002e7 | Trac #29617: don't set ECLDIR in sage-env. |
cd04a8c | Trac #29617: move MAXIMA_FAS to sage_conf. |
482ad99 | Trac #29617: modify Kenzo interface to support system ECL. |
683abb9 | Trac #29617: fix kenzo SPKG build. |
1615691 | Trac #29617: treat empty MAXIMA_FAS and KENZO_FAS as None. |
Sure, all done.
So this fixes a regression for the kenzo package introduced by the ECL update?
It would be good to update the ticket description.
Also, run this ticket on GH Actions please so that we can see if the system packages are accepted correctly
Replying to @mkoeppe:
So this fixes a regression for the
kenzopackage introduced by the ECL update?
Kenzo is optional so I wouldn't guess as to when it stopped working. Some of its tests fail; for example, sage/interfaces/kenzo.py has
EXAMPLES::
sage: from sage.interfaces.kenzo import MooreSpace # optional - kenzo
sage: m24 = MooreSpace(2,4) # optional - kenzo
sage: m24 # optional - kenzo
[K10 Simplicial-Set]
but when I run it I get
sage: from sage.interfaces.kenzo import MooreSpace
sage: m24 = MooreSpace(2,4)
sage: m24
[K1 Simplicial-Set]
Tox run: https://github.com/orlitzky/sage/actions/runs/712735969
Replying to @orlitzky:
Replying to @mkoeppe:
So this fixes a regression for the
kenzopackage introduced by the ECL update?Kenzo is optional so I wouldn't guess as to when it stopped working.
We do run CI tests on GH Actions for all optional packages (just the installation + spkg-check). See for example https://github.com/sagemath/sage/runs/2223044706 for 9.3.rc1. I do not see any build failures for kenzo. Were just some tests of the sagelib testsuite failing when kenzo is installed (the GH Actions do not test this), or what is the issue?
Replying to @mkoeppe:
We do run CI tests on GH Actions for all optional packages (just the installation + spkg-check). See for example https://github.com/sagemath/sage/runs/2223044706 for 9.3.rc1. I do not see any build failures for kenzo. Were just some tests of the sagelib testsuite failing when kenzo is installed (the GH Actions do not test this), or what is the issue?
Digging deeper, I probably just broke kenzo with the ECL spkg.
Our ECL spkg bundles ASDF, and the bundled version works OK. But if the system copy of ECL is used and if that system copy does not bundle ASDF, then there's a backwards-incompatible API change in the ASDF make-build function which has been deprecated for a while (and now exists only for the sake of backwards-compatibility with ECL).
Ultimately the make-build function takes a type argument (was :fasl but I changed it to :fasb) and passes it to another function called select-bundle-operation. In the bundled copy of ASDF, it looks like
;; Find the operation that produces a given bundle-type
(defun select-bundle-operation (type &optional monolithic)
(ecase type
((:dll :shared-library)
(if monolithic 'monolithic-dll-op 'dll-op))
((:lib :static-library)
(if monolithic 'monolithic-lib-op 'lib-op))
((:fasl)
(if monolithic 'monolithic-compile-bundle-op 'compile-bundle-op))
((:image)
'image-op)
((:program)
'program-op)))but in my unbundled ASDF, it's
;; Find the operation that produces a given bundle-type
(defun select-bundle-operation (type &optional monolithic)
(ecase type
((:dll :shared-library)
(if monolithic 'monolithic-dll-op 'dll-op))
((:lib :static-library)
(if monolithic 'monolithic-lib-op 'lib-op))
((:fasb)
(if monolithic 'monolithic-compile-bundle-op 'compile-bundle-op))
((:image)
'image-op)
((:program)
'program-op))))To avoid having to support both, one idea that comes to mind is that we could take a newer version of asdf.lisp and copy it over the bundled version in the ECL spkg. Then :fasb would work in both of the situations above. However it would still fail if the system ECL is using a bundled asdf.lisp. Patching kenzo's compile.lisp to try both is likely the only reliable way.
Does Kenzo have an active upstream maintainer?
Replying to @orlitzky:
we could take a newer version of
asdf.lispand copy it over the bundled version in the ECL spkg. Then:fasbwould work in both of the situations above.
As we currently do not have plans to ship additional Common Lisp libraries in the Sage distribution, I think we should not do unbundling work on ECL but stick with upstream.
Patching kenzo's
compile.lispto try both is likely the only reliable way.
Yes, I agree, this seems to be the way to go. You can use HANDLER-CASE for this http://clhs.lisp.se/Body/m_hand_1.htm
It would probably also be good to add a kenzo/spkg-check.in that at least tries to load it into ECL.
Replying to @mkoeppe:
Does Kenzo have an active upstream maintainer?
Paging @miguelmarco :o
- The original kenzo upstream is at https://www-fourier.ujf-grenoble.fr/~sergerar/Kenzo/
- Our SPKG points to https://github.com/gheber/kenzo
- The release itself comes from https://github.com/miguelmarco/kenzo/
The ASDF docs say that program-op and program-system should replace make-build. Those functions are available in the bundled copy of ASDF as well, so another good long-term option would be for kenzo upstream to switch to them (if possible).
- The original kenzo upstream is at https://www-fourier.ujf-grenoble.fr/~sergerar/Kenzo/
- Our SPKG points to https://github.com/gheber/kenzo
- The release itself comes from https://github.com/miguelmarco/kenzo/
The ASDF docs say that
program-opandprogram-systemshould replacemake-build. Those functions are available in the bundled copy of ASDF as well, so another good long-term option would be for kenzo upstream to switch to them (if possible).
To clarify this:
What we maintain (by "we" I mean Ana Romeno, Julian Cuevas, Jose Divason and myself) is a small fork of Kenzo, modified to be compatible with ECL, easier to install, and some extra features to allow a better interface with Sage.
I am not fluent in lisp myself, but will talk about this issues with the rest of the team. Maybe we can modify the package to make it more installation friendly.
Replying to @orlitzky:
The ASDF docs say that
program-opandprogram-systemshould replacemake-build. Those functions are available in the bundled copy of ASDF as well, so another good long-term option would be for kenzo upstream to switch to them (if possible).
Yes, this sounds the like the right solution.
Replying to @mkoeppe:
Replying to @orlitzky:
The ASDF docs say that
program-opandprogram-systemshould replacemake-build. Those functions are available in the bundled copy of ASDF as well, so another good long-term option would be for kenzo upstream to switch to them (if possible).Yes, this sounds the like the right solution.
I have done some tests on that. Might release a new version with this change in a few days, after some testing.
So, to make sure: can you confirm that changing from
(asdf:make-build :kenzo :type :fasl :monolithic t :move-here #P".")
to
(asdf:operate 'asdf:monolithic-compile-bundle-op :kenzo)
in the compile.lisp file would solve the problem?
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.
is kenzo build fixed? I'm getting (on macOS/homebrew), with ecl from Homebrew
[kenzo-1.1.9] Package 'kenzo' is currently not installed
[kenzo-1.1.9] No legacy uninstaller found for 'kenzo'; nothing to do
[kenzo-1.1.9] sed: 1: "compile.lisp": command c expects \ followed by text
[kenzo-1.1.9] ********************************************************************************
[kenzo-1.1.9] failed to update make-build invocation
Replying to @miguelmarco:
I have done some tests on that. Might release a new version with this change in a few days, after some testing.
So, to make sure: can you confirm that changing from
(asdf:make-build :kenzo :type :fasl :monolithic t :move-here #P".")to
(asdf:operate 'asdf:monolithic-compile-bundle-op :kenzo)in the
compile.lispfile would solve the problem?
It fixes the build for my system ECL (with ASDF unbundled). If it also works with sage's ECL spkg, I think we have a solution. Thanks!
I just made a new release. You can get the tarball at https://github.com/miguelmarco/kenzo/releases/download/1.1.10/kenzo-1.1.10.tar.gz
Please check that it solves the problem, and then we can update the kenzo Sage package.
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
27727ac | build/pkgs/ecl/distros/arch.txt: New |
38cc46d | Trac #29617: check for ecl-config instead of ecl. |
18cbcee | Trac #29617: compile a check for ECL_VERSION_NUMBER. |
444d828 | Trac #29617: substitute ecl-config path into sage_conf. |
4061f54 | Trac #29617: remove failing ECL test. |
edd246f | Trac #29617: don't set ECLDIR in sage-env. |
133aab7 | Trac #29617: move MAXIMA_FAS to sage_conf. |
9b595c0 | Trac #29617: modify Kenzo interface to support system ECL. |
b4ae35e | Trac #29617: treat empty MAXIMA_FAS and KENZO_FAS as None. |
2d40f36 | Trac #29617: upgrade kenzo to v1.1.10. |