From 61892f10c424a56282ece815b18b5780abb9cfce Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 10 Jan 2024 10:19:47 +0100
Subject: [PATCH] Fix readline related workspace issue

... by reverting a previous change that made us disable readline
when stdin is not connected to a terminal (see
https://github.com/gap-system/gap/pull/4495). While that still
seems useful, and avoids certain issues when piping GAP output
into files, it needs a better implication that avoids the issues
described in https://github.com/gap-system/gap/issues/5014

Also add a separate CI test suite target "testworkspace" for testing
for this issue, and potentially other workspace related issues.
---
 .github/workflows/CI.yml |  2 +-
 Makefile.rules           |  2 +-
 dev/ci.sh                | 12 +++++++++++-
 src/gap.c                |  2 +-
 src/system.c             |  6 ++++--
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
index 4e7583bc49e..6af88454d32 100644
--- a/.github/workflows/CI.yml
+++ b/.github/workflows/CI.yml
@@ -53,7 +53,7 @@ jobs:
             "testmanuals",
 
             # test error reporting and compiling as well as libgap
-            "testexpect testmockpkg testspecial test-compile testlibgap testkernel",
+            "testexpect testmockpkg testspecial test-compile testlibgap testkernel testworkspace",
 
             # compile packages and run GAP tests
             # don't use --enable-debug to prevent the tests from taking too long
diff --git a/Makefile.rules b/Makefile.rules
index bd217470454..9bb68d3a6fa 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -952,7 +952,7 @@ check: all
 # citests runs a subset of the tests run by CI -- this is SLOW and still doesn't
 # quite cover everything (e.g. we don't test out-of-tree builds and `make install`)
 citests: all
-	SRCDIR=$(abs_srcdir) dev/ci.sh testexpect testmockpkg testspecial test-compile testlibgap testkernel testinstall
+	SRCDIR=$(abs_srcdir) dev/ci.sh testexpect testmockpkg testspecial test-compile testlibgap testkernel testworkspace testinstall
 
 LIBGAPTESTS := $(addprefix tst/testlibgap/,basic api wscreate wsload trycatch)
 
diff --git a/dev/ci.sh b/dev/ci.sh
index 0e582497df2..bf7db7a517d 100755
--- a/dev/ci.sh
+++ b/dev/ci.sh
@@ -353,8 +353,14 @@ GAPInput
 
     # if there were any failures, abort now.
     [[ $TESTMANUALSPASS = yes ]] || error "reference manual tests failed"
+    ;;
+
+  testworkspace)
 
-    # while we are at it, also test the workspace code
+    # test saving a workspace, with stdin redirected (this tests for an issue
+    # where we used to disable readline in this case, which lead to issues later
+    # on when loading the workspace without redirected stdin; for details,
+    # see https://github.com/gap-system/gap/issues/5014)
     $GAP -A $(gap_cover_arg workspace) <<GAPInput
         SetUserPreference("ReproducibleBehaviour", true);
         # Also test a package banner
@@ -363,6 +369,10 @@ GAPInput
         QuitGap(0);
 GAPInput
 
+    # ... and test loading a workspace, then run testinstall in there
+    # (make sure to not redirect stdin here, see comment above)
+    $GAP -A -L test.wsp $(gap_cover_arg workspace) $SRCDIR/tst/testinstall.g
+
     ;;
 
   testlibgap)
diff --git a/src/gap.c b/src/gap.c
index 6246a26ef02..9e6577a4457 100644
--- a/src/gap.c
+++ b/src/gap.c
@@ -1209,7 +1209,7 @@ void UpdateTime(UInt startTime)
 
 // UPDATE_STAT lets code assign the special variables which GAP
 // automatically sets in interactive sessions. This is for demonstration
-// code which wants to look like iteractive usage of GAP. Using this
+// code which wants to look like interactive usage of GAP. Using this
 // function will not stop GAP automatically changing these variables as
 // usual.
 static Obj FuncUPDATE_STAT(Obj self, Obj name, Obj newStat)
diff --git a/src/system.c b/src/system.c
index 6ffe29ef443..cc3dfefebeb 100644
--- a/src/system.c
+++ b/src/system.c
@@ -671,8 +671,10 @@ void InitSystem (
     if (SyWindow)
         SyUseReadline = 0;
     // don't use readline if stdin is not attached to a terminal
-    else if (!isatty(fileno(stdin)))
-        SyUseReadline = 0;
+    // FIXME: disabled this, as it breaks certain workspaces (see also
+    // issue https://github.com/gap-system/gap/issues/5014)
+    //else if (!isatty(fileno(stdin)))
+    //    SyUseReadline = 0;
 #endif
 
     InitSysFiles();