Remove unnecessary system calls via os.path.exists()/os.path.isdir()

Rather than checking a file exists, then reading it, simply read it and
handle the potential ENOENT OS error. This reduces the number of system
calls from:

    1. check file exists
    2. read file

To:

    1. read file

Reducing system calls is an optimization.

More importantly, this also avoids race conditions that were previously
not handled. If the file was deleted after the existence check but
before reading it, an unhandled FileNotFound error would be raised.

This approach better follows Python pattern "Easier to ask for
forgiveness than permission" rather than the "Look before you leap"
approach. See: https://docs.python.org/3/glossary.html#term-eafp

The same pattern applies to os.makedirs(). A compatibility shim was
added to allow using the exist_ok parameter on Python 2.

piptools/_compat/__init__.py

@@ -2,6 +2,9 @@
 # flake8: noqa
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import errno
+import os
+
 from pip._vendor import six
 
 from .pip_compat import PIP_VERSION, parse_requirements
@@ -10,3 +13,14 @@
     from .tempfile import TemporaryDirectory
 else:
     from tempfile import TemporaryDirectory
+
+
+def makedirs(name, mode=0o777, exist_ok=False):
+    if six.PY2:
+        try:
+            os.makedirs(name, mode)
+        except OSError as e:
+            if not exist_ok or e.errno != errno.EEXIST:
+                raise
+    else:
+        os.makedirs(name, mode, exist_ok)

piptools/cache.py

@@ -1,13 +1,15 @@
 # coding: utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import errno
 import json
 import os
 import platform
 import sys
 
 from pip._vendor.packaging.requirements import Requirement
 
+from ._compat import makedirs
 from .exceptions import PipToolsError
 from .utils import as_tuple, key_from_req, lookup_table
 
@@ -62,8 +64,7 @@ class DependencyCache(object):
     """
 
     def __init__(self, cache_dir):
-        if not os.path.isdir(cache_dir):
-            os.makedirs(cache_dir)
+        makedirs(cache_dir, exist_ok=True)
         cache_filename = "depcache-{}.json".format(_implementation_name())
 
         self._cache_file = os.path.join(cache_dir, cache_filename)
@@ -101,9 +102,11 @@ def as_cache_key(self, ireq):
 
     def read_cache(self):
         """Reads the cached contents into memory."""
-        if os.path.exists(self._cache_file):
+        try:
             self._cache = read_cache_file(self._cache_file)
-        else:
+        except IOError as e:
+            if e.errno != errno.ENOENT:
+                raise
             self._cache = {}
 
     def write_cache(self):

piptools/repositories/pypi.py

@@ -24,7 +24,7 @@
 from pip._internal.utils.urls import path_to_url, url_to_path
 from pip._vendor.requests import RequestException
 
-from .._compat import PIP_VERSION, TemporaryDirectory, contextlib
+from .._compat import PIP_VERSION, TemporaryDirectory, contextlib, makedirs
 from ..click import progressbar
 from ..exceptions import NoCandidateFound
 from ..logging import log
@@ -238,12 +238,9 @@ def get_dependencies(self, ireq):
                 download_dir = None
             else:
                 download_dir = self._get_download_path(ireq)
-                if not os.path.isdir(download_dir):
-                    os.makedirs(download_dir)
-            if PIP_VERSION[:2] <= (20, 2) and not os.path.isdir(
-                self._wheel_download_dir
-            ):
-                os.makedirs(self._wheel_download_dir)
+                makedirs(download_dir, exist_ok=True)
+            if PIP_VERSION[:2] <= (20, 2):
+                makedirs(self._wheel_download_dir, exist_ok=True)
 
             with global_tempdir_manager():
                 wheel_cache = WheelCache(self._cache_dir, self.options.format_control)

tests/test_cache.py

@@ -1,5 +1,6 @@
+import os
+import sys
 from contextlib import contextmanager
-from os import remove
 from shutil import rmtree
 from tempfile import NamedTemporaryFile
 
@@ -28,7 +29,7 @@ def _read_cache_file_helper(to_write):
 
     finally:
         # Delete the file on exit
-        remove(cache_file.name)
+        os.remove(cache_file.name)
 
 
 def test_read_cache_file_not_json():
@@ -62,6 +63,22 @@ def test_read_cache_file_successful():
         assert "success" == read_cache_file(cache_file_name)
 
 
+def test_read_cache_does_not_exist(tmpdir):
+    cache = DependencyCache(cache_dir=str(tmpdir))
+    assert cache.cache == {}
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32", reason="os.fchmod() not available on Windows"
+)
+def test_read_cache_permission_error(tmpdir):
+    cache = DependencyCache(cache_dir=str(tmpdir))
+    with open(cache._cache_file, "w") as fp:
+        os.fchmod(fp.fileno(), 0o000)
+    with pytest.raises(IOError, match="Permission denied"):
+        cache.read_cache()
+
+
 def test_reverse_dependencies(from_line, tmpdir):
     # Since this is a test, make a temporary directory. Converting to str from py.path.
     tmp_dir_path = str(tmpdir)

tests/test_compat.py

@@ -0,0 +1,21 @@
+import os
+
+import pytest
+
+from piptools._compat import makedirs
+
+
+def test_makedirs_exist_ok_true(tmpdir):
+    path = str(tmpdir / "test")
+    makedirs(path, exist_ok=True)
+    assert os.path.isdir(path)
+    makedirs(path, exist_ok=True)
+    assert os.path.isdir(path)
+
+
+def test_makedirs_exist_ok_false(tmpdir):
+    path = str(tmpdir / "test")
+    makedirs(path)
+    assert os.path.isdir(path)
+    with pytest.raises(OSError, match="exists"):
+        makedirs(path)