sds / overcommit

A fully configurable and extendable Git hook manager
MIT License
3.9k stars 279 forks source link

YamlLint seems to issue only warnings, not errors #737

Closed Gui13 closed 3 years ago

Gui13 commented 3 years ago

I have activated yamllint in overcommit:


PreCommit:
  YamlLint:
    enabled: true

Running it on a definitely bad file still continues to the commit message, although it clearly returns an exit code of 1 and an error:

image
maintenance/1.7$ OVERCOMMIT_DEBUG=1 overcommit -r 
git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "xxx\n"
STDERR: ""
git config --local --get overcommit.configuration.signature
EXIT STATUS: 0
STDOUT: "zzz\n"
STDERR: ""
git config --local --get overcommit.configuration.signature
EXIT STATUS: 0
STDOUT: "zzz\n"
STDERR: ""
Running pre-commit hooks
git config --get user.name
git config --get user.email
EXIT STATUS: 0
STDOUT: "Guillaume B\n"
STDERR: ""
EXIT STATUS: 0
STDOUT: "xxx@yyy.com\n"
STDERR: ""
git rev-parse HEAD
grep -IHn ^<<<<<<<[     ] ... (1606 splittable args)
yamllint --format=parsable --strict ... (749 splittable args)
EXIT STATUS: 0
STDOUT: "zzz\n"
STDERR: ""
git ls-tree --name-only HEAD ... (529 splittable args)
EXIT STATUS: 0
STDOUT: ".ansible-lint\n.gitignore\n.gitlab-ci.yml\n.yamllint\nCHANGELOG.md\nREADME.md\nyadda yadda\n"
STDERR: ""
EXIT STATUS: 1
STDOUT: ""
STDERR: ""
EXIT STATUS: 1
STDOUT: "/xxx/rp/tasks/main.yml:3:9: [error] syntax error: mapping values are not allowed here\n"
STDERR: ""
Analyze with YAMLlint......................................[YamlLint] WARNING
/xxx/rp/tasks/main.yml:3:9: [error] syntax error: mapping values are not allowed here

⚠ All pre-commit hooks passed, but with warnings

YamlSyntax correctly chokes on my malformed Yaml, but I expected YamlLint to do so too.

This is my malformed Yaml (this is an ansible playbook):

---
- name: Ensure directories exists
    file: ## this block is indented although it shouldnt
      state: directory
      owner: root
      group: www-data
      mode: 0750
      dest: "{{ item }}"
    with_items:
      - /etc/nginx/scripts/
      - /etc/nginx/common/
      - /etc/nginx/conf.d/
Gui13 commented 3 years ago

All right I found the issue:

https://github.com/sds/overcommit/blob/master/lib/overcommit/hook/pre_commit/yaml_lint.rb#L14

This should probably look like

return [ :error, result.stdout]