From 3a740d7df7984eb0a8d8e0155262ed76a3eb1cb9 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 19 Oct 2026 13:08:32 +0200 Subject: [PATCH] Fix `Process` changing GAP's directory on Cygwin spawnve cannot set the child's working directory, so the Cygwin variant of SyExecuteProcess changes GAP's own, and never back: every `Process` or `RunProcess` call with a directory leaves GAP there. Change the directory only around spawnve and restore it afterwards, keeping errno from chdir or spawnve for the caller. Doing both after the descriptor setup leaves no early return between them. Assisted-by: Generative AI --- src/iostream.c | 29 ++++++++++++++++++++--------- tst/testinstall/process.tst | 6 +++++- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/iostream.c b/src/iostream.c index 08af67d05b..ea1b039036 100644 --- a/src/iostream.c +++ b/src/iostream.c @@ -628,12 +628,6 @@ SyExecuteProcess(Char * dir, Char * prg, Int in, Int out, Char * args[]) int res; SyProcessResult result = { 0, 0, 0 }; - // change the working directory - if (chdir(dir) == -1) { - result.execError = errno; - return result; - } - // if is -1 open "/dev/null" if (in == -1) tin = open("/dev/null", O_RDONLY); @@ -671,6 +665,7 @@ SyExecuteProcess(Char * dir, Char * prg, Int in, Int out, Char * args[]) fcntl(0, F_SETFD, 0); } + savestdout = -1; if (tout != 1) { savestdout = dup(1); if (savestdout == -1 || dup2(tout, 1) == -1) { @@ -690,9 +685,25 @@ SyExecuteProcess(Char * dir, Char * prg, Int in, Int out, Char * args[]) } FreezeStdin = 1; - // now try to execute the program - res = spawnve(_P_WAIT, prg, (const char * const *)args, - (const char * const *)environ); + + // spawnve cannot set the child's working directory, so change GAP's own + // just for the call + int oldwd = open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); + if (oldwd == -1 || chdir(dir) == -1) + res = -1; + else + res = spawnve(_P_WAIT, prg, (const char * const *)args, + (const char * const *)environ); + + // restore it, keeping errno from chdir or spawnve + if (oldwd != -1) { + int savedErrno = errno; + if (fchdir(oldwd) == -1) + PErr("SyExecuteProcess: cannot restore working directory"); + close(oldwd); + errno = savedErrno; + } + // remember why, before the repairs below overwrite errno int spawnErrno = errno; diff --git a/tst/testinstall/process.tst b/tst/testinstall/process.tst index 18e21455f8..44a3057641 100644 --- a/tst/testinstall/process.tst +++ b/tst/testinstall/process.tst @@ -41,11 +41,15 @@ rec( status := 0 ) gap> out; "hi\n" -# the working directory can be chosen +# the working directory can be chosen, without changing GAP's own (ask the +# kernel, as DirectoryCurrent() is cached) +gap> cwd := GAP_getcwd();; gap> dir := DirectoryTemporary();; gap> PrintTo(Filename(dir, "marker"), ""); gap> RunProcess("ls", rec(directory := dir)).output; "marker\n" +gap> GAP_getcwd() = cwd; +true # error cases gap> RunProcess();