Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure Dalmatian’s Bash scripts run under a “modern” Bash (4.0+) by switching shebangs to use the user’s bash from PATH, adding a runtime Bash-version guard, and ensuring Homebrew installs Bash on macOS.
Changes:
- Updated many scripts’ shebangs from
#!/bin/bashto#!/usr/bin/env bash. - Added a sourced guard (
lib/bash-functions/check_bash_version.sh) that exits if Bash major version is < 4. - Added
bashto theBrewfileto make a modern Bash available viabrew bundle install.
Reviewed changes
Copilot reviewed 21 out of 139 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test.sh | Switch shebang to env bash for consistent execution environment. |
| support/zsh-completion.sh | Switch shebang to env bash. |
| support/bash-completion.sh | Switch shebang to env bash. |
| lib/php-sql-munge.sh | Switch shebang to env bash. |
| lib/bash-functions/yes_no.sh | Switch shebang to env bash. |
| lib/bash-functions/warning.sh | Switch shebang to env bash. |
| lib/bash-functions/resource_prefix_hash.sh | Switch shebang to env bash. |
| lib/bash-functions/resolve_aws_profile.sh | Switch shebang to env bash. |
| lib/bash-functions/read_prompt_with_setup_default.sh | Switch shebang to env bash. |
| lib/bash-functions/pick_ecs_instance.sh | Switch shebang to env bash. |
| lib/bash-functions/log_msg.sh | Switch shebang to env bash. |
| lib/bash-functions/log_info.sh | Switch shebang to env bash. |
| lib/bash-functions/is_installed.sh | Switch shebang to env bash. |
| lib/bash-functions/install_session_manager.sh | Switch shebang to env bash. |
| lib/bash-functions/err.sh | Switch shebang to env bash. |
| lib/bash-functions/check_bash_version.sh | Add Bash >= 4 runtime guard (new file). |
| lib/bash-functions/aws_epoch.sh | Switch shebang to env bash. |
| lib/bash-functions/append_sso_config_file.sh | Switch shebang to env bash. |
| lib/bash-functions/append_sso_config_file_assume_role.sh | Switch shebang to env bash. |
| lib/bash-functions/append_import_block.sh | Switch shebang to env bash. |
| Brewfile | Add bash formula to ensure modern Bash is installed via Brew. |
| bin/waf/v1/set-ip-rule | Switch shebang to env bash. |
| bin/waf/v1/list-blocked-requests | Switch shebang to env bash. |
| bin/waf/v1/delete-ip-rule | Switch shebang to env bash. |
| bin/waf/v1/cf-ip-block | Switch shebang to env bash. |
| bin/utilities/v2/run-command | Switch shebang to env bash. |
| bin/util/v1/list-security-group-rules | Switch shebang to env bash. |
| bin/util/v1/ip-port-exposed | Switch shebang to env bash. |
| bin/util/v1/generate-four-words | Switch shebang to env bash. |
| bin/util/v1/exec | Switch shebang to env bash. |
| bin/util/v1/env | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/view-tfvars | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/set-tfvars | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/set-global-tfvars | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/run-terraform-command | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/link-import-file | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/initialise | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/get-tfvars | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/create-import-file | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/clone | Switch shebang to env bash. |
| bin/terraform-dependencies/v2/clean-tfvars-cache | Switch shebang to env bash. |
| bin/service/v2/set-environment-variables | Switch shebang to env bash. |
| bin/service/v2/list-services | Switch shebang to env bash. |
| bin/service/v2/get-environment-variables | Switch shebang to env bash. |
| bin/service/v2/deploy | Switch shebang to env bash. |
| bin/service/v2/container-access | Switch shebang to env bash. |
| bin/service/v1/show-deployment-status | Switch shebang to env bash. |
| bin/service/v1/set-environment-variable | Switch shebang to env bash. |
| bin/service/v1/run-container-command | Switch shebang to env bash. |
| bin/service/v1/restart-containers | Switch shebang to env bash. |
| bin/service/v1/rename-environment-variable | Switch shebang to env bash. |
| bin/service/v1/pull-image | Switch shebang to env bash. |
| bin/service/v1/list-pipelines | Switch shebang to env bash. |
| bin/service/v1/list-environment-variables | Switch shebang to env bash. |
| bin/service/v1/list-domains | Switch shebang to env bash. |
| bin/service/v1/list-container-placement | Switch shebang to env bash. |
| bin/service/v1/get-environment-variable | Switch shebang to env bash. |
| bin/service/v1/force-deployment | Switch shebang to env bash. |
| bin/service/v1/ecr-vulnerabilities | Switch shebang to env bash. |
| bin/service/v1/deploy-status | Switch shebang to env bash. |
| bin/service/v1/deploy-build-logs | Switch shebang to env bash. |
| bin/service/v1/deploy | Switch shebang to env bash. |
| bin/service/v1/delete-environment-variable | Switch shebang to env bash. |
| bin/service/v1/container-access | Switch shebang to env bash. |
| bin/s3/v2/list-bucket-properties | Switch shebang to env bash. |
| bin/s3/v2/empty-and-delete-bucket | Switch shebang to env bash. |
| bin/rds/v2/shell | Switch shebang to env bash. |
| bin/rds/v2/run-command | Switch shebang to env bash. |
| bin/rds/v1/start-sql-backup-to-s3 | Switch shebang to env bash. |
| bin/rds/v1/shell | Switch shebang to env bash. |
| bin/rds/v1/set-root-password | Switch shebang to env bash. |
| bin/rds/v1/list-instances | Switch shebang to env bash. |
| bin/rds/v1/list-databases | Switch shebang to env bash. |
| bin/rds/v1/import-dump | Switch shebang to env bash. |
| bin/rds/v1/get-root-password | Switch shebang to env bash. |
| bin/rds/v1/export-dump | Switch shebang to env bash. |
| bin/rds/v1/download-sql-backup | Switch shebang to env bash. |
| bin/rds/v1/create-database | Switch shebang to env bash. |
| bin/rds/v1/count-sql-backups | Switch shebang to env bash. |
| bin/elasticache/v1/reboot | Switch shebang to env bash. |
| bin/elasticache/v1/list-clusters | Switch shebang to env bash. |
| bin/ecs/v2/ec2-access | Switch shebang to env bash. |
| bin/ecs/v1/upload-to-transfer-bucket | Switch shebang to env bash. |
| bin/ecs/v1/remove-from-transfer-bucket | Switch shebang to env bash. |
| bin/ecs/v1/instance-refresh | Switch shebang to env bash. |
| bin/ecs/v1/file-upload | Switch shebang to env bash. |
| bin/ecs/v1/file-download | Switch shebang to env bash. |
| bin/ecs/v1/efs-restore | Switch shebang to env bash. |
| bin/ecs/v1/ec2-access | Switch shebang to env bash. |
| bin/ec2/v2/port-forward | Switch shebang to env bash. |
| bin/deploy/v2/list-infrastructures | Switch shebang to env bash. |
| bin/deploy/v2/list-accounts | Switch shebang to env bash. |
| bin/deploy/v2/infrastructure | Switch shebang to env bash. |
| bin/deploy/v2/delete-default-resources | Switch shebang to env bash. |
| bin/deploy/v2/account-bootstrap | Switch shebang to env bash. |
| bin/dalmatian-refresh-config | Switch shebang to env bash. |
| bin/dalmatian | Switch shebang to env bash. |
| bin/configure-commands/v2/version | Switch shebang to env bash. |
| bin/configure-commands/v2/update | Switch shebang to env bash. |
| bin/configure-commands/v2/setup | Switch shebang to env bash. |
| bin/configure-commands/v1/login | Switch shebang to env bash. |
| bin/config/v1/services-to-tsv | Switch shebang to env bash. |
| bin/config/v1/list-services-by-buildspec | Switch shebang to env bash. |
| bin/config/v1/list-services | Switch shebang to env bash. |
| bin/config/v1/list-infrastructures | Switch shebang to env bash. |
| bin/config/v1/list-environments | Switch shebang to env bash. |
| bin/cloudtrail/v2/query | Switch shebang to env bash. |
| bin/cloudfront/v2/logs | Switch shebang to env bash. |
| bin/cloudfront/v1/logs | Switch shebang to env bash. |
| bin/cloudfront/v1/generate-basic-auth-password-hash | Switch shebang to env bash. |
| bin/cloudfront/v1/clear-cache | Switch shebang to env bash. |
| bin/ci/v1/deploy-status | Switch shebang to env bash. |
| bin/ci/v1/deploy-build-logs | Switch shebang to env bash. |
| bin/certificate/v1/list | Switch shebang to env bash. |
| bin/certificate/v1/delete | Switch shebang to env bash. |
| bin/certificate/v1/create | Switch shebang to env bash. |
| bin/aws/v2/run-command | Switch shebang to env bash. |
| bin/aws/v2/login | Switch shebang to env bash. |
| bin/aws/v2/list-profiles | Switch shebang to env bash. |
| bin/aws/v2/generate-config | Switch shebang to env bash. |
| bin/aws/v2/exec | Switch shebang to env bash. |
| bin/aws/v2/account-init | Switch shebang to env bash. |
| bin/aws/v1/mfa | Switch shebang to env bash. |
| bin/aws/v1/key-age | Switch shebang to env bash. |
| bin/aws/v1/instance-shell | Switch shebang to env bash. |
| bin/aws/v1/exec | Switch shebang to env bash. |
| bin/aws/v1/awscli-version | Switch shebang to env bash. |
| bin/aws/v1/assume-infrastructure-role | Switch shebang to env bash. |
| bin/aurora/v1/start-sql-backup-to-s3 | Switch shebang to env bash. |
| bin/aurora/v1/shell | Switch shebang to env bash. |
| bin/aurora/v1/set-root-password | Switch shebang to env bash. |
| bin/aurora/v1/list-instances | Switch shebang to env bash. |
| bin/aurora/v1/list-databases | Switch shebang to env bash. |
| bin/aurora/v1/import-dump | Switch shebang to env bash. |
| bin/aurora/v1/get-root-password | Switch shebang to env bash. |
| bin/aurora/v1/export-dump | Switch shebang to env bash. |
| bin/aurora/v1/download-sql-backup | Switch shebang to env bash. |
| bin/aurora/v1/create-database | Switch shebang to env bash. |
| bin/aurora/v1/count-sql-backups | Switch shebang to env bash. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+9
| # This script is sourced by dalmatian and subcommands. | ||
| # It ensures that a modern version of Bash is used. | ||
|
|
||
| if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then | ||
| echo "Error: Bash 4.0 or newer is required." >&2 |
There was a problem hiding this comment.
The PR description mentions that a test was added to ensure Bash >= 4.0, but the repo doesn’t appear to include any new test/CI assertion for this (CI is still shellcheck-only). Consider either adding a CI check (e.g., run a small script that asserts ${BASH_VERSINFO[0]} >= 4) or updating the PR description to reflect that this is enforced at runtime via this sourced guard.
Comment on lines
+1
to
+8
| #!/usr/bin/env bash | ||
| set -e | ||
| set -o pipefail | ||
|
|
||
| # This script is sourced by dalmatian and subcommands. | ||
| # It ensures that a modern version of Bash is used. | ||
|
|
||
| if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then |
There was a problem hiding this comment.
This file lives under lib/bash-functions, but it doesn’t define any exported functions and instead runs logic at source-time. Since bin/dalmatian sources every file in this directory as part of its function-loading loop, this introduces a side effect in what is otherwise a “function definitions only” directory. Consider making this a check_bash_version function that bin/dalmatian calls explicitly near startup, or moving the guard to a dedicated preflight script/directory.
MacOS ships with an older version of bash (3.2) which lacks many features and has some quirks compared to modern versions of bash (4.0+). This commit updates the shebangs in our bash scripts to use `#!/usr/bin/env bash` which will use the version of bash found in the user's environment, allowing users to use a modern version of bash if they have it installed. A test was also added to check that the version of bash being used is at least 4.0, and the Brewfile was updated to include bash as a dependency, so that users on MacOS will have a modern version of bash available when they run `brew bundle install`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MacOS ships with an older version of bash (3.2) which lacks many
features and has some quirks compared to modern versions of bash (4.0+).
This commit updates the shebangs in our bash scripts to use
#!/usr/bin/env bashwhich will use the version of bash found in theuser's environment, allowing users to use a modern version of bash if
they have it installed.
A test was also added to check that the version of bash being used is at
least 4.0, and the Brewfile was updated to include bash as a dependency,
so that users on MacOS will have a modern version of bash available when
they run
brew bundle install.