Should ksh provide builtins for external commands like `cat` or `mkdir`?

[siteshwar]

ksh93 provides number of shell builtins for standard commands under this directory. Only some of them are compiled in legacy build system by default and listed under special path /opt/ast/bin:

/opt/ast/bin/basename
/opt/ast/bin/cat
/opt/ast/bin/chmod
/opt/ast/bin/cmp
/opt/ast/bin/cut
/opt/ast/bin/dirname
/opt/ast/bin/getconf
/opt/ast/bin/head
/opt/ast/bin/logname
/opt/ast/bin/sync
/opt/ast/bin/uname
/opt/ast/bin/wc

To enable these builtins you have to add /opt/ast/bin to $PATH. We should investigate if we should continue to provide shell builtins for these standard commands.

[krader1961]

Some food for thought....

When the fish-shell project considers adding a new builtin they carefully consider whether the builtin name might conflict with a widely used external command.

Fish also realized that not every platform provides a realpath command. But rather than make it a builtin that always overrides an external realpath command they used the function autoload mechanism to dynamically check for a) a grealpath command (in case someone had installed the GNU version under that name), b) a realpath command, and only if those aren't found c) fallback to the builtin. This minimizes the chance of surprising the user by ensuring they get the full featured implementation provided by their OS if one is available. If not fallback to the builtin so that fish scripts can assume they can at least do realpath /a/symlinked/path.

Most of those commands aren't too hard to justify as being optionally available as a builtin in terms of portability and/or performance. But sync is a really odd command to have as a builtin. When was the last time anyone used that command? Regardless it can't be justified as a builtin on the grounds or portability or performance. So why is it included?

[dannyweldon]

Most of those commands aren't too hard to justify as being optionally available as a builtin in terms of portability and/or performance. But sync is a really odd command to have as a builtin. When was the last time anyone used that command? Regardless it can't be justified as a builtin on the grounds or portability or performance. So why is it included?

One possibility is if ksh is being used as a root shell in single user mode, having sync built in could be useful if no other commands are available, to allow root to get the filesystem saved for consistency before powering off the machine.

[krader1961]

@dannyweldon, If no other commands are available I have a hard time believing a sync builtin is needed. If the root filesystem (which should have a sync command) isn't available the system is so screwed up the inability to run that command is the least of the admin's worries. Sorry, but I don't buy that argument. Nice try, but I'm unconvinced we should be maintaining that code and bloating the ksh size, even if just by a few hundred bytes, for something that 99.9999% of ksh users will never need.

[dannyweldon]

I am a professional Unix/Linux administrator and kept running into this situation recently repeatedly when the glusterfs filesystem driver was having issues causing all file I/O on a Linux system to freeze. Running any local commands would hang, however I could kill them with Ctrl-C. But I could still run "commands" that were built into the shell, of which in bash was only "echo"; not very useful, but can be a crude form of ls. I have worked a long time ago previously when we did all root system administration on various Unix flavours using the Korn shell. It would have been nice to have had builtin commands available so that I could actually do something useful, which is why I think the sync command has been added as a builtin, and I believe I am correct. Whether anyone will really use it very often nowadays is another matter.

[krader1961]

@dannyweldon, I am still unconvinced the sync builtin is worth maintaining as a core feature. There is a nearly infinite number of commands we could include as builtins using the criteria from your comment. But that does not justify the cost of making them builtins for everyone. I have no objection to making it easy for third-parties to extend ksh with such commands where they have specialized requirements such as you describe. Quite the contrary. Making it easy to adapt ksh to such needs is more likely to keep it relevant. I just don't see why the core ksh project should pay the cost of maintaining that code.

Also, if "Running any local commands would hang" it is likely that running sync, even as a builtin, will also "hang". How likely is it that even if it forces a sync of the filesystem(s) you care about before it blocks syncing the filesystem(s) that are causing problems you would even notice? You also wrote, perhaps imprecisely, that the glusterfs filesystem driver was having issues causing all file I/O on a Linux system to freeze. If it was causing all I/O to stall what use is sync even as a builtin?

That you could not run external commands is usually an indication that one or more filesystems in $PATH is unresponsive. In my experience as a sysadmin, and UNIX support tech, not being able to run the sync command in that situation was never a concern. Even before journaled filesystems were the norm. So I remain unconvinced that the core ksh code should retain sync as a builtin. But should make it easy for people like you to build a version that includes it as a builtin if they deem the command useful enough to justify the overhead.

[krader1961]

Followup to my previous comments...

Keep in mind the kernel typically performs periodic syncs on the order of every 30 to 60 seconds. So even if the sync command isn't available all you have to do is stop doing anything and wait a minute before rebooting the system. The primary purpose of the sync(2) API is to hasten a graceful shutdown. If you're dealing with an abnormal situation, such as where some filesystem I/O is blocked, it is not unreasonable to just "count to 10" before forcing the system to halt (e.g., by pressing the "big red button").

[jelmd]

I also like to keep them, because they also provide some add. feature, which might be usable in certain situations, make life easier (e.g. see getconf --man etc.). ksh scripts may rely on them - most distro provided commands do not have all the options, which the ast cmds provide - so removing them may break a lot of ksh scripts. Last but not least I can understand @dannyweldon : around 2000 +-5 we used static ksh with all its builtins enabled as our busybox - advantage was, that commands worked as expected and even having a man page for these cmds w/o having access to all the groff/troff stuff. Not used anymore, because we PXE boot, ... now - however, with such builtins incl. dyn. cmd load ksh has the potential to be used as a much better busybox as well ... So please do not close this door!

[krader1961]

I have no objection to making it easy for ksh to be a viable alternative to busybox. Busybox is the basis of the CLI for the Asuswrt-Merlin firmware project that I use on my WiFi router. Not to mention a lot of other hardware that uses Linux and Busybox. I'd be thrilled if ksh were used instead. But that does not justify encumbering the general purpose ksh code with features that 99.9999% of users won't use.

We should make it easy to build a statically linked ksh binary that includes a lot of builtin commands (e.g., sync and cat) that can't be justified as builtins in any other situation. And those builtins should be enabled by default. But I see no reason why those commands should be available as builtins of a ksh command used in typical situations. And even if made available in those typical situations they should not automatically supplant the external commands of the same name.

[krader1961]

Note that I had to revert commit 734c1108 because the "heredoc" unit test expected behavior of the builtin cat command that is apparently not provided by the external command. But it's not obvious from the failure message that the difference in behavior has anything at all to do with the options supported by cat. This seems to be a good example for why including such builtins by default is a bad idea. A unit test that uses the cat command should not be affected by whether it is a builtin or an external command unless the test is explicitly testing the behavior of the builtin. Which the heredoc test is not doing AFAICT.

[lkoutsofios]

having builtins of system commands is one of the ways ksh93 made it possible to write system independent scripts that also ran much faster than with other shells. Glenn and Dave tried to keep these commands as close to posix as possible. when a builtin was discovered to violate the standard, they just fixed the builtin. builtins were always optional, but they are a documented feature of ksh93 so I think they have to stay.

[colstrom]

I agree with @jelmd and @lkoutsofios: The builtin's provided by ksh93 are a boon to portable scripting (portable between systems, not shells), and the embedded documentation (via --man) is consistently helpful. It would be a shame to lose either of these things.

[dannyweldon]

With regards to the sync builtin, a very common problem scenario is when a server runs very low on memory such that it cannot fork processes any more. That's when it becomes very important to have shell builtins, the most notable being the kill builtin, which is even in bash. The reason being that without it, it would be impossible to kill any errant processes.

So the sync builtin has obviously been added for completeness in such situations where it can be used as a last ditch effort to minimise loss of data before power cycling. While filesystem journaling will keep your filesystem from being corrupted, it will not prevent the data loss in files from buffers not being flushed.

Now of course, you would have to have had a root ksh shell open before the problem became too serious, but it could also be used by a system ksh script that runs continuously and runs the sync builtin at some predetermined pattern, or perhaps after certain file operations and would be guaranteed not to fail in low memory situations.

Also, the builtins of standard utilities are documented here:

(Note: debian/ubuntu users need to install the mm and ms macros using: apt install groff)

nroff -mm PROMO.mm | less -R

      · Improved  performance:  KSH‐93  executes  many  scripts
        faster  than  the System V Bourne shell. A major reason
        for this is that many of  the  standard  utilities  are
        built‐in.   To  reduce  the time to initiate a command,
        KSH‐93 allows commands to be added as built‐ins at  run
        time  on  systems  that support dynamic loading such as
        System V Release 4.

[krader1961]

BTW, There are a bunch of commands in src/cmd/builtins which are both ksh builtins and external commands with apparently one exception: pty. From a ksh built using the legacy build system from the beta branch:

$ type head
head is a shell builtin version of /usr/bin/head
$ type pty
darwin.i386-64/src/cmd/ksh93/ksh: whence: pty: not found
$ builtin pty
builtin: pty: not found
$ type rev
rev is a shell builtin version of /usr/bin/rev
$ type tee
tee is a shell builtin version of /usr/bin/tee
$ type grep
grep is a tracked alias for /usr/local/bin/grep

I don't understand why pty isn't available as a builtin along with related commands like grep and tee. We definitely do not want to be providing external programs (e.g., grep) as part of this project which might shadow the distro provided version of the program. That the pty command is not implemented as a builtin means it has to be provided as an external command. Which I don't understand.

[siteshwar]

This is one of the cases where having a builtin for external command can be used to workaround os limitations.

[siteshwar]

With #474 I have enabled builtins for all commands mentioned in the description except cut and wc. They were changed after the last stable release and I need to take a closer look at how multibyte characters are handled with those changes. There are other builtins too for standard commands, but those mentioned in the description were the only ones that were built by default.

[krader1961]

This is one of the cases where having a builtin for external command can be used to workaround os limitations.

I don't find that a compelling argument. Or at least the example isn't compelling since it does something so strange that if a coworker tried to get it merged I'd wonder if they were drunk.

[siteshwar]

I noticed that ksh on opensuse is built with all the builtins. This line in the ksh.spec file enables it:

    #define SHOPT_CMDLIB_DIR    "/%{_lib}/ast/bin"

It does 2 totally different things:

1. Modify path of builtins from /opt/ast/bin to /lib64/ast/bin.
2. Enable building all the builtins provided under this directory.

@bitstreamout Did you add this line to change path of builtins or to enable building all builtins ? Since you are modifying default path, I have to assume that you did not intend to build all builtins. Using a single macro to do 2 complete different things is another bad choice by original ksh developers.

[siteshwar]

These builtins are now built by default:

/opt/ast/bin/basename
/opt/ast/bin/cat
/opt/ast/bin/chmod
/opt/ast/bin/cmp
/opt/ast/bin/cut
/opt/ast/bin/dirname
/opt/ast/bin/getconf
/opt/ast/bin/head
/opt/ast/bin/logname
/opt/ast/bin/mkdir
/opt/ast/bin/sync
/opt/ast/bin/uname
/opt/ast/bin/wc

mkdir is missing from the description of the issue because it was kept disabled by a downstream patch in fedora. Now we have all the builtins that were built by default in the last stable release, so I am going to close this issue.

[saper]

Are those test failures related to this?

122/272 alias/shcomp                            OK       0.12 s

--- command ---
SHELL='/home/saper/sw/ast/build/src/cmd/ksh93/ksh' LANG='C.UTF-8' SHCOMP='/home/saper/sw/ast/build/src/cmd/ksh93/shcomp' LD_LIBRARY_PATH='/home/saper/sw/ast/build/src/lib/libast:/home/saper/sw/ast/build/src/lib/libcmd:/home/saper/sw/ast/build/src/lib/libcoshell:/home/saper/sw/ast/build/src/lib/libdll' LIBSAMPLE_PATH='/home/saper/sw/ast/build/src/lib/libdll/libsample.so' /home/saper/sw/ast/build/src/cmd/ksh93/ksh /home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh shcomp alias
--- stderr ---

/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 56: mkfifo: not found
/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 57: mkfifo: not found
/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 62: mkdir: not found
/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 83: cat: not found
/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 84: cat: not found
/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 85: cat: not found
/home/saper/sw/ast/src/cmd/ksh93/tests/util/run_test.sh: line 86: chmod: not found

(this is master - 46885145)

[krader1961]

@saper No those tests mean that standard utilities that should be installed on your system could not be found. The run_test.sh script does not depend on any ksh builtins. Those errors imply that $PATH does not have the expected directories; e.g., /bin, /usr/bin, etc. The unit tests should also not be treated as succeeding if those basic utilities can't be found. I'm sort of surprised the test is passing for you. When I simulate what I think is happening with meson-logs/testlog.txt I get the expected failure because the constructed script can't be found. This is the last stderr line in testlog.txt:

/Users/krader/projects/3rd-party/ast/build/src/cmd/ksh93/ksh: /alias.sh.comp: not found