From f9f8273fc8e3719044c3341a4c7fea732f7eb226 Mon Sep 17 00:00:00 2001 From: Ed Savage Date: Wed, 22 Jul 2026 10:11:30 +1200 Subject: [PATCH 1/3] Snapshot version-bump helpers before release-branch checkout Patch bumps start from main then git checkout the target branch tip, which replaced create_github_pull_request.sh with an older copy that rejects --label. Keep pipeline-revision helpers in a temp snapshot instead. Co-authored-by: Cursor --- dev-tools/bump_main_minor_freeze.sh | 17 +++- dev-tools/bump_version.sh | 15 +++- dev-tools/unittest/test_version_bump_lib.py | 98 +++++++++++++++++++++ dev-tools/version_bump_lib.sh | 28 ++++++ docs/changelog/3082.yaml | 5 ++ 5 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 dev-tools/unittest/test_version_bump_lib.py create mode 100644 docs/changelog/3082.yaml diff --git a/dev-tools/bump_main_minor_freeze.sh b/dev-tools/bump_main_minor_freeze.sh index e03481f20..705341c43 100755 --- a/dev-tools/bump_main_minor_freeze.sh +++ b/dev-tools/bump_main_minor_freeze.sh @@ -29,10 +29,19 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPT_DIR}/version_bump_lib.sh" PYTHON="${PYTHON:-python3}" -VALIDATION_PY="${SCRIPT_DIR}/version_bump_validation.py" -UPDATE_BACKPORTRC_PY="${SCRIPT_DIR}/update_backportrc.py" -UPDATE_CATALOG_PY="${SCRIPT_DIR}/update_catalog_snapshot.py" -CREATE_PR_SH="${SCRIPT_DIR}/create_github_pull_request.sh" + +# Snapshot before any release-branch checkout replaces worktree helpers. +HELPERS_DIR="$(version_bump_snapshot_helpers "$SCRIPT_DIR" \ + version_bump_validation.py \ + update_backportrc.py \ + update_catalog_snapshot.py \ + create_github_pull_request.sh \ + ensure_github_cli.sh)" +trap 'rm -rf "${HELPERS_DIR}"' EXIT +VALIDATION_PY="${HELPERS_DIR}/version_bump_validation.py" +UPDATE_BACKPORTRC_PY="${HELPERS_DIR}/update_backportrc.py" +UPDATE_CATALOG_PY="${HELPERS_DIR}/update_catalog_snapshot.py" +CREATE_PR_SH="${HELPERS_DIR}/create_github_pull_request.sh" : "${NEW_VERSION:?NEW_VERSION must be set}" : "${BRANCH:?BRANCH must be set}" diff --git a/dev-tools/bump_version.sh b/dev-tools/bump_version.sh index 720340fbe..2197f0b7b 100755 --- a/dev-tools/bump_version.sh +++ b/dev-tools/bump_version.sh @@ -23,6 +23,10 @@ # else — immediate gh pr merge --squash (legacy / escape hatch) # Does not modify .backportrc.json (reserved for future release automation). # +# Helpers used after `git checkout` of the release branch (validation + PR +# creation) are snapshotted from this script's directory first so an older +# branch tip cannot replace them mid-run. +# # Environment: # NEW_VERSION, BRANCH — required # DRY_RUN — true to skip push and PR creation @@ -51,8 +55,15 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${SCRIPT_DIR}/version_bump_lib.sh" PYTHON="${PYTHON:-python3}" -VALIDATION_PY="${SCRIPT_DIR}/version_bump_validation.py" -CREATE_PR_SH="${SCRIPT_DIR}/create_github_pull_request.sh" + +# Snapshot before any release-branch checkout replaces worktree helpers. +HELPERS_DIR="$(version_bump_snapshot_helpers "$SCRIPT_DIR" \ + version_bump_validation.py \ + create_github_pull_request.sh \ + ensure_github_cli.sh)" +trap 'rm -rf "${HELPERS_DIR}"' EXIT +VALIDATION_PY="${HELPERS_DIR}/version_bump_validation.py" +CREATE_PR_SH="${HELPERS_DIR}/create_github_pull_request.sh" : "${NEW_VERSION:?NEW_VERSION must be set}" : "${BRANCH:?BRANCH must be set}" diff --git a/dev-tools/unittest/test_version_bump_lib.py b/dev-tools/unittest/test_version_bump_lib.py new file mode 100644 index 000000000..9f174cf5e --- /dev/null +++ b/dev-tools/unittest/test_version_bump_lib.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License +# 2.0 and the following additional limitation. Functionality enabled by the +# files subject to the Elastic License 2.0 may only be used in production when +# invoked by an Elasticsearch process with a license key installed that permits +# use of machine learning features. You may not use this file except in +# compliance with the Elastic License 2.0 and the foregoing additional +# limitation. + +"""Tests for helpers in dev-tools/version_bump_lib.sh.""" + +from __future__ import annotations + +import os +import subprocess +import textwrap +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parents[2] +_LIB_SH = _REPO_ROOT / "dev-tools" / "version_bump_lib.sh" + + +def _run_lib_snippet(snippet: str, *, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]: + script = textwrap.dedent( + f"""\ + set -euo pipefail + # shellcheck source=../version_bump_lib.sh + source "{_LIB_SH}" + {snippet} + """ + ) + run_env = os.environ.copy() + if env: + run_env.update(env) + return subprocess.run( + ["bash", "-c", script], + check=False, + capture_output=True, + text=True, + env=run_env, + ) + + +def test_snapshot_helpers_copies_files_and_is_independent_of_src(tmp_path: Path) -> None: + src = tmp_path / "src" + src.mkdir() + (src / "helper_a.sh").write_text("#!/bin/bash\necho a\n", encoding="utf-8") + (src / "helper_b.py").write_text("print('b')\n", encoding="utf-8") + + completed = _run_lib_snippet( + f""" + dest=$(version_bump_snapshot_helpers "{src}" helper_a.sh helper_b.py) + printf '%s\\n' "$dest" + test -f "$dest/helper_a.sh" + test -f "$dest/helper_b.py" + test -x "$dest/helper_a.sh" + # Mutating the source must not affect the snapshot. + echo mutated > "{src}/helper_a.sh" + grep -q mutated "$dest/helper_a.sh" && exit 2 || true + rm -rf "$dest" + """ + ) + assert completed.returncode == 0, completed.stderr + dest_line = completed.stdout.strip().splitlines()[-1] + assert dest_line + assert not Path(dest_line).exists() + + +def test_snapshot_helpers_fails_on_missing_file(tmp_path: Path) -> None: + src = tmp_path / "src" + src.mkdir() + (src / "present.sh").write_text("#!/bin/bash\n", encoding="utf-8") + + completed = _run_lib_snippet( + f'version_bump_snapshot_helpers "{src}" present.sh missing.sh' + ) + assert completed.returncode != 0 + assert "missing helper to snapshot" in completed.stderr + + +def test_create_github_pull_request_in_snapshot_finds_ensure_github_cli() -> None: + """Regression: after checkout, create_pr must resolve ensure_github_cli next to itself.""" + src = _REPO_ROOT / "dev-tools" + completed = _run_lib_snippet( + f""" + dest=$(version_bump_snapshot_helpers "{src}" \\ + create_github_pull_request.sh \\ + ensure_github_cli.sh) + bash -n "$dest/create_github_pull_request.sh" + # Snapshot must keep modern --label parsing (missing on older release branches). + grep -q -- '--label)' "$dest/create_github_pull_request.sh" + # ensure_github_cli must sit beside it for SCRIPT_DIR lookup. + test -f "$dest/ensure_github_cli.sh" + rm -rf "$dest" + """ + ) + assert completed.returncode == 0, completed.stderr diff --git a/dev-tools/version_bump_lib.sh b/dev-tools/version_bump_lib.sh index 32ec1c93b..7c31915fb 100755 --- a/dev-tools/version_bump_lib.sh +++ b/dev-tools/version_bump_lib.sh @@ -88,3 +88,31 @@ read_elasticsearch_version_from_ref() { local ref=$1 git show "${ref}:gradle.properties" | grep '^elasticsearchVersion=' | head -1 | cut -d= -f2 | tr -d '[:space:]' || true } + +# Copy named helper files from \p src_dir into a fresh temp directory and print +# that path. Patch/minor bump pipelines check out the *target* release branch +# tip after starting from main; without a snapshot, later subprocesses would +# execute older copies of these helpers from the release branch (e.g. a +# create_github_pull_request.sh that does not accept --label). +# +# Usage: dest=$(version_bump_snapshot_helpers "$SCRIPT_DIR" file1 file2 ...) +version_bump_snapshot_helpers() { + local src_dir="$1" + shift + if [[ $# -lt 1 ]]; then + echo "ERROR: version_bump_snapshot_helpers requires at least one file name" >&2 + return 1 + fi + local dest name + dest="$(mktemp -d "${TMPDIR:-/tmp}/ml-cpp-version-bump-helpers.XXXXXX")" + for name in "$@"; do + if [[ ! -f "${src_dir}/${name}" ]]; then + echo "ERROR: missing helper to snapshot: ${src_dir}/${name}" >&2 + rm -rf "${dest}" + return 1 + fi + cp "${src_dir}/${name}" "${dest}/${name}" + chmod +x "${dest}/${name}" + done + printf '%s' "$dest" +} diff --git a/docs/changelog/3082.yaml b/docs/changelog/3082.yaml new file mode 100644 index 000000000..c56ce1406 --- /dev/null +++ b/docs/changelog/3082.yaml @@ -0,0 +1,5 @@ +area: Build +issues: [] +pr: 0 +summary: Snapshot version-bump helpers before release-branch checkout +type: bug From 77266297b146855edda3899f36ef2e9faa5574f0 Mon Sep 17 00:00:00 2001 From: Ed Savage Date: Wed, 22 Jul 2026 10:12:08 +1200 Subject: [PATCH 2/3] Add changelog for #3083 Co-authored-by: Cursor --- docs/changelog/3082.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/3082.yaml b/docs/changelog/3082.yaml index c56ce1406..10d8aac07 100644 --- a/docs/changelog/3082.yaml +++ b/docs/changelog/3082.yaml @@ -1,5 +1,5 @@ area: Build issues: [] -pr: 0 +pr: 3083 summary: Snapshot version-bump helpers before release-branch checkout type: bug From 4a0aae2f1293a62b07632fc42e7071db052fb7c5 Mon Sep 17 00:00:00 2001 From: Ed Savage Date: Wed, 22 Jul 2026 11:42:49 +1200 Subject: [PATCH 3/3] [ML] Address review comments on version-bump helper snapshotting - Remove docs/changelog/3082.yaml: this is a CI/tooling change that does not need a changelog entry, and the filename/pr field mismatched anyway. - Harden version_bump_snapshot_helpers: handle cp/chmod failure explicitly so a copy error removes the temp dir and returns non-zero instead of aborting under set -e before the caller installs its cleanup trap. Co-authored-by: Cursor --- dev-tools/version_bump_lib.sh | 9 +++++++-- docs/changelog/3082.yaml | 5 ----- 2 files changed, 7 insertions(+), 7 deletions(-) delete mode 100644 docs/changelog/3082.yaml diff --git a/dev-tools/version_bump_lib.sh b/dev-tools/version_bump_lib.sh index 7c31915fb..4ad8dcfb5 100755 --- a/dev-tools/version_bump_lib.sh +++ b/dev-tools/version_bump_lib.sh @@ -111,8 +111,13 @@ version_bump_snapshot_helpers() { rm -rf "${dest}" return 1 fi - cp "${src_dir}/${name}" "${dest}/${name}" - chmod +x "${dest}/${name}" + # Handle cp/chmod failure explicitly: under `set -e` an aborted copy would + # skip the caller's cleanup trap (not yet installed) and leak ${dest}. + if ! cp "${src_dir}/${name}" "${dest}/${name}" || ! chmod +x "${dest}/${name}"; then + echo "ERROR: failed to stage helper ${name} into ${dest}" >&2 + rm -rf "${dest}" + return 1 + fi done printf '%s' "$dest" } diff --git a/docs/changelog/3082.yaml b/docs/changelog/3082.yaml deleted file mode 100644 index 10d8aac07..000000000 --- a/docs/changelog/3082.yaml +++ /dev/null @@ -1,5 +0,0 @@ -area: Build -issues: [] -pr: 3083 -summary: Snapshot version-bump helpers before release-branch checkout -type: bug