-
Notifications
You must be signed in to change notification settings - Fork 280
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
Avoid runtime exception caused by newer pathspec #548
Conversation
Hello @ssbarnea, thanks for the fix! Could you give a small example of a directory of YAML files that triggers the bug with pathspec 0.11.1? |
I will try to add a test for it. I was hoping you will forget to ask me about a test ;) |
The test might not be needed? It depends on what breaks. I'm curious about a simple example here, and we'll see whether a test is needed. |
@adrienverge I added the test, it is needed, if you remove the code change the test will fail with exception. It was a good call to ask me to add a test. |
As reported in #548, there might be a problem with pathspec 0.11.1 which does't allow calling `match_file()` with argument `None` anymore. The `linter.run()` function shouldn't call `YamlLintConfig.is_file_ignored(None)` anyway.
I'm sorry to ask again, but please, could you provide a real-life example of a directory structure + yamllint conf that causes a failure? The added test does't explain it, not why |
That happens for anyone trying to call
Sorry for having to mention it but, lack of typing support in yamllint combined with lack of testing for invalid arguments, can cause bugs like this. The linter is expected to allow linting content that does not come from a specific file (stdin), but once you have a config file that has an That is the kind of bug that can be prevented when using typing/mypy. I hope this helps explaining it. |
OK, so from my understanding this is not a bug with command-line tool The current version of this pull request still doesn't seem appropriate to me, checking that |
As reported in #548, there might be a problem with pathspec 0.11.1 which does't allow calling `match_file()` with argument `None` anymore. The `linter.run()` function shouldn't call `YamlLintConfig.is_file_ignored(None)` anyway.
If yesterday's linter fix 15f8204 is a solution (it looks like it is) then can we expect a new PyPI release soon? |
Hello @alanbchristie, I just released yamllint 1.30.0, which includes the fix 👍 |
Thanks - that's now fixed it! |
Some changes made in the minor pathspec update recently released break yamllint. This makes the code work with newer version too.
See: https://github.com/ansible/ansible-lint/actions/runs/4439000759/jobs/7790864220?pr=3194