From 9f306996e4293e5cee64512d073261bf24a7792e Mon Sep 17 00:00:00 2001 From: Nimish Date: Mon, 6 Jul 2026 21:10:41 +0530 Subject: [PATCH 1/2] fix: preserve freshly-installed v2 binary during v1 legacy cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v1 (Python/PyInstaller) binary-zip layout installed `phase` alongside an `_internal/` directory in /usr/local/bin. A v2 *binary* install lands in that same directory, but cleanup_legacy() removed `${dir}/phase` whenever `${dir}/_internal` was present — deleting the v2 binary it had just installed. The installer still printed "installed successfully" and exited 0, leaving the machine with no `phase` at all. This only bit distros that fall through to the binary install path (Arch, and any OS/distro without a matching deb/rpm/apk package); package installs put v2 in /usr/bin and were unaffected. cleanup_legacy() now takes the absolute path of the just-installed binary and never deletes it — guarding both the _internal removal loop and the stale-symlink branch. Also drops the old, misleading package-manager advisory blocks (they fired even after a successful v2 install). Verified against real v1.21.3 and v2.3.0 release artifacts across binary, deb, rpm and apk installs, real v1->v2 package upgrades, the stale-symlink path, and idempotent re-runs. --- install.sh | 59 +++++++++++++++++------------------------------------- 1 file changed, 18 insertions(+), 41 deletions(-) diff --git a/install.sh b/install.sh index 5699d6ec..597384b5 100644 --- a/install.sh +++ b/install.sh @@ -182,56 +182,32 @@ get_latest_version() { # --- Install --- cleanup_legacy() { - # Clean up remnants of the old Python CLI (v1.x). - # - # Package-managed installs (deb/rpm/apk): the v2.0 packages use - # --replaces, so the package manager handles the upgrade cleanly. - # We just nudge the user to use the package instead of this script. - # - # Binary zip installs (old arm64, Arch): clean up the leftover - # _internal/ directory and stale symlinks. - - if command -v dpkg > /dev/null 2>&1 && dpkg -s phase > /dev/null 2>&1; then - echo "" - info "Detected Phase CLI v1 installed via DEB package." - info "Upgrade with the v2 package instead: dpkg -i phase__.deb" - info "Download from: https://github.com/${REPO}/releases" - echo "" - elif command -v rpm > /dev/null 2>&1 && rpm -q phase > /dev/null 2>&1; then - echo "" - info "Detected Phase CLI v1 installed via RPM package." - info "Upgrade with the v2 package instead: rpm -Uvh phase--1..rpm" - info "Download from: https://github.com/${REPO}/releases" - echo "" - elif command -v apk > /dev/null 2>&1 && apk info -e phase > /dev/null 2>&1; then - echo "" - info "Detected Phase CLI v1 installed via APK package." - info "Upgrade with the v2 package instead: apk add --allow-untrusted phase__.apk" - info "Download from: https://github.com/${REPO}/releases" - echo "" - fi - - # Clean up binary zip leftovers (_internal/ directory and the old binary). - # The old v1 binary zip installed phase + _internal/ side by side. - # The new v2 packages install to /usr/bin/phase, so we must remove the - # old binary from /usr/local/bin (or /usr/bin) to avoid PATH shadowing. + # Clean up remnants of the old Python CLI (v1.x). $1 is the absolute path of + # the v2 binary we just installed; it must be preserved. The old v1 + # binary-zip layout (`phase` + `_internal/`) lived in /usr/local/bin — which + # is also where a v2 *binary* install lands — so we must not delete the + # freshly-installed binary while clearing the leftovers. + keep="$1" + + # The old v1 binary zip installed phase + _internal/ side by side. Remove the + # stale _internal/, and the sibling v1 binary too (it would otherwise shadow + # a /usr/bin package install on PATH) — but never the v2 binary at $keep. for dir in /usr/local/bin /usr/local/sbin /usr/bin; do if [ -d "${dir}/_internal" ]; then info "Removing legacy ${dir}/_internal/..." do_install rm -rf "${dir}/_internal" - # The presence of _internal/ proves this is the old PyInstaller v1 - # binary. Always remove it — the new v2 binary is installed - # elsewhere (e.g. /usr/bin via package manager). - if [ -f "${dir}/phase" ]; then + if [ -f "${dir}/phase" ] && [ "${dir}/phase" != "$keep" ]; then info "Removing old v1 binary ${dir}/phase..." do_install rm -f "${dir}/phase" fi fi done - # Clean up stale symlinks pointing to old /usr/lib/phase/ - if [ -L "/usr/bin/phase" ]; then + # Clean up stale symlinks pointing to old /usr/lib/phase/ — but never the + # binary we just installed (on the package path $keep is /usr/bin/phase, so + # guard against deleting it if the pkg manager ever left it symlink-shaped). + if [ -L "/usr/bin/phase" ] && [ "/usr/bin/phase" != "$keep" ]; then link_target=$(readlink "/usr/bin/phase" 2>/dev/null || true) case "$link_target" in *lib/phase/phase*) @@ -292,7 +268,8 @@ install_package() { ;; esac - cleanup_legacy + # v2 package installs the binary to /usr/bin/phase (nfpm bindir). + cleanup_legacy "/usr/bin/${BINARY_NAME}" echo "" info "Phase CLI v${version} installed successfully." @@ -328,7 +305,7 @@ install_binary() { info "Installing to ${INSTALL_DIR}/${BINARY_NAME}..." do_install install -m 755 "${tmpdir}/${asset_name}" "${INSTALL_DIR}/${BINARY_NAME}" - cleanup_legacy + cleanup_legacy "${INSTALL_DIR}/${BINARY_NAME}" echo "" info "Phase CLI v${version} installed successfully." From f60eff3b8c96eb1f6a826ae359b737ce582d49d8 Mon Sep 17 00:00:00 2001 From: Nimish Date: Mon, 6 Jul 2026 21:38:49 +0530 Subject: [PATCH 2/2] fix(install): harden legacy cleanup against symlinked-dir deletes Follow-up to the keep-guard fix, addressing review of #309: - Compare inodes (-ef) instead of paths when deciding whether ${dir}/phase is the just-installed binary. On the (exotic) topology where a loop dir is a symlink onto another (e.g. /usr/local/bin -> /usr/bin), a v1 leftover and the fresh v2 binary are the same physical file under two paths; the old string compare missed that and deleted v2 on the package path. Verified on dash and busybox. - Add a safety net: if $keep no longer exists after cleanup, die loudly instead of printing "installed successfully" over a broken install. This fails closed for any future regression in this class. - Drop the non-load-bearing string guard on the stale-symlink branch (a successful package install has already replaced the symlink with a regular file, so [ -L ] is false there); the -ef check now covers it. --- install.sh | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/install.sh b/install.sh index 597384b5..a366c839 100644 --- a/install.sh +++ b/install.sh @@ -192,12 +192,15 @@ cleanup_legacy() { # The old v1 binary zip installed phase + _internal/ side by side. Remove the # stale _internal/, and the sibling v1 binary too (it would otherwise shadow # a /usr/bin package install on PATH) — but never the v2 binary at $keep. + # Compare inodes (-ef), not strings: on a host where one of these dirs is a + # symlink onto another, ${dir}/phase and $keep can be the same physical file + # reached by two different paths, which a string compare would miss. for dir in /usr/local/bin /usr/local/sbin /usr/bin; do if [ -d "${dir}/_internal" ]; then info "Removing legacy ${dir}/_internal/..." do_install rm -rf "${dir}/_internal" - if [ -f "${dir}/phase" ] && [ "${dir}/phase" != "$keep" ]; then + if [ -f "${dir}/phase" ] && ! [ "${dir}/phase" -ef "$keep" ]; then info "Removing old v1 binary ${dir}/phase..." do_install rm -f "${dir}/phase" fi @@ -205,9 +208,8 @@ cleanup_legacy() { done # Clean up stale symlinks pointing to old /usr/lib/phase/ — but never the - # binary we just installed (on the package path $keep is /usr/bin/phase, so - # guard against deleting it if the pkg manager ever left it symlink-shaped). - if [ -L "/usr/bin/phase" ] && [ "/usr/bin/phase" != "$keep" ]; then + # binary we just installed (same-file guard, as above). + if [ -L "/usr/bin/phase" ] && ! [ "/usr/bin/phase" -ef "$keep" ]; then link_target=$(readlink "/usr/bin/phase" 2>/dev/null || true) case "$link_target" in *lib/phase/phase*) @@ -216,6 +218,13 @@ cleanup_legacy() { ;; esac fi + + # Safety net: legacy cleanup must never remove the binary we just installed. + # If some unusual layout defeated the guards above, fail loudly instead of + # printing "installed successfully" over a now-broken install. + if [ ! -e "$keep" ]; then + die "Legacy cleanup removed the just-installed binary at ${keep}. Aborting — please report this." + fi } # Asset naming convention: phase_cli_{version}_{os}_{arch}[.ext]