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

Make script POSIX compliant (and switch shebang to sh) #751

Draft
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Dec 14, 2022

Pull Request

May not merge in absence of any user feedback, but wanted to check it was possible!

Problem

n is currently written as a bash script. Bash isn't always available by default, and is an old version on macOS.

Related: #344 #648 #673

Solution

Rewrite as a POSIX shell script so runs with whatever (POSIX) shell is installed.

All tests pass on Ubuntu using dash as the shell. Have addressed all shellcheck issues with flags passed to utilities.

Currently still using local (a lot) which is commonly available but not POSIX.

https://stackoverflow.com/a/18600920/1082434

It is normally done with the local keyword, which is, as you seem to know, not defined by POSIX. Here is an informative discussion about adding 'local' to POSIX.

However, even the most primitive POSIX-compliant shell I know of which is used by some GNU/Linux distributions as the /bin/sh default, dash (Debian Almquist Shell), supports it. FreeBSD and NetBSD use ash, the original Almquist Shell, which also supports it. OpenBSD uses a ksh implementation for /bin/sh which also supports it. So unless you're aiming to support non-GNU non-BSD systems like Solaris, or those using standard ksh, etc., you could get away with using local.

ChangeLog

@shadowspawn shadowspawn changed the title Feature/posix Make script POSIX compliant (and switch shebang to sh) Dec 14, 2022
@shadowspawn
Copy link
Collaborator Author

Issue with "super barebones" setup and only sh available: #782

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants