sk- / git-lint

improving source code one step at a time
Apache License 2.0
237 stars 78 forks source link

If a cache directory already exists then git-lint stack traces. #117

Closed rawrgulmuffins closed 6 years ago

rawrgulmuffins commented 6 years ago

Current Behavior

Traceback (most recent call last):
  File "/opt/anaconda/envs/jenkins/bin/git-lint", line 21, in <module>
    sys.exit(gitlint.main(sys.argv))
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/gitlint/__init__.py", line 249, in main
    filename in sorted(modified_files.keys())]):
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/concurrent/futures/_base.py", line 641, in result_iterator
    yield fs.pop().result()
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/concurrent/futures/_base.py", line 455, in result
    return self.__get_result()
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/concurrent/futures/thread.py", line 63, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/gitlint/__init__.py", line 183, in process_file
    filename, modified_lines, gitlint_config)
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/gitlint/linters.py", line 188, in lint
    linter_output = linter(filename, lines)
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/gitlint/linters.py", line 93, in lint_command
    utils.save_output_in_cache(name, filename, output)
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/gitlint/utils.py", line 113, in save_output_in_cache
    with _open_for_write(cache_filename) as f:
  File "/opt/anaconda/envs/jenkins/lib/python2.7/site-packages/gitlint/utils.py", line 68, in _open_for_write
    os.makedirs(dirname)
  File "/opt/anaconda/envs/jenkins/lib/python2.7/os.py", line 157, in makedirs
    mkdir(name, mode)
OSError: [Errno 17] File exists: '/var/lib/jenkins/.git-lint/cache/pylint/var/lib/jenkins/workspace/build_apps/apps_repo/crepes/apt/apt/utils'

Expected Behavior

Git lint should skip then creation step

git-lint version

$ git lint --version
git-lint v0.0.9
$ git --version
git version 2.16.2
sk- commented 6 years ago

Thanks Alex for the bug report. I think this may be related to the change to execute multiple linters in parallel. So, it could be a race condition in which two linters are trying to save some output.

In python 3 os.makedirs accepts the argument exist_ok, but that's not supported in python 2.7.

Probably the cleanest solution would be to use pathlib2 as explained in this answer https://stackoverflow.com/a/45547008/1413687.

rawrgulmuffins commented 6 years ago

Thanks for the quick reply. Also, yeah we're still using 2.7. =(

Another alternative is to catch the exception at this line rather than doing an if check. No race condition that way.

I'll go submit a PR for it

Edit: Should have read the SO link before making a PR. =P

I'll go use Pathlib instead of OSError catching.

rawrgulmuffins commented 6 years ago

@sk- Added the Pathlib solution you suggested.