Restoring saved workspaces broken in GAP 4.12?

[zickgraf]

When creating a workspace and trying to load it with GAP 4.12, the following error occurs:

Panic in src/saveload.c:618: This workspace is not compatible with GAP kernel (4.12.0, present: 4.12.0 with readline)

For me this looks like the version number is not handled consistently (once with "with readline" and once without). If you cannot reproduce, I will of course provide more details.

 ┌───────┐   GAP 4.12.0 of 2022-08-18
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.4, AutoDoc 2022.07.10, AutPGrp 1.11, Browse 1.8.14, 
             CaratInterface 2.3.4, CRISP 1.4.5, Cryst 4.1.25, CrystCat 1.1.10, CTblLib 1.3.4, FactInt 1.6.3, 
             FGA 1.4.0, Forms 1.2.8, GAPDoc 1.6.6, genss 1.6.7, IO 4.7.2, IRREDSOL 1.4.3, LAGUNA 3.9.5, orb 4.8.5, 
             Polenta 1.3.10, Polycyclic 2.16, PrimGrp 3.4.2, RadiRoot 2.9, recog 1.3.2, ResClasses 4.7.3, 
             SmallGrp 1.5, Sophus 1.27, SpinSym 1.5.2, TomLib 1.2.9, TransGrp 3.6.3, utils 0.76
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'

[ChrisJefferson]

Just for confirmation, I assume you downloaded the tarball from www.gap-system.org?

[zickgraf]

Just for confirmation, I assume you downloaded the tarball from www.gap-system.org?

Yes, and this gave me https://github.com/gap-system/gap/releases/download/v4.12.0/gap-4.12.0.tar.gz

[ChrisJefferson]

It would be good some more details -- I tried reproducing this but it seemed to work fine (I'm on Ubuntu in WSL). A full terminal cut, like this, would be helpful if yours is different:

gap-4.12.0/ $ ./gap
 ┌───────┐   GAP 4.12.0 of 2022-08-18
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.4, AutoDoc 2022.07.10, AutPGrp 1.11, CRISP 1.4.5, Cryst 4.1.25,
             CrystCat 1.1.10, CTblLib 1.3.4, FactInt 1.6.3, FGA 1.4.0, Forms 1.2.8, GAPDoc 1.6.6, genss 1.6.7,
             IO 4.7.2, IRREDSOL 1.4.3, LAGUNA 3.9.5, orb 4.8.5, Polenta 1.3.10, Polycyclic 2.16, PrimGrp 3.4.2,
             RadiRoot 2.9, recog 1.3.2, ResClasses 4.7.3, SmallGrp 1.5, Sophus 1.27, SpinSym 1.5.2, TomLib 1.2.9,
             TransGrp 3.6.3, utils 0.76
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
gap> x := "test";
"test"
gap> SaveWorkspace("ws");
true
gap>
gap-4.12.0/ $ ./gap -L ws
 ┌───────┐   GAP 4.12.0 of 2022-08-18
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loaded workspace: ws
 Packages:   AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.4, AutoDoc 2022.07.10, AutPGrp 1.11, CRISP 1.4.5, Cryst 4.1.25,
             CrystCat 1.1.10, CTblLib 1.3.4, FactInt 1.6.3, FGA 1.4.0, Forms 1.2.8, GAPDoc 1.6.6, genss 1.6.7,
             IO 4.7.2, IRREDSOL 1.4.3, LAGUNA 3.9.5, orb 4.8.5, Polenta 1.3.10, Polycyclic 2.16, PrimGrp 3.4.2,
             RadiRoot 2.9, recog 1.3.2, ResClasses 4.7.3, SmallGrp 1.5, Sophus 1.27, SpinSym 1.5.2, TomLib 1.2.9,
             TransGrp 3.6.3, utils 0.76
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
gap> x;
"test"

[zickgraf]

Aaaah, the way I am calling GAP in a script seems to disable readline:

#!/bin/sh
#                                                           Frank L�beck
#
# Use this script inside the $GAPROOT directory (the top directory
# containing 'bin', 'lib', 'pkg' and so on as subdirectories) to 
# generate a workspace file:
#     bin/wsgap4

echo Creating new workspace file ... 

# the -r option avoids to store user specific settings in the workspace
current/bin/gap.sh -r <<EOF

# load here all packages you want to include in the standard workspace
LoadPackage("io");

# load help book infos with a dummy help query
??blablfdfhskhks

# a small trick to make everything sensible available to the TAB completion
function() local a; for a in NamesGVars() do if ISB_GVAR(a) then
VAL_GVAR(a); fi;od;end;
last();

# save the workspace
SaveWorkspace("current/bin/wsgap4");

quit;
EOF

I checked an old workspace, there readline support was not disabled by this way of calling GAP. Do you have a suggestion on how to call GAP with multiline input from a script and keep readline support enabled? (I tried the -E flag which does not seem to make a difference).

In any case, thanks for the super fast reply and sorry for the false alarm!

[ChrisJefferson]

In terms of fixing your issue, you could replace running GAP with something like this:

current/bin/gap.sh -c 'LoadPackage("io");' -c '??blablfd' -c '(function() local a; for a in NamesGVars() do if ISB_GVAR(a) then VAL_GVAR(a); fi;od;end)();' -c 'SaveWorkspace("current/bin/wsgap4");' -c 'QuitGap();'

However, leave this issue open for now, as I'm interested why this used to work, and now doesn't, particularly if (based on @frankluebeck 's name), this is a script multiple people might be using.

[ChrisJefferson]

Having looked through history, this is caused by system.c line 826, introduced by @fingolfin , who (quite reasonably) stopped us using readline when GAP is not run from a proper terminal. It's just unfortunate that interacts with workspaces, but I think might be something we don't want to change.

[frankluebeck]

I will update the rsync distribution to GAP 4.12 and then investigate. Thanks for the hints already given.

[frankluebeck]

I agree that it is sensible to disable readline by default when GAP's input is not a terminal. But why does GAP ignore an explicit -E argument and still does not load readline?

For the rsync distribution I have now changed the script that generates the workspace (the script does no longer redirect the standard input but just reads a file): see this commit

[ChrisJefferson]

One minor point, actually -E disables readline (this happens in 4.11 and 4.12), as readline is enabled by default.

If people want this, and want something less confusing, we could easily add --readline and --no-readline, so make clear how to turn readline on and off (and make --readline force readline on even when reading from a file).

[fingolfin]

This is a general concern I have with the GAP command line options: a lot of them toggle behavior, which can make it difficult to reliable ensure things are really turned on or off (think of gap being a shell script which already sets e.g. -E. So having longform options to more (all?) GAP options which normally toggle behavior would be nice. I'd tend to names like --enable-readline or --disable-readline. Of course they are not nice for interactive usage, but for scripts can be super handy for the reason I just gave

[frankluebeck]

Indeed, toggling options are only useful when the default is fixed (which was the case in "old" GAP versions).

[fingolfin]

So, how do we want to deal with this?

For saveload.c I think we really just want to store whether it is available ? The check was added in PR #2720 to fix two crash reports. But perhaps we can do this differently...

E.g. first we use a patch like this:

diff --git a/src/saveload.c b/src/saveload.c
index 606b24ee8..2d6df4507 100644
--- a/src/saveload.c
+++ b/src/saveload.c
@@ -499,9 +499,9 @@ static Char * GetKernelDescription(void)
 {
     static Char SyKernelDescription[256];
     strcpy(SyKernelDescription, SyKernelVersion);
-    if (SyUseReadline) {
-        strcat(SyKernelDescription, " with readline");
-    }
+#ifdef HAVE_LIBREADLINE
+    strcat(SyKernelDescription, " with readline");
+#endif
     return SyKernelDescription;
 }
 

Next, when restoring savespaces, we ignore the actual -E command line arguments, and instead initialize SyUseReadline to match the value of GAPInfo.UseReadline in the workspace being restored.

As an alternative to the second part, we could change the code in lib/cmdledit.g to use CallAndInstallPostRestore in order to setup GAPInfo.UseReadline and GAPInfo.CommandLineEditFunctions if necessary also after restoring a workspace?

[ChrisJefferson]

My concern (and it's hard to know without tracing the code) is if this will reintroduce the bugs we were fixing by adding this -- there are some separate things, did we build with readline, and are we actually using it, and the question is will GAP misbehave if, when we load in the saved workspace, we start using readline when we weren't before?

[fingolfin]

Punting to 4.12.2 as nobody worked on a fix.

[mohamed-barakat]

Now that I managed to create the workspace using the workaround suggested by Frank the history saving does not work.

[fingolfin]

Nobody has voiced intention to work on this, and so it won't be fixed in 4.12.2, and realistically not in any 4.12.x. I am going to punt it to the 4.13 milestone, but that won't fix it magically either, it'll just ensure it'll be revisited by me before that release.... We'll see what happens