Fix the bash compatibility mode

[McDutchie]

Does anyone care about ksh's bash compatibility mode?

It's not compiled in my default. The RELEASE file says it's incomplete and experimental.

bash is pretty ubiquitous. I think if people want to run bash scripts, they'll run bash.

Would anyone object to getting rid?

[posguy99]

Are distributions enabling it? I mean, if *I* wanted bash, I'd run bash, but I also use macOS, so I'd be building bash anyway.

…

-- Marc Wilson

On Jun 13, 2020, at 5:48 PM, Martijn Dekker ***@***.***> wrote:  Does anyone care about ksh's bash compatibility mode? It's not compiled in my default. The RELEASE file says it's incomplete and experimental. bash is pretty ubiquitous. I think if people want to run bash scripts, they'll run bash. Would anyone object to getting rid? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[McDutchie]

Are distributions enabling it?

It would surprise me if there are any that do. It's easy to check: ksh sets a $BASH_VERSION variable if it's enabled and adds a shopt builtin.

…Oh. Now it would surprise me even more, because I just found out it doesn't even compile:

$ bin/package make CCFLAGS='-Os -DSHOPT_BASH=1'
[...]
/usr/local/src/ksh93/ksh/src/cmd/ksh93/sh/init.c:1397:14: warning: comparison of distinct pointer types ('Shell_t *' (aka 'struct Shell_s *') and 'Shinit_f' (aka 'void (*)(struct Shell_s *, int)')) [-Wcompare-distinct-pointer-types]
                        shp>userinit = userinit = bash_init;
                        ~~~^~~~~~~~~
/usr/local/src/ksh93/ksh/src/cmd/ksh93/sh/init.c:1397:24: error: expression is not assignable
                        shp>userinit = userinit = bash_init;
                        ~~~~~~~~~~~~ ^
1 warning and 1 error generated.

…that's a straight-up typo: that should be shp->userinit. It's clear the Bell Labs folks didn't bother to give this the most rudimentary testing before releasing ksh 93u+.

After fixing the typo, compiling ksh succeeds but linking it fails:

[...]
+ cc -O -L. -L/usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib -o ksh pmain.o libshell.a /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libdll.a /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libcoshell.a /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libcmd.a -lutil /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv -lutil /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv -liconv /usr/local/src/ksh93/ksh/arch/darwin.i386-64/lib/libast.a -liconv
Undefined symbols for architecture x86_64:
  "_e_bash_rc", referenced from:
      _sh_main in libshell.a(main.o)
  "_sh_bash1", referenced from:
      _infof in libshell.a(args.o)
  "_sh_bash2", referenced from:
      _infof in libshell.a(args.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
mamake [cmd/ksh93]: *** exit code 1 making ksh
mamake: *** exit code 1 making cmd/ksh93
mamake: *** exit code 1 making all

[DavidMorano]

Does anyone care about ksh's bash compatibility mode?

I am (possibly one of the people) continuing the development of ksh2020 and I have not bothered to even attempt to turn on the BASH compatibility mode, nor do I have any desire to do so. But I am interested in any other responses that people might make here.

[McDutchie]

There has been an objection on the korn-shell list against removing the broken bash compatibility mode. Here is my reply.

In short: It falls to the people interested in using this to fix it, or have it fixed. Either someone fixes this before we release, or it gets removed shortly before release.

So, this is now a TODO item.

[JohnoKing]

Well, FWIW it is probably worth the effort to fix the local builtin present in the Bash compatibility mode. This is quoted from @bk2204's comment in att#220:

I'll point out a few things about the local keyword:

1. There's a request to the Austin Group (the POSIX folks) about standardizing it as
   part of POSIX which has garnered a lot of comments in favor. ksh has typically
   been capable of running POSIX commands, so it may be worthwhile to implement
   for that reason.

2. Debian requires all shells that can be used as /bin/sh to implement local (and
   they don't build with SHOPT_BASH). Consequently, it isn't possible to use ksh
   as /bin/sh on a Debian or Ubuntu system.

3. Git requires a shell providing local in order to run the testsuite and shell
   commands. Consequently, it isn't possible to use ksh as the shell for Git.

4. All other open source POSIX-compliant shells that I'm aware of implement
   local: bash, dash, mksh, pdksh (and derivatives, like OpenBSD's ksh),
   and zsh.

The first point seems to be referencing this request on the Austin Group Defect Tracker. The other points are all perfectly valid reasons to have a local builtin, although this project does have a 'bug fixes only' policy, as in the ksh93u+m README:

1. No new features; bug fixes only (but see items 3 and 4). Feature development is for a future separate branch.

The feature development branch hasn't been created yet, so improvements to the local builtin will have to be placed behind the SHOPT_BASH ifdef with the rest of the Bash compatibility code, as the local builtin isn't enabled by default.

[McDutchie]

Well, I am not a policy absolutist. I think policies should be broken if there are solid reasons why following them is worse than not following them.

Adding a local builtin just might be such a case, precisely because POSIX is likely to mandate one, one of these years. POSIX compliance was one of Korn's stated core objectives, and is one of ours.

However, to be anything like compatible with other shells, local should implement dynamic scoping, not static scoping like typeset. See Part III, Q28 in this FAQ, to know the difference.

In standard ksh mode, local should then only work in POSIX functions (defined like this() { ...; }), and it should implement dynamic scoping. The behaviour of typeset would be unchanged.

In bash compatibility mode (if it gets fixed), local should simply be a synonym of typeset without options, and both should implement dynamic scoping, even in ksh functions defined with the function keyword – because that is how typeset a.k.a. declare behaves in bash, mksh, yash, and zsh. Which means ksh static scoping would not be available if ksh is compiled in bash compatibility mode, but that is the price. You can't have both in one typeset. (edit: unless we add a new shell option for that instead…)

[JohnoKing]

I have created a WIP patch to add local and declare as special builtins. At the moment dynamic scoping isn't enabled as it is hidden behind SH_BASH (like in ksh93v-), although I did fix a few bugs (local now works correctly in POSIX functions and isn't broken when ${.sh.fun} is unset, unlike in ksh2020). The patch is included below:
local_builtin.patch.txt

[McDutchie]

BTW, I don't want to add a declare builtin except as part of a bash compatibility mode, as that has zero chance of ever being added to POSIX.

Nobody is showing up to fix SHOPT_BASH, so it's looking increasingly likely to be removed before release. In which case we should add local only. I don't think we should add bashisms as part of standard ksh; we're not trying to change the ksh language here.

[posguy99]

Does ksh need local? There doesn't seem to be any problem with having local variables right now, although you have to use the non-POSIX function declaration.

[McDutchie]

Yes, local is set to be added to POSIX so we're going to need it if we're to keep the goal of POSIX compliance. It is practically standard already, ksh93 is the only shell that doesn't have it. It needs to work with POSIX functions and have dynamic scoping, not static scoping like typeset with ksh functions (see Part III, Q28 in this FAQ for the difference).

[JohnoKing]

I have been trying to get local working with dynamic scoping limited to functions, but I haven't made any progress with POSIX functions (although dynamic scoping now works in ksh functions). The local builtin (present in my local-builtin branch) currently makes all variables assigned in POSIX functions visible outside of the function:

foo() {
    local var=bar
}
foo; echo $var

In most shells that support local (dash, bash, etc.) this script only prints a newline. The current ksh local builtin outputs 'bar' (a bug also present in ksh93v- and ksh2020) because local makes $var visible outside of functions. This is due to a limitation in POSIX functions. Ksh does not give POSIX functions a function-local scope at all, only ksh functions get one:

ksh/src/cmd/ksh93/sh/xec.c

Lines 3201 to 3224 in e805c7d

	if(nv_isattr(np,NV_FPOSIX))
	{
	char *save;
	int loopcnt = shp->st.loopcnt;
	shp->posix_fun = np;
	save = argv[-1];
	argv[-1] = 0;
	shp->st.funname = nv_name(np);
	shp->last_root = nv_dict(DOTSHNOD);
	nv_putval(SH_FUNNAMENOD, nv_name(np),NV_NOFREE);
	opt_info.index = opt_info.offset = 0;
	error_info.errors = 0;
	shp->st.loopcnt = 0;
	b_dot_cmd(argn+1,argv-1,&shp->bltindata);
	shp->st.loopcnt = loopcnt;
	argv[-1] = save;
	}
	else
	{
	fun.env = envlist;
	fun.node = np;
	fun.nref = 0;
	sh_funscope(argn,argv,0,&fun,execflg);
	}

Ksh93v- made the following change to give POSIX functions proper scoping in the Bash compatibility mode:

@@ -3387,7 +3411,7 @@ static void sh_funct(Shell_t *shp,Namval_t *np,int argn, char *argv[],struct arg
 	SH_LEVELNOD->nvalue.s = lp->maxlevel;
 	shp->st.lineno = error_info.line;
 	np->nvalue.rp->running  += 2;
-	if(nv_isattr(np,NV_FPOSIX))
+	if(nv_isattr(np,NV_FPOSIX) && !sh_isoption(shp,SH_BASH))
 	{
 		char *save;
 		int loopcnt = shp->st.loopcnt;

This change breaks backward compatibility because it also affects typeset, so a different fix will be necessary.

[McDutchie]

Since the local built-in is shaping up to be a big project of its own, it's probably best dealt with in a separate issue. It remains to be seen whether we can make it work before the first release. I'd like to put out a first beta test version in the next month or so.

So, since no one has shown up to fix SHOPT_BASH in 2.5 months, I'm working on getting rid, and that will close this issue.

A couple of things are worth keeping. IMO this includes the handy and popular &>file shorthand for >file 2>&1. As a matter of fact, ksh93 already supports this natively, but only while running rc/profile/login scripts, and it issues a warning. I'd like to make it globally available and remove the warning. mksh fully supports it by default, as do zsh and (of course) bash, so I think it's time to fall in line there.

We should also keep the -o posix option (see #20) and it should disable the &>file shorthand, as it is technically a grammatical incompatibility with POSIX. For instance, POSIXly,

ls &>files.txt

is equivalent to

ls &
>files.txt

– meaning ls is run as a background job while an empty file files.txt is created. This is also what ksh93 currently does (except when running an rc/profile/login script). But nobody would write it like in the one-line version above – in real-world scripts there would be a newline separating those, or at least a space. So I seriously doubt that the &> operator would ever bite anyone. (In fact, bash and zsh don't even bother to disable &> in their POSIX modes, though mksh does disable it and so should we.)