sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.35k stars 460 forks source link

Update FriCAS to 1.3.8 #34051

Closed mantepse closed 2 years ago

mantepse commented 2 years ago

This ticket is to upgrade to FriCAS 1.3.8 and track dependencies.

Tarball: see checksums.ini

Note that FriCAS has switched to using Git and the primary repository is now the one at GitHub, but the homepage remains at Sourceforge:

Our last update was to Fricas 1.3.7 in #32049.

To try this ticket from your Sage root directory, with git-trac-command installed:

  1. git trac try
  2. SAGE_SPKG="sage-spkg -o" ./sage -i fricas (or use ./configure --enable-download-from-upstream-url first)
  3. ./sage -t --optional=fricas,sage src/sage/interfaces/fricas.py

Depends on #34017

CC: @fchapoton

Component: packages: optional

Keywords: FriCAS

Author: Martin Rubey

Branch/Commit: d7df5ec

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/34051

mantepse commented 2 years ago

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@

 Note that FriCAS has switched to using Git and the primary repository is now the one at GitHub, but the homepage remains at Sourceforge:

-- https://fricas.sourceforge.net
+- http://fricas.sf.net/download.html 
 - https://github.com/fricas/fricas/releases

 Our last update was to Fricas 1.3.7 in #32049.
mantepse commented 2 years ago

Branch: u/mantepse/update_fricas_to_1_3_8

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Commit: 4a579cf

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

4a579cfadapt doctests
mantepse commented 2 years ago
comment:4

not completely sure whether package-version.txt shouldn't contain 1.3.8p1, but if I do, it tries to download fricas-1.3.8p1 from upstream, which obviously fails.

mkoeppe commented 2 years ago
comment:5

To indicate a patchlevel from patches added in Sage, you would use 1.3.8.p1 (note the extra dot)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 4a579cf to b48a4ac

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

b48a4accorrect patch level
mantepse commented 2 years ago
comment:7

Please test! I removed two patches, because it is unclear to me what they do and it works without them on my system (ubuntu 21.04), but I can imagine that they are needed on other systems.

I did not touch macos_list.patch.

I am slightly unsure whether it would be better to use a tolerance instead of an ellipsis here:

@@ -1563,7 +1563,7 @@ class FriCASElement(ExpectElement):
             sage: psi(1.0)
             -0.577215664901533
             sage: fricas.polygamma(1, 1.0)                                      # optional - fricas
-            1.6449340668482269
+            1.644934066848226...
             sage: psi(1, 1).n()
             1.64493406684823
mkoeppe commented 2 years ago
comment:8

Fails on macOS: https://github.com/mkoeppe/sage/runs/7056483127?check_suite_focus=true

checking host system type... x86_64-apple-darwin19.6.0
checking target system type... x86_64-apple-darwin19.6.0
checking for in-tree build on case insensitive file system... configure: error: in tree build on case insensitive file system is not supported. Use out-of-source-tree build instead.
********************************************************************************
mantepse commented 2 years ago
comment:10

I don't know how to do that: I think we want to pass enable_case_insensitive_file_system_check=no to configure.

mantepse commented 2 years ago
comment:11

Could you try the following?

diff --git a/build/pkgs/fricas/spkg-install.in b/build/pkgs/fricas/spkg-install.in
index df98bfdb00f..5bd4f903148 100644
--- a/build/pkgs/fricas/spkg-install.in
+++ b/build/pkgs/fricas/spkg-install.in
@@ -3,6 +3,6 @@ cd src
 # Use newer version of config.guess and config.sub (see Trac #23847)
 cp "$SAGE_ROOT"/config/config.* config

-sdh_configure --with-lisp=ecl
+sdh_configure --with-lisp=ecl --enable_case_insensitive_file_system_check=no
 sdh_make
 sdh_make_install -j1
mkoeppe commented 2 years ago
comment:12

I first tried the out-of-source-tree build that was suggested by that error message:

diff --git a/build/pkgs/fricas/spkg-install.in b/build/pkgs/fricas/spkg-install.in
index df98bfdb00..bf9f559cc7 100644
--- a/build/pkgs/fricas/spkg-install.in
+++ b/build/pkgs/fricas/spkg-install.in
@@ -3,6 +3,9 @@ cd src
 # Use newer version of config.guess and config.sub (see Trac #23847)
 cp "$SAGE_ROOT"/config/config.* config

-sdh_configure --with-lisp=ecl
+mkdir build
+cd build
+ln -s ../configure .
+sdh_configure --srcdir=.. --with-lisp=ecl
 sdh_make
 sdh_make_install -j1

But it appears to be broken:

[fricas-1.3.8.p1] FRICAS-LISP> 
[fricas-1.3.8.p1] ;;;
[fricas-1.3.8.p1] ;;; Compiling fricas-lisp.lisp.
[fricas-1.3.8.p1] ;;; OPTIMIZE levels: Safety=0, Space=0, Speed=3, Debug=0
[fricas-1.3.8.p1] ;;;
[fricas-1.3.8.p1] ;;; Style warning:
[fricas-1.3.8.p1] ;;;   in file fricas-lisp.lisp, position 2874
[fricas-1.3.8.p1] ;;;   at (DEFUN SAVE-CORE-RESTART ...)
[fricas-1.3.8.p1] ;;;   ! The variable RESTART is not used.
[fricas-1.3.8.p1] ;;; Style warning:
[fricas-1.3.8.p1] ;;;   in file fricas-lisp.lisp, position 2874
[fricas-1.3.8.p1] ;;;   at (DEFUN SAVE-CORE-RESTART ...)
[fricas-1.3.8.p1] ;;;   ! The variable CORE-IMAGE is not used.
[fricas-1.3.8.p1] ;;; Style warning:
[fricas-1.3.8.p1] ;;;   in file fricas-lisp.lisp, position 11341
[fricas-1.3.8.p1] ;;;   at (DEFUN quiet_load_alien ...)
[fricas-1.3.8.p1] ;;;   ! The variable S is not used.
[fricas-1.3.8.p1] ;;; End of Pass 1.
[fricas-1.3.8.p1] ;;; Internal error:
[fricas-1.3.8.p1] ;;;   ** Error code 1 when executing
[fricas-1.3.8.p1] ;;; (EXT:RUN-PROGRAM "clang" ("-I." "-I/usr/local/Cellar/ecl/21.2.1_2/include/" "-I/usr/local/opt/gmp/include" "-I/usr/local/opt/libffi/include" "-I/usr/local/opt/bdw-gc/include" "-g" "-O2" "-fPIC" "-fno-common" "-D_THREAD_SAFE" "-Ddarwin" "-O2" "-c" "fricas-lisp.c" "-o" "fricas-lisp.o" "-I//Users/mkoeppe/s/sage/sage-rebasing/worktree-rebase/local/var/tmp/sage/build/fricas-1.3.8.p1/src/src/include/" "-I//Users/mkoeppe/s/sage/sage-rebasing/worktree-rebase/local/var/tmp/sage/build/fricas-1.3.8.p1/src/src/../config/")):
[fricas-1.3.8.p1] ;;; In file included from fricas-lisp.c:6:
[fricas-1.3.8.p1] ;;; In file included from ./fricas-lisp.eclh:7:
[fricas-1.3.8.p1] ;;; //Users/mkoeppe/s/sage/sage-rebasing/worktree-rebase/local/var/tmp/sage/build/fricas-1.3.8.p1/src/src/include/com.h:45:10: fatal error: 'fricas_c_macros.h' file not found
[fricas-1.3.8.p1] ;;; #include "fricas_c_macros.h"
[fricas-1.3.8.p1] ;;;          ^~~~~~~~~~~~~~~~~~~
[fricas-1.3.8.p1] ;;; 1 error generated.
[fricas-1.3.8.p1] NIL
[fricas-1.3.8.p1] T
[fricas-1.3.8.p1] T
mkoeppe commented 2 years ago
comment:13

But your fix (after a spelling fix) also does not work:

diff --git a/build/pkgs/fricas/spkg-install.in b/build/pkgs/fricas/spkg-install.in
index df98bfdb00..62a15704f1 100644
--- a/build/pkgs/fricas/spkg-install.in
+++ b/build/pkgs/fricas/spkg-install.in
@@ -3,6 +3,6 @@ cd src
 # Use newer version of config.guess and config.sub (see Trac #23847)
 cp "$SAGE_ROOT"/config/config.* config

-sdh_configure --with-lisp=ecl
+sdh_configure --with-lisp=ecl --enable-case-insensitive-file-system-check=no
 sdh_make
 sdh_make_install -j1
[fricas-1.3.8.p1] ;;; Compiling fricas-lisp.lisp.
[fricas-1.3.8.p1] ;;; OPTIMIZE levels: Safety=0, Space=0, Speed=3, Debug=0
[fricas-1.3.8.p1] ;;;
[fricas-1.3.8.p1] ;;; Style warning:
[fricas-1.3.8.p1] ;;;   in file fricas-lisp.lisp, position 2874
[fricas-1.3.8.p1] ;;;   at (DEFUN SAVE-CORE-RESTART ...)
[fricas-1.3.8.p1] ;;;   ! The variable RESTART is not used.
[fricas-1.3.8.p1] ;;; Style warning:
[fricas-1.3.8.p1] ;;;   in file fricas-lisp.lisp, position 2874
[fricas-1.3.8.p1] ;;;   at (DEFUN SAVE-CORE-RESTART ...)
[fricas-1.3.8.p1] ;;;   ! The variable CORE-IMAGE is not used.
[fricas-1.3.8.p1] ;;; Style warning:
[fricas-1.3.8.p1] ;;;   in file fricas-lisp.lisp, position 11341
[fricas-1.3.8.p1] ;;;   at (DEFUN quiet_load_alien ...)
[fricas-1.3.8.p1] ;;;   ! The variable S is not used.
[fricas-1.3.8.p1] ;;; End of Pass 1.
[fricas-1.3.8.p1] ;;; Internal error:
[fricas-1.3.8.p1] ;;;   ** Error code 1 when executing
[fricas-1.3.8.p1] ;;; (EXT:RUN-PROGRAM "clang" ("-I." "-I/usr/local/Cellar/ecl/21.2.1_2/include/" "-I/usr/local/opt/gmp/include" "-I/usr/local/opt/libffi/include" "-I/usr/local/opt/bdw-gc/include" "-g" "-O2" "-fPIC" "-fno-common" "-D_THREAD_SAFE" "-Ddarwin" "-O2" "-c" "fricas-lisp.c" "-o" "fricas-lisp.o" "-I//Users/mkoeppe/s/sage/sage-rebasing/worktree-rebase/local/var/tmp/sage/build/fricas-1.3.8.p1/src/src/include/" "-I//Users/mkoeppe/s/sage/sage-rebasing/worktree-rebase/local/var/tmp/sage/build/fricas-1.3.8.p1/src/src/../config/")):
[fricas-1.3.8.p1] ;;; fricas-lisp.c:639:25: error: implicit declaration of function 'remove_directory' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
[fricas-1.3.8.p1] ;;;   value0 = ecl_make_int(remove_directory(ecl_base_string_pointer_safe(v2)));
[fricas-1.3.8.p1] ;;;                         ^
[fricas-1.3.8.p1] ;;; /usr/local/Cellar/ecl/21.2.1_2/include/ecl/external.h:1107:23: note: expanded from macro 'ecl_make_int'
[fricas-1.3.8.p1] ;;; # define ecl_make_int ecl_make_int32_t
[fricas-1.3.8.p1] ;;;                       ^
[fricas-1.3.8.p1] ;;; fricas-lisp.c:639:25: note: did you mean 'L25_remove_directory_'?
[fricas-1.3.8.p1] ;;; /usr/local/Cellar/ecl/21.2.1_2/include/ecl/external.h:1107:23: note: expanded from macro 'ecl_make_int'
[fricas-1.3.8.p1] ;;; # define ecl_make_int ecl_make_int32_t
[fricas-1.3.8.p1] ;;;                       ^
[fricas-1.3.8.p1] ;;; ./fricas-lisp.eclh:38:18: note: 'L25_remove_directory_' declared here
[fricas-1.3.8.p1] ;;; static cl_object L25_remove_directory_(cl_object );
[fricas-1.3.8.p1] ;;;                  ^
[fricas-1.3.8.p1] ;;; fricas-lisp.c:921:25: error: implicit declaration of function 'makedir' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
[fricas-1.3.8.p1] ;;;   value0 = ecl_make_int(makedir(ecl_base_string_pointer_safe(v2)));
[fricas-1.3.8.p1] ;;;                         ^
[fricas-1.3.8.p1] ;;; /usr/local/Cellar/ecl/21.2.1_2/include/ecl/external.h:1107:23: note: expanded from macro 'ecl_make_int'
[fricas-1.3.8.p1] ;;; # define ecl_make_int ecl_make_int32_t
[fricas-1.3.8.p1] ;;;                       ^
[fricas-1.3.8.p1] ;;; 2 errors generated.
[fricas-1.3.8.p1] NIL
[fricas-1.3.8.p1] T
[fricas-1.3.8.p1] T
mantepse commented 2 years ago
comment:14

OK, so we cannot quite remove the patch in extdecls.patch. Could you please try to put the following into a file extdecls.patch in the patches directory.

diff --git a/src/include/cfuns-c.H1 b/src/include/cfuns-c.H1
index af23933a..8e458c6c 100644
--- a/src/include/cfuns-c.H1
+++ b/src/include/cfuns-c.H1
@@ -1,2 +1,4 @@
 extern int directoryp(char *);
 extern int writeablep(char *);
+extern int remove_directory(char *);
+extern int makedir(char *);
mkoeppe commented 2 years ago
comment:15

That out-of-source check in the makefile seems broken too: https://github.com/mkoeppe/fricas/runs/7099432851?check_suite_focus=true

mantepse commented 2 years ago
comment:16

sorry, that's way over my head :-(

mkoeppe commented 2 years ago
comment:17

Replying to @mkoeppe:

That out-of-source check in the makefile seems broken too: https://github.com/mkoeppe/fricas/runs/7099432851?check_suite_focus=true

I'm taking this back.

mkoeppe commented 2 years ago
comment:18

Looks like FriCAS does not have sufficient CI coverage. I have prepared one at https://github.com/fricas/fricas/pull/102, please take a look

mantepse commented 2 years ago
comment:19

I am not really a fricas developer, and I know nothing about CI :-(

mkoeppe commented 2 years ago

Dependencies: #34017

mkoeppe commented 2 years ago

Changed branch from u/mantepse/update_fricas_to_1_3_8 to u/mkoeppe/update_fricas_to_1_3_8

mkoeppe commented 2 years ago
comment:22

Replying to @mantepse:

OK, so we cannot quite remove the patch in extdecls.patch. Could you please try to put the following into a file extdecls.patch in the patches directory.

diff --git a/src/include/cfuns-c.H1 b/src/include/cfuns-c.H1
index af23933a..8e458c6c 100644
--- a/src/include/cfuns-c.H1
+++ b/src/include/cfuns-c.H1
@@ -1,2 +1,4 @@
 extern int directoryp(char *);
 extern int writeablep(char *);
+extern int remove_directory(char *);
+extern int makedir(char *);

How about sending this as a PR to FriCAS


New commits:

4f8939fbuild/pkgs/fricas/spkg-install.in: Use --enable-case-insensitive-file-system-check=no
58d62f4build/bin/write-dockerfile.sh: ADD src/VERSION.txt
4e12f40Merge #34017
mkoeppe commented 2 years ago

Changed commit from b48a4ac to 4e12f40

mantepse commented 2 years ago
comment:23

re comment:14: does it work? I don't have access to a mac, so I cannot say.

Provided it does: FriCAS 1.3.8 is released, and the content of cfuns-c.H1 changed so much that the original patch did not even apply. So, yes, we should mention it to the FriCAS people, but it doesn't make sense to wait for the next release.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 4e12f40 to d7df5ec

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

d7df5ecbuild/pkgs/fricas/patches/extdecls.patch: Re-add (updated)
mkoeppe commented 2 years ago
comment:25

Yes, it works

mkoeppe commented 2 years ago

Reviewer: Matthias Koeppe

mkoeppe commented 2 years ago
comment:27

Also ./sage -tp src/sage/interfaces/fricas.py src/sage/calculus src/sage/functions src/sage/symbolic:

All tests passed!
mantepse commented 2 years ago
comment:28

Great! Thank you so much!

Is there any other platform which should absolutely be tested? If not so, please go ahead!

mkoeppe commented 2 years ago
comment:29

Take a look at the discussion in https://github.com/fricas/fricas/pull/102 - using the CI, it was discovered that fricas is broken on recent-ish Fedora when the Fedora-provided ECL is in use.

That's a regression in platform support. See https://github.com/sagemath/sage/runs/6443849430?check_suite_focus=true - where fricas built successfully in Sage 9.6 for one of the affected platforms.

vbraun commented 2 years ago

Changed branch from u/mkoeppe/update_fricas_to_1_3_8 to d7df5ec