test, lint: check bash version before executing codespell #24610
Conversation
3c8b40d to
8c2b102
Compare
|
Concept ACK. |
8c2b102 to
a5e5b6d
Compare
|
Concept ACK Code reviewed. |
What system / OS was this? Does everything else work? Judging by https://repology.org/project/bash/versions, there is no recently released version of any OS that ships with Bash Given that, I'm ~0 on adding more code to warn for this case. |
@fanquake I use MacOS and most versions (I think) are stuck in 3.x giving its license. So, since mapfile doesn't work in 3.x, the script doesn't "fail", it runs ignoring that certain files should be skipped, giving me some false flags. |
macOS swapped it's default shell away from bash > 2 years ago, with the release of |
Very good point, I had to download the bash source from the official GNU site to even get a binary I could test with. I agree its probably not worth adding code for this. |
|
Thanks, however I'm going to close this PR. I don't think we should be adding more code to our scripts, to better support Bash 3. I would suggest either swapping to the macOS default shell (zsh), or installing and using a modern Bash, i.e Bash 5.1 via homebrew: https://formulae.brew.sh/formula/bash. |
I ran into this today and from my understanding it's still annoying, especially if you are starting out on a new macOS system. I am suggesting at least adding some docs on this since it would have saved me some time, see #24750 |
|
This kind of issues keep coming up. Can we please port scripts like this to Python… |
|
Started in #24762 |
When executing
lint-spelling.shI noticedmapfilecommand wasn't working so it wasn't ignoring the files that should be ignored giving me some false flags. I noticed the problem is the version of bash,mapfilewas introduced in bash 4 and my machine had the 3 one. So, I added a verification to skip it if the version of bash is less than 4.