From 7cb7a6a5ff224457aa9e26471d0f8e86f8a2f10b Mon Sep 17 00:00:00 2001 From: Wu Sheng Date: Sun, 30 Aug 2026 21:47:46 +0800 Subject: [PATCH] Make the release scripts actually run Running them for the first time. They had been written and reviewed but never executed, and every one of these was found by execution, not by reading. release.sh died silently immediately after preflight's first log line. preflight ended on `git rev-parse "$TAG" && die` / `svn ls .../$VERSION && die`, and on the normal path -- tag absent, version not yet staged -- the left side fails. set -e is exempt for the left side of an && list, but the list's status becomes the FUNCTION's return value, and `preflight` is called as a bare statement, so set -e killed the run there with no message. Both guards are now if-form, which returns 0 when the condition is false. This has been present since the script was added, so preflight had never once completed. The temp-dir cleanup was registered as an EXIT trap over a `local`. A trap runs after the function has returned and cannot see a local, so under set -u the trap itself failed: exit status 1 on the success path, and the directory holding the signed artifacts leaked every run. The variable and the trap are now both at script scope. The earlier RETURN trap had the mirror-image flaw -- right scope, but RETURN does not fire when set -e kills the shell mid-function. The Linux-only guard is removed. It was added when `make clean` was a backslash-continued rm whose later -rf tokens sat mid-argument-list, which GNU rm permutes and BSD rm does not. That recipe is now a single rm -rf over one operand list, which both accept, and the guard outlived its reason. Verified by running rather than by reading: release.sh --dry-run, from a pristine clone, on macOS: exit 0, all nine stages, both signatures verify under gpg --batch --verify, both checksums under shasum -c, the packaged chart renders 33 resources, all six artifacts staged for svn, and no temp directory left behind. release-passed.sh against a local svnadmin repo mirroring the real layout -- a staged 5.0.0 in dev, 4.9.0 plus loose 4.8.0 files in release. --dry-run walks every stage and changes nothing; declining the promotion aborts and changes nothing; accepting moves all six files, removes both the 4.9.0 directory and the loose file, and declining the GitHub release aborts before creating it. The macOS build path end to end: make clean exits 0 and removes everything, make release-src produces a tarball with no AppleDouble, .DS_Store, .git or build artifacts, make release signs and checksums all six files. Nothing was uploaded to dist.apache.org and no tag was pushed; the svn work all happened in a throwaway checkout, and svn add is local until svn commit. --- docs/contributing/release.md | 10 +++++---- tools/releasing/release.sh | 39 ++++++++++++++++++++++++------------ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/docs/contributing/release.md b/docs/contributing/release.md index a25eb21..acdc2a9 100644 --- a/docs/contributing/release.md +++ b/docs/contributing/release.md @@ -137,8 +137,10 @@ gpg --list-secret-keys --keyid-format=long ## 3. Build, verify, tag, upload and call the vote — `release.sh` -**On Linux, from a pristine clone of the release commit.** Preflight aborts unless `uname -s` is -`Linux`, and there is no override flag. +**From a pristine clone of the release commit.** Linux and macOS both work — the build was +verified end to end on each. A pristine clone matters for a different reason: preflight refuses to +run against a dirty tree, and `release-src` archives the working *tree*, so any untracked file +sitting in the checkout would otherwise be shipped inside the source release. ```shell git clone git@github.com:apache/skywalking-helm && cd skywalking-helm @@ -168,7 +170,6 @@ irreversible step comes last, and a failure leaves nothing on the remote to clea Preflight refuses to start when: -- the host is not Linux; - any of `helm`, `gpg`, `shasum`, `svn`, `git`, `make`, `tar`, `awk` is missing. All of them are reported in one message — finding them one at a time costs one failed run per package; - `helm` is older than 3.8. The chart ships only as an OCI artifact, and `helm push` to an `oci://` @@ -571,7 +572,8 @@ applied and `-f` swallowed the stray `rm` operands, and the target did what it r macOS — stops option parsing at the first operand, so `-r` never took effect: it failed with `rm: bin/: is a directory`, left `bin/` and `chart/skywalking/charts/` behind, and exited `1`, which aborted `make clean`, `make release-src` and `make release`. The recipe is one `rm -rf` again and -survives BSD `rm`; the Linux requirement now lives in `release.sh`'s preflight, not in this recipe. +survives BSD `rm`. `release.sh` carried a Linux-only guard for as long as that recipe was broken; +with the recipe fixed the guard was obsolete, and it has been removed. If you touch it, keep it a single `rm -rf` and re-read the whole thing rather than one line. Because `clean` wipes `charts/` and `Chart.lock`, the next `package` re-resolves dependencies from diff --git a/tools/releasing/release.sh b/tools/releasing/release.sh index 51c8ef0..e79b07c 100755 --- a/tools/releasing/release.sh +++ b/tools/releasing/release.sh @@ -43,16 +43,25 @@ log() { echo " $*"; } step() { echo; echo "=== $* ==="; } die() { echo "ERROR: $*" >&2; exit 1; } +# upload_to_svn stages the signed artifacts in a temp directory. The cleanup is registered here, +# at script scope, over a script-scope variable: an EXIT trap runs after the function has already +# returned, so it cannot see a `local`, and under `set -u` the unbound name would make the trap +# fail and take the script's exit status with it. +WORKDIR="" +cleanup() { [[ -n "${WORKDIR}" ]] && rm -rf "${WORKDIR}"; return 0; } +trap cleanup EXIT + # --------------------------------------------------------------------------- preflight() { step "Preflight" - # make clean is a prerequisite of make release, and its recipe is a single - # backslash-continued rm whose later -rf tokens sit mid-argument-list. GNU rm - # permutes those; BSD rm does not. So on macOS it exits 2 and leaves - # chart/skywalking/charts/ behind, and the release is built from a dirty tree. - [[ "$(uname -s)" == "Linux" ]] || die "build the release on Linux -- 'make clean' does not work on macOS (BSD rm), see docs/contributing/release.md" + # No OS check. There used to be one: `make clean` was a backslash-continued rm + # whose later -rf tokens sat mid-argument-list, which GNU rm permutes and BSD rm + # does not, so on macOS it exited 2 and left chart/skywalking/charts/ behind. + # That recipe is now a single rm -rf over one operand list, which both accept, + # and the whole path -- clean, release-src, package, gpg sign, shasum, and the + # verify checks below -- has been run through to completion on macOS. # Report every missing tool at once. Dying on the first means one failed run # per package, and this check exists precisely to spend zero of them. @@ -102,14 +111,21 @@ ${strays}" TAG="v${VERSION}" log "version ${VERSION} (from chart/skywalking/Chart.yaml)" - git rev-parse "${TAG}" >/dev/null 2>&1 && die "tag ${TAG} already exists -- bump Chart.yaml or delete the tag" + # `if`, not `X && die`. An && list that fails on its left side returns non-zero, and when it is + # the last statement in a function that becomes the function's return value -- which `set -e` + # then treats as a failed call, killing the run with no message. `if` returns 0 when the + # condition is false, which is the normal path here. + if git rev-parse "${TAG}" >/dev/null 2>&1; then + die "tag ${TAG} already exists -- bump Chart.yaml or delete the tag" + fi # A re-run after a partial upload would mkdir a local ${VERSION} over a path that already exists # in svn, and only find out at commit time -- after the build, the signing and the tag push. # Safe to read a non-zero exit as "not there" only because the svn check above already # established that the repository is reachable and the credentials work. - svn ls "${SVN_DEV_URL}/helm/${VERSION}" >/dev/null 2>&1 \ - && die "${SVN_DEV_URL}/helm/${VERSION} already exists -- delete it, or bump the version" + if svn ls "${SVN_DEV_URL}/helm/${VERSION}" >/dev/null 2>&1; then + die "${SVN_DEV_URL}/helm/${VERSION} already exists -- delete it, or bump the version" + fi } @@ -167,11 +183,8 @@ upload_to_svn() { step "Upload to ${SVN_DEV_URL}/helm/${VERSION}" cd "${PROJECT_DIR}" - # EXIT, not RETURN: a RETURN trap does not fire when `set -e` kills the shell part-way through - # the function, which would leave a temp directory holding a copy of the signed artifacts. - local workdir - workdir=$(mktemp -d) - trap 'rm -rf "${workdir}"' EXIT + WORKDIR=$(mktemp -d) + local workdir="${WORKDIR}" # Sparse checkout: a full checkout of dist/dev/skywalking pulls every # sub-project's staging area, which is gigabytes.