From 87da941348e1b345d1be348b091d3ae65ac66bb7 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 9 Dec 2021 15:16:18 +0000 Subject: [PATCH] Clean up daemon handling Split `common.sh` into the vars and functions definitions vs starting the daemon (and possibly other initialization logic). This way, `init.sh` can just `source` the former. Trying to start the daemon before `nix.conf` is written will fail because `nix daemon` requires `--experimental-features 'nix-command'`. `killDaemon` is idempotent, so it's safe to call when no daemon is running. `startDaemon` and `killDaemon` use the PID (which is now exported to subshells) to decide whether there is work to be done, rather than `NIX_REMOTE`, which might conceivably be set differently even if a daemon is running. `startDaemon` and `killDaemon` can save/restore the old `NIX_REMOTE` as `NIX_REMOTE_OLD`. `init.sh` kills daemon before deleting everything (including the daemon socket). --- .gitignore | 2 +- tests/common.sh | 12 ++++++ .../vars-and-functions.sh.in} | 43 +++++++++++-------- tests/db-migration.sh | 1 - tests/init.sh | 9 ++-- tests/local.mk | 4 +- 6 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 tests/common.sh rename tests/{common.sh.in => common/vars-and-functions.sh.in} (81%) diff --git a/.gitignore b/.gitignore index 83313fe0a..e326966d6 100644 --- a/.gitignore +++ b/.gitignore @@ -75,7 +75,7 @@ perl/Makefile.config # /tests/ /tests/test-tmp -/tests/common.sh +/tests/common/vars-and-functions.sh /tests/result* /tests/restricted-innocent /tests/shell diff --git a/tests/common.sh b/tests/common.sh new file mode 100644 index 000000000..68b90a85f --- /dev/null +++ b/tests/common.sh @@ -0,0 +1,12 @@ +set -e + +if [[ -z "${COMMON_SH_SOURCED-}" ]]; then + +COMMON_SH_SOURCED=1 + +source "$(readlink -f "$(dirname "${BASH_SOURCE[0]}")")/common/vars-and-functions.sh" +if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then + startDaemon +fi + +fi # COMMON_SH_SOURCED diff --git a/tests/common.sh.in b/tests/common/vars-and-functions.sh.in similarity index 81% rename from tests/common.sh.in rename to tests/common/vars-and-functions.sh.in index 939f2fce1..0deef4c1c 100644 --- a/tests/common.sh.in +++ b/tests/common/vars-and-functions.sh.in @@ -1,8 +1,8 @@ set -e -if [[ -z "$COMMON_SH_SOURCED" ]]; then +if [[ -z "${COMMON_VARS_AND_FUNCTIONS_SH_SOURCED-}" ]]; then -COMMON_SH_SOURCED=1 +COMMON_VARS_AND_FUNCTIONS_SH_SOURCED=1 export PS4='+(${BASH_SOURCE[0]}:$LINENO) ' @@ -25,7 +25,7 @@ if [[ -n $NIX_STORE ]]; then fi export _NIX_IN_TEST=$TEST_ROOT/shared export _NIX_TEST_NO_LSOF=1 -export NIX_REMOTE=$NIX_REMOTE_ +export NIX_REMOTE=${NIX_REMOTE_-} unset NIX_PATH export TEST_HOME=$TEST_ROOT/test-home export HOME=$TEST_HOME @@ -90,13 +90,14 @@ clearCacheCache() { startDaemon() { # Don’t start the daemon twice, as this would just make it loop indefinitely - if [[ "$NIX_REMOTE" == daemon ]]; then - return + if [[ "${_NIX_TEST_DAEMON_PID-}" != '' ]]; then + return fi # Start the daemon, wait for the socket to appear. rm -f $NIX_DAEMON_SOCKET_PATH - PATH=$DAEMON_PATH nix-daemon& - pidDaemon=$! + PATH=$DAEMON_PATH nix-daemon & + _NIX_TEST_DAEMON_PID=$! + export _NIX_TEST_DAEMON_PID for ((i = 0; i < 300; i++)); do if [[ -S $NIX_DAEMON_SOCKET_PATH ]]; then DAEMON_STARTED=1 @@ -108,25 +109,35 @@ startDaemon() { fail "Didn’t manage to start the daemon" fi trap "killDaemon" EXIT + # Save for if daemon is killed + NIX_REMOTE_OLD=$NIX_REMOTE export NIX_REMOTE=daemon } killDaemon() { - kill $pidDaemon + # Don’t fail trying to stop a non-existant daemon twice + if [[ "${_NIX_TEST_DAEMON_PID-}" == '' ]]; then + return + fi + kill $_NIX_TEST_DAEMON_PID for i in {0..100}; do - kill -0 $pidDaemon 2> /dev/null || break + kill -0 $_NIX_TEST_DAEMON_PID 2> /dev/null || break sleep 0.1 done - kill -9 $pidDaemon 2> /dev/null || true - wait $pidDaemon || true + kill -9 $_NIX_TEST_DAEMON_PID 2> /dev/null || true + wait $_NIX_TEST_DAEMON_PID || true + rm -f $NIX_DAEMON_SOCKET_PATH + # Indicate daemon is stopped + unset _NIX_TEST_DAEMON_PID + # Restore old nix remote + NIX_REMOTE=$NIX_REMOTE_OLD trap "" EXIT } restartDaemon() { - [[ -z "${pidDaemon:-}" ]] && return 0 + [[ -z "${_NIX_TEST_DAEMON_PID:-}" ]] && return 0 killDaemon - unset NIX_REMOTE startDaemon } @@ -190,10 +201,6 @@ enableFeatures() { set -x -if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then - startDaemon -fi - onError() { set +x echo "$0: test failed at:" >&2 @@ -205,4 +212,4 @@ onError() { trap onError ERR -fi # COMMON_SH_SOURCED +fi # COMMON_VARS_AND_FUNCTIONS_SH_SOURCED diff --git a/tests/db-migration.sh b/tests/db-migration.sh index 3f9dc8972..92dd4f3ba 100644 --- a/tests/db-migration.sh +++ b/tests/db-migration.sh @@ -9,7 +9,6 @@ fi source common.sh killDaemon -unset NIX_REMOTE # Fill the db using the older Nix PATH_WITH_NEW_NIX="$PATH" diff --git a/tests/init.sh b/tests/init.sh index da0568ed9..fea659516 100755 --- a/tests/init.sh +++ b/tests/init.sh @@ -1,14 +1,13 @@ -set -e -o pipefail +set -eu -o pipefail -source common.sh +# Don't start the daemon +source common/vars-and-functions.sh test -n "$TEST_ROOT" if test -d "$TEST_ROOT"; then chmod -R u+w "$TEST_ROOT" # We would delete any daemon socket, so let's stop the daemon first. - if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then - killDaemon - fi + killDaemon rm -rf "$TEST_ROOT" fi mkdir "$TEST_ROOT" diff --git a/tests/local.mk b/tests/local.mk index 4ef071f96..a4537cf09 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -129,9 +129,9 @@ endif install-tests += $(foreach x, $(nix_tests), tests/$(x)) -clean-files += $(d)/common.sh $(d)/config.nix $(d)/ca/config.nix +clean-files += $(d)/tests/common/vars-and-functions.sh $(d)/config.nix $(d)/ca/config.nix -test-deps += tests/common.sh tests/config.nix tests/ca/config.nix +test-deps += tests/common/vars-and-functions.sh tests/config.nix tests/ca/config.nix tests/plugins/libplugintest.$(SO_EXT) ifeq ($(BUILD_SHARED_LIBS), 1) test-deps += tests/plugins/libplugintest.$(SO_EXT)