[PATCH v3 0/3] t: improve compatibility with NixOS

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 5864 bytes --]

Hi,

this is the third version of my patch series to improve compatibility of
our tests with NixOS.

Changes compared to v2:

    - Patch 1: We now check for both httpd and apache2 binaries via
      PATH.

    - Patch 1: We're a bit more defensive and will check whether
      variables are empty before passing them to either `test -d` or
      `test -x`.

    - Patch 3: Clarified why PATH is unset.

    - Patch 3: Instead of writing PATH into the hook we now write it
      into the `hook-env` configuration file read by Subversion.

Patrick

Patrick Steinhardt (3):
  t/lib-httpd: dynamically detect httpd and modules path
  t/lib-httpd: stop using legacy crypt(3) for authentication
  t9164: fix inability to find basename(1) in Subversion hooks

 t/lib-httpd.sh                        | 17 +++++++++++++----
 t/lib-httpd/passwd                    |  2 +-
 t/lib-httpd/proxy-passwd              |  2 +-
 t/t9164-git-svn-dcommit-concurrent.sh |  9 ++++++++-
 4 files changed, 23 insertions(+), 7 deletions(-)

Range-diff against v2:
1:  cee8fbebf84 ! 1:  e4c75c492dd t/lib-httpd: dynamically detect httpd and modules path
    @@ t/lib-httpd.sh: fi
      HTTPD_PARA=""
      
     -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
    -+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' "$(command -v httpd)"
    ++for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \
    ++			  '/usr/sbin/apache2' \
    ++			  "$(command -v httpd)" \
    ++			  "$(command -v apache2)"
      do
    - 	if test -x "$DEFAULT_HTTPD_PATH"
    +-	if test -x "$DEFAULT_HTTPD_PATH"
    ++	if test -n "$DEFAULT_HTTPD_PATH" -a -x "$DEFAULT_HTTPD_PATH"
      	then
    -@@ t/lib-httpd.sh: do
    + 		break
      	fi
      done
      
    @@ t/lib-httpd.sh: do
     +				 '/usr/libexec/httpd' \
     +				 "${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}"
      do
    - 	if test -d "$DEFAULT_HTTPD_MODULE_PATH"
    +-	if test -d "$DEFAULT_HTTPD_MODULE_PATH"
    ++	if test -n "$DEFAULT_HTTPD_MODULE_PATH" -a -d "$DEFAULT_HTTPD_MODULE_PATH"
      	then
    + 		break
    + 	fi
2:  f4c6c754f1e = 2:  3dce76da477 t/lib-httpd: stop using legacy crypt(3) for authentication
3:  361f1bd9c88 ! 3:  6891e254155 t9164: fix inability to find basename(1) in hooks
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    t9164: fix inability to find basename(1) in hooks
    +    t9164: fix inability to find basename(1) in Subversion hooks
     
    -    The post-commit hook in t9164 is executed with a default environment.
    -    To work around this issue we already write the current `PATH` value into
    -    the hook script. But this is done at a point where we already tried to
    -    execute non-built-in commands, namely basename(1). While this seems to
    -    work alright on most platforms, it fails on NixOS.
    +    Hooks executed by Subversion are spawned with an empty environment. By
    +    default, not even variables like PATH will be propagated to them. In
    +    order to ensure that we're still able to find required executables, we
    +    thus write the current PATH variable into the hook script itself and
    +    then re-export it in t9164.
     
    -    Set the `PATH` variable earlier to fix this issue. Note that we do this
    -    via two separate heredocs. This is done because in the first one we do
    -    want to expand variables, whereas in the second one we don't.
    +    This happens too late in the script though, as we already tried to
    +    execute the basename(1) utility before exporting the PATH variable. This
    +    tends to work on most platforms as the fallback value of PATH for Bash
    +    (see `getconf PATH`) is likely to contain this binary. But on more
    +    exotic platforms like NixOS this is not the case, and thus the test
    +    fails.
    +
    +    While we could work around this issue by simply setting PATH earlier, it
    +    feels fragile to inject a user-controlled value into the script and have
    +    the shell interpret it. Instead, we can refactor the hook setup to write
    +    a `hooks-env` file that configures PATH for us. Like this, Subversion
    +    will know to set up the environment as expected for all hooks.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## t/t9164-git-svn-dcommit-concurrent.sh ##
     @@ t/t9164-git-svn-dcommit-concurrent.sh: setup_hook()
    + 			"passed to setup_hook" >&2 ; return 1; }
      	echo "cnt=$skip_revs" > "$hook_type-counter"
      	rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
    - 	hook="$rawsvnrepo/hooks/$hook_type"
    --	cat > "$hook" <<- 'EOF1'
    -+	cat >"$hook" <<-EOF
    - 		#!/bin/sh
    - 		set -e
    -+
    -+		PATH=\"$PATH\"
    -+		export PATH
    -+	EOF
     +
    -+	cat >>"$hook" <<-'EOF'
    - 		cd "$1/.."  # "$1" is repository location
    - 		exec >> svn-hook.log 2>&1
    - 		hook="$(basename "$0")"
    -@@ t/t9164-git-svn-dcommit-concurrent.sh: setup_hook()
    - 		cnt="$(($cnt - 1))"
    - 		echo "cnt=$cnt" > ./$hook-counter
    - 		[ "$cnt" = "0" ] || exit 0
    --EOF1
    ++	# Subversion hooks run with an empty environment by default. We thus
    ++	# need to propagate PATH so that we can find executables.
    ++	cat >"$rawsvnrepo/conf/hooks-env" <<-EOF
    ++	[default]
    ++	PATH = ${PATH}
     +	EOF
     +
    + 	hook="$rawsvnrepo/hooks/$hook_type"
    + 	cat > "$hook" <<- 'EOF1'
    + 		#!/bin/sh
    +@@ t/t9164-git-svn-dcommit-concurrent.sh: EOF1
      	if [ "$hook_type" = "pre-commit" ]; then
      		echo "echo 'commit disallowed' >&2; exit 1" >>"$hook"
      	else

base-commit: dadef801b365989099a9929e995589e455c51fed
-- 
2.42.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]