Use /usr/bin/env bash for widely compatible shebang#674
Conversation
Signed-off-by: Sandro Jäckel <sandro.jaeckel@gmail.com>
SuperSandro2000
force-pushed
the
bin-bash
branch
from
ff5c880 to
fb2c2c0
Compare
|
Huge thanks for opening this PR and for the work you put into it. The maintainer shop is currently full, so this may sit for a bit before it gets a proper review. We will come back to this as soon as possible with real feedback; I wanted to make sure it did not sit unacknowledged in the meantime. |
DeusData
added
bug
|
Thanks. This looks like a focused portability change. Since it touches build scripts and generated hook scripts, please link the tracking issue or add a short rationale for the NixOS compatibility need, and we’ll need normal CI validation before merge. One security-hygiene nit: please change the fake |
|
Thanks for converging the remaining holdouts — we verified the repo already uses env-form on ~20 newer scripts, so this brings full consistency, and MSYS2 provides /usr/bin/env so the Windows legs are safe. Three small asks and this merges: (1) the two open questions from earlier (a rationale/tracking link, and the ghp_-shaped fixture string); (2) please keep at least one '#!/bin/bash' case in the parser tests — the current sweep removed the only coverage for absolute-path shebang extraction in infra_parse_shell; (3) FYI the hook-script hunks change runtime resolution on every user's machine (PATH-resolved bash instead of pinned /bin/bash) — standard practice and fine by us, just worth a sentence in the PR body. Thanks! Update: to keep momentum on the bug backlog, we're going to carry the changes above over the line ourselves shortly — a distilled follow-up on current main implementing the notes in this thread, with you credited as |
On NixOS (and other non-FHS systems) /bin/bash does not exist, so scripts with an absolute shebang fail to run. Switch the remaining holdouts to /usr/bin/env bash: eleven scripts/*.sh, test-infrastructure/run.sh, and the three Claude Code hook scripts emitted by src/cli/cli.c (gate, session reminder, subagent reminder). Distilled from PR DeusData#674, with parser-test coverage preserved: the infra_parse_shell* fixtures in tests/test_pipeline.c intentionally keep #!/bin/bash so absolute-path shebang extraction stays covered, and tests/repro fixtures are untouched. Also replace the GitHub-PAT-shaped fixture string flagged in the DeusData#674 thread with an obviously fake placeholder (ghp_FAKE...) that still matches the ghp_ + 36-alnum secret detector. Co-authored-by: Sandro Jäckel <sandro.jaeckel@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
Thanks for converging the shebang holdouts — NixOS users will appreciate it. We carried it over the line in 010ac9b (PR #812) with you credited as co-author, with two small deltas from the review: the absolute-path shebang parser fixtures keep one #!/bin/bash case (so that extraction path stays covered), and the ghp_-shaped fixture string became an obviously-fake placeholder that still trips the secret detector. A third hook heredoc had appeared since your branch (SubagentStart) — converted too, so the tree is now fully consistent at 0 raw /bin/bash shebangs outside fixtures. Closing in favor of the distill — thanks again! |
2 participants
What does this PR do?
Fix the shebangs of scripts to work on eg NixOS.
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)