From de1abae78eec47471c6a1308a00def201e4cf9da Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Fri, 17 Jul 2026 00:42:41 +0200 Subject: [PATCH] fix(scripts): resolve go version wrappers via GOBIN, prune stale ones GO_VERSION=1.26 install reported before/after and skipped both the SDK download and the go1.26 cycle symlink whenever GOPATH/bin was off PATH: the post-install check used 'have go1.26.5' (a PATH lookup) although go install always lands in GOBIN. Every auto-update run then left another orphaned goX.Y.Z wrapper (12 accumulated in the field). - resolve wrappers via go env GOBIN/GOPATH explicitly (install check, SDK download, symlink update, and the before/after version probes) - prune superseded same-cycle wrappers and their ~/sdk trees after a successful update; other cycles untouched - drop unused TOOL var (pre-existing SC2034 blocking the shellcheck hook on this file) Signed-off-by: Sebastian Mendel --- scripts/install_go.sh | 60 +++++++++++--- tests/test_go_multiversion.py | 146 ++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 tests/test_go_multiversion.py diff --git a/scripts/install_go.sh b/scripts/install_go.sh index e14e353..3518f2a 100755 --- a/scripts/install_go.sh +++ b/scripts/install_go.sh @@ -6,7 +6,6 @@ DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" have() { command -v "$1" >/dev/null 2>&1; } -TOOL="go" ACTION="${1:-install}" # Support version-specific installation via GO_VERSION env var @@ -22,16 +21,33 @@ else DISPLAY_NAME="go" fi -# Get current version of the specific binary +# GOBIN (default GOPATH/bin) — where `go install` puts version wrappers. +# May be off PATH, so never rely on `have` alone for these binaries. +go_bin_dir() { + have go || return 0 + local dir + dir="$(go env GOBIN 2>/dev/null || true)" + [ -z "$dir" ] && dir="$(go env GOPATH 2>/dev/null || true)/bin" + echo "$dir" +} + +# Get current version of the specific binary (PATH first, then GOBIN) get_go_version() { local bin="$1" if have "$bin"; then "$bin" version 2>/dev/null | head -1 || true + return 0 + fi + local gobin + gobin="$(go_bin_dir)" + if [ -n "$gobin" ] && [ -x "$gobin/$bin" ]; then + "$gobin/$bin" version 2>/dev/null | head -1 || true fi } install_go() { - local before="$(get_go_version "$BINARY")" + local before + before="$(get_go_version "$BINARY")" # Version-specific Go installation (e.g., go1.24 alongside go1.25) if [ -n "$TARGET_CYCLE" ]; then @@ -65,17 +81,34 @@ install_go() { echo "Installing ${FULL_BINARY} via go install golang.org/dl/${FULL_BINARY}@latest..." if go install "golang.org/dl/${FULL_BINARY}@latest"; then - # The go1.XX.YY command needs to download its SDK on first run - if have "$FULL_BINARY"; then + # go install lands in GOBIN, which may be off PATH — resolve explicitly + local gobin full_bin + gobin="$(go_bin_dir)" + full_bin="$gobin/$FULL_BINARY" + if [ -x "$full_bin" ]; then + # The go1.XX.YY command needs to download its SDK on first run echo "Downloading Go ${FULL_VERSION} SDK..." - "$FULL_BINARY" download || true + "$full_bin" download || true # Update symlink from go1.24 -> go1.24.12 - GOBIN="$(go env GOPATH)/bin" - if [ -x "$GOBIN/$FULL_BINARY" ]; then - ln -sf "$FULL_BINARY" "$GOBIN/$BINARY" 2>/dev/null || true - echo "Updated symlink: $BINARY -> $FULL_BINARY" + ln -sf "$FULL_BINARY" "$gobin/$BINARY" 2>/dev/null || true + echo "Updated symlink: $BINARY -> $FULL_BINARY" + + # Remove superseded wrappers (and SDKs) of the same cycle so + # repeated updates don't accumulate go1.26.0, go1.26.2, ... + if [[ "$TARGET_CYCLE" =~ ^[0-9]+\.[0-9]+$ ]]; then + local stale stale_name + for stale in "$gobin/go${TARGET_CYCLE}".*; do + [ -e "$stale" ] || continue + stale_name="$(basename "$stale")" + [ "$stale_name" = "$FULL_BINARY" ] && continue + rm -f "$stale" + rm -rf "$HOME/sdk/$stale_name" 2>/dev/null || true + echo "Removed superseded wrapper: $stale_name" + done fi + else + echo "Error: ${FULL_BINARY} not found in $gobin after go install" >&2 fi else echo "Failed to install ${FULL_BINARY}" >&2 @@ -140,8 +173,11 @@ install_go() { rm -rf "$TMP" 2>/dev/null || true fi - local after="$(get_go_version "$BINARY")" - local path="$(command -v "$BINARY" 2>/dev/null || true)" + local after + + after="$(get_go_version "$BINARY")" + local path + path="$(command -v "$BINARY" 2>/dev/null || true)" printf "[%s] before: %s\n" "$DISPLAY_NAME" "${before:-}" printf "[%s] after: %s\n" "$DISPLAY_NAME" "${after:-}" if [ -n "$path" ]; then printf "[%s] path: %s\n" "$DISPLAY_NAME" "$path"; fi diff --git a/tests/test_go_multiversion.py b/tests/test_go_multiversion.py new file mode 100644 index 0000000..c82e508 --- /dev/null +++ b/tests/test_go_multiversion.py @@ -0,0 +1,146 @@ +"""Tests for multi-version go installs (GO_VERSION=1.26 install_go.sh). + +After `go install golang.org/dl/go1.26.5@latest`, the script verified the +wrapper via PATH (`have go1.26.5`) — but go installs into GOBIN +(default GOPATH/bin), which may be off PATH. The SDK download and the +go1.26 cycle symlink were then silently skipped, before/after reported +``, and every auto-update run left another orphaned goX.Y.Z wrapper. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +skip_on_windows = pytest.mark.skipif( + sys.platform == "win32", reason="Shell script tests require POSIX shell" +) + +SCRIPT = Path(__file__).parent.parent / "scripts" / "install_go.sh" + +# Template for the goX.Y.Z wrapper the `go` stub "installs" into GOPATH/bin +WRAPPER_TEMPLATE = """#!/usr/bin/env bash +# real golang.org/dl wrappers report their own version even when invoked +# through the cycle symlink (go1.26 -> go1.26.5) +me="$(basename "$(readlink -f "$0")")" +echo "$me $*" >> "$GO_STUB_LOG" +case "$1" in + download) exit 0 ;; + version) echo "go version $me linux/amd64" ;; +esac +""" + +GO_STUB = """#!/usr/bin/env bash +echo "go $*" >> "$GO_STUB_LOG" +case "$1" in + env) + case "$2" in + GOBIN) echo "" ;; + GOPATH) echo "$FAKE_GOPATH" ;; + esac + ;; + install) + pkg="${2#golang.org/dl/}" + pkg="${pkg%@latest}" + mkdir -p "$FAKE_GOPATH/bin" + cp "$WRAPPER_TEMPLATE_PATH" "$FAKE_GOPATH/bin/$pkg" + chmod +x "$FAKE_GOPATH/bin/$pkg" + ;; + version) + echo "go version go1.26.1 linux/amd64" + ;; +esac +exit 0 +""" + + +@skip_on_windows +class TestGoMultiVersionInstall: + def _setup(self, tmp_path: Path) -> tuple[dict, Path, Path]: + stub_dir = tmp_path / "stubs" + stub_dir.mkdir() + gopath = tmp_path / "gopath" + (gopath / "bin").mkdir(parents=True) + home = tmp_path / "home" + home.mkdir() + log = tmp_path / "stub.log" + log.touch() + + template = tmp_path / "wrapper.template" + template.write_text(WRAPPER_TEMPLATE) + + for name, body in ( + ("go", GO_STUB), + ("curl", '#!/usr/bin/env bash\necho \'[{"version":"go1.26.5"}]\'\n'), + ("python3", "#!/usr/bin/env bash\nexit 0\n"), + ): + stub = stub_dir / name + stub.write_text(body) + stub.chmod(0o755) + + env = { + "HOME": str(home), + # GOPATH/bin deliberately NOT on PATH — the bug's trigger + "PATH": f"{stub_dir}:/usr/bin:/bin", + "GO_VERSION": "1.26", + "FAKE_GOPATH": str(gopath), + "GO_STUB_LOG": str(log), + "WRAPPER_TEMPLATE_PATH": str(template), + } + return env, gopath, log + + def _run(self, env: dict) -> subprocess.CompletedProcess: + return subprocess.run( + ["bash", str(SCRIPT), "install"], + capture_output=True, text=True, timeout=60, env=env, + ) + + def test_sdk_download_and_symlink_despite_gobin_off_path(self, tmp_path): + env, gopath, log = self._setup(tmp_path) + result = self._run(env) + assert result.returncode == 0, result.stderr + + assert "go1.26.5 download" in log.read_text(), ( + "SDK download must run even when GOPATH/bin is off PATH" + ) + cycle_link = gopath / "bin" / "go1.26" + assert cycle_link.is_symlink(), "go1.26 cycle symlink must be created" + assert os.readlink(cycle_link) == "go1.26.5" + + def test_before_after_report_not_none(self, tmp_path): + env, _, _ = self._setup(tmp_path) + result = self._run(env) + out = result.stdout + result.stderr + # "before: " is correct on a fresh install — the bug was the + # after-probe returning because GOPATH/bin is off PATH + assert "[go@1.26] after: go version go1.26.5" in out, ( + f"after-version must probe GOPATH/bin, got: {out}" + ) + + def test_superseded_wrappers_of_same_cycle_are_removed(self, tmp_path): + env, gopath, _ = self._setup(tmp_path) + # Orphans from earlier runs (the wrapper zoo) + for old in ("go1.26.0", "go1.26.2"): + stale = gopath / "bin" / old + stale.write_text("#!/bin/sh\n") + stale.chmod(0o755) + (Path(env["HOME"]) / "sdk" / old).mkdir(parents=True) + # A different cycle must be untouched + other = gopath / "bin" / "go1.25.11" + other.write_text("#!/bin/sh\n") + other.chmod(0o755) + + result = self._run(env) + assert result.returncode == 0, result.stderr + + assert not (gopath / "bin" / "go1.26.0").exists() + assert not (gopath / "bin" / "go1.26.2").exists() + assert not (Path(env["HOME"]) / "sdk" / "go1.26.0").exists() + assert (gopath / "bin" / "go1.26.5").exists() + assert (gopath / "bin" / "go1.25.11").exists(), ( + "other cycles must not be touched" + )