qa: Pin shellcheck version #15166

Pin shellcheck version.

Can't we lock the installed version instead?

@promag The shellcheck on Travis is pre-installed AFAIK - we don't install it explicitly in our Travis conf. Also since we don't control the version the user has installed pinning wouldn't have prevented the version problem described in https://github.com/bitcoin/bitcoin/pull/14115/commits/908a559f33cf9d774953ce46e159e66fc9bc758d.

@practicalswift IMO what the user has it not a problem, instead it helps if the user has a newer version so he can update the travis one if necessary.

We could run in a docker container?

@promag Yes, pinning the version by going the Docker route would be an alternative way to solve this. I'll let @laanwj chime in here.

I think it makes sense to allow for some variability in shellcheck versions for people that want to run this locally. (this isn't an argument against pinning as well, though)

Just to be clear, in travis run the linter in a container with a specific version of shellcheck, locally use the one available in the path?

So we'd still had to support multiple versions of shellcheck

@MarcoFalke why? The official supported would be the travis IMO. If the linter fails for the user then he either upgrades/downgrades his version or submit a pull to fix the (most likely) new version.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #13728 (WIP: Scripts and tools: Run the CI lint stage on mac by Empact)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@promag The proposed solution allows for some variability in shellcheck versions. Could you describe what problems foresee with allowing for such variability? If I know the problems I can perhaps help solve them :-)

As the shellcheck author I would definitely recommend pinning versions rather than filtering suggestions. This is because the scope of existing suggestions often changes through improvements or bugfixes.

For example, the SC2054 warning that commas won't work as an array separator recently had its detection criteria changed, so now shellcheck master (and therefore the next release) will start triggering on test/lint/lint-format-strings.sh due to its array containing unquoted FatalError,0 fprintf,1 ..

By pinning, you can upgrade at your leisure without surprise build breaks.

@koalaman Thanks for the input! What is the best way to achieve version pinning of shellcheck in Travis? Travis pre-installs shellcheck.

@practicalswift Unfortunately I'm not very familiar with Travis myself.

Someone contributed a recipe which just downloads a specific release version (which could obviously be improved by verifying the checksum):

export scversion="stable" # or "v0.4.7", or "latest"
wget "https://storage.googleapis.com/shellcheck/shellcheck-${scversion}.linux.x86_64.tar.xz"
tar --xz -xvf shellcheck-"${scversion}".linux.x86_64.tar.xz
cp shellcheck-"${scversion}"/shellcheck /usr/bin/
shellcheck --version

Specific ShellCheck releases can also be run via Docker, if that helps.

We already use docker so I guess we could use the one available in bionic?

Now pinning the shellcheck version instead. Please re-review :-)

ACK, we do the same for the pip packages

I guess checksum verification is not necessary. Still curious why not docker. utACK a517541 though.

@promag We don't use Docker for any of the linters. The non-Docker setup is simpler.