fix: Make hooks bash 3.2 compatible

[MaxymVlasov]

Put an x into the box if that apply:

• This PR introduces breaking change.
• This PR fixes a bug.
• This PR adds new functionality.
• This PR enhances existing functionality.

Description of your changes

In new versions, macOS use zsh by default and include bash 3.2 for compatibility reasons.
That OS will never upgrade bash to 4+, during license issues.
theverge.com/2019/6/4/18651872/apple-macos-catalina-zsh-bash-shell-replacement-features

So looks like we need to maintain compatibility with bash 3.2 🙄
Or require bash 4.2+ as deps.

Fixes #337

How can we test changes

Run on macOS:

repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: 48bc03ca3f0f2f782d2f430069868019a6892062
  hooks:
    - id: terraform_fmt

[yermulnik]

ℹ️ IMHO: we EITHER officially declare support for Bash 3 and hence test every change using Bash 3 as well as modern Bash OR we set Bash >= 4 as a requirement for pre-commit-terraform and don't add more toil.

[antonbabenko]

@yermulnik I agree that toil is bad here but I am also not sure how hard it is for users to install Bash 4+ on various systems where pre-commit is executed.

[yermulnik]

@antonbabenko From my knowledge it's either very old and outdated version of existing distros, or MacOS only. The latter has at least four more or less easy and more or less user friendly and/or widely used ways to upgrade to a modern version of Bash.
I also would like to mention there are OSes where Bash is not installed by default either. Should we support these OSes as well relying only on fully POSIX-compatible shell features and utilities?

If we'd support outdated version of Bash for only one "brand" of OS, someone may also ask us to provide support for e.g. non-Bash shells (there are a plenty of and zsh and fish are only a few of them which are widely used) or for embedded systems where the base system misses a lot of OS or shell tools and features.

[yermulnik]

Also those systems which lack modern Bash (and any other tool used by pre-commit and pre-commit-terraform to bring the best UX and functionality) should probably consider looking into using pre-commit-terraform as Docker container — it seems the only sensible meaning for maintaining this Docker image, eh?

[pshamus]

Also those systems which lack modern Bash (and any other tool used by pre-commit and pre-commit-terraform to bring the best UX and functionality) should probably consider looking into using pre-commit-terraform as Docker container — it seems the only sensible meaning for maintaining this Docker image, eh?

@yermulnik That isn't going to scale well in organizations where not everyone has Docker installed.

I understand the sentiment not wanting to have to support old versions of bash. However, this should be introduced as a breaking change. We have hundreds of projects using these hooks and to all of the sudden now require them to install an updated version of bash (and hope they got it right) is a big burden.

[yermulnik]

@pshamus

(apologies for long-read)

We have hundreds of projects using these hooks and to all of the sudden now require them to install an updated version of bash

It is not about the number of projects, but about the number of engineers.
If you have hundreds engineers who work with at least one repository with pre-commit enabled and who use company-managed macos-based laptops, you may want to leverage packaging automation and mass-installing (e.g. https://github.com/scriptingosx/GNU-bash-mac-installer).
If this is about a dozen of engineers working with hundreds of projects, they most probably already use Homebrew or MacPorts. Or at least they may be interested to start using it, as it's a one of the 1st things engineers would want on the freshly installed macos since macos ships with a plenty of quite old tools in base.

And yep, I fully agree such a change should have been announced in advance along with proper semantic versioning for changes with backward incompatibility.

As I already stated before, my personal opinion (I'm not founder or owner, I'm collaborator) is we either revisit this entire repo and convert it to become fully POSIX-compliant so that we rely upon /bin/sh on every single POSIX-compliant system (what to do with those which are not? 🤔), or we drop support for ancient versions (not old, but ancient) of required tools and continue developing this project and comply with modern toolset.

ps: one of the main reasons against bash3 support would be the need to maintain and test every single change and release with bash3 which is not impossible, but, you know, «That isn't going to scale well in organizations where not everyone has Docker installed» — I see Docker as the only sensible approach to run bash3 w/o littering our workstations with ancient software and quirks to temporarily fool local environment about which bash version should come up via /usr/bin/env bash 😉

[MaxymVlasov]

@antonbabenko let's fix breaking changes introduced in v1.63 for some Mac users, by merging that PR.
Got confirmation in #337 (comment) that the code works fine.

@yermulnik could you please check in a week, how much effort need to make code POSIX-compatible?
If it needs from us to rewrite too much stuff or make code unreadable, I suppose to officially break support for bash <4.2 in v2.0.0

[yermulnik]

@yermulnik could you please check in a week, how much effort need to make code POSIX-compatible?

Do we have any plans to switch to /bin/sh instead of relying on bash?
Otherwise it doesn't make any sense to even look into making in strictly POSIX-compatible/compliant.

What's needed to make code compatible with Bash v3 I (by a surprising coincident) already described in #337 (comment) today 😃 Let me know if that's what you mean.

I suppose to officially break support for bash <4.2 in v2.0.0

That's definitely the way to go w/o any constraints: we should drop support of ancient Bash in whichever major release of pre-commit-terraform 👍🏻

[MaxymVlasov]

Do we have any plans to switch to /bin/sh instead of relying on bash?

If it is not too difficult - why not

[yermulnik]

Do we have any plans to switch to /bin/sh instead of relying on bash?

If it is not too difficult - why not

This may make sense, btw. Though we need to inspect our code and ensure it complies to POSIX standard and rip off any bashisms including those that belong to bash 3.
We can start with leveraging e.g. https://github.com/duggan/shlint (or any other alike tool) to test compatibility and (if possible) create GH action to run this tool on PRs/commits in container with most common /bin/sh implementations installed (zsh ksh bash dash sh).

[antonbabenko]

This PR is included in version 1.64.1 🎉