Skip to content

etc: Use _execute for bison download to get log only on failure as for others. #10777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
titan73 wants to merge 2 commits into
base: master
Choose a base branch
from
+23 −19
Open

etc: Use _execute for bison download to get log only on failure as for others. #10777

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f72d3d
etc: Use _execute for bison download to get log only on failure as fo…
titan73 Jun 29, 2026
5fa351d
etc: Turn (error ...) into a warn with returning an error status.
titan73 Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 22 additions & 19 deletions etc/DependencyInstaller.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,26 +338,29 @@ _install_bison() {
if [[ "${bison_installed_version}" != "${BISON_VERSION}" ]]; then
(
cd "${BASE_DIR}"
local mirrors=(
"https://ftp.gnu.org/gnu/bison"
"https://ftpmirror.gnu.org/bison"
"https://mirrors.kernel.org/gnu/bison"
"https://mirrors.dotsrc.org/gnu/bison"
)
local success=0
for mirror in "${mirrors[@]}"; do
local url="${mirror}/bison-${BISON_VERSION}.tar.gz"
log "Trying to download bison from: $url"
if wget $OPT_NOCERT "$url"; then
success=1
break
else
warn "Failed to download from $mirror"
_download_bison() {
local mirrors=(
"https://ftp.gnu.org/gnu/bison"
"https://ftpmirror.gnu.org/bison"
"https://mirrors.kernel.org/gnu/bison"
"https://mirrors.dotsrc.org/gnu/bison"
)
local success=0
for mirror in "${mirrors[@]}"; do
local url="${mirror}/bison-${BISON_VERSION}.tar.gz"
log "Trying to download bison from: $url"
if wget $OPT_NOCERT "$url"; then
success=1
break
else
warn "Failed to download from $mirror"
fi
done
if [[ ${success} -ne 1 ]]; then
(error "Could not download bison-${BISON_VERSION}.tar.gz from any mirror.")
fi
Comment on lines +359 to 362

@gemini-code-assist gemini-code-assist Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using a subshell wrapper (error ...) to prevent the script from exiting immediately is an anti-pattern and can be confusing. Since _download_bison is executed via _execute (which handles logging and final termination on failure), the function should simply print the error message to stderr and return a non-zero exit status (e.g., return 1) to signal failure to the caller.

Suggested change
if [[ ${success} -ne 1 ]]; then
(error "Could not download bison-${BISON_VERSION}.tar.gz from any mirror.")
fi
if [[ ${success} -ne 1 ]]; then
echo "Could not download bison-${BISON_VERSION}.tar.gz from any mirror." >&2
return 1
fi

@titan73 titan73 Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. I used "warn ..." instead of an echo though.

done
if [[ ${success} -ne 1 ]]; then
error "Could not download bison-${BISON_VERSION}.tar.gz from any mirror."
fi
}
_execute "Downloading Bison" _download_bison
_verify_checksum "${BISON_CHECKSUM}" "bison-${BISON_VERSION}.tar.gz" || error "Bison checksum failed."
_execute "Extracting Bison..." tar xf "bison-${BISON_VERSION}.tar.gz"
cd "bison-${BISON_VERSION}"
Expand Down
Loading