Skip to content

Commit 10982b3

Browse files
wu-shengclaude
andcommitted
Name the vote-passed versions explicitly; never take them positionally
`vote-passed <old_version>` published the version resolved from the tag and deleted the version passed as the argument. Two versions that do opposite things, and only one of them positional - the deleted one. Reading `vote-passed 9.7.0` as "release 9.7.0" is the natural interpretation, and it would have promoted 9.7.0 into dist/release and then immediately svn rm'd it. Take them by name, or ask for them: vote-passed --release 9.7.0 --old_version 9.6.0 vote-passed --release 9.7.0 --no-cleanup vote-passed # asks for both Anything not named is prompted for, each prompt saying what will happen to that version. Positional arguments are now an error pointing at the named form, so existing muscle memory fails loudly rather than quietly meaning something else. Equal versions are refused and the release version must already be tagged. RELEASE_VERSION and OLD_VERSION still work, with the flags winning over them. Versions are validated by one anchored regex, shared with cleanup. A glob such as [0-9]*.[0-9]*.[0-9]* accepts "9.7.0/", which is textually different from "9.7.0" and so slips past the equality guard, yet SVN canonicalises the trailing slash - cleanup would have deleted the release promote had just published. Anchoring also rejects whitespace, v-prefixes, traversal and command characters before any of it reaches an svn path. The defaults behind the prompts were wrong too. The release version came from the highest version tag, but releases are not monotonic: a 9.6.1 patch cut from the 9.6.0 line after 9.7.0 has shipped is newer in time but lower in version, so the suggestion would have been the already-released 9.7.0. Order by tag creation date instead - maven-release-plugin writes annotated tags, so that timestamp belongs to the tag and survives fetches. The old version now comes from dist/release, the only place that knows what this release actually replaces, with the version being released filtered out so re-running after a partial failure cannot offer to delete it. Both detectors are genuinely best effort. Under `set -euo pipefail` a failed svn ls, or a grep matching nothing - which is exactly the state after a partial failure, when only the new release remains - aborted the release instead of yielding no suggestion, leaving the "could not read dist/release" branch unreachable. They now return success with empty output. Testing the flags uncovered a bug that predates them and reached every command. error() wrote to stdout, and resolve_version is always called as `version=$(resolve_version ...)`, so its message was captured into the variable instead of shown: releasing an untagged version aborted, correctly, but printed nothing about why. info/warn/error now write to stderr. The email templates use plain echo, so they stay on stdout and remain pipeable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent afede1b commit 10982b3

2 files changed

Lines changed: 183 additions & 26 deletions

File tree

docs/en/contribution/release-java-agent.md

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ The release script `tools/releasing/release.sh` automates the full release workf
4747

4848
# (send vote email to dev@skywalking.apache.org, wait 72h for vote to pass)
4949

50-
# Step 2: Promote, push Docker images, generate announce email, and clean up
51-
./tools/releasing/release.sh vote-passed [old_version_to_remove]
50+
# Step 2: Promote, publish the GitHub Release, generate announce email, and clean up
51+
./tools/releasing/release.sh vote-passed
5252
```
5353

5454
Run `./tools/releasing/release.sh` without arguments to see all available commands, including individual steps if you need to run them separately.
@@ -104,9 +104,29 @@ Every step after `prepare` identifies the release by its **tag** (`vx.y.z`), nev
104104
checked-out branch. By the time you run `vote-passed`, the release PR has normally been
105105
merged and `release/x.y.z` deleted, and `main` has already moved on to the next
106106
`-SNAPSHOT`; the tag is the only thing that still pins the release. The version defaults to
107-
the highest `vx.y.z` tag in the repository, and can be overridden with a positional
107+
the most recently created `vx.y.z` tag, and can be overridden with a positional
108108
argument (`./release.sh docker 9.7.0`) or `RELEASE_VERSION=9.7.0`.
109109

110+
`vote-passed` never takes the versions positionally — they do opposite things, and swapping
111+
them would delete the release that was just promoted. Name them, or be asked:
112+
113+
```shell
114+
./tools/releasing/release.sh vote-passed --release 9.7.0 --old_version 9.6.0
115+
./tools/releasing/release.sh vote-passed --release 9.7.0 --no-cleanup
116+
./tools/releasing/release.sh vote-passed # asks for both
117+
```
118+
119+
- **Release version** — the one being published. Defaults to the most recently *created*
120+
`vx.y.z` tag, not the highest one: a `9.6.1` patch cut from the `9.6.0` line after `9.7.0`
121+
has shipped is newer in time but lower in version.
122+
- **Old version** — removed from `dist/release`, which ASF policy keeps to just the current
123+
release. Defaults to what is published there now, excluding the version being released.
124+
Answer `none` to skip.
125+
126+
`RELEASE_VERSION` and `OLD_VERSION` are honoured too; the flags win over them. Both versions
127+
are validated as `x.y.z`, the release version must already be tagged, and the two being equal
128+
is refused.
129+
110130
After the vote passes, run `vote-passed` which executes:
111131
1. **promote** — move packages from `dist/dev` to `dist/release` in Apache SVN (prompts for SVN credentials), then release the Nexus staging repository at https://repository.apache.org and update the website download page
112132
2. **github-release** — publish the GitHub Release for the tag, using `changes/changes-x.y.z.md` as its notes

tools/releasing/release.sh

Lines changed: 160 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@
2727
# ./release.sh prepare-vote Run prepare + stage + upload, then generate vote email
2828
# ./release.sh email [vote|announce] Generate email content
2929
# ./release.sh promote Move from dist/dev to dist/release in SVN
30-
# ./release.sh docker Build and push Docker images
31-
# ./release.sh vote-passed Run promote + docker, then generate announce email
30+
# ./release.sh github-release Publish the GitHub Release (pushes Docker images via CI)
31+
# ./release.sh docker Push Docker images locally (fallback)
32+
# ./release.sh vote-passed [--release x.y.z] [--old_version x.y.z]
3233
# ./release.sh cleanup <old_version> Remove old release from dist/release
3334

3435
set -euo pipefail
@@ -42,9 +43,69 @@ GREEN='\033[0;32m'
4243
YELLOW='\033[1;33m'
4344
NC='\033[0m' # No Color
4445

45-
info() { echo -e "${GREEN}[INFO]${NC} $*"; }
46-
warn() { echo -e "${YELLOW}[WARN]${NC} $*"; }
47-
error() { echo -e "${RED}[ERROR]${NC} $*"; exit 1; }
46+
# Diagnostics go to stderr. resolve_version and the detect_* helpers are called
47+
# inside $( ), which captures stdout - an error printed there would be swallowed
48+
# into the variable instead of reaching the release manager.
49+
info() { echo -e "${GREEN}[INFO]${NC} $*" >&2; }
50+
warn() { echo -e "${YELLOW}[WARN]${NC} $*" >&2; }
51+
error() { echo -e "${RED}[ERROR]${NC} $*" >&2; exit 1; }
52+
53+
# ============================================================
54+
# validate_version — the only accepted shape for a version
55+
# ============================================================
56+
# Anchored, so nothing can ride along after the digits. A glob like
57+
# [0-9]*.[0-9]*.[0-9]* accepts "9.7.0/", which is textually different from
58+
# "9.7.0" and so slips past the equality guard in vote-passed, yet SVN
59+
# canonicalises the trailing slash - cleanup would then delete the release that
60+
# promote had just published. Every version entering an svn path goes through
61+
# here.
62+
validate_version() {
63+
local version="$1"
64+
local what="${2:-version}"
65+
if [[ ! "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
66+
error "${what} must be exactly x.y.z, got: '${version}'"
67+
fi
68+
}
69+
70+
# ============================================================
71+
# detect_release_version — best guess at the release in flight
72+
# ============================================================
73+
# Ordered by when the tag was made, not by version number. Releases are not
74+
# monotonic: a 9.6.1 patch cut from the 9.6.0 line after 9.7.0 has shipped is
75+
# newer in time but lower in version, and sorting by version would pick the
76+
# already-released 9.7.0. maven-release-plugin writes annotated tags, so
77+
# creatordate is the tag's own timestamp and is stable across fetches.
78+
#
79+
# This is only ever a default offered to the release manager, never the final
80+
# word - vote-passed asks them to confirm it.
81+
detect_release_version() {
82+
local found
83+
found=$(git for-each-ref --sort=-creatordate --format='%(refname:short)' \
84+
'refs/tags/v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null | head -1 | sed 's/^v//') || found=""
85+
printf '%s' "$found"
86+
}
87+
88+
# ============================================================
89+
# detect_old_version — the release currently published in dist/release
90+
# ============================================================
91+
# ASF policy keeps only the current release in dist/release; older ones are
92+
# served from archive.apache.org. Whatever is there now is therefore what this
93+
# release replaces. Excludes the version being released, so re-running after a
94+
# partial failure - when promote has already copied it in - does not offer to
95+
# delete the release itself. Best effort: no network, no default.
96+
detect_old_version() {
97+
local exclude="${1:-}"
98+
local found
99+
# Under `set -euo pipefail` a failed svn ls, or a grep that matches nothing,
100+
# would abort the whole release rather than simply yield no suggestion. This
101+
# is only ever a hint, so swallow both and return empty.
102+
found=$(svn ls "https://dist.apache.org/repos/dist/release/skywalking/java-agent/" 2>/dev/null \
103+
| sed 's#/$##' \
104+
| grep -E '^[0-9]+\.[0-9]+\.[0-9]+$' \
105+
| grep -vx "$exclude" \
106+
| sort -V | tail -1) || found=""
107+
printf '%s' "$found"
108+
}
48109

49110
# ============================================================
50111
# resolve_version — identify the release from its tag
@@ -60,8 +121,8 @@ error() { echo -e "${RED}[ERROR]${NC} $*"; exit 1; }
60121
# That would aim SVN moves and Docker pushes at the wrong version. Tags are
61122
# branch-independent and outlive the release branch, so select from the tag list.
62123
#
63-
# Order of precedence: explicit argument, then $RELEASE_VERSION, then the highest
64-
# vX.Y.Z tag in the repository.
124+
# Order of precedence: explicit argument, then $RELEASE_VERSION, then the most
125+
# recently created vX.Y.Z tag (see detect_release_version for why not the highest).
65126
resolve_version() {
66127
local explicit="${1:-}"
67128
[ -z "$explicit" ] && explicit="${RELEASE_VERSION:-}"
@@ -71,17 +132,14 @@ resolve_version() {
71132
version="${explicit#v}"
72133
else
73134
local latest
74-
latest=$(git tag -l 'v[0-9]*.[0-9]*.[0-9]*' --sort=-v:refname | head -1)
135+
latest=$(detect_release_version)
75136
if [ -z "$latest" ]; then
76137
error "No vX.Y.Z release tag found. Pass the version explicitly, e.g. '$0 <command> 9.7.0'."
77138
fi
78-
version="${latest#v}"
139+
version="$latest"
79140
fi
80141

81-
case "$version" in
82-
[0-9]*.[0-9]*.[0-9]*) ;;
83-
*) error "Version must look like x.y.z, got: ${version}" ;;
84-
esac
142+
validate_version "$version" "Release version"
85143

86144
# Refuse to act on a release that was never tagged.
87145
if ! git rev-parse -q --verify "refs/tags/v${version}" >/dev/null 2>&1; then
@@ -672,6 +730,7 @@ cmd_cleanup() {
672730
if [ -z "$old_version" ]; then
673731
error "Usage: $0 cleanup <old_version> (e.g., 9.5.0)"
674732
fi
733+
validate_version "$old_version" "Old version"
675734

676735
info "Removing old release ${old_version} from dist/release..."
677736

@@ -710,15 +769,87 @@ cmd_prepare_vote() {
710769
# vote-passed — run all steps after the vote passes
711770
# ============================================================
712771
cmd_vote_passed() {
713-
local old_version="${1:-}"
714-
715772
cd "$PROJECT_ROOT"
716773

717-
# Resolved from the release tag, so this works after the release branch has
718-
# been merged and deleted. Show it before touching SVN or Docker Hub, both of
719-
# which are public and awkward to undo.
720-
local version
721-
version=$(resolve_version "")
774+
# Two versions, and they do opposite things: one is published, the other is
775+
# deleted. Never take them positionally - `vote-passed 9.7.0` while releasing
776+
# 9.7.0 reads as "release this" but would svn rm what promote just copied in.
777+
# Name them, or be asked for them. Detection only supplies defaults.
778+
local version="${RELEASE_VERSION:-}"
779+
local old_version="${OLD_VERSION:-}"
780+
local old_version_given=0
781+
[ -n "$old_version" ] && old_version_given=1
782+
783+
while [ "$#" -gt 0 ]; do
784+
case "$1" in
785+
--release|--release-version)
786+
[ -n "${2:-}" ] || error "$1 needs a version, e.g. --release 9.7.0"
787+
version="$2"; shift 2 ;;
788+
--old_version|--old-version)
789+
[ -n "${2:-}" ] || error "$1 needs a version, e.g. --old_version 9.6.0 (or --no-cleanup)"
790+
old_version="$2"; old_version_given=1; shift 2 ;;
791+
--no-cleanup)
792+
old_version=""; old_version_given=1; shift ;;
793+
-h|--help)
794+
echo "Usage: $0 vote-passed [--release x.y.z] [--old_version x.y.z | --no-cleanup]"
795+
echo ""
796+
echo " --release the version being published; must already be tagged"
797+
echo " --old_version the version removed from dist/release"
798+
echo " --no-cleanup leave dist/release alone"
799+
echo ""
800+
echo "Anything not given is asked for. RELEASE_VERSION and OLD_VERSION are"
801+
echo "honoured as well, and the flags win over them."
802+
return 0 ;;
803+
-*)
804+
error "Unknown option: $1
805+
Usage: $0 vote-passed [--release x.y.z] [--old_version x.y.z | --no-cleanup]" ;;
806+
*)
807+
error "'$0 vote-passed' does not take positional arguments - it is too easy to
808+
confuse the version being released with the one being deleted. Name them:
809+
$0 vote-passed --release <new> --old_version <old>
810+
or run it with no arguments and answer the prompts." ;;
811+
esac
812+
done
813+
814+
info "Publishing a release. Two versions are needed."
815+
echo ""
816+
817+
if [ -z "$version" ]; then
818+
local suggested
819+
suggested=$(detect_release_version)
820+
echo " The version being released. It must already be tagged and voted on."
821+
if [ -n "$suggested" ]; then
822+
echo " Most recently tagged: ${suggested} (tag v${suggested})"
823+
fi
824+
read -rp " Release version${suggested:+ [$suggested]}: " version
825+
version="${version:-$suggested}"
826+
fi
827+
[ -z "$version" ] && error "No release version given."
828+
version=$(resolve_version "$version")
829+
echo ""
830+
831+
if [ "$old_version_given" -eq 0 ]; then
832+
local current
833+
current=$(detect_old_version "$version")
834+
echo " The version to remove from dist/release. ASF policy keeps only the"
835+
echo " current release there; older ones are served from archive.apache.org."
836+
if [ -n "$current" ]; then
837+
echo " Currently published in dist/release: ${current}"
838+
else
839+
echo " Could not read dist/release, so there is no suggestion."
840+
fi
841+
read -rp " Old version to remove${current:+ [$current]} (or 'none' to skip): " old_version
842+
old_version="${old_version:-$current}"
843+
fi
844+
# 'none' is the explicit opt out; blank accepts the suggestion above.
845+
[ "$old_version" = "none" ] && old_version=""
846+
[ -n "$old_version" ] && validate_version "$old_version" "Old version ('none' or --no-cleanup skips cleanup);"
847+
848+
# The mistake this whole prompt exists to prevent.
849+
if [ -n "$old_version" ] && [ "$old_version" = "$version" ]; then
850+
error "Old version and release version are both ${version}; that would delete the release being published."
851+
fi
852+
echo ""
722853

723854
info "Publishing release ${version}:"
724855
echo " Release tag : v${version}"
@@ -781,11 +912,14 @@ main() {
781912
echo "Quick start (two-step release):"
782913
echo " $0 prepare-vote 9.7.0 [9.8.0] # before vote (next version auto-calculated if omitted)"
783914
echo " (wait for 72h vote to pass)"
784-
echo " $0 vote-passed [old_version] # after vote"
915+
echo " $0 vote-passed # after vote (asks for the versions)"
916+
echo " $0 vote-passed --release 9.7.0 --old_version 9.6.0"
785917
echo ""
786918
echo "Every command after 'prepare' identifies the release by its tag (vX.Y.Z), not by the"
787919
echo "checked-out branch, so they still work once release/x.y.z has been merged and deleted."
788-
echo "The version defaults to the highest vX.Y.Z tag; override with an argument or RELEASE_VERSION."
920+
echo "The version defaults to the most recently created vX.Y.Z tag - not the highest, since a"
921+
echo "patch release can be newer in time but lower in version. Override with an argument or"
922+
echo "RELEASE_VERSION."
789923
echo ""
790924
echo "Individual commands:"
791925
echo " preflight Check tools and environment"
@@ -799,7 +933,10 @@ main() {
799933
echo " the Docker images, via publish-docker.yaml"
800934
echo " docker [ver] Push Docker images from this machine (fallback"
801935
echo " for when the workflow fails)"
802-
echo " vote-passed [old_ver] Run promote + github-release + announce [+ cleanup]"
936+
echo " vote-passed [--release x.y.z] [--old_version x.y.z | --no-cleanup]"
937+
echo " Run promote + github-release + announce [+ cleanup]."
938+
echo " Anything not named is asked for. Never positional:"
939+
echo " the two versions do opposite things."
803940
echo " cleanup <old_version> Remove old release from dist/release"
804941
;;
805942
esac

0 commit comments

Comments
 (0)