Changes to make the tests pass

This includes changes to make the b_alias, b_basename, and builtins unit
tests pass in light of the previous changes.

.travis.yml

@@ -25,7 +25,7 @@ matrix:
           compiler: gcc
           env:
             - DISTRO_TYPE=opensuse/tumbleweed
-              INSTALL_REQUIREMENTS="zypper refresh; zypper in -y gcc python3 python3-pip ninja sudo glibc-locale glibc-devel ed ncurses-utils vim findutils which netcat expect git; pip3 install meson==0.44.0"
+              INSTALL_REQUIREMENTS="zypper refresh; zypper in -y gcc python3 python3-pip ninja sudo glibc-locale glibc-devel ed ncurses-utils vim findutils which netcat expect git man groff; pip3 install meson==0.44.0"
           before_install:
               - docker pull ${DISTRO_TYPE}
 
@@ -35,7 +35,7 @@ matrix:
           compiler: gcc
           env:
             - DISTRO_TYPE=ubuntu
-              INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git procps; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
+              INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git procps man groff; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
           before_install:
               - docker pull ${DISTRO_TYPE}
 
@@ -45,7 +45,7 @@ matrix:
           compiler: gcc
           env:
             - DISTRO_TYPE=debian
-              INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 python3-pip ninja-build sudo locales ed vim procps findutils debianutils netcat-openbsd expect git; pip3 install meson==0.44.0; sed -i '/# en_US.UTF-8 UTF-8/s/..//' /etc/locale.gen && locale-gen"
+              INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 python3-pip ninja-build sudo locales ed vim procps findutils debianutils netcat-openbsd expect git man groff; pip3 install meson==0.44.0; sed -i '/# en_US.UTF-8 UTF-8/s/..//' /etc/locale.gen && locale-gen"
           before_install:
               - docker pull ${DISTRO_TYPE}
 
@@ -57,7 +57,7 @@ matrix:
           compiler: gcc
           env:
             - DISTRO_TYPE=i386/ubuntu
-              INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
+              INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git man groff; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
           before_install:
               - docker pull ${DISTRO_TYPE}
 

scripts/build-on-docker.sh

@@ -18,7 +18,7 @@ mkdir build
 cd build
 
 echo ==== Configuring the build
-if ! meson -Dwarnings-are-errors=true
+if ! meson -Dwarnings-are-errors=true --prefix=$(mktemp -dt ksh.XXXXXX)
 then
     cat meson-logs/meson-log.txt
     exit 1
@@ -34,6 +34,7 @@ CORE_COUNT=$(nproc)
 export MESON_TESTTHREADS=$(( 4 * ${CORE_COUNT:-1} ))
 echo MESON_TESTTHREADS=$MESON_TESTTHREADS
 
+ninja install
 if ! meson test --setup=malloc
 then
     cat meson-logs/testlog-malloc.txt

scripts/build-on-macos.sh

@@ -9,7 +9,7 @@ mkdir build
 cd build
 
 echo ==== Configuring the build
-if ! meson -Dwarnings-are-errors=true
+if ! meson -Dwarnings-are-errors=true --prefix=$(mktemp -dt ksh.XXXXXX)
 then
     cat meson-logs/meson-log.txt
     exit 1
@@ -33,6 +33,7 @@ echo "/tmp/ksh_auditfile;$(id -u)" | sudo tee /etc/ksh_audit
 # newer. macOS 10.12 doesn't honor the MallocLogFile=/dev/null env var which
 # results in lots of malloc debug messages being written to stderr which
 # breaks the unit tests.
+ninja install
 if ! meson test
 then
     # cat meson-logs/testlog-malloc.txt

scripts/install_aux_files.sh

@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Do installation steps that are hard to do via the standard Meson function
+# (e.g., `install_subdir()`).
+#
+ksh_aux_dir="$MESON_INSTALL_PREFIX/share/ksh"
+ksh_man_dir="$ksh_aux_dir/man"
+ksh_man_src="$MESON_SOURCE_ROOT/src/cmd/ksh93/docs"
+umask 022
+
+set -x
+cd "$ksh_man_src"
+
+# At some point we'll want to uncomment this statement. Probably after we've
+# converted all the documentation to Sphinx format. At that point we'll want
+# to stop committing the `make man` artifacts when the docs are changed.
+# Instead, require that be done at install time.
+#
+# Note: I can't figure out how to make Sphinx `make man` emit the
+# documentation in the expected hierachy under the _build dir. Since we
+# currently only care about section one (i.e., command) documentation force it
+# to be installed in that directory.
+#
+# make man
+
+mkdir -p -m 755 "$ksh_man_dir/man1"
+cp _build/man/*.1 "$ksh_man_dir/man1"
+
+# Old versions of Meson (e.g., 0.44) don't recognize the `install_mode` option
+# of functions like `install_subdir()` and don't use sensible default
+# permissions (e.g., no public read/execute). So try to ensure all the aux
+# files have reasonable permissions.
+find "$ksh_aux_dir" -type d -print0 | xargs -0 chmod 755
+find "$ksh_aux_dir" -type f -print0 | xargs -0 chmod 644

src/cmd/ksh93/bltins/alias.c

@@ -54,7 +54,7 @@ int b_alias(int argc, char *argv[], Shbltin_t *context) {
     tdata.argnum = 0;
     tdata.aflag = *argv[1];
 
-    optind = 1;
+    optind = 0;  // WTF? This is needed on Linux.
     while ((opt = getopt_long(argc, argv, short_options, long_options, NULL)) != -1) {
         switch (opt) {
             case 1: {

src/cmd/ksh93/docs/meson.build

@@ -3,13 +3,6 @@
 # default man page directory: /usr/local/share/man/man1. The man pages for individual builtins will
 # be installed in /usr/local/share/ksh/man.
 #
-install_man('ksh.1')
+install_man('ksh.1', install_mode: 'rw-r--r--')
 
-# I can't figure out how to make Sphinx `make man` emit the documentation in the expected hierachy
-# under the _build dir. Since we currently only care about section one (i.e., command) documentation
-# force it to be installed in that directory.
-#
-# Note: We can use the Meson `add_install_script()` directive if and when we need to handle more
-# than section one man pages. Such a script could split the contents of _build/man into separate
-# subdirs based on the file name extensions.
-install_subdir('_build/man', install_dir: 'share/ksh/man/man1', strip_directory: true)
+# See also scripts/install_aux_files.sh which takes care of installing the private man pages.

src/cmd/ksh93/functions/_ksh_print_help

@@ -6,9 +6,14 @@
 #
 function _ksh_print_help {
     local cmd=$1
-    local fname="${.sh.install_prefix}/share/ksh/man/man1/${cmd}.1"
+    local man_dir="${.sh.install_prefix}/share/ksh/man"
+    local fname="$man_dir/man1/${cmd}.1"
 
-    [[ -f $fname ]] || return 1  # can't find the man page; give up
+    if [[ ! -f $fname ]]
+    then
+        print -u2 Unable to find man page for $cmd command
+        return 1  # can't find the man page; give up
+    fi
 
     # Find the `ul` command to translate x\cHx sequences, etc., into bold chars and such. Pagers
     # like `less` do this but better to not rely on them. If `ul` is not available just pipe the
@@ -27,7 +32,13 @@ function _ksh_print_help {
     # Convert the nroff source for the command into something that looks good on the terminal.
     # Specifically, by elimating all the text outside of the SYNOPSIS and FLAGS sections while
     # attempting to preserve some of the highlighting understood by the terminal.
-    nroff -c -man -rLL=${cols}n "$fname" |
+    local nroff_cmd=$(whence -p nroff)
+    if [[ -n $nroff_cmd ]]
+    then
+        nroff -c -man -rLL=${cols}n "$fname"
+    else
+        MANPATH="$man_dir" man "$cmd" 2> /dev/null
+    fi |
         sed -n \
             -e '/^S.*Y.*N.*O.*P.*S.*I.*S$/,/^[^ ]/ {
                     /^S/p

src/cmd/ksh93/meson.build

@@ -39,8 +39,9 @@ shcomp_exe = executable('shcomp', ['sh/shcomp.c'], c_args: shared_c_args,
     dependencies: [libm_dep, libexecinfo_dep, libdl_dep, libsocket_dep, libnsl_dep],
     install: true)
 
-install_subdir('functions', install_dir: 'share/ksh')
-install_data(['data/config.ksh'], install_dir: 'share/ksh')
+install_subdir('functions', install_dir: 'share/ksh', install_mode: 'rwxr-xr-x')
+install_data(['data/config.ksh'], install_dir: 'share/ksh', install_mode: 'rw-r--r--')
+meson.add_install_script(source_dir + '/scripts/install_aux_files.sh')
 
 test_dir = join_paths(meson.current_source_dir(), 'tests')
 test_driver = join_paths(test_dir, 'util', 'run_test.sh')

src/cmd/ksh93/tests/b_alias.sh

@@ -95,20 +95,20 @@ then
     [[ $(alias -t | grep rm= ) ]] && log_error 'tracked alias not cleared'
 fi
 
-if hash -r 2>/dev/null && [[ ! $(hash) ]]
+if hash -r && [[ ! $(hash) ]]
 then
     PATH=$TEST_DIR:/bin:/usr/bin
     for i in foo -foo --
     do
         print ':' > $TEST_DIR/$i
         chmod +x $TEST_DIR/$i
-        hash -r -- $i 2>/dev/null || log_error "hash -r -- $i failed"
+        hash -r -- $i || log_error "hash -r -- $i failed with wrong status" 0 $?
         expect="$i=$TEST_DIR/$i"
         actual="$(hash)"
         [[ $actual == $expect ]] || log_error "hash -r -- $i failed" "$expect" "$actual"
     done
 else
-    log_error 'hash -r failed'
+    log_error 'hash -r failed' '' "$(hash)"
 fi
 
 ( alias :pr=print) 2> /dev/null || log_error 'alias beginning with : fails'

src/cmd/ksh93/tests/builtins.sh

@@ -29,8 +29,10 @@ for name in $(builtin -l |
 
     actual=$($name --this-option-does-not-exist 2>&1)
     expect="Usage: $name"
+    # The first pattern supports the legacy optget() --help output. The second supports the new
+    # output from the _ksh_print_help function.
     [[ "$actual" =~ "$expect" ||
-       "$actual" =~ "SYNOPSIS".*"$name: Unknown flag '--this-option-does-not-exist'" ]] ||
+       "$actual" =~ ^SYNOPSIS.*$name..Unknown.flag ]] ||
         log_error "$name should show usage info on unrecognized options" "$expect" "$actual"
 done
 

src/cmd/ksh93/tests/meson.build

@@ -160,8 +160,10 @@ foreach testspec : all_tests
 
         # Run the test without compiling the script (which is how most people use ksh).
         lang_var = 'LANG=en_US.UTF-8'
+        # Note the use of `-p` to keep the shell from sourcing dot scripts. Without this the
+        # platform's ksh config files could cause problems.
         test(test_name, ksh93_exe, timeout: timeout, is_parallel: parallel,
-            args: [test_driver, test_name],
+            args: ['-p', test_driver, test_name],
             env: [shell_var, lang_var, src_root, test_root, ld_library_path, libsample_path])
 
         # The shcomp variants are only applicable to the non-interactive tests.
@@ -177,7 +179,7 @@ foreach testspec : all_tests
             if not skip_test
                 # Run the test after compiling the script with `shcomp`.
                 test(test_name + '/shcomp', ksh93_exe, timeout: timeout, is_parallel: parallel,
-                    args: [ test_driver, 'shcomp', test_name],
+                    args: ['-p', test_driver, 'shcomp', test_name],
                     env: [shell_var, lang_var, src_root, test_root, shcomp_var, ld_library_path,
                           libsample_path])
             endif

src/cmd/ksh93/tests/util/run_test.sh

@@ -180,12 +180,12 @@ typeset -a failure_lines
 
 # Run a ksh API test. Modeled loosely on the older run_interactive function.
 function run_api {
-    $test_path >$test_name.out 2>$test_name.err
+    $test_path > $test_name.out 2> $test_name.err
     exit_status=$?
 
     if [[ -e $TEST_ROOT/$test_name.out ]]
     then
-        if ! diff -q $TEST_ROOT/$test_name.out $test_name.out >/dev/null
+        if ! diff -q $TEST_ROOT/$test_name.out $test_name.out > /dev/null
         then
             log_error "Stdout for $test_name had unexpected differences:"
             diff -U3 $TEST_ROOT/$test_name.out $test_name.out >&2
@@ -200,7 +200,7 @@ function run_api {
 
     if [[ -e $TEST_ROOT/$test_name.err ]]
     then
-        if ! diff -q $TEST_ROOT/$test_name.err $test_name.err >/dev/null
+        if ! diff -q $TEST_ROOT/$test_name.err $test_name.err > /dev/null
         then
             log_error "Stderr for $test_name had unexpected differences:"
             diff -U3 $TEST_ROOT/$test_name.err $test_name.err >&2
@@ -242,12 +242,12 @@ function run_interactive {
     fi
 
     expect -n -c "source $TEST_ROOT/util/interactive.expect.rc" -f $test_path \
-        >$test_name.out 2>$test_name.err
+        > $test_name.out 2> $test_name.err
     exit_status=$?
 
     if [[ -e $TEST_ROOT/$test_name.out ]]
     then
-        if ! diff -q $TEST_ROOT/$test_name.out $test_name.out >/dev/null
+        if ! diff -q $TEST_ROOT/$test_name.out $test_name.out > /dev/null
         then
             log_error "Stdout for $test_name had unexpected differences:"
             diff -U3 $TEST_ROOT/$test_name.out $test_name.out >&2
@@ -262,7 +262,7 @@ function run_interactive {
 
     if [[ -e $TEST_ROOT/$test_name.err ]]
     then
-        if ! diff -q $TEST_ROOT/$test_name.err $test_name.err >/dev/null
+        if ! diff -q $TEST_ROOT/$test_name.err $test_name.err > /dev/null
         then
             log_error "Stderr for $test_name had unexpected differences:"
             diff -U3 $TEST_ROOT/$test_name.err $test_name.err >&2
@@ -338,7 +338,7 @@ fi
 if [[ $test_name == *.exp ]]
 then
     # Interactive test.
-    if ! command -v expect >/dev/null
+    if ! command -v expect > /dev/null
     then
         log_info "Skipping $test_name because no expect command could be found"
         exit 0
@@ -401,12 +401,12 @@ else
     chmod 755 $test_script
     if [[ $shcomp == false ]]
     then
-        $TEST_DIR/$test_script $test_name < /dev/null
+        $SHELL -p $TEST_DIR/$test_script $test_name < /dev/null
         exit_status=$?
     elif [[ $shcomp != skip ]]
     then
         $SHCOMP $test_script > $test_script.comp || exit
-        $SHELL $TEST_DIR/$test_script.comp $test_name < /dev/null
+        $SHELL -p $TEST_DIR/$test_script.comp $test_name < /dev/null
         exit_status=$?
     else
         exit_status=0