Require bash 4+ explicitly instead of failing silently under /bin/bash - #3
Open
systemjack wants to merge 1 commit into
Open
Require bash 4+ explicitly instead of failing silently under /bin/bash#3systemjack wants to merge 1 commit into
systemjack wants to merge 1 commit into
Conversation
localdev-mounts.sh uses `declare -A` for mount-target collision detection, so it needs bash 4.0+. Every launcher declared `#!/bin/bash`, which on macOS is bash 3.2.57 -- the one interpreter that cannot run it. The failure was silent and wrong rather than loud. Under 3.2 `declare -A` fails, USED_TARGETS degrades to an indexed array, subscript expansion is evaluated as arithmetic, and .localdev-mounts.toml entries are dropped with no error. The container then comes up missing mounts the user believes are present -- observed as a workspace whose read-only reference repo simply was not there, with nothing in the output to say so beyond two stray `declare: -A: invalid option` lines. The same breakage silently disabled the collision guard itself, so two host directories claiming one container target went undetected on macOS for as long as the file-mount feature has existed. Two changes: - Shebangs become `#!/usr/bin/env bash` in all four launchers and the library, so a PATH-resolved bash 4+ (Homebrew's, typically) is preferred over /bin/bash. - The library asserts BASH_VERSINFO >= 4 and exits with an actionable message naming the running version and interpreter. `#!/usr/bin/env bash` alone would still fail silently for anyone without a modern bash on PATH, and silently dropping mounts is not an acceptable failure mode. Verified: launched by name, the .localdev-mounts.toml entry is now present in the podman args as :ro; invoked as `/bin/bash localdev` it aborts with the version error rather than proceeding; and the collision guard now correctly aborts on two mounts sharing a target, which it never did on macOS before. This affects every macOS host, since /bin/bash there is 3.2 -- anyone relying on .localdev-mounts.toml has been silently losing mounts. Also documents the requirement in README Prerequisites. The runtime guard tells you after the fact; a macOS user needs to know before their first launch that `brew install bash` is a setup step, not an optional nicety.
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.
localdev-mounts.sh uses
declare -Afor mount-target collision detection, so itneeds bash 4.0+. Every launcher declared
#!/bin/bash, which on macOS is bash3.2.57 -- the one interpreter that cannot run it.
The failure was silent and wrong rather than loud. Under 3.2
declare -Afails,USED_TARGETS degrades to an indexed array, subscript expansion is evaluated as
arithmetic, and .localdev-mounts.toml entries are dropped with no error. The
container then comes up missing mounts the user believes are present -- observed
as a workspace whose read-only reference repo simply was not there, with nothing
in the output to say so beyond two stray
declare: -A: invalid optionlines.The same breakage silently disabled the collision guard itself, so two host
directories claiming one container target went undetected on macOS for as long
as the file-mount feature has existed.
Two changes:
#!/usr/bin/env bashin all four launchers and the library, soa PATH-resolved bash 4+ (Homebrew's, typically) is preferred over /bin/bash.
naming the running version and interpreter.
#!/usr/bin/env bashalone wouldstill fail silently for anyone without a modern bash on PATH, and silently
dropping mounts is not an acceptable failure mode.
Verified: launched by name, the .localdev-mounts.toml entry is now present in the
podman args as :ro; invoked as
/bin/bash localdevit aborts with the versionerror rather than proceeding; and the collision guard now correctly aborts on two
mounts sharing a target, which it never did on macOS before.
This affects every macOS host, since /bin/bash there is 3.2 -- anyone relying on
.localdev-mounts.toml has been silently losing mounts.
Also documents the requirement in README Prerequisites. The runtime guard tells
you after the fact; a macOS user needs to know before their first launch that
brew install bashis a setup step, not an optional nicety.