Skip to content

fix(release): use find instead of bash-4 globstar in artifact collection#8

Closed
gocanto wants to merge 1 commit into
mainfrom
fix/release-globstar
Closed

fix(release): use find instead of bash-4 globstar in artifact collection#8
gocanto wants to merge 1 commit into
mainfrom
fix/release-globstar

Conversation

@gocanto

@gocanto gocanto commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Problem

The Release workflow's collect-release-artifacts.sh used shopt -s globstar + ** recursive globs. macOS GitHub runners (macos-14) ship bash 3.2, where globstar doesn't exist, so the step failed with shopt: globstar: invalid shell option name and the release aborted after building the DMG but before publishing.

Observed in the v0.1.1-rc1 run (Release workflow's first-ever execution).

Fix

Replace the recursive globs with find, which works on bash 3.2+. Verified locally on bash 3.2.57 (same as the runner): finds artifacts, canonicalizes names, writes SHASUMS256.txt, exits 0.

Test plan

  • bash -n syntax check on bash 3.2.57
  • Functional run against fake out/make artifacts
  • Cut a v0.1.1-rcN prerelease after merge to validate end-to-end

macOS GitHub runners ship bash 3.2, which lacks `globstar`, so the
`shopt -s globstar` + `**` recursive globs in collect-release-artifacts.sh
errored ('shopt: globstar: invalid shell option name') and failed the
release before publishing. Replace the recursive globs with `find`, which
works on bash 3.2.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the use of bash globstar (**) with find in collect-release-artifacts.sh to ensure compatibility with macOS GitHub runners running bash 3.2. Feedback was provided regarding the use of find ... | head -n1 in combination with set -euo pipefail, which can cause the script to crash unexpectedly due to SIGPIPE or if the target directory is missing. It is recommended to append || true inside the command substitutions to handle these cases gracefully.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +14 to +15
src_dmg="$(find "$out" -type f -name '*.dmg' | head -n1)"
src_zip="$(find "$out" -type f -name '*.zip' | head -n1)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using find ... | head -n1 in combination with set -euo pipefail can lead to flaky failures or unexpected early exits:

  1. SIGPIPE / pipefail issue: If find produces multiple lines of output, head -n1 will exit early after reading the first line. This closes the pipe, causing find to receive a SIGPIPE signal and exit with status 141. Under pipefail, this non-zero exit status propagates and triggers set -e, crashing the script.
  2. Bypassing error handling: If the $out directory does not exist, find will fail. Instead of reaching the friendly error message and exit 1 on lines 16-20, the script will crash immediately on line 14 or 15.

Appending || true to the command inside the command substitution ensures the assignment always succeeds, allowing the script to handle missing files or directories gracefully in the subsequent if block.

Suggested change
src_dmg="$(find "$out" -type f -name '*.dmg' | head -n1)"
src_zip="$(find "$out" -type f -name '*.zip' | head -n1)"
src_dmg="$(find "$out" -type f -name '*.dmg' | head -n1 || true)"
src_zip="$(find "$out" -type f -name '*.zip' | head -n1 || true)"

@gocanto

gocanto commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #10, which combines this with #9 on a fresh branch off main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant