math/maxima: Add ECL and SAGE options

[salvadore]

Summary

• Add an ECL option to build maxima using ecl lisp. Option enabled by default so that Sage can work properly when using packages. [1]
• Add a SAGE option (implies ECL) to apply some patches so that Sage can work properly. This option is also enabled by default to get working Sage packages. [1]
• Fix a bug in xmaxima about documentation. This also introduces an XMAXIMA option (enabled by default) because to have xmaxima working we need to ensure that DOCS and TEST are enabled, which is done by using XMAXIMA_IMPLIES. This also has the advantage to remove USES= tk for users who want to disable XMAXIMA. [2]
• Modify TEST option so that it only installs tests, but it does not run them any more in the post-build-TEST-on target. Tests can still be run using "make test".

[1] Based on an original submission by thierry.
[2] Reported by: thierry

[thierry]

Note: math/maxima and math/maxima-sage install the same files: a CONFLICT line should be added in math/maxima.

[salvadore]

The solution you propose seems a lot of work.
While I appreciate that you offered yourself to maintain the new port you create (so that indeed you take care of the extra work), I think a much easier solution can be achieved: we could add an ECL option to the maxima port that already exists and make it the default option.

If, for some reason, we found out that it is a bad idea to set ecl as the default lisp (I will investigate on that) other easier solutions might be:

• create maxima-sage as a slave port of maxima;
• maybe use flavors?

In any case, I think it would be a good idea to create the ECL option in the traditional maxima port and I would thank you if could do that :)

[thierry]

In D24959#549674, @salvadore wrote:

If, for some reason, we found out that it is a bad idea to set ecl as the default lisp (I will investigate on that) other easier solutions might be:

• create maxima-sage as a slave port of maxima;
• maybe use flavors?

In any case, I think it would be a good idea to create the ECL option in the traditional maxima port and I would thank you if could do that :)

OK, that would be fine!

But if you plan to update ECL, I would not risk to block you if Sage is not ready for that.

[salvadore]

Ok, let's add the ECL option then and set it as default.
Do you want to modify your review yourself or do you prefer that I commandeer the revision and make the changes?

[thierry]

In D24959#550269, @salvadore wrote:

Ok, let's add the ECL option then and set it as default.
Do you want to modify your review yourself or do you prefer that I commandeer the revision and make the changes?

Do not hesitate to do it!

I have not been able to make this myself with the current port, and it's why I proposed to fork it - but I you succeeded, it's better, and I say bravo.

[salvadore]

Ok, thanks.
I will work on that very soon.

[salvadore]

This should work. I still have to finish testing it, but I had the first successfull poudriere build.
With ECL on I got a file linked to libgc-threaded.so and thus I should add libgc-threaded.so:devel/boehm-gc-threaded dependency. However I suspect that this happens only when ecl is built with THREADS on (it's the default). In that case, I think I should not declare the dependecy since:

• if ECL is installed with THREADS on libgc-threaded.so installed as well;
• if ECL is installed with THREADS off maxima is not linked to libgc-threaded.so.

Is that correct?

I add my mentors as reviewers.

[salvadore]

Remove a useless line that I had left commented and a comment not so useful.

[salvadore]

Please consider it as a work in progress. There are still a few things I have to check and fix.

[thierry]

In D24959#554634, @salvadore wrote:

This should work. I still have to finish testing it, but I had the first successfull poudriere build.
With ECL on I got a file linked to libgc-threaded.so and thus I should add libgc-threaded.so:devel/boehm-gc-threaded dependency. However I suspect that this happens only when ecl is built with THREADS on (it's the default). In that case, I think I should not declare the dependecy since:

• if ECL is installed with THREADS on libgc-threaded.so installed as well;
• if ECL is installed with THREADS off maxima is not linked to libgc-threaded.so.

Is that correct?

Building ECL and Maxima with the default options produces this error:
/usr/local/lib/maxima/5.43.2/binary-ecl/maxima is linked to /usr/local/lib//libgc-threaded.so.1 from devel/boehm-gc-threaded but it is not declared as a dependency

Since ECL is built by default with threads, this dependency should be declared.

[thierry]

Good job! I have rebuilt Sage with your version of Maxima, without any problem, and the tests cases works fine.

I just noticed two points:

1. maxima-index.lisp is installed under $PREFIX/share/info (OK)

but is searched under /usr/local/info and produced this message:

;;; Warning: Maxima is unable to set up the help system.
(Details: CL-INFO::LOAD-PRIMARY-INDEX: Filesystem error with pathname #P"/usr/local/info/maxima-index.lisp".
Either
 1) the file does not exist, or
 2) we are not allowed to access the file, or
 3) the pathname points to a broken symbolic link.)

2. It's impossible to launch xmaxima (not needed by Sage):

$ xmaxima
exec: wish: not found

(See my initial patch: I fixed that with

--with-wish=${WISH}

.

[salvadore]

I confirm that maxima links to libgc-threaded.so.1 if and only if ecl is built with threads.

Building ECL and Maxima with the default options produces this error:
/usr/local/lib/maxima/5.43.2/binary-ecl/maxima is linked to /usr/local/lib//libgc-threaded.so.1 from devel/boehm-gc-threaded but it is not declared as a dependency

Since ECL is built by default with threads, this dependency should be declared.

I am still unsure about that: if we do not declare it everything works fine, we only get that message. On the other hand, if we declare it, a user might build ecl without threads and then install devel/bohem-gc-threaded as a dependency for maxima, although maxima does not depend on it at all. I feel installing an unused dependency is worse than getting a QA error message.

In D24959#554697, @thierry wrote:

Good job! I have rebuilt Sage with your version of Maxima, without any problem, and the tests cases works fine.

I just noticed two points:

1. maxima-index.lisp is installed under $PREFIX/share/info (OK)

but is searched under /usr/local/info and produced this message:

;;; Warning: Maxima is unable to set up the help system.
(Details: CL-INFO::LOAD-PRIMARY-INDEX: Filesystem error with pathname #P"/usr/local/info/maxima-index.lisp".
Either
 1) the file does not exist, or
 2) we are not allowed to access the file, or
 3) the pathname points to a broken symbolic link.)

Can you please tell me how do you produce that error message? I do not get that error by running maxima and trying, for example, the describe command. Is that a sage specific thing?

In D24959#554697, @thierry wrote:

2. It's impossible to launch xmaxima (not needed by Sage):

$ xmaxima
exec: wish: not found

(See my initial patch: I fixed that with

--with-wish=${WISH}

.

Ok, thanks. I fixed it in my local copy and it will be in the review next time I update it.

Another thing about which I am unsure is the python dependency: it is not there in the actual version but you introduced it. Do you need it? I tried building maxima on ecl without python and running it works. Unless you have a reason to introduce it, let's avoid it so that we do not get too heavy on dependencies for nothing.

[thierry]

About the info dir: it is from Sage, when running maxima_console. But you're right, I don't see it directly from Maxima: I'll investigate it.

About Python: I don't know what it does, and it could be a no-op... I just defined it because Sage does that and I replicated their options.

And about gc-threaded, the package needs it because this is the default for ECL.
If you want to avoid a dependency for the ports when threading is disabled, it is possible to grep the output of

ecl-config --libs

[salvadore]

About info directory: I haven't done anything. Let me know if you need anything once your investigations are done.

About python: I removed it from the patch. Let me know if it there are issues.

About gc-threaded: I could not use ecl-config as it assumes ecl to be installed, which is not necessarily the case (for example I think poudriere computes all dependecies for a build before installing them). So I checked the THREAD option through make -C ${.CURDIR}/../../lang/ecl -V PORT_OPTIONS:THREAD. It is still possible to break dependencies, but is is harder and in particular everything should work fine for packages from pkg.freebsd.org.

I am testing it with poudriere. Up to now, everything works fine.

[thierry]

About info directory: I haven't done anything. Let me know if you need anything once your investigations are done.

I found this patch in Sage tarball (to be put under math/maxima/files/patch-src_init-cl.lisp:

--- src/init-cl.lisp.orig       2019-10-27 16:13:48 UTC
+++ src/init-cl.lisp
@@ -137,7 +137,7 @@ When one changes, the other does too."
          (setq libdir     (combine-path maxima-prefix-env "lib"))
          (setq libexecdir (combine-path maxima-prefix-env "libexec"))
          (setq datadir    (combine-path maxima-prefix-env "share"))
-         (setq infodir    (combine-path maxima-prefix-env #+(or cygwin windows win32 win64) "share" "info")))
+         (setq infodir    (combine-path maxima-prefix-env "share" "info")))
        (progn
          (setq libdir     (maxima-parse-dirstring *autoconf-libdir*))
          (setq libexecdir (maxima-parse-dirstring *autoconf-libexecdir*))

I don't understand how it could work, because it looks like it only removes a comment… but it does, and the problem is fixed!

About python: I removed it from the patch. Let me know if it there are issues.

Everything seems OK!

No more problem for me.

[salvadore]

In D24959#554776, @thierry wrote:

I don't understand how it could work, because it looks like it only removes a comment… but it does, and the problem is fixed!

I am not a lisp expert, but comments in lisp are introduced by ; not by #. The notation #+ introduces something else: https://stackoverflow.com/questions/40198974/what-is-the-meaning-of-and-in-common-lisp .

I am going to add that patch. Thanks!

[salvadore]

• Add init-cl.lisp patch. I have added a comment about the patch being borrowed from SageMath as you did for all other files. See however https://reviews.freebsd.org/D25180 (I think you forgot a reference for lang/ecl).

• Introduce SAGE option (enabled by default, implies ECL), so that users can also choose to build maxima on ecl without Sage's patches.

[thierry]

This is fine to be used by math/sage!

But the problem with xmaxima reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246599#c5 is still there:

when I launch xmaxima (not wxmaxima as written in the PR!), it displays the error:

Documentation not found in '/usr/local/share/maxima/5.43.2'

Either the online doc must be moved from $DOCSDIR to $DATADIR, as it was done in my initial proposition, or the code must be patched to find it.

[salvadore]

In D24959#554935, @thierry wrote:

But the problem with xmaxima reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246599#c5 is still there:

when I launch xmaxima (not wxmaxima as written in the PR!), it displays the error:

Documentation not found in '/usr/local/share/maxima/5.43.2'

Either the online doc must be moved from $DOCSDIR to $DATADIR, as it was done in my initial proposition, or the code must be patched to find it.

I will try to patch the code, I think it is preferable. Thanks for reporting that bug on xmaxima. However, I discovered that documentation for wxmaxima is also broken.
But let's do one thing at the time and focus on sage now: this patch is already complex enough on its own. I will fix those documentation bugs in a later review if they do not affect maxima support on sage.

Testing is going fine up to now.

@gerald, @tcberner: I think the patch is ready enough to be reviewed for a mentor approval.

[salvadore]

On second thought, I am going to try to fix the documentation bug for xmaxima now. I hope it to be easy enough and if it is it will save me some testing.

[thierry]

OK. As far as math/sage is concerned, it is OK (Sage does not use xmaxima).

[salvadore]

Fix xmaxima bug about documentation. This also introduces an XMAXIMA option (enabled by default) because to have xmaxima working we need to ensure that DOCS and TEST are on, which is done by using XMAXIMA_IMPLIES. This also has the advantage to remove USES= tk for users who want to disable XMAXIMA.

And I won't touch this patch anymore except for fixes. Improvements are still possible, for example by stripping binaries, but today I received a mail to inform me that maxima has been updated upstream, so whatever additional improvement I might think I will do it when updating the port.

[thierry]

Great! With this revision xmaxima works fine.

[salvadore]

Change CONFIGURE_ARGS+= --with-wish=${WISH} into XMAXIMA_CONFIGURE_WITH= wish=${WISH}.

Thanks @tcberner!

[salvadore]

head/math/maxima/Makefile

59

@mat: Can you please confirm that I can use such a line? While discussing this review with tcberner (my co-mentor) doubts arised on it. This line was the result of a conversation with thierry that you can read in the comments of this review. Thanks!

[thierry]

Since we cannot have an answer from mat@, I would follow tcberner's advice and suggest this simple patch:

diff -urN math/maxima.orig/Makefile math/maxima/Makefile
--- math/maxima.orig/Makefile   2020-06-13 20:56:46.842159000 +0200
+++ math/maxima/Makefile        2020-06-13 20:58:44.161099000 +0200
@@ -55,10 +55,8 @@
 ECL_RUN_DEPENDS=       ecl:lang/ecl
 ECL_LIB_DEPENDS=       libgmp.so:math/gmp      \
                        libgc.so:devel/boehm-gc \
-                       libffi.so:devel/libffi
-.if ${:!make -C ${.CURDIR}/../../lang/ecl -V PORT_OPTIONS:MTHREADS!}
-ECL_LIB_DEPENDS+=      libgc-threaded.so:devel/boehm-gc-threaded
-.endif
+                       libffi.so:devel/libffi  \
+                       libgc-threaded.so:devel/boehm-gc-threaded
 ECL_CONFIGURE_ON=      --with-ecl=ecl
 ECL_PLIST_SUB=         BINDIR=binary-ecl BINNAME=maxima
 GET_ECLLIB=            ecl -eval "(princ (SI:GET-LIBRARY-PATHNAME))" -eval "(quit)"

ECL is built threaded by default; the only problem would be when someone forces to build it unthreaded, and then builds maxima: we would add an unnecessary dependency on boehm-gc-threaded.
But this is not a big deal: this port is much smaller than Maxima, and it even shares its distfile with devel/boehm-gc (it's just a variant).

[salvadore]

I edited the patch following thierry's suggestion and added a comment to
remember what's the issue with that dependency.

[tcberner]

With the offending part backed out, I see no reason why this should not land :)