diff --git a/get-build-number/get_build_number.sh b/get-build-number/get_build_number.sh index 8a534bf6..082259ed 100755 --- a/get-build-number/get_build_number.sh +++ b/get-build-number/get_build_number.sh @@ -6,19 +6,46 @@ set -euo pipefail : "${GITHUB_REPOSITORY:?}" GH_API_VERSION_HEADER="X-GitHub-Api-Version: 2022-11-28" BUILD_NUMBER_FILE="${BUILD_NUMBER_FILE:-.build_number.txt}" +PROPERTIES_API_URL="repos/${GITHUB_REPOSITORY}/properties/values" +# The custom-properties API has no conditional/atomic update (no If-Match support), so +# concurrent runs (e.g. GitHub Stacked PRs) can race on the read-increment-write cycle and +# claim the same number. Verify after writing and retry on a detected collision to close +# most of that window; see PREQ-7781. +MAX_ATTEMPTS="${MAX_ATTEMPTS:-10}" + +get_property_value() { + gh api -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" --jq '.[] | select(.property_name == "build_number") | .value' +} echo "Fetching build number from repository properties..." -PROPERTIES_API_URL="repos/${GITHUB_REPOSITORY}/properties/values" -BUILD_NUMBER=$(gh api -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" --jq '.[] | select(.property_name == "build_number") | .value') -echo "Current build number from repo: ${BUILD_NUMBER:=0}" -if ! [[ "$BUILD_NUMBER" =~ ^[0-9]+$ ]]; then - echo "::error title=Invalid build number::Build number '${BUILD_NUMBER}' is not a valid positive integer." >&2 - exit 1 -fi - -BUILD_NUMBER=$((BUILD_NUMBER + 1)) -gh api --method PATCH -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" \ - -f "properties[][property_name]=build_number" \ - -f "properties[][value]=${BUILD_NUMBER}" -echo "Incremented 'build_number' repository property to ${BUILD_NUMBER}" -echo "${BUILD_NUMBER}" > "$BUILD_NUMBER_FILE" + +attempt=1 +while true; do + BUILD_NUMBER=$(get_property_value) + echo "Current build number from repo: ${BUILD_NUMBER:=0}" + if ! [[ "$BUILD_NUMBER" =~ ^[0-9]+$ ]]; then + echo "::error title=Invalid build number::Build number '${BUILD_NUMBER}' is not a valid positive integer." >&2 + exit 1 + fi + + NEXT_BUILD_NUMBER=$((BUILD_NUMBER + 1)) + gh api --method PATCH -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" \ + -f "properties[][property_name]=build_number" \ + -f "properties[][value]=${NEXT_BUILD_NUMBER}" + + CONFIRMED_BUILD_NUMBER=$(get_property_value) + if [[ "$CONFIRMED_BUILD_NUMBER" == "$NEXT_BUILD_NUMBER" ]]; then + echo "Incremented 'build_number' repository property to ${NEXT_BUILD_NUMBER}" + echo "${NEXT_BUILD_NUMBER}" > "$BUILD_NUMBER_FILE" + exit 0 + fi + + if (( attempt >= MAX_ATTEMPTS )); then + echo "::error title=Build number race::Could not obtain a unique build number after ${MAX_ATTEMPTS} attempts; concurrent runs kept overwriting each other." >&2 + exit 1 + fi + + echo "Concurrent update detected (expected ${NEXT_BUILD_NUMBER}, found ${CONFIRMED_BUILD_NUMBER}); retrying (attempt $((attempt + 1))/${MAX_ATTEMPTS})..." + sleep "0.$((RANDOM % 900 + 100))" + attempt=$((attempt + 1)) +done diff --git a/spec/get_build_number_spec.sh b/spec/get_build_number_spec.sh index 54222de8..3ed37d1b 100755 --- a/spec/get_build_number_spec.sh +++ b/spec/get_build_number_spec.sh @@ -11,25 +11,29 @@ End Describe 'get_build_number.sh' It 'should increment and return the build number' + export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_1.txt" + rm -f "$GH_GET_CALLS_FILE" Mock gh if [[ "$*" =~ "api --method PATCH" ]]; then echo "gh $*" elif [[ "$*" =~ "properties/values" ]]; then - echo '42' + count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" > "$GH_GET_CALLS_FILE" + if [[ "$count" -eq 1 ]]; then + echo '42' + else + echo '43' + fi else echo "gh $*" fi End - # shellcheck disable=SC2317 -# preserve() { %preserve BUILD_NUMBER; } -# AfterRun preserve When run script get-build-number/get_build_number.sh The line 1 should include "Fetching build number" The line 2 should equal "Current build number from repo: 42" The line 3 should include "43" The path "$BUILD_NUMBER_FILE" should be file The contents of file "$BUILD_NUMBER_FILE" should equal "43" -# The variable BUILD_NUMBER should equal "43" End It 'should return an error if BUILD_NUMBER is invalid' @@ -43,8 +47,22 @@ Describe 'get_build_number.sh' End It 'should handle empty build number' + export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_3.txt" + rm -f "$GH_GET_CALLS_FILE" Mock gh - echo '' + if [[ "$*" =~ "api --method PATCH" ]]; then + echo "gh $*" + elif [[ "$*" =~ "properties/values" ]]; then + count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" > "$GH_GET_CALLS_FILE" + if [[ "$count" -eq 1 ]]; then + echo '' + else + echo '1' + fi + else + echo "gh $*" + fi End When run script get-build-number/get_build_number.sh The status should be success @@ -52,4 +70,53 @@ Describe 'get_build_number.sh' # Ignore empty line from second call to gh The line 4 should include "1" End + + It 'should retry when a concurrent run wins the race, then succeed' + export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_4.txt" + rm -f "$GH_GET_CALLS_FILE" + Mock gh + if [[ "$*" =~ "api --method PATCH" ]]; then + echo "gh $*" + elif [[ "$*" =~ "properties/values" ]]; then + count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" > "$GH_GET_CALLS_FILE" + case "$count" in + 1) echo '42' ;; # our initial read + 2) echo '99' ;; # a concurrent run raced ahead of our write + 3) echo '99' ;; # our retry read picks up their value + *) echo '100' ;; # our second write is now confirmed + esac + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Concurrent update detected (expected 43, found 99)" + The output should include "Incremented 'build_number' repository property to 100" + The path "$BUILD_NUMBER_FILE" should be file + The contents of file "$BUILD_NUMBER_FILE" should equal "100" + End + + It 'should fail after exhausting retries if the race never resolves' + export MAX_ATTEMPTS=3 + export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_5.txt" + rm -f "$GH_GET_CALLS_FILE" + Mock gh + if [[ "$*" =~ "api --method PATCH" ]]; then + echo "gh $*" + elif [[ "$*" =~ "properties/values" ]]; then + # Every read disagrees with what we just wrote, simulating permanent contention. + count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1)) + echo "$count" > "$GH_GET_CALLS_FILE" + echo "$((count * 100))" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "Concurrent update detected" + The stderr should include "::error title=Build number race::Could not obtain a unique build number after 3 attempts" + End End