fix: use universal shebangs for better portability #2127
Conversation
Replace hardcoded #\!/bin/bash shebangs with #\!/usr/bin/env bash across all shell scripts to improve cross-platform compatibility. This allows the scripts to work on systems where bash may be installed in different locations.
rawkode
requested review from
a team,
JivusAyrus,
Noroth,
StarpTech,
devsergiy,
jensneuse and
wilsonrivera
as code owners
WalkthroughUpdated shebangs in many shell scripts from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
scripts/configurations/kubernetes.sh (1)
4-7: Avoid committing secrets; require env at runtimeCOSMO_API_KEY appears hardcoded. Even if this is a dev value, don’t commit secrets. Source from environment and fail if missing.
- export COSMO_API_KEY=cosmo_669b576aaadc10ee1ae81d9193425705 + # Require COSMO_API_KEY to be set in the environment before sourcing this file + : "${COSMO_API_KEY:?Set COSMO_API_KEY in your environment before sourcing scripts/configurations/kubernetes.sh}" + export COSMO_API_KEYOptionally move all values into a non-committed .env file and source it locally.
🧹 Nitpick comments (23)
router/__schemas/plugin.sh (1)
1-3: Add strict mode for safer executionEnable early failure and better error surfacing in CI.
#!/usr/bin/env bash +set -Eeuo pipefailscripts/update-demo.sh (1)
1-3: Harden script optionsConsider stricter flags to catch unset vars and pipeline errors.
- set -e + set -Eeuo pipefailexamples/full-cosmo-helm/create_demo.sh (1)
1-1: Avoid global npm install for wgc (optional)Global installs often fail in CI or restricted environments. Prefer on-demand execution or a presence check.
Example (outside this hunk):
# Prefer npx (no global install) or fallback if missing if ! command -v wgc >/dev/null 2>&1; then npx -y wgc@latest --version >/dev/null || npm install -g wgc@latest fiscripts/demo-router.sh (1)
1-7: Improve robustness and signal handlingAdd strict mode and use exec so the script forwards signals to the Go process cleanly.
#!/usr/bin/env bash +set -Eeuo pipefail cd "../router" -go run main.go -override-env=.env.demo +exec go run main.go -override-env=.env.demostudio/entrypoint.sh (1)
1-1: Portable shebang — approved; note one portability caveatShebang change is correct. Minor portability note: elsewhere in this script,
sed -i(BSD vs GNU) differs; on macOS/BSD it typically needs an argument (e.g.,-i ''), while GNU sed accepts bare-i. Consider using-i.bakand removing backups afterward to keep this script fully cross-platform.examples/full-cosmo-docker/destroy.sh (1)
4-4: Make path change robust to invocation directory (optional)Current relative cd can fail if the script is run from another CWD. Consider basing it on the script’s location.
Apply:
-cd ../.. && make full-demo-down +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +cd "${SCRIPT_DIR}/../.." && make full-demo-downscripts/delete-kubernetes-demo.sh (1)
6-6: Optional: source via script directory for robustnessSourcing with a relative CWD can fail if invoked from another directory. Consider resolving relative to the script’s location.
-. ./scripts/configurations/kubernetes.sh +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)" +. "${SCRIPT_DIR}/configurations/kubernetes.sh".github/scripts/setup-keycloak.sh (1)
1-5: Optional: harden the script (strict mode + curl failure on HTTP errors)Add strict mode and make curl fail on non-2xx:
#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t' export KC_VERSION=25.0.1 -curl -LO https://github.com/keycloak/keycloak/releases/download/"${KC_VERSION}"/keycloak-"${KC_VERSION}".zip +curl -fsSLO "https://github.com/keycloak/keycloak/releases/download/${KC_VERSION}/keycloak-${KC_VERSION}.zip"demo/update_token.sh (1)
2-2: Optional: add pipefailYou already use -eu; pipefail helps catch failures in pipelines.
-set -eu +set -euo pipefailscripts/bench-delete.sh (1)
2-2: Optional: strict mode consistencyConsider enabling -u and pipefail to fail fast in CI.
-set -e +set -euo pipefaildemo/bump-deps.sh (1)
1-5: Optional: add strict modeThis script manipulates git and go modules; better to fail early.
#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t' GIT_REV=$(git show-ref main --heads -s)router-tests/bump-deps.sh (1)
2-10: Consider failing fast on errorsAdd strict mode so failures in git/go commands stop the script early.
#!/usr/bin/env bash + +set -euo pipefail + GIT_REV=$(git rev-parse --short HEAD) echo "Updating dependencies to $GIT_REV"demo/compose.sh (1)
3-7: Harden script execution
- Add strict mode.
- Ensure cd failure exits (matches pattern used elsewhere in repo).
#!/usr/bin/env bash -cd "../cli" +set -euo pipefail + +cd "../cli" || exit pnpm wgc router compose -i ../demo/graph.yaml -o ../demo/config.jsonscripts/bench-update.sh (1)
2-12: Use pipefail and nounset as wellYou already use set -e; add -u and pipefail for robustness when sourcing config and running pnpm.
- set -e +set -euo pipefailrouter-tests/update-config-no-edg.sh (1)
7-26: Add strict mode to fail fastEnsures pnpm/jq errors stop the script instead of continuing.
#!/usr/bin/env bash + +set -euo pipefailexamples/router-simple/start.sh (1)
2-2: Optional: harden failure modesConsider enabling stricter bash flags for more predictable failures in pipelines and unset vars.
-set -e +set -Eeuo pipefailrouter/__schemas/compose.sh (1)
1-2: Shebang portability — LGTM; add strict mode (optional)The env-based shebang is a portability win. Consider adding strict mode early in the script:
#!/usr/bin/env bash + +set -Eeuo pipefailrouter-tests/update-config.sh (1)
1-2: Portable shebang — LGTM; ensure robust error handling (optional)Add strict mode so failures in compose/format steps abort the script:
#!/usr/bin/env bash + +set -Eeuo pipefailscripts/create-cloud-demo.sh (1)
1-2: Shebang portability — LGTM; tighten strict flags (optional)Upgrade to stricter flags for safer execution:
#!/usr/bin/env bash -set -e +set -Eeuo pipefailscripts/delete-local-demo.sh (2)
1-2: Portable shebang — LGTM; strengthen error handling (optional)Consider stricter flags:
#!/usr/bin/env bash -set -e +set -Eeuo pipefail
6-7: Optional: source configuration robustly regardless of current working directorySourcing via a relative path can fail when invoked from another directory. Resolve via the script’s own directory:
-. ./scripts/configurations/local.sh +script_dir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)" +. "$script_dir/configurations/local.sh"docker/clickhouse/init-db.sh (1)
1-1: Portability shebang looks good – consider stricter shell options.
#!/usr/bin/env bashsolves the portability problem.
While touching the header, you might also addset -uo pipefail(alongside-e) to fail fast on unset vars and piped errors.scripts/setup-fulldemo.sh (1)
25-29:wgcavailability block works but duplicates logic – extract to a helper.You now have (almost) identical “ensure-wgc-in-PATH” code here and in
create-cli-demo.sh. Duplicating install logic across scripts risks drift.Consider moving this snippet into a dedicated helper script or Make target and source/ invoke it from both places.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
.github/scripts/setup-keycloak.sh(1 hunks)data_migrations/controlplane/1747776337_rbac_groups/migrate.sh(1 hunks)demo/bump-deps.sh(1 hunks)demo/compose.sh(1 hunks)demo/deploy.sh(1 hunks)demo/update_token.sh(1 hunks)docker/clickhouse/init-db.sh(1 hunks)docker/nats/build-push.sh(1 hunks)examples/full-cosmo-docker/destroy.sh(1 hunks)examples/full-cosmo-docker/start.sh(1 hunks)examples/full-cosmo-helm/create_demo.sh(1 hunks)examples/full-cosmo-helm/delete_demo.sh(1 hunks)examples/router-simple/start.sh(1 hunks)helm/cosmo/README.md(1 hunks)helm/cosmo/values.yaml(1 hunks)keycloak/theme/build.sh(1 hunks)router-tests/bump-deps.sh(1 hunks)router-tests/testdata/tls/makecert.sh(1 hunks)router-tests/update-config-no-edg.sh(1 hunks)router-tests/update-config.sh(1 hunks)router/__schemas/compose.sh(1 hunks)router/__schemas/full-plugin-project.sh(1 hunks)router/__schemas/plugin.sh(1 hunks)scripts/bench-create.sh(1 hunks)scripts/bench-delete.sh(1 hunks)scripts/bench-router.sh(1 hunks)scripts/bench-update.sh(1 hunks)scripts/configurations/kubernetes.sh(1 hunks)scripts/configurations/local.sh(1 hunks)scripts/create-cli-demo.sh(1 hunks)scripts/create-cloud-demo.sh(1 hunks)scripts/create-docker-demo.sh(1 hunks)scripts/create-full-local-demo.sh(1 hunks)scripts/create-kubernetes-demo.sh(1 hunks)scripts/create-local-demo.sh(1 hunks)scripts/delete-docker-demo.sh(1 hunks)scripts/delete-full-local-demo.sh(1 hunks)scripts/delete-kubernetes-demo.sh(1 hunks)scripts/delete-local-demo.sh(1 hunks)scripts/demo-router.sh(1 hunks)scripts/setup-fulldemo.sh(3 hunks)scripts/update-demo.sh(1 hunks)studio/entrypoint.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build_image_fork (nonroot)
- GitHub Check: build_test_fork
- GitHub Check: build_test
- GitHub Check: build_image_fork
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
🔇 Additional comments (35)
scripts/create-full-local-demo.sh (2)
1-1: Shebang portability change approvedSwitching to
#!/usr/bin/env bashimproves portability across macOS/BSD/Nix/Guix and similar environments.
1-3: Repo-wide script verification complete: no issues detected
- All scripts use
#!/usr/bin/env bash(no hardcoded/bin/bash)- All
.shfiles are executable- No CRLF line endings in shell scripts (the only CRLF result was in a Windows batch file:
client-tests/apollo-kotlin/gradlew.bat, which is expected)router/__schemas/plugin.sh (1)
1-1: Shebang portability change approvedStandardizing on
#!/usr/bin/env bashis the right call here.scripts/update-demo.sh (1)
1-1: Shebang portability change approvedGood move to prefer
env-based resolution.examples/full-cosmo-helm/create_demo.sh (1)
1-1: Shebang portability change approvedConsistent with the PR goal; no behavior change.
scripts/demo-router.sh (1)
1-1: Shebang portability change approvedLooks good.
scripts/create-local-demo.sh (1)
1-1: Env-based shebang applied consistently across the repo; no/bin/bashshebangs foundCross-repo sanity check confirms there are no remaining hardcoded
#!/bin/bashlines.router-tests/testdata/tls/makecert.sh (1)
1-1: Shebang updated to /usr/bin/env bash — looks goodImproves interpreter resolution without altering behavior.
keycloak/theme/build.sh (1)
1-1: Env-based shebang is correct here — approvedConsistent with the rest of the repo and keeps
set -euo pipefailsemantics intact.router/__schemas/full-plugin-project.sh (1)
1-1: Good switch to /usr/bin/env bashEnhances portability for environments where bash isn’t in /bin.
scripts/create-kubernetes-demo.sh (1)
1-1: Portable shebang verification successfulA repository-wide scan found no hardcoded bash shebangs (
/bin/bash,/usr/bin/bash,/usr/local/bin/bash); all scripts now use#!/usr/bin/env bash. LGTM.examples/full-cosmo-docker/destroy.sh (1)
1-1: Portable shebang — LGTMCorrect, consistent with the PR goal. No behavior change.
scripts/delete-full-local-demo.sh (1)
1-1: Portable shebang — LGTMGood portability improvement; no logic changes.
data_migrations/controlplane/1747776337_rbac_groups/migrate.sh (1)
1-1: Portable shebang — LGTMAppropriate for environments where bash isn’t at a fixed path. Script already uses bash-specific features, so this aligns well.
examples/full-cosmo-helm/delete_demo.sh (1)
1-1: Portable shebang — LGTMMatches the repository-wide standardization. No functional changes.
scripts/create-docker-demo.sh (1)
1-1: Repo-wide shell script checks passed — no action neededAll scripts were scanned for legacy shebangs, executable bits, and CRLF endings:
- ✓ No
#!/bin/bashshebangs found- ✓ All tracked
.shfiles are executable- ✓ No CRLF line endings detected
Excellent portability improvement—LGTM!
scripts/bench-create.sh (1)
1-1: Portability shebang change — LGTM.
Consistent with the PR’s goal; no behavior change.docker/nats/build-push.sh (1)
1-1: Env-based shebang for Bash — LGTM.
Appropriate for cross-platform environments; no functional changes.scripts/delete-docker-demo.sh (1)
1-1: Portable shebang — LGTM.
Matches the repo-wide migration; script behavior unchanged.scripts/bench-router.sh (1)
1-1: Use of /usr/bin/env bash — LGTM.
Aligns with portability objective; no other modifications observed..github/scripts/setup-keycloak.sh (1)
1-1: Shebang portability LGTMUsing /usr/bin/env bash is the right call for CI runners and diverse images.
demo/update_token.sh (1)
1-1: Shebang portability LGTMMatches the PR objective, no behavioral changes.
scripts/bench-delete.sh (1)
1-1: Shebang portability LGTMConsistent with other scripts, no logic change.
demo/bump-deps.sh (1)
1-1: Shebang portability LGTMRight move for developer machines and CI.
router-tests/bump-deps.sh (1)
1-1: Shebang portability LGTMSwitching to /usr/bin/env bash improves cross-platform compatibility. Good change.
demo/compose.sh (1)
1-1: Shebang portability LGTMStandard, portable shebang. Approved.
scripts/bench-update.sh (1)
1-1: Shebang portability LGTMConsistent with the repo-wide update. Looks good.
router-tests/update-config-no-edg.sh (1)
1-1: Shebang portability LGTMGood standardization to /usr/bin/env bash.
scripts/configurations/kubernetes.sh (1)
1-1: Shebang portability LGTMMatches the stated PR goal. Approved.
examples/router-simple/start.sh (1)
1-1: No remaining/bin/bashshebangs; optional script hardeningI ran the suggested repository-wide search and confirmed there are no hardcoded
/bin/bashshebangs left—everything now uses#!/usr/bin/env bash.
examples/router-simple/start.shbegins with#!/usr/bin/env bash- Repository check:
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' -e '^#!\s*/bin/bash\b' # → no matchesOptional: consider adding
set -o pipefailandset -uat the top of your scripts for stronger error detection.scripts/configurations/local.sh (1)
1-1: LGTM: switched to env-based bash for portabilityUsing /usr/bin/env makes this script more robust across environments. No functional changes introduced.
demo/deploy.sh (1)
1-1: LGTM: env-based shebang improves cross-platform supportInterpreter discovery via env is preferred on macOS/BSD/NixOS/GUIX. No behavior change.
examples/full-cosmo-docker/start.sh (1)
1-1: LGTM: portable shebang; EOF newline addition is fine
- Shebang update is correct and portable.
- Newline at EOF (Line 4) is a good hygiene fix; no logic changes.
Also applies to: 4-4
helm/cosmo/README.md (1)
47-47: Sync README template and regenerate Helm docsThe values table now reflects the portable shebang in
db-init.sh. Note thathelm/cosmo/README.mdis generated by helm-docs fromhelm/cosmo/README.md.gotmpl(seehelm/Makefile:34). Please update the template (andvalues.yamlif needed) and regenerate the docs:cd helm/cosmo helm-docs . -o README.md # or from repo root: make -C helm docshelm/cosmo/values.yaml (1)
425-425: Confirm/usr/bin/envavailability or switch to a fixed bash shebangThe Bitnami ClickHouse image uses
/bin/bashto run yourinitdbScripts, and/usr/bin/envmay not be present in every variant. To avoid surprises at runtime:
- Verify that your target image includes
env(for example:If this returns nothing,docker run --rm bitnami/clickhouse:latest which env/usr/bin/envisn’t available.)- If
envisn’t present, update your script’s shebang to#!/bin/bash.- Ensure the script is executable (e.g.
chmod +x initdb.sh) before mounting it.Locations to check:
• helm/cosmo/values.yaml: line 425 (yourinitdb-script entry)
| @@ -1,4 +1,4 @@ | |||
| #!/bin/bash | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Shebang portability LGTM
Switching to env-based bash improves cross-distro portability (macOS, BSD, Nix, Guix, containers). No functional impact here.
To ensure no hardcoded /bin/bash shebangs remain, run:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n '^\#\!\s*/bin/bash(\s|$)' || echo "No /bin/bash shebangs left"Length of output: 209
Shebang portability LGTM – update verification regex
File: scripts/delete-kubernetes-demo.sh
Lines: 1-1
#!/usr/bin/env bash
Shebang portability LGTM
Switching to an env-based bash shebang improves cross-distro portability (macOS, BSD, Nix, Guix, containers). No functional impact here.
To ensure there are no remaining hardcoded /bin/bash shebangs, update the verification command to use a Rust-compatible regex:
#!/usr/bin/env bash
set -euo pipefail
rg -n '^#!\s*/bin/bash(\s|$)' || echo "No /bin/bash shebangs left"🤖 Prompt for AI Agents
In scripts/delete-kubernetes-demo.sh at line 1, the shebang is correctly updated
to use /usr/bin/env bash for portability. To complete this improvement, add the
suggested verification command using ripgrep with the Rust-compatible regex to
check for any remaining hardcoded /bin/bash shebangs. This involves appending
the lines setting strict bash options and the rg command as shown, ensuring the
script can verify no hardcoded shebangs remain.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
ysmolski
requested review from
endigma,
miklosbarabas and
thisisnithin
as code owners
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
| export NPM_CONFIG_PREFIX="${NPM_CONFIG_PREFIX:-$HOME/.npm-global}" | ||
| export PATH="$NPM_CONFIG_PREFIX/bin:$PATH" | ||
| fi | ||
|
|
There was a problem hiding this comment.
I think its better to move changes to wgc to a separate commit.
Also rebase instead of pulling merge commits.
(Just my suggestions, I just came across this pr and not a contributor to this repo)
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
hi @rawkode, thanks for you contribution. |
4 participants
Replace hardcoded
#\!/bin/bashshebangs with#\!/usr/bin/env bashacross all shell scripts to improve cross-platform compatibility.This allows the scripts to work on systems where bash may be installed in different locations, such as BSD, macOS, NixOS, and GUIX.
Summary by CodeRabbit
Checklist