wire-server
wire-server copied to clipboard
WPB-2568 Fix shellcheck linting problems on all shell scripts
This PR activates shellcheck linting for almost all shell scripts.
It also fixes various suggestions on the shell scripts that are now shellchecked.
Tracked by https://wearezeta.atlassian.net/browse/WPB-2568
Checklist
- [x] Add a new entry in an appropriate subdirectory of
changelog.d - [x] Read and follow the PR guidelines
It is annoying that this wouldn't respect gitignore and could overwrite files in dist-newstyle, etc. I am looking into why it the git walker doesn't work. So far I have found this:
Running treefmt --no-cache -vv --walk=git -f shellcheck prints this:
DEBU format | shellcheck: executing: /home/axeman/workspace/wire-server/.env/bin/shellcheck docs/convert/convert.sh docs/convert/revert.sh hack/bin/helm_overrides.sh
INFO format | shellcheck: 3 file(s) processed in 21.992731ms
traversed 6499 files
emitted 6499 files for processing
But if we ask git about how many files it has by running git ls-files | wc -l, it says: 6671.
When using the filesystem walker it says this:
DEBU format | shellcheck: executing: /home/axeman/workspace/wire-server/.env/bin/shellcheck changelog.d/mk-cleanup.sh deploy/dockerephemeral/db-migrate/brig-index.sh deploy/dockerephemeral/db-migrate/brig-schema.sh deploy/dockerephemeral/db-migrate/galley-schema.sh deploy/dockerephemeral/db-migrate/gundeck-schema.sh deploy/dockerephemeral/db-migrate/spar-schema.sh deploy/dockerephemeral/init_vhosts.sh deploy/dockerephemeral/run.sh docs/convert/convert.sh docs/convert/revert.sh hack/bin/cabal-run-all-tests.sh hack/bin/cabal-run-integration.sh hack/bin/certchain.sh hack/bin/create_team.sh hack/bin/create_team_request_code.sh hack/bin/find-latest-docker-tag.sh hack/bin/gen-certs.sh hack/bin/generate-local-nix-packages.sh hack/bin/helm_overrides.sh hack/bin/integration-cleanup.sh hack/bin/integration-logs-relevant-bits.sh hack/bin/integration-test.sh hack/bin/kind-upload-image.sh hack/bin/kind-upload-images.sh hack/bin/kubectl-get-debug-info.sh hack/bin/mk-drv.sh hack/bin/nix-hls.sh hack/bin/oauth_test.sh hack/bin/register_idp.sh hack/bin/set-wire-server-image-version.sh hack/bin/update.sh hack/bin/upload-image.sh hack/bin/upload-images.sh integration/scripts/integration-dynamic-backends-brig-index.sh integration/scripts/integration-dynamic-backends-db-schemas.sh integration/scripts/integration-dynamic-backends-s3.sh integration/scripts/integration-dynamic-backends-ses.sh integration/scripts/integration-dynamic-backends-vhosts.sh libs/http2-manager/test/resources/gen-certs.sh services/background-worker/fill_rabbit.sh services/background-worker/shutdown_test.sh services/federator/test/resources/unit/gen-certs.sh services/federator/test/resources/unit/gen-multidomain-certs.sh services/gen-aws-conf.sh services/proxy/test/scripts/proxy-test.sh tools/hlint.sh
INFO format | shellcheck: 46 file(s) processed in 602.981369ms
traversed 41030 files
emitted 41030 files for processing
formatted 46 files (0 changed) in 1.517s
I didn't want us to find weird problems in future so I did a a timeboxed stare at treefmt's code and found that this code basically ignores any file which doesn't have the mode 100644, which means it excludes executable files causing our shellscripts to not be found :facepalm:
Can we please not do the changes to treefmt args and instead do either of these things:
- Wait for https://github.com/numtide/treefmt/pull/389 to be merged and released and fly without treefmt for shell scripts until then
- Patch treefmt in nix with the bug fix
Thanks a lot for fixing all the shellscripts tho :heart:
I cut a new release, which includes the fix for this: https://github.com/numtide/treefmt/releases/tag/v2.0.5.
You can wait until it propagates through Hydra, or add treefmt as a source (looks like you're using niv) and use the tag directly in the short-term: https://nixpkgs-tracker.ocfox.me/?pr=336307
After rebasing on bumped nixpkgs we can use --walk=git to match all our scripts.
Randomly requested review from people I know that know bash