Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.35.0 broke the "ignore" feature #657

Open
baurmatt opened this issue Feb 16, 2024 · 8 comments
Open

1.35.0 broke the "ignore" feature #657

baurmatt opened this issue Feb 16, 2024 · 8 comments

Comments

@baurmatt
Copy link

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

@baurmatt baurmatt changed the title 1.35.0 broken the "ignore" feature 1.35.0 broke the "ignore" feature Feb 16, 2024
@adrienverge
Copy link
Owner

Hello Matthias, thanks for the clear explanation, with a reproducer 👍
(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. #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
Copy link

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

  • hook id: check-yaml-lint
  • exit code: 1

.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
Copy link

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
Copy link
Owner

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
Copy link

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
Copy link

@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 added a commit that referenced this issue Feb 16, 2024
Commit 2344380 "Cleanly skip broken symlinks that are ignored" fixed a
problem on symbolic links but also introduced a change on how ignored
files should be treated, depending on whether they're explicitely passed
as command-line arguments or not [^1].

This change is annoying for users that dynamically build the list of
files to pass as arguments, e.g. [^2]:

    find -name '*\.yaml' | xargs yamllint

The present commit adds unit tests for `yamllint [FILES]...` and
`yamllint --list-files [FILES]...`, that passed with previous version
1.34.0, and restore the behavior of this version.

As a result it also reverts the API change of commit 2344380 on
`yamllint.linter.run(stream, config)`.

[^1]: #657 (comment)
[^2]: #657 (comment)
adrienverge added a commit that referenced this issue Feb 16, 2024
Commit 2344380 "Cleanly skip broken symlinks that are ignored" fixed a
problem on symbolic links but also introduced a change on how ignored
files should be treated, depending on whether they're explicitely passed
as command-line arguments or not [^1].

This change is annoying for users that dynamically build the list of
files to pass as arguments, e.g. [^2]:

    find -name '*\.yaml' | xargs yamllint

The present commit adds unit tests for `yamllint [FILES]...` and
`yamllint --list-files [FILES]...`, that passed with previous version
1.34.0, and restore the behavior of this version.

As a result it also reverts the API change of commit 2344380 on
`yamllint.linter.run(stream, config)`.

[^1]: #657 (comment)
[^2]: #657 (comment)
@adrienverge
Copy link
Owner

@hardoverflow thanks, this is indeed a use-case that seems legitimate. I created #658 to restore the previous behavior, and released it in yamllint version 1.35.1.

@hardoverflow
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants