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@xxxxxx>
## 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@xxxxxx>
## 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:
signature.asc
Description: PGP signature
