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

Fix reading from a closed stdin buffer #36

Merged
merged 1 commit into from
Oct 12, 2019
Merged

Conversation

seporaitis
Copy link
Contributor

@seporaitis seporaitis commented Mar 12, 2019

Summary: Couple of flake8 plugins rely on pycodestyle.stdin_get_value, which does not cache the string, after reading it from stdin and closing the buffer. This means that if there are two plugins using this function - the second plugin relying on this function, cannot read from stdin, because it was closed, and ValueError: I/O operation on closed file gets raised.

Fix: use flake8.utils.stdin_get_value, which caches the stdin, so that multiple plugins can use it. It is based on flake8-commas that does not seem to have this issue.

Steps to reproduce:

requirements.txt:

flake8==3.7.6
flake8-tuple==0.2.13
flake8-print==3.1.0

Run:

python -m flake8 - < path/to/python/file.py

Expected output: lint successfully.
Actual output:

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.2_2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.2_2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/__main__.py", line 4, in <module>
    cli.main()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/cli.py", line 18, in main
    app.run(argv)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/application.py", line 394, in run
    self._run(argv)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/application.py", line 382, in _run
    self.run_checks()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/main/application.py", line 301, in run_checks
    self.file_checker_manager.run()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 330, in run
    self.run_serial()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 314, in run_serial
    checker.run_checks()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 608, in run_checks
    self.run_ast_checks()
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8/checker.py", line 504, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8_tuple.py", line 48, in run
    lines = get_lines(self.filename)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/flake8_tuple.py", line 33, in get_lines
    return pep8.stdin_get_value().splitlines(True)
  File "/Users/julius/Work/workenv/lib/python3.7/site-packages/pycodestyle.py", line 1726, in stdin_get_value
    return TextIOWrapper(sys.stdin.buffer, errors='ignore').read()
ValueError: I/O operation on closed file

Additional comments:

In my environment this error got triggered in flake8-print and flake8-tuple. I am simultaneously doing a similar fix to flake8-tuple as well. However, there must be a third plugin that uses pycodestyle.stdin_get_value first time. I will investigate and do a similar fix as well.

I bumped version up as it would be great to release this fix quickly - it would save my eyes from emacs+flycheck blowing up with an error taking half the buffer on every file save. :)

Let me know if I can improve this to expedite the release.

Thank you.

@adamchainz
Copy link
Contributor

Presumably the try/except ImportError is due to the function moving between different versions of flake8?

@seporaitis
Copy link
Contributor Author

If understand correctly this pull request on flake8-commas the answer is yes.

@seporaitis
Copy link
Contributor Author

@JBKahn is there anything I could do to improve this PR so it is easier for you to merge it?

@seporaitis
Copy link
Contributor Author

@JBKahn ping

1 similar comment
@nndii
Copy link

nndii commented Sep 24, 2019

@JBKahn ping

@JBKahn JBKahn merged commit d35957c into JBKahn:master Oct 12, 2019
@JBKahn
Copy link
Owner

JBKahn commented Oct 12, 2019

Sorry it took so long but this didn't pass the tests and I had no time to really look at it @nndii . I have to at least ensure tests pass before merging.

In this case, the tests weren't passing with older versions of flake8 that I'm willing to simply stop supporting. Thank you for your patience and thanks for the PR @seporaitis

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

Successfully merging this pull request may close these issues.

4 participants