Use shfmt (for ./hack only)

[kolyshkin]

As suggested in #28760 (comment), let's try to use shfmt for shell scripts under ./hack/ only. For the previous (rejected) attempt, see #26340. To reiterate the discussion in there, the main reasons to reject were:

• we only gain readability (and even that is questionable);
• it raises barrier for contribution.

The second point is probably of less concern for ./hack (as it is rarely modified). Also, make validate-source (or make validatepr, or make shfmt) automatically fixes all the formatting issues, so the barrier is probably not that high.

shfmt configuration is intentionally kept minimal. Yet the diff is big, mostly because of indentation differences (previously we only enforced the use of spaces instead of tabs, but how many spaces was used was not in any way enforced or suggested).

Feel free to close w/o further discussion if you don't like it; this is not a hill I will die on.

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

• Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
  commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
  the sign-off email address. See CONTRIBUTING.md
  for more information.
• Referenced issues using Fixes: #00000 in commit message (if applicable)
• Tests have been added/updated (or no tests are needed)
• Documentation has been updated (or no documentation changes are needed)
• All commits pass make validatepr (format/lint checks)
• Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

NONE

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[Honny1]

LGTM, Please rebase. I will add the label No New Tests.

[Luap99]

I am fine with limiting the scope to just hack/ but is there a way to turn of the redirection rule > /dev/null is perfectly readable IMO, removing the spaces for the file in in each redirection does not feel worth it to me?

[kolyshkin]

Rebased; add space_redirects = true.

[Luap99]

Rebased; add space_redirects = true.

This is the opposite problem with that now though, no? I was thinking more in terms both ways should be allowed if possible.

[kolyshkin]

Rebased; add space_redirects = true.

This is the opposite problem with that now though, no? I was thinking more in terms both ways should be allowed if possible.

Alas it's not possible; it's either with or without a space (which makes sense as it enforces a coding style).

Let me know which one you prefer. I like it more with the space, but I also like the minimized config (TBH I'd reformat everything without .editorconfig at all but that is extreme).

[Luap99]

Alas it's not possible; it's either with or without a space (which makes sense as it enforces a coding style).

Well sure but to me this really does not make any difference readability wise and then it means all contributors need to remember this is a thing now and format accordingly which means it will almost certainly result in people having to repush for a trivial thing that IMO has no upside.

Anyhow I do not care that much if it is limited to hack/, if you ask me what way to choose I would just say the one with the smaller diff.

[kolyshkin]

and then it means all contributors need to remember this is a thing now and format accordingly which means it will almost certainly result in people having to repush for a trivial thing that IMO has no upside.

Aren't contributors supposed to run make validatepr or something similar before they push?

We can also add a git hook to format these files.

Anyhow I do not care that much if it is limited to hack/, if you ask me what way to choose I would just say the one with the smaller diff.

With space_redirects = true:

28 files changed, 262 insertions(+), 256 deletions(-)

Without:

 28 files changed, 268 insertions(+), 264 deletions(-)

So I guess let's keep it.

[Luap99]

Aren't contributors supposed to run make validatepr or something similar before they push?

Sure but the fact is the majority of people wont, even maintainers.

We can also add a git hook to format these files.

That is an option but lets not increase the scope of it here I guess.

[kolyshkin]

Rebased

[mheon]

LGTM

[Luap99]

Should we hold this for after we land all the new CI changes? I don't want to add more work on us atm with reformatting the commits, and likely fixing merge conflicts over there then

[Honny1]

LGTM