-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue with ksh93 on OpenBSD #776
Comments
oBSD's A good discussion by Stéphane Chazelas is here. |
Yes, that's right. Adding In the meantime, you can solve the problem by editing |
Of course that's only a partial solution. The user's Maybe a better solution would be: on systems where the default ksh on |
I don't agree. You are special-casing real What's next, |
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.
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 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
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. |
I don't think there's a problem with (BTW I will never stop thinking that the collection of how rc files work in |
Yeah, that was my original idea. I agree with you that a runtime decision is better.
I never claimed bash is flawless :) |
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. |
@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 Patch v1diff --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)
{ |
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. |
OpenBSD port maintainer here. I do agree that we should probably patch out sourcing of So I think the simplest solution would be to just make the value of |
Thanks for the feedback. I will give this some more thought. |
Oh, and … does the |
ksh93 has always worked this way by default, even in the AT&T days. You can edit
It isn't. |
Then it is broken since AT&T days. A The check for This is exactly one of those behaviours this project should fix by providing some sensible default. |
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. |
It's just not right that it depends on the presence of /etc/ksh.kshrc or even /etc/bash.bashrec on the build machine whether /etc/ksh.kshrc is going to be loaded at startup on the users' machine. src/cmd/ksh93/Mamfile: - Remove SHOPT_SYSRC probe code. src/cmd/ksh93/SHOPT.sh: - Enable SHOPT_SYSRC by default. README.md: - Document that SHOPT_SYSRC should be disabled on systems with a default incompatible /etc/ksh.kshrc. Resolves: #776
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
orexit
, 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
andexit
are still executed despite the error, thepwd
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 bytypeset
.The text was updated successfully, but these errors were encountered: