adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.79k stars 265 forks source link

1.35.0 broke the "ignore" feature #657

Open baurmatt opened 5 months ago

baurmatt commented 5 months ago

Hey,

first of all: thanks for this project! Basically all of our CI pipelines use it! :)

Sadly we found that the latest (1.35.0) release broken the "Ignoring paths" feature which we use to get some of your pipeline green :) I've created a reproduction case in the hope that this helps to narrow down the problem:

$ mkdir bar
$ cd bar/
$ echo -e "extends: default\n\nignore: | \n test.yaml" > .yamllint
$ rm .yamllint
$ echo "foo: bar"> test.yaml
$ python3 -m venv .venv
$ source .venv/bin/activate
$ pip3 install yamllint==1.34.0
Collecting yamllint==1.34.0
  Using cached yamllint-1.34.0-py3-none-any.whl.metadata (4.2 kB)
Collecting pathspec>=0.5.3 (from yamllint==1.34.0)
  Using cached pathspec-0.12.1-py3-none-any.whl.metadata (21 kB)
Collecting pyyaml (from yamllint==1.34.0)
  Using cached PyYAML-6.0.1-cp311-cp311-macosx_11_0_arm64.whl.metadata (2.1 kB)
Using cached yamllint-1.34.0-py3-none-any.whl (66 kB)
Using cached pathspec-0.12.1-py3-none-any.whl (31 kB)
Using cached PyYAML-6.0.1-cp311-cp311-macosx_11_0_arm64.whl (167 kB)
Installing collected packages: pyyaml, pathspec, yamllint
Successfully installed pathspec-0.12.1 pyyaml-6.0.1 yamllint-1.34.0

[notice] A new release of pip is available: 23.3.1 -> 24.0
[notice] To update, run: pip install --upgrade pip
$ yamllint --version
yamllint 1.34.0
$ yamllint test.yaml
test.yaml
  1:1       warning  missing document start "---"  (document-start)
$ echo -e "extends: default\n\nignore: | \n test.yaml" > .yamllint
$ yamllint test.yaml
$ pip3 install yamllint==1.35.0
Collecting yamllint==1.35.0
  Using cached yamllint-1.35.0-py3-none-any.whl.metadata (4.2 kB)
Requirement already satisfied: pathspec>=0.5.3 in ./.venv/lib/python3.11/site-packages (from yamllint==1.35.0) (0.12.1)
Requirement already satisfied: pyyaml in ./.venv/lib/python3.11/site-packages (from yamllint==1.35.0) (6.0.1)
Using cached yamllint-1.35.0-py3-none-any.whl (66 kB)
Installing collected packages: yamllint
  Attempting uninstall: yamllint
    Found existing installation: yamllint 1.34.0
    Uninstalling yamllint-1.34.0:
      Successfully uninstalled yamllint-1.34.0
Successfully installed yamllint-1.35.0

[notice] A new release of pip is available: 23.3.1 -> 24.0
[notice] To update, run: pip install --upgrade pip
$ yamllint --version
yamllint 1.35.0
$ yamllint test.yaml
test.yaml
  1:1       warning  missing document start "---"  (document-start)

Thanks!

Regards, Matthias

adrienverge commented 5 months ago

Hello Matthias, thanks for the clear explanation, with a reproducer :+1: (I think you added rm .yamllint by mistake)

With this configuration:

extends: default
ignore: test.yaml

test.yaml is correctly ignored when running yamllint in "file discovery" mode, e.g. yamllint .. But it is debatable that test.yaml should be ignored when it is passed explicitely as a cli argument, e.g. yamllint test.yaml. https://github.com/adrienverge/yamllint/pull/654 fixed a strange behavior about symbolic links, and this one at the same time.

May I ask how you encounter the problem? How you call yamllint? With the failing YAML file passed as an explicit argument?

I'm not sure whether the old or the new behavior is better. On the one hand, it doesn't make sense to ignore a file that is explicitely passed as argument to the command. But on the other hand, doing is more consistent between how the global ignore and the per-rule ignore work:

extends: default
ignore: test.yaml
rules:
  key-duplicates:
    ignore: test.yaml

(in the above example, key-duplicates will ignore test.yaml, whether it's linted with yamllint . or yamllint test.yaml)

chuismiguel commented 5 months ago

Same problem here. This .yamllint code block

ignore: |
  .clang-format
  .clang-tidy
  .yamllint

Is not ignoring the files correctly, as in my pre-commit run, the file is not ignored:

Checking and linting yaml files internals................................Failed

.clang-format 52:141 error line too long (694 > 140 characters) (line-length) 55:141 error line too long (767 > 140 characters) (line-length)

Duxy1996 commented 5 months ago

I'm having the same problem. I got a clang-format line, which exceeds 140 characters. It should be ignored by the yamllint. But it is not ignored in my Jenkins, and it is braking the build.

ignore: | .clang-format .clang-tidy .yamllint

The version working version is: 1.31

adrienverge commented 5 months ago

Luis, Carlos, same question: how do you encounter the problem? How do you call yamllint? With the failing YAML file passed as an explicit argument?

hardoverflow commented 5 months ago

Hi Adrien,

we currently use the following call for yamllint that is explict:

find . -type f \( -name '*\.yaml' -or -name '*\.yml' \) -and -not \( -path './*templates/*' -or -name '*helmfile.yaml' \) -print0 | xargs -0 yamllint

/cc @baurmatt

Duxy1996 commented 5 months ago

@adrienverge We got a pipline on a Jenkins which uses pre-commit. On the pre-commit config we got this section:

[options]
packages = find:
install_requires =
    mdformat-myst>=0.1.5
    yamlfix>=1.9.0
    yamllint>=1.31.0
    cmakelang>=0.6.13
python_requires = >=3.8

Each time that the Jenkins is execute it clears the pre-commit and updates the hooks. This morning was failing and we have fixied the problem doing this:

packages = find:
install_requires =
    mdformat-myst>=0.1.5
    yamlfix>=1.9.0
    yamllint==1.31.0
    cmakelang>=0.6.13
python_requires = >=3.8
adrienverge commented 5 months ago

@hardoverflow thanks, this is indeed a use-case that seems legitimate. I created https://github.com/adrienverge/yamllint/pull/658 to restore the previous behavior, and released it in yamllint version 1.35.1.

hardoverflow commented 5 months ago

Thanks for the quick response and the bug fix. Thanks also for such a great tool! From my point of view, the issue can be closed. The bugfix works as expected.