Skip to content

fix: fix bash syntax error when running run_macaron.sh on MacOS #528

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.

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

Merged
merged 3 commits into from
Oct 25, 2023

Conversation

nathanwn
Copy link
Member

@nathanwn nathanwn commented Oct 25, 2023

The bash pattern matching syntax @(...) requires the shopt option extglob to be set. Although this option is set by default on newer versions of bash, it is not always the case. One known case is on MacOS, where the default bash version is bash 3.2. (even on the latest version of MacOS).

Reproduction: Run the following bash script in the bash:3.2 docker container.

#!/usr/bin/env bash

if [[ "${DOCKER_PULL}" != @(always|missing|never) ]]; then
    echo "DOCKER_PULL must be one of: always, missing, never (default: always)"
    exit 1
fi

Signed-off-by: Nathan Nguyen <nathan.nguyen@oracle.com>
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 25, 2023
Copy link
Member

@behnazh-w behnazh-w left a comment

Choose a reason for hiding this comment

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

It's good to add shopt -s extglob for older versions of bash, but we should add a requirement for the bash version for running run_macaron.sh. In general running old bash versions is not a good idea for security reasons, etc. and it's better if people upgrade their bash version. Can you please add a check for the bash version and "recommend" to update it if it's below our supported version?

Signed-off-by: Nathan Nguyen <nathan.nguyen@oracle.com>
@nathanwn
Copy link
Member Author

It's good to add shopt -s extglob for older versions of bash, but we should add a requirement for the bash version for running run_macaron.sh. In general running old bash versions is not a good idea for security reasons, etc. and it's better if people upgrade their bash version. Can you please add a check for the bash version and "recommend" to update it if it's below our supported version?

I have added a warning in efccf1d.

@nathanwn nathanwn self-assigned this Oct 25, 2023
@nathanwn nathanwn marked this pull request as ready for review October 25, 2023 06:51
@nathanwn nathanwn requested a review from tromai as a code owner October 25, 2023 06:51
Signed-off-by: Nathan Nguyen <nathan.nguyen@oracle.com>
@nathanwn
Copy link
Member Author

A note about bash version has been added to our installation instructions in cd73e23.

@nathanwn nathanwn merged commit 2abd8b7 into staging Oct 25, 2023
@nathanwn nathanwn deleted the set-shopt-extglob-option branch October 25, 2023 23:47
art1f1c3R pushed a commit that referenced this pull request Nov 29, 2024
Signed-off-by: Nathan Nguyen <nathan.nguyen@oracle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants