Skip to content

Commit

Permalink
Fix readline related workspace issue
Browse files Browse the repository at this point in the history
... by reverting a previous change that made us disable readline
when stdin is not connected to a terminal (see
#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 #5014

Also add a separate CI test suite target "testworkspace" for testing
for this issue, and potentially other workspace related issues.
  • Loading branch information
fingolfin committed Jan 10, 2024
1 parent e00b01f commit 61892f1
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile.rules
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
12 changes: 11 additions & 1 deletion dev/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/gap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions src/system.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 61892f1

Please sign in to comment.