Skip to content

Conversation

dszakallas
Copy link
Contributor

Update conditional checks in the build environment script to use double brackets.

Single brackets errors with

./tools/macos_buildenv.sh:32: = not found

I am using

bash --version
GNU bash, version 5.3.0(1)-release (aarch64-apple-darwin24.5.0)
Copyright (C) 2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

@Copilot Copilot AI review requested due to automatic review settings September 13, 2025 17:02
@github-actions github-actions bot added the developer experience Issues, bugs and PRs related to the development process, development environment & developer docs label Sep 13, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fix shell script compatibility issues by updating conditional syntax from single to double brackets to resolve bash parsing errors on macOS.

  • Replace single bracket [ with double bracket [[ in conditional statements
  • Ensure script compatibility with GNU bash version 5.3.0

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 33 to 34
if [ -n "${BUILDENV_ARM64_CROSS}" ]; then
if [ -n "${BUILDENV_RELEASE}" ]; then
Copy link
Preview

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

[nitpick] For consistency with the updated conditional checks, consider updating these remaining single bracket conditionals to use double brackets [[ ]] syntax as well.

Copilot uses AI. Check for mistakes.

fi
elif [ "$HOST_ARCH" == "arm64" ]; then
elif [[ "$HOST_ARCH" == "arm64" ]]; then
if [ -n "${BUILDENV_RELEASE}" ]; then
Copy link
Preview

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

[nitpick] For consistency with the updated conditional checks, consider updating this remaining single bracket conditional to use double brackets [[ ]] syntax as well.

Suggested change
if [ -n "${BUILDENV_RELEASE}" ]; then
if [[ -n "${BUILDENV_RELEASE}" ]]; then

Copilot uses AI. Check for mistakes.

@daschuer
Copy link
Member

Thank you. The fix looks good. Can you rebase it to the 2.5 branch and change the merge target to 2.5 via the edit button above?

@dszakallas
Copy link
Contributor Author

Rebasing results in conflicts. Do we need a separate PR in this case?

@JoergAtGithub JoergAtGithub changed the base branch from main to 2.6 September 20, 2025 07:53
@JoergAtGithub JoergAtGithub changed the base branch from 2.6 to main September 20, 2025 07:54
@JoergAtGithub
Copy link
Member

JoergAtGithub commented Sep 20, 2025

The code differs between 2.5 and 2.6. I recommend to rebase this to 2.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer experience Issues, bugs and PRs related to the development process, development environment & developer docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants