Upon new shell or TERM change, command completion fails outside of the default path

[pghvlaans]

This bug appears to have been introduced in f8faa2a.

In both line editor modes, immediately after starting the shell or after changing the $TERM variable, command completion fails for executables in places other than the default path (/bin:/usr/bin). This effect lasts until a new prompt is reached by carriage return executing a command or ctrl+C.

[McDutchie]

Thanks for the report.

Using a normal command substitution instead of a shared-state command substitution to invoke tput seems to be an effective workaround for the bug. So there must still be some bug involving command -p and/or shared-state command substitutions somewhere, but about an hour of searching got me nowhere so far.

diff --git a/src/cmd/ksh93/edit/edit.c b/src/cmd/ksh93/edit/edit.c
index ce21a8f07..1dac5391b 100644
--- a/src/cmd/ksh93/edit/edit.c
+++ b/src/cmd/ksh93/edit/edit.c
@@ -413,7 +413,7 @@ static void get_tput(char *tp, char **cpp)
 	sh_offoption(SH_RESTRICTED);
 	sh_offoption(SH_VERBOSE);
 	sh_offoption(SH_XTRACE);
-	sfprintf(sh.strbuf,".sh.value=${ \\command -p tput %s 2>/dev/null;}",tp);
+	sfprintf(sh.strbuf,".sh.value=$(\\command -p tput %s 2>/dev/null)",tp);
 	sh_trap(sfstruse(sh.strbuf),0);
 	if((cp = nv_getval(SH_VALNOD)) && (!*cpp || strcmp(cp,*cpp)!=0))
 	{

[McDutchie]

Another workaround: execute a dummy command (:) after the shared-state comsub.

diff --git a/src/cmd/ksh93/edit/edit.c b/src/cmd/ksh93/edit/edit.c
index ce21a8f07..9aed564ac 100644
--- a/src/cmd/ksh93/edit/edit.c
+++ b/src/cmd/ksh93/edit/edit.c
@@ -413,7 +413,7 @@ static void get_tput(char *tp, char **cpp)
 	sh_offoption(SH_RESTRICTED);
 	sh_offoption(SH_VERBOSE);
 	sh_offoption(SH_XTRACE);
-	sfprintf(sh.strbuf,".sh.value=${ \\command -p tput %s 2>/dev/null;}",tp);
+	sfprintf(sh.strbuf,".sh.value=${ \\command -p tput %s 2>/dev/null;};:",tp);
 	sh_trap(sfstruse(sh.strbuf),0);
 	if((cp = nv_getval(SH_VALNOD)) && (!*cpp || strcmp(cp,*cpp)!=0))
 	{

[pghvlaans]

I found a third workaround: add sh_offstate(SH_DEFPATH) after restoring the shell options.

diff --git a/src/cmd/ksh93/edit/edit.c b/src/cmd/ksh93/edit/edit.c
index ce21a8f0..cb07c670 100644
--- a/src/cmd/ksh93/edit/edit.c
+++ b/src/cmd/ksh93/edit/edit.c
@@ -429,6 +429,7 @@ static void get_tput(char *tp, char **cpp)
        }
        nv_unset(SH_VALNOD);
        sh.options = o;
+       sh_offstate(SH_DEFPATH);
        sigrelease(SIGINT);
 }

EDIT: Is this something that should be handled in whence.c?

[pghvlaans]

I suppose that means a command -p bug per se can be ruled out, since the SH_DEFPATH state doesn't do any damage with ordinary command substitution.

[McDutchie]

Of course SH_DEFPATH is the problem. I need more caffeine...

EDIT: Is this something that should be handled in whence.c?

No, the actual running of commands via command is handled in sh_exec(); whence.c handles command -v and command -V.

I tried this patch (click triangle) that turns off SH_DEFPATH at the end of every sh_exec() invocation and in sh_exit() instead of at the beginning of sh_exec(), but that causes a few regression test failures as sh_exec() heavily uses recursion.

diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c
index f024f6520..b26b9c11f 100644
--- a/src/cmd/ksh93/sh/fault.c
+++ b/src/cmd/ksh93/sh/fault.c
@@ -564,6 +564,7 @@ void sh_exit(int xno)
 	struct checkpt	*pp = (struct checkpt*)sh.jmplist;
 	int		sig=0;
 	Sfio_t		*pool;
+	sh_offstate(SH_DEFPATH);
 	/* POSIX requires exit status >= 2 for error in 'test'/'[' */
 	if(xno==1 && sh.bltinfun==b_test)
 		sh.exitval = 2;
diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index 6fb0b9371..b8f474853 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -893,7 +893,6 @@ int sh_exec(const Shnode_t *t, int flags)
 			job.curjobid = 0;
 			flags &= ~sh_state(SH_INTERACTIVE);
 		}
-		sh_offstate(SH_DEFPATH);
 		if(!(flags & sh_state(SH_ERREXIT)))
 			sh_offstate(SH_ERREXIT);
 		sh.exitval=0;
@@ -1443,6 +1442,7 @@ int sh_exec(const Shnode_t *t, int flags)
 #endif
 				if(sh.topfd > topfd && !(sh.subshell && (np==SYSEXEC || np==SYSREDIR)))
 					sh_iorestore(topfd,jmpval);  /* avoid leaking unused file descriptors */
+				sh_offstate(SH_DEFPATH);
 				exitset();
 				break;
 			}

So I think the following is probably the best we can do: when turning on the command completion state (SH_COMPLETE), make sure SH_DEFPATH is off.

diff --git a/src/cmd/ksh93/edit/completion.c b/src/cmd/ksh93/edit/completion.c
index bc6ae9c4c..ffb898793 100644
--- a/src/cmd/ksh93/edit/completion.c
+++ b/src/cmd/ksh93/edit/completion.c
@@ -385,6 +385,7 @@ int ed_expand(Edit_t *ep, char outbuff[],int *cur,int *eol,int mode, int count)
 		{
 			cmd_completion=1;
 			sh_onstate(SH_COMPLETE);
+			sh_offstate(SH_DEFPATH);
 		}
 		if(ep->e_nlist)
 		{

[pghvlaans]

Cool, that seems to work. Thank you!

FWIW: Adding the two sh_offstate in the first patch but not removing the one at the beginning of sh_exec() doesn't fail the path tests.