fix: make a renamed runner's bin and externals links relative

bin/runpool

@@ -43,7 +43,7 @@ export RUNPOOL_INVOKED
 # The released version, and the only place it is written. The Homebrew formula
 # builds from a git tag, so a tag without a matching bump here ships a binary
 # that misreports itself.
-RUNPOOL_VERSION="0.12.0"
+RUNPOOL_VERSION="0.12.1"
 
 # shellcheck source=lib/common.sh
 . "${RUNPOOL_ROOT}/lib/common.sh"

lib/lifecycle.sh

@@ -571,20 +571,25 @@ _rp_move_dir() {
   mv "${from}" "${to}"
 }
 
-_rp_migrate_rewrite_runner_links() {
+# GitHub's runner updater points `bin` and `externals` at its versioned
+# directories with ABSOLUTE symlinks, so a runner directory that moves takes
+# two dangling links with it and `./config.sh` dies on a missing
+# Runner.Listener. Rewriting them relative makes the installation
+# self-contained, and therefore safe to move again.
+#
+# Every caller that relocates a runner directory has to do this, and do it
+# before running config.sh rather than after.
+_rp_rewrite_runner_links() {
   local runner_dir="$1" name link target target_name
   for name in bin externals; do
     link="${runner_dir}/${name}"
     [ -L "${link}" ] || continue
     target=$(readlink "${link}") || return 1
     target_name="${target##*/}"
     [ -n "${target_name}" ] && [ -d "${runner_dir}/${target_name}" ] || {
-      _rp_err "migration copied an unresolved runner link: ${link} -> ${target}"
+      _rp_err "unresolved runner link: ${link} -> ${target}"
       return 1
     }
-    # GitHub's runner updater creates absolute links to its versioned bin and
-    # externals directories. Make the copied installation self-contained so
-    # removing the legacy tree cannot break Runner.Listener afterwards.
     ln -sfn "${target_name}" "${link}" || return 1
   done
 }
@@ -697,7 +702,7 @@ _rp_migrate_storage() {
     while [ "${i}" -le "${POOL_COUNT}" ]; do
       runner_dir="${new_dir}/runner-${i}"
       cache_dir="${cache_pool}/runner-${i}"
-      _rp_migrate_rewrite_runner_links "${runner_dir}" || return 1
+      _rp_rewrite_runner_links "${runner_dir}" || return 1
       mkdir -p "${cache_dir}" || return 1
       # A work tree is regenerable but not necessarily portable. Compilers can
       # bake its absolute path into build artifacts, and a remote cache can
@@ -1074,6 +1079,10 @@ CONF
       _rp_log "${new} runner-${i}: past the pool's count of ${POOL_COUNT}, deregistered and left on disk"
       continue
     }
+    # Before config.sh, not after: the runner's bin and externals links are
+    # absolute and now point into the directory this pool used to live in, so
+    # ./config.sh cannot even start until they are made relative.
+    _rp_rewrite_runner_links "${runner_dir}" || return 1
     _rp_migrate_update_work_folder "${runner_dir}" "$(_rp_runner_work_dir "${new}" "${i}")" || return 1
     runner_name="$(hostname -s)-${new}-${i}"
     _rp_log "${new} runner-${i}: registering as '${runner_name}'"

tests/pool-rename.sh

@@ -81,12 +81,27 @@ build_pool() {
       >"${dir}/runner-${i}/.runner"
     # A config.sh that behaves like the real one: refuses nothing, writes a
     # .runner, and records exactly what it was asked for.
+    # Faithful to the real one in the way that matters here: it runs
+    # ./bin/Runner.Listener, so a dangling bin link makes it fail before it
+    # registers anything. A stub that ignored the links would pass whether or
+    # not they were rewritten, and prove nothing.
     cat >"${dir}/runner-${i}/config.sh" <<STUB
 #!/bin/bash
+[ -x ./bin/Runner.Listener ] || { echo "./config.sh: line 80: ./bin/Runner.Listener: No such file or directory" >&2; exit 1; }
 echo "\$*" >> "${cfg_log}"
 printf '{"agentId": 99, "workFolder": "x"}\n' > .runner
 STUB
     chmod +x "${dir}/runner-${i}/config.sh"
+    # GitHub's runner updater points bin and externals at versioned directories
+    # with ABSOLUTE links. A directory that moves therefore takes two dangling
+    # links with it, and config.sh dies on a missing Runner.Listener before it
+    # can register anything. The fixture has to carry them or the rename looks
+    # like it works.
+    mkdir -p "${dir}/runner-${i}/bin.2.337.0" "${dir}/runner-${i}/externals.2.337.0"
+    printf '#!/bin/bash\n' >"${dir}/runner-${i}/bin.2.337.0/Runner.Listener"
+    chmod +x "${dir}/runner-${i}/bin.2.337.0/Runner.Listener"
+    ln -sfn "${dir}/runner-${i}/bin.2.337.0" "${dir}/runner-${i}/bin"
+    ln -sfn "${dir}/runner-${i}/externals.2.337.0" "${dir}/runner-${i}/externals"
     printf 'plist for %s runner-%s\n' "${name}" "${i}" \
       >"${RUNPOOL_AGENT_DIR}/runpool.${name}.${i}.plist"
     i=$(( i + 1 ))
@@ -136,6 +151,21 @@ absent  "${RUNPOOL_BASE}/runners/alpha"          "the old runner tree"
 present "${RUNPOOL_CACHE_DIR}/pools/bravo"       "the cache moved"
 absent  "${RUNPOOL_CACHE_DIR}/pools/alpha"       "the old cache"
 
+# The links have to be relative afterwards, and they have to resolve. An
+# absolute link left pointing into the old path is a runner that cannot start,
+# and config.sh fails before registering, which leaves the pool half renamed.
+for r in 1 2; do
+  link="${RUNPOOL_BASE}/runners/bravo/runner-${r}/bin"
+  [ -L "${link}" ] || fail "runner-${r}: bin is not a symlink"
+  case "$(readlink "${link}")" in
+    /*) fail "runner-${r}: bin is still an absolute link into the old path" ;;
+  esac
+  [ -d "${link}/" ] || fail "runner-${r}: bin does not resolve after the move"
+  ok
+  [ -d "${RUNPOOL_BASE}/runners/bravo/runner-${r}/externals/" ] || fail "runner-${r}: externals does not resolve"
+  ok
+done
+
 # Written by _rp_write_plist against the new label, and the old ones removed by
 # hand: _rp_rewrite_plists only ever writes.
 present "${RUNPOOL_AGENT_DIR}/runpool.bravo.1.plist" "the new launch agent"