Scope dist/release cleanup to older patches of the same minor

RussellSpitzer · RussellSpitzer · commit 22fb3e7bb1ca · 2026-05-08T16:49:46.000-05:00
Previously svn_list_old_releases returned every apache-parquet-X.Y.Z
directory other than the version being released, which would have
swept the entire dist/release/parquet/ tree on first run and would
clobber a newer minor on a back-patch release (e.g. shipping 1.9.4
after 1.10.0 is out). Restrict the policy to "older patches of the
same minor branch": releasing 1.9.2 removes 1.9.0 and 1.9.1 but
never touches 1.8.x, 1.10.x, or a higher 1.9.x patch.

The new implementation validates the input with VERSION_REGEX, then
uses awk with literal-prefix matching (index($0, prefix) == 1) and
numeric tail comparison so '.' is never interpreted as a regex
metacharacter and double-digit patches sort correctly. The
publish-release.sh step summary now reads
"Patch Release Cleanup (parquet-X.Y.x)" so the operator can see the
policy at a glance. Adds seven new bats cases covering same-minor
cleanup, leaving higher patches alone, releasing .0 (no-op),
ignoring -rc and sister-project siblings, double-digit patches, and
invalid input. Full suite at 119 tests, all green.
diff --git a/release/bin/publish-release.sh b/release/bin/publish-release.sh
@@ -216,29 +216,33 @@ svn_promote_rc_to_release "${version}" "${rc_num}"
 step_summary "Promoted \`${rc_tag}\` -> \`${final_tag}\` on dist.apache.org"
 
 # ---------------------------------------------------------------------------
-# Step 2: Clean up old releases from dist/release
+# Step 2: Clean up superseded patch releases on the same minor branch
 # ---------------------------------------------------------------------------
+# Policy: only patch releases of the *same* minor (parquet-${major}.${minor}.x)
+# that are older than ${version} are removed from dist/release/parquet/.
+# Releases on other minor branches are left in place and must be retired
+# manually. See svn_list_old_releases for the full rationale.
 step_summary ""
-step_summary "### Old Release Cleanup"
+step_summary "### Patch Release Cleanup (parquet-${major}.${minor}.x)"
 
 if [[ ${DRY_RUN:-1} -ne 1 ]]; then
   if ! old_versions=$(svn_list_old_releases "${version}"); then
     exit 1
   fi
 
   if [[ -n "${old_versions}" ]]; then
-    step_summary "Removing old releases:"
+    step_summary "Removing superseded patches on parquet-${major}.${minor}.x:"
     while IFS= read -r old_dir; do
       [[ -z "${old_dir}" ]] && continue
       svn_remove_release "${old_dir}" "${version}"
       step_summary "- Removed \`${old_dir}\`"
     done <<< "${old_versions}"
   else
-    step_summary "No old releases to clean up"
+    step_summary "No superseded ${major}.${minor}.x patches to clean up"
   fi
 else
-  print_command "Dry-run, WOULD clean up old releases from dist/release"
-  step_summary "Would clean up old releases (dry-run)"
+  print_command "Dry-run, WOULD remove superseded ${major}.${minor}.x patches from dist/release"
+  step_summary "Would remove superseded ${major}.${minor}.x patches (dry-run)"
 fi
 
 # ---------------------------------------------------------------------------
diff --git a/release/libs/_svn.sh b/release/libs/_svn.sh
@@ -124,8 +124,17 @@ function svn_remove_rc {
 # svn_list_old_releases <version_to_keep>
 #   Echoes (one per line, on stdout) the names of release directories
 #   under dist/release/parquet/ that should be removed when promoting
-#   <version_to_keep> as the new latest release. Returns non-zero only
-#   on real svn-listing failure.
+#   <version_to_keep> as the new latest release.
+#
+#   Policy: only older patch releases of the *same* minor branch are
+#   considered eligible for cleanup. Releasing 1.9.2 cleans up 1.9.0 and
+#   1.9.1; it never touches releases on a different X.Y branch (e.g.
+#   1.8.x, 1.10.x) and never touches a higher patch on the same branch
+#   (e.g. 1.9.3 if it somehow exists). This matches "only one patch per
+#   supported minor on dist.apache.org/release"; older minor branches are
+#   left in place and must be retired manually.
+#
+#   Returns non-zero only on real svn-listing failure or invalid input.
 function svn_list_old_releases {
   local version_to_keep="$1"
   local release_base_url="${APACHE_DIST_URL}${APACHE_DIST_RELEASE_PATH}"
@@ -135,6 +144,14 @@ function svn_list_old_releases {
     return 0
   fi
 
+  if [[ ! "${version_to_keep}" =~ ^${VERSION_REGEX}$ ]]; then
+    print_error "svn_list_old_releases: invalid version '${version_to_keep}'"
+    return 1
+  fi
+  local keep_major="${BASH_REMATCH[1]}"
+  local keep_minor="${BASH_REMATCH[2]}"
+  local keep_patch="${BASH_REMATCH[3]}"
+
   local listing
   if ! listing=$(svn list \
       --username "${SVN_USERNAME}" --password "${SVN_PASSWORD}" --non-interactive \
@@ -143,8 +160,18 @@ function svn_list_old_releases {
     return 1
   fi
 
-  echo "${listing}" | grep -E "^${TAG_PREFIX}[0-9]" | sed 's|/$||' \
-    | grep -v "^${TAG_PREFIX}${version_to_keep}\$" || true
+  # Match only `apache-parquet-${keep_major}.${keep_minor}.<patch>/` entries
+  # with a numeric patch strictly less than ${keep_patch}. The literal-prefix
+  # check via awk's `index` avoids any chance of `.` being interpreted as a
+  # regex metacharacter and matching a sibling tree.
+  local minor_prefix="${TAG_PREFIX}${keep_major}.${keep_minor}."
+  echo "${listing}" \
+    | sed 's|/$||' \
+    | awk -v prefix="${minor_prefix}" -v keep_patch="${keep_patch}" '
+        index($0, prefix) == 1 {
+          tail = substr($0, length(prefix) + 1)
+          if (tail ~ /^[0-9]+$/ && (tail + 0) < (keep_patch + 0)) print $0
+        }'
 }
 
 # svn_remove_release <release_dir> <new_version>
diff --git a/release/tests/svn.bats b/release/tests/svn.bats
@@ -112,6 +112,11 @@ setup() {
 }
 
 # ---- svn_list_old_releases ----
+#
+# Policy under test: cleanup is scoped to *older patch releases on the same
+# minor branch*. Releasing 1.9.2 cleans up 1.9.0 and 1.9.1; it must never
+# touch other minors (1.8.x, 1.10.x), the kept version itself, or any
+# higher patch on the same minor.
 
 @test "svn_list_old_releases: dry-run returns empty without calling svn" {
   DRY_RUN=1
@@ -121,25 +126,124 @@ setup() {
   [ -z "$(echo "$output" | grep -v Dry-run | grep -v WOULD)" ]
 }
 
-@test "svn_list_old_releases: filters to apache-parquet-* dirs and excludes the kept version" {
+@test "svn_list_old_releases: returns older patches on the same minor only" {
   DRY_RUN=0
   svn() {
     if [[ "$1" == "list" ]]; then
-      printf 'apache-parquet-1.16.0/\napache-parquet-1.17.0/\napache-parquet-1.18.0/\nKEYS\nREADME\n'
+      printf '%s\n' \
+        'apache-parquet-1.8.3/' \
+        'apache-parquet-1.9.0/' \
+        'apache-parquet-1.9.1/' \
+        'apache-parquet-1.9.2/' \
+        'apache-parquet-1.10.0/' \
+        'KEYS' 'README'
       return 0
     fi
     return 99
   }
   export -f svn
-  run svn_list_old_releases "1.18.0"
+  run svn_list_old_releases "1.9.2"
   [ "$status" -eq 0 ]
-  [[ "$output" == *"apache-parquet-1.16.0"* ]]
-  [[ "$output" == *"apache-parquet-1.17.0"* ]]
-  [[ "$output" != *"apache-parquet-1.18.0"* ]]
+  [[ "$output" == *"apache-parquet-1.9.0"* ]]
+  [[ "$output" == *"apache-parquet-1.9.1"* ]]
+  # Same-minor current and other-minor entries must not be returned.
+  [[ "$output" != *"apache-parquet-1.9.2"* ]]
+  [[ "$output" != *"apache-parquet-1.8.3"* ]]
+  [[ "$output" != *"apache-parquet-1.10.0"* ]]
   [[ "$output" != *"KEYS"* ]]
   [[ "$output" != *"README"* ]]
 }
 
+@test "svn_list_old_releases: never touches a higher patch on the same minor" {
+  DRY_RUN=0
+  svn() {
+    if [[ "$1" == "list" ]]; then
+      printf '%s\n' \
+        'apache-parquet-1.9.1/' \
+        'apache-parquet-1.9.2/' \
+        'apache-parquet-1.9.3/'
+      return 0
+    fi
+    return 99
+  }
+  export -f svn
+  run svn_list_old_releases "1.9.2"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"apache-parquet-1.9.1"* ]]
+  [[ "$output" != *"apache-parquet-1.9.2"* ]]
+  [[ "$output" != *"apache-parquet-1.9.3"* ]]
+}
+
+@test "svn_list_old_releases: returns empty when releasing the .0 of a minor" {
+  DRY_RUN=0
+  svn() {
+    if [[ "$1" == "list" ]]; then
+      printf '%s\n' \
+        'apache-parquet-1.8.5/' \
+        'apache-parquet-1.9.0/' \
+        'apache-parquet-1.10.0/'
+      return 0
+    fi
+    return 99
+  }
+  export -f svn
+  run svn_list_old_releases "1.9.0"
+  [ "$status" -eq 0 ]
+  [ -z "$(echo "$output" | grep -v '^$')" ]
+}
+
+@test "svn_list_old_releases: ignores rc and otherwise-malformed siblings" {
+  DRY_RUN=0
+  svn() {
+    if [[ "$1" == "list" ]]; then
+      printf '%s\n' \
+        'apache-parquet-1.9.0/' \
+        'apache-parquet-1.9.1-rc0/' \
+        'apache-parquet-1.9.x/' \
+        'apache-parquet-format-2.10.0/' \
+        'apache-parquet-cpp-1.5.0/'
+      return 0
+    fi
+    return 99
+  }
+  export -f svn
+  run svn_list_old_releases "1.9.2"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"apache-parquet-1.9.0"* ]]
+  # Only the bare X.Y.Z/ sibling on the same minor counts.
+  [[ "$output" != *"rc0"* ]]
+  [[ "$output" != *"1.9.x"* ]]
+  [[ "$output" != *"format"* ]]
+  [[ "$output" != *"cpp"* ]]
+}
+
+@test "svn_list_old_releases: handles double-digit patch numbers numerically" {
+  DRY_RUN=0
+  svn() {
+    if [[ "$1" == "list" ]]; then
+      printf '%s\n' \
+        'apache-parquet-1.9.2/' \
+        'apache-parquet-1.9.9/' \
+        'apache-parquet-1.9.10/'
+      return 0
+    fi
+    return 99
+  }
+  export -f svn
+  run svn_list_old_releases "1.9.10"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"apache-parquet-1.9.2"* ]]
+  [[ "$output" == *"apache-parquet-1.9.9"* ]]
+  [[ "$output" != *"apache-parquet-1.9.10"* ]]
+}
+
+@test "svn_list_old_releases: rejects an invalid version-to-keep" {
+  DRY_RUN=0
+  run svn_list_old_releases "1.9"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"invalid version"* ]]
+}
+
 @test "svn_list_old_releases: returns empty when only the kept version exists" {
   DRY_RUN=0
   svn() {
