Issue with ksh93 on OpenBSD

[daviduhden]

I encountered a serious problem after changing the root user's shell to ksh93 1.0.8 on OpenBSD 7.5 using the chpass(1) command.

When executing basic commands such as cd or exit, the following error messages are displayed:

For cd:

-ksh93: wcd[93]: _ignore_: line 89: local: not found

For exit:

-ksh93: wsu[97]: _ignore_: line 89: local: not found

Additionally, when executing the pwd command, the following error is displayed:

-ksh93: pwd: cannot determine present working directory [No such file or directory]

While the actions for cd and exit are still executed despite the error, the pwd command becomes completely unusable.

This issue appears to stem from the OpenBSD global initialization script for KornShell located in /etc/ksh.kshrc. The script contains a command that is not supported by ksh93. Specifically, the command local. In order to fix this problem you have to replace it by typeset.

[posguy99]

oBSD's ksh is pdksh, not ksh88 or ksh93 (which each have different ideas about scopes).

A good discussion by Stéphane Chazelas is here.

[McDutchie]

This issue appears to stem from the OpenBSD global initialization script for KornShell located in /etc/ksh.kshrc. The script contains a command that is not supported by ksh93. Specifically, the command local. In order to fix this problem you have to replace it by typeset.

Yes, that's right. Adding local is definitely on our to-do list, but since it is expected to have dynamic scoping which ksh93 doesn't support, this is a puzzle we haven't solved yet (it does static scoping only, see Q28 here).

In the meantime, you can solve the problem by editing src/cmd/ksh93/SHOPT.sh and changing SHOPT SYSRC= to SHOPT SYSRC=0. That will stop it from detecting /etc/ksh.kshrc at compile time and disable the behaviour of loading it at startup.

[McDutchie]

Of course that's only a partial solution. The user's ~/.kshrc may be incompatible as well.

Maybe a better solution would be: on systems where the default ksh on $PATH is not ksh93 at build time, compile ksh to use variant names like /etc/ksh93.kshrc and ~/.ksh93rc by default. What do you think?

[posguy99]

I don't agree. You are special-casing real ksh behavior in favor of things that call themselves ksh but aren't. The place for that behavior is in the shell script itself, not in the shell. Not that you'd ever get oBSD to go along with that, since pdksh is in their base.

What's next, /etc/profile and ~/.profile? That way just leads to the mess that is bash.

[McDutchie]

things that call themselves ksh but aren't

That ship sailed more than three decades ago. If AT&T hadn't insisted on such an outdated and destructive proprietary mindset from the get go, neither pdksh nor bash would ever have existed, and the community would have stepped in to fix the mess that AT&T was making of ksh.

It's AT&T that screwed up by making ksh93 incompatible with ksh88, so it's not for us now to take the arrogant position that pdksh isn't a “real” ksh. It's a 99.9% accurate clone of the ksh88 of the days (even emulating some of its bugs), which is very impressive given that the source code was a closely guarded corporate secret then.

The place for that behavior is in the shell script itself, not in the shell. Not that you'd ever get oBSD to go along with that, since pdksh is in their base.

Exactly. Rigid adherence to principles is often a good thing, but I'm not convinced of that in this particular case. This is not the first time this issue annoys people. I've run into it myself as well. Plus, I'm the one who is going to have to deal with the reports that are going to keep coming in if something isn't done about this.

It's not as if there isn't a precedent, anyway. It's common for the shell binary to be called ksh93 on systems where ksh might be pdksh or mksh. Why should the rc scripts not follow suit?

Which reminds me of the fact that ksh is already detecting what it's called at runtime to decide whether or not to default to POSIX mode (when invoked as sh). So should be easy to add a little code for it to adapt the rc script names at runtime. If it's called ksh93, it should look for matching ksh93rc script names, otherwise nothing changes.

What's next, /etc/profile and ~/.profile?

profile scripts are specified by POSIX and are expected to work on all the POSIX shells by limiting themselves to sh(1) commands. I can promise you that nothing is going to change there.

That way just leads to the mess that is bash.

Compared to the incredible mess that the AT&T folks left for us to tidy up, bash (for all its flaws) is a paragon of correctness and reliability. Chet Ramey and the FSF simply did a better job than AT&T did.

Even after fixing well over 1000 AT&T bugs, as long as we have not managed to fix bugs/design flaws like #771, we have no business calling any other shell a mess, full stop.

By the way, I do appreciate you contributing your opinion, even if I don't end up agreeing with it. In fact, it was useful as it caused me to think this whole thing through more deeply.

[posguy99]

I don't think there's a problem with ksh adapting to the name of the binary at run-time, but I do feel 100% there's a problem with the behavior being a compile-time decision. Your comment in https://github.com/ksh93/ksh/issues/776#issuecomment-2267592548 sounded like the latter.

(BTW I will never stop thinking that the collection of how rc files work in bash is completely broken.)

[McDutchie]

I don't think there's a problem with ksh adapting to the name of the binary at run-time, but I do feel 100% there's a problem with the behavior being a compile-time decision. Your comment in #776 (comment) sounded like the latter.

Yeah, that was my original idea. I agree with you that a runtime decision is better.

(BTW I will never stop thinking that the collection of how rc files work in bash is completely broken.)

I never claimed bash is flawless :)

[daviduhden]

On systems where the default ksh on $PATH is not ksh93 at build time, compile ksh to use variant names like /etc/ksh93.kshrc and ~/.ksh93rc by default.

What would need to be modified to implement this? I would like to propose a patch in the OpenBSD port to fix this problem. It might be better if you could solve this problem in the original project by adding conditionals for operating systems that use another KornShell implementation by default, such as OpenBSD, NetBSD and MirBSD.

[McDutchie]

@daviduhden, here is a patch for you to test. Please let me know how it works for you.

As discussed above with @posguy99, it makes the decision at runtime, based on whether ksh is invoked as ksh93. If so, it looks for /etc/ksh93.kshrc and ~/.ksh93rc instead of the names without the 93. Since our ksh is generally installed as ksh93 on systems that have another ksh as the default, this should meet your needs.

Patch v1

```diff diff --git a/src/cmd/ksh93/Mamfile b/src/cmd/ksh93/Mamfile index 5a21008c..8cc3cc44 100644 --- a/src/cmd/ksh93/Mamfile +++ b/src/cmd/ksh93/Mamfile @@ -119,8 +119,8 @@ make install virtual exec - writedef PRINTF_LEGACY 1 ;; exec - esac ;; exec - 'SYSRC=') - exec - # if one of these exists, make SHOPT_SYSRC load /etc/ksh.kshrc by default - exec - if test -f /etc/ksh.kshrc || test -f /etc/bash.bashrc + exec - # if one of these exists, make SHOPT_SYSRC load /etc/ksh{,93}.kshrc by default + exec - if test -f /etc/ksh.kshrc || test -f /etc/ksh93.kshrc || test -f /etc/bash.bashrc exec - then writedef SYSRC 1 exec - fi ;; exec - # some other SHOPTs may be probed for using feature tests in features/options diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c index 698daeb0..7407fe8b 100644 --- a/src/cmd/ksh93/data/msg.c +++ b/src/cmd/ksh93/data/msg.c @@ -181,6 +181,7 @@ const char e_suidprofile[] = "/etc/suid_profile"; #endif #if SHOPT_SYSRC const char e_sysrc[] = "/etc/ksh.kshrc"; +const char e_sysrc93[] = "/etc/ksh93.kshrc"; #endif #if !SHOPT_SCRIPTONLY const char hist_fname[] = "/.sh_history"; diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index 568d6aee..39b0925d 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -92,6 +92,7 @@ extern char* sh_setenviron(const char*); #define SH_TYPE_KSH 002 #define SH_TYPE_POSIX 004 #define SH_TYPE_LOGIN 010 +#define SH_TYPE_KSH93 020 #define SH_TYPE_RESTRICTED 040 #ifndef PIPE_BUF diff --git a/src/cmd/ksh93/include/io.h b/src/cmd/ksh93/include/io.h index f3c8ca78..7a0dc91a 100644 --- a/src/cmd/ksh93/include/io.h +++ b/src/cmd/ksh93/include/io.h @@ -110,6 +110,7 @@ extern const char e_profile[]; extern const char e_sysprofile[]; #if SHOPT_SYSRC extern const char e_sysrc[]; +extern const char e_sysrc93[]; #endif extern const char e_stdprompt[]; extern const char e_supprompt[]; diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index 0792a047..694b1cb7 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -403,6 +403,9 @@ struct Shell_s #if SHOPT_REGRESS struct Regress_s *regress; #endif /* SHOPT_REGRESS */ +#if SHOPT_SYSRC + char *sysrc; /* path to system-wide kshrc */ +#endif }; /* used for builtins */ diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 6dce5b41..03eed38a 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -1258,7 +1258,10 @@ int sh_type(const char *path) s++; t |= SH_TYPE_SH; if ((t & SH_TYPE_KSH) && *s == '9' && *(s+1) == '3') + { s += 2; + t |= SH_TYPE_KSH93; + } #if _WINIX if (*s == '.' && *(s+1) == 'e' && *(s+2) == 'x' && *(s+3) == 'e') s += 4; @@ -1375,9 +1378,12 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit) } /* read the environment */ env_init(); +#if SHOPT_SYSRC + sh.sysrc = type & SH_TYPE_KSH93 ? e_sysrc93 : e_sysrc; +#endif if(!ENVNOD->nvalue.cp) { - sfprintf(sh.strbuf,"%s/.kshrc",nv_getval(HOME)); + sfprintf(sh.strbuf, type & SH_TYPE_KSH93 ? "%s/.ksh93rc" : "%s/.kshrc", nv_getval(HOME)); nv_putval(ENVNOD,sfstruse(sh.strbuf),NV_RDONLY); } /* increase SHLVL */ diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c index 4bd97845..820db4cf 100644 --- a/src/cmd/ksh93/sh/main.c +++ b/src/cmd/ksh93/sh/main.c @@ -209,7 +209,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit) name = *name ? sh_strdup(name) : NULL; #if SHOPT_SYSRC if(!strmatch(name, "?(.)/./*")) - sh_source(iop, e_sysrc); + sh_source(iop, sh.sysrc); #endif if(name) { ```

[daviduhden]

Here is a patch for you to test. Please let me know how it works for you.

I have tested the patch on OpenBSD 7.6-beta, and ksh93u+m now functions perfectly. I had to make some adjustments to the diff for init.c as it did not apply to version 1.0.10. The patches have already been proposed for the OpenBSD port along with an update to version 1.0.10. Thank you for your attention. It is rare to see a developer work so diligently to resolve issues that arise with the software they maintain.

[pstumpf]

OpenBSD port maintainer here.

I do agree that we should probably patch out sourcing of /etc/ksh.kshrc (if need be, even just locally in the OpenBSD port), but changing the default user initialisation file to $HOME/.ksh93rc is way over the top and will break existing user setups not only on OpenBSD, but most Linux distributions. One should generally be able to maintain a .kshrc that works with different versions of ksh.

So I think the simplest solution would be to just make the value of e_sysrc[] configurable at build time.

[McDutchie]

Thanks for the feedback. I will give this some more thought.

[pstumpf]

Oh, and … does the Mamfile really check for existence of /etc/ksh.kshrc or /etc/bash.bashrc on the build machine and base its decision of whether to include the sysrc feature on the existence of one of those files? That's just wrong. I think SHOPT_SYSRC should always be included. It’s also documented this way.

[McDutchie]

Oh, and … does the Mamfile really check for existence of /etc/ksh.kshrc or /etc/bash.bashrc on the build machine and base its decision of whether to include the sysrc feature on the existence of one of those files? That's just wrong. I think SHOPT_SYSRC should always be included.

ksh93 has always worked this way by default, even in the AT&T days. You can edit src/cmd/ksh93/SHOPT.sh and set SYSRC to 1 to always enable /etc/ksh.kshrc or to 0 to always disable it. The way to do that for a port would be through a local patch.

It’s also documented this way.

It isn't. src/cmd/ksh93/README quite clearly documents the current practice. In SHOPT.sh itself, it's mentioned that an empty value means probe.

[pstumpf]

Then it is broken since AT&T days. A README in the source code is not user-facing documentation, and the man page does not give any clear indication if ksh supports the sysrc feature on a given system.

The check for /etc/ksh.kshrc may have made some semblance of sense "back in the day" -- nowadays it just leads to confusion and unexpected behaviour for both package maintainers and users. You’ll even end up with a different Korn Shell depending on if bash is installed or not on the build machine.

This is exactly one of those behaviours this project should fix by providing some sensible default.

[McDutchie]

I concede the point. The SHOPT_SYSRC probe is broken.

As for the rest of this issue, it seems like someone is going to be unhappy no matter what I decide, so I'm deciding to change as little as necessary to achieve sanity.

The SHOPT_SYSRC probe is deleted from the Mamfile and SHOPT_SYSRC is now enabled by default. On systems such as NetBSD and OpenBSD, where /etc/ksh.kshrc is incompatible by default, users building ksh93 will have to either change its value to 0 to disable loading /etc/ksh.kshrc at startup, or edit /etc/ksh.kshrc to make it compatible with ksh93.

I will add a paragraph to README.md to make users (and packagers) aware of this.