.github/workflow/reviewdog: Add shellcheck GitHub Action

[lukegb]

There's been some interest in running Shellcheck as part of the review workflow on discrete .sh scripts as a review aid. In particular, this would cover stage-1-init.sh and stage-2-init.sh.

The use of reviewdog here is to parse the output of Shellcheck and, crucially, only report failures on lines modified by the PR author, which allows for this to be introduced now and for Shellcheck lint failures to gradually be driven down over time (or through a separate concerted effort).

This is run in the default github-pr-check mode. If we used the github-pr-review mode instead then Shellcheck's suggestions would be in the form of review comments, but I don't think the tradeoff is worth it (and I don't really like the automatically generated commits that GitHub generates when you accept review comments...).

cc @SuperSandro2000 @grahamc

[WIP, but deliberately not draft so codeowners see it]

[SuperSandro2000]

Will this mark the check red if there is an error and yellow if there is a warning?

Can we configure shellcheck and exclude things we definitely do not want globally?

[lukegb]

Will this mark the check red if there is an error and yellow if there is a warning?

No, because checks checks are pass/fail/running, not really pass/error/warn.

Can we configure shellcheck and exclude things we definitely do not want globally?

Yes.

[grahamc]

Can you add a commit demonstrating where it triggers?

[zowoq]

How does it handle these errors?

SC1008: This shebang was unrecognized. ShellCheck only supports sh/bash/dash/ksh. Add a 'shell' directive to specify.

SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

[lukegb]

Can you add a commit demonstrating where it triggers?

I have a test PR: lukegb#1

How does it handle these errors?

SC1008: This shebang was unrecognized. ShellCheck only supports sh/bash/dash/ksh. Add a 'shell' directive to specify.

SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

At the moment, those are implicitly silenced for review purposes by virtue of being preexisting - if someone edits the shebang line then it'll complain, though.

[grahamc]

I love it!

[SuperSandro2000]

How do we handle things like https://github.com/lukegb/nixpkgs/pull/1/files#annotation_652947792? This is clearly a false positive and we have lots of those.

[FRidh]

How does it handle @variables@ that need to be substituted? It won't error on those?

Seems its the same as @SuperSandro2000 referred to

How do we handle things like https://github.com/lukegb/nixpkgs/pull/1/files#annotation_652947792? This is clearly a false positive and we have lots of those.

[SuperSandro2000]

How does it handle @variables@ that need to be substituted? It won't error on those?

Seems its the same as @SuperSandro2000 referred to

How do we handle things like lukegb/nixpkgs#1 (files)? This is clearly a false positive and we have lots of those.

Yes I am. This is the big bummer about this PR because we will have a lot of those false positives and writing a comment above every single one of them is not fesable.

[FRidh]

If its not possible to instruct it to avoid these substitutions I think we should just stick with or convert to using shellcheck in derivations.

[stale]

I marked this as stale due to inactivity. → More info

[Mic92]

Not a show stopper but is there a way to only run this ci job if an actually .sh file changed? I have not checked if github provides some support in this direction.

[stale]

I marked this as stale due to inactivity. → More info