Skip to content
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

BUG: avoid use of non-portable which in the tests #424

Closed
wants to merge 1 commit into from

Conversation

thesamesam
Copy link
Contributor

which is not a standard POSIX utility, and indeed, each of these test scripts uses #!/bin/bash as its shebang, so we can use type -P which has the same behaviour as which for free.

(If the tests used POSIX shell, we could do command -v, its only caveat is that it'll pick up functions in the user's shell, which doesn't matter 99% of the time anyway.)

Distributions like Debian [0] and Gentoo [1] are looking to remove which from their base set of packages.

[0] https://lwn.net/Articles/874049/
[1] https://bugs.gentoo.org/646588

which is not a standard POSIX utility, and indeed, each of these test scripts
uses #!/bin/bash as its shebang, so we can use `type -P` which has the same
behaviour as `which` for free.

(If the tests used POSIX shell, we could do `command -v`, its only caveat is
that it'll pick up functions in the user's shell, which doesn't matter 99% of
the time anyway.)

Distributions like Debian [0] and Gentoo [1] are looking to remove `which`
from their base set of packages.

[0] https://lwn.net/Articles/874049/
[1] https://bugs.gentoo.org/646588

Signed-off-by: Sam James <[email protected]>
@pcmoore pcmoore changed the title tests: avoid use of non-portable which BUG: avoid use of non-portable which in the tests Jan 7, 2024
@pcmoore pcmoore added this to the v2.6.0 milestone Jan 7, 2024
@pcmoore
Copy link
Member

pcmoore commented Jan 7, 2024

Hi @thesamesam, first off, great branch name :)

Other than that, this patch looks good to me, and considering the trivial nature I'm going to go ahead and merge this into main via 47ca644. @drakenclimber if you have any concerns/objections please let me know and we can fix things up, or revert this patch if needed.

Thanks all!

@pcmoore pcmoore closed this Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants