search

jumanjihouse / pre-commit-hooks

git pre-commit hooks that work with http://pre-commit.com/
MIT License
114 stars 52 forks source link

Fix shellcheck hook by removing additional dependencies #81

Closed vgautam-dialpad closed 3 years ago

vgautam-dialpad commented 3 years ago

The shellcheck hook breaks with the latest version of pre-commit (2.10.0) because of pre-commit/pre-commit#1771, which errors out when you use language: script and also specify additional dependencies.

From the pre-commit docs on script:

This hook type will not be given a virtual environment to work with – if it needs additional dependencies the consumer must install them manually.

I've also added an error message if shellcheck isn't locally installed, to match what already exists for the shfmt hook (https://github.com/jumanjihouse/pre-commit-hooks/blob/master/pre_commit_hooks/shfmt#L13-L16)

[The CI failure is because of ruby + rubocop stuff and is unrelated to this PR - shellcheck passes]

avdv commented 3 years ago

Same problem here, CI suddenly failed this morning...

To mitigate, either downgrade pre-commit to <2.10, or override the additional_dependencies in your .pre-commit-config.yaml:

  - repo: http://github.com/jumanjihouse/pre-commit-hooks
    rev: 2.1.4
    hooks:
      - id: shellcheck
        additional_dependencies: []
askb commented 3 years ago

LGTM, team can we get a review on this please.

Look for any errors in the above output or in docs/_build/linkcheck/output.txt
pre-commit installed: appdirs==1.4.4,cfgv==3.2.0,distlib==0.3.1,filelock==3.0.12,identify==1.5.13,importlib-metadata==3.4.0,nodeenv==1.5.0,pre-commit==2.10.1,PyYAML==5.4.1,six==1.15.0,toml==0.10.2,typing-extensions==3.7.4.3,virtualenv==20.4.2,zipp==3.4.0
pre-commit run-test-pre: PYTHONHASHSEED='491141206'
pre-commit run-test: commands[0] | pre-commit run --all-files --show-diff-on-failure
[ERROR] The hook `shellcheck` specifies `additional_dependencies` but is using language `script` which does not install an environment.  Perhaps you meant to use a specific language?
ERROR: InvocationError for command /home/user/git/common-packer/.tox/pre-commit/bin/pre-commit run --all-files --show-diff-on-failure (exited with code 1)
__________________________________________________________________________________________________________________ summary ___________________________________________________________________________________________________________________
  docs: commands succeeded
  docs-linkcheck: commands succeeded
ERROR:   pre-commit: commands failed
jumanjiman commented 3 years ago

Thanks everybody!

  1. I merged the PR to update rubocop (and was causing this PR to fail), then
  2. rebased this PR on master and opened https://github.com/jumanjihouse/pre-commit-hooks/pull/85
  3. merged 85

(apologies that i didn't see this before...i missed the notification in email for it)

jumanjiman commented 3 years ago

tag 2.1.5 has the fixes