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

Running scripts on systems with bash version older than v4 causes errors #680

Open
obi1kenobi opened this issue Mar 2, 2024 · 3 comments

Comments

@obi1kenobi
Copy link
Owner

          Running `./scripts/regenerate_test_rustdocs.sh` also gives the error,

cargo-semver-checks/scripts/regenerate_test_rustdocs.sh: line 23: shopt: globstar: invalid shell option name

Originally posted by @vasucp1207 in #679 (comment)

The globstar option was introduced by Bash 4, released 15 years ago. Unfortunately, some systems still ship with only Bash 3 and will have this error.

Our scripts should check the installed bash version and exit with an error if it's older than v4. If it's possible to accommodate other shells (e.g. zsh, fish, nushell, etc.), ideally we should do that as well.

@jw013
Copy link
Contributor

jw013 commented Mar 3, 2024

@vasucp1207 can confirm but based on what I saw in the comments I am almost certain they are on Mac OS, which ships an ancient version of Bash by default (3.something, apparently for GPL-avoidance reasons) as well as BSD flavors of command line tools which differ from GNU versions. The missing Cargo.toml is likely due to a difference in sed behavior between Mac and GNU, https://stackoverflow.com/questions/5694228/sed-in-place-flag-that-works-both-on-mac-bsd-and-linux

Writing reliable cross-platform scripts can be a rather tedious rabbit hole to dive into but there are some easy things we can do without going too deep.

  • The #!/usr/bin/env bash at the top of the scripts should ensure that some version of bash is running the scripts so I don't think we need to worry about non-bash shells (zsh/fish/nushell etc) unless users go out of their way, e.g. zsh some_script.sh instead of ./some_script.sh.

  • Adding a check for Bash version 4+ is simple, basically checking that $BASH_VERSION exists and that ${BASH_VERSINFO[0]} -ge 4. This should also catch any non-bash shells too since they would not set those variables.

  • The real tedium comes from trying to ensure that every external tool is installed and supports the required features. I took a brief look into the scripts directory and see grep, sort, curl, jq, and of course sed. There may be more I missed. Not sure this level of effort is worth it for development-only scripts. Perhaps an easier approach would be to document in the Contributing guide that the scripts expect updated bash + a list of the required external programs.

@obi1kenobi
Copy link
Owner Author

Great points! If you have a few minutes, would you care to open some PRs for the bash version check and the contributing guide suggestions? I'd be thrilled to merge them.

@obi1kenobi
Copy link
Owner Author

#681 mostly addressed this — thank you @jw013! The only open question remaining is if we can do something about our incompatible use of sed.

If I correctly understand the suggested fix at the link you shared, it seems like we might be able to make a copy of the files we intend to edit with sed and apply our change to the copy? Perhaps we make the copy reside in /tmp somewhere, so that a failure of the script won't pollute the git repo?

This is a bit hard for me to test since I don't have a Mac, so I'd love some help. If neither of you @jw013 and @vasucp1207 is up for trying a PR, I can try to whip something up next weekend if I have a bit of time.

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

No branches or pull requests

2 participants