Repository: jazzband/pip-tools
Pull request #1068 (2020-02-14 15:25:18)

Some cleanup
Some cleanup that was pulled out of a no-longer-useful branch I had been working on:

- Fixed a docstring
- Standardized on `key_from_ireq` for use with `InstallRequirement`s. It had been a mix of `key_from_ireq(ireq)` and `key_from_req(ireq.req)`.
- Ensured that a `finally` block can run even if `NamedTemporaryFile` construction raises

**Changelog-friendly one-liner**: Minor cleanup, including standardizing on key_from_ireq


Patch:
diff --git a/piptools/_compat/__init__.py b/piptools/_compat/__init__.py
index a26e531..e835b1e 100644
--- a/piptools/_compat/__init__.py
+++ b/piptools/_compat/__init__.py
@@ -28,6 +28,7 @@ from .pip_compat import (
     is_dir_url,
     is_file_url,
     is_vcs_url,
+    normalize_path,
     parse_requirements,
     path_to_url,
     stdlib_pkgs,
diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py
index 1e45d3a..f465c59 100644
--- a/piptools/_compat/pip_compat.py
+++ b/piptools/_compat/pip_compat.py
@@ -72,6 +72,7 @@ Link = do_import("models.link", "Link", old_path="index")
 Session = do_import("_vendor.requests.sessions", "Session")
 Resolver = do_import("legacy_resolve", "Resolver", old_path="resolve")
 WheelCache = do_import("cache", "WheelCache", old_path="wheel")
+normalize_path = do_import("utils.misc", "normalize_path", old_path="utils")
 
 # pip 18.1 has refactored InstallRequirement constructors use by pip-tools.
 if PIP_VERSION < (18, 1):
diff --git a/piptools/repositories/local.py b/piptools/repositories/local.py
index f389784..961bc0b 100644
--- a/piptools/repositories/local.py
+++ b/piptools/repositories/local.py
@@ -6,7 +6,7 @@ from contextlib import contextmanager
 from .._compat import FAVORITE_HASH
 from .base import BaseRepository
 
-from piptools.utils import as_tuple, key_from_req, make_install_requirement
+from piptools.utils import as_tuple, key_from_ireq, make_install_requirement
 
 
 def ireq_satisfied_by_existing_pin(ireq, existing_pin):
@@ -56,7 +56,7 @@ class LocalRequirementsRepository(BaseRepository):
         self.repository.freshen_build_caches()
 
     def find_best_match(self, ireq, prereleases=None):
-        key = key_from_req(ireq.req)
+        key = key_from_ireq(ireq)
         existing_pin = self.existing_pins.get(key)
         if existing_pin and ireq_satisfied_by_existing_pin(ireq, existing_pin):
             project, version, _ = as_tuple(existing_pin)
@@ -70,7 +70,7 @@ class LocalRequirementsRepository(BaseRepository):
         return self.repository.get_dependencies(ireq)
 
     def get_hashes(self, ireq):
-        key = key_from_req(ireq.req)
+        key = key_from_ireq(ireq)
         existing_pin = self.existing_pins.get(key)
         if existing_pin and ireq_satisfied_by_existing_pin(ireq, existing_pin):
             hashes = existing_pin.options.get("hashes", {})
diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py
index 2aa2a1d..d90a5ff 100644
--- a/piptools/repositories/pypi.py
+++ b/piptools/repositories/pypi.py
@@ -24,6 +24,7 @@ from .._compat import (
     is_dir_url,
     is_file_url,
     is_vcs_url,
+    normalize_path,
     path_to_url,
     url_to_path,
 )
@@ -62,6 +63,8 @@ class PyPIRepository(BaseRepository):
         # and pre) are deferred to pip.
         self.command = create_install_command()
         self.options, _ = self.command.parse_args(pip_args)
+        if self.options.cache_dir:
+            self.options.cache_dir = normalize_path(self.options.cache_dir)
 
         self.session = self.command._build_session(self.options)
         self.finder = self.command._build_package_finder(
@@ -81,7 +84,7 @@ class PyPIRepository(BaseRepository):
 
         # Setup file paths
         self.freshen_build_caches()
-        self._cache_dir = cache_dir
+        self._cache_dir = normalize_path(cache_dir)
         self._download_dir = fs_str(os.path.join(self._cache_dir, "pkgs"))
         self._wheel_download_dir = fs_str(os.path.join(self._cache_dir, "wheels"))
 
diff --git a/piptools/resolver.py b/piptools/resolver.py
index 4cf3f2a..6cfcd93 100644
--- a/piptools/resolver.py
+++ b/piptools/resolver.py
@@ -17,7 +17,6 @@ from .utils import (
     is_pinned_requirement,
     is_url_requirement,
     key_from_ireq,
-    key_from_req,
 )
 
 green = partial(click.style, fg="green")
@@ -31,7 +30,7 @@ class RequirementSummary(object):
 
     def __init__(self, ireq):
         self.req = ireq.req
-        self.key = key_from_req(ireq.req)
+        self.key = key_from_ireq(ireq)
         self.extras = str(sorted(ireq.extras))
         self.specifier = str(ireq.specifier)
 
@@ -274,12 +273,10 @@ class Resolver(object):
         if has_changed:
             log.debug("")
             log.debug("New dependencies found in this round:")
-            for new_dependency in sorted(diff, key=lambda req: key_from_req(req.req)):
+            for new_dependency in sorted(diff, key=key_from_ireq):
                 log.debug("  adding {}".format(new_dependency))
             log.debug("Removed dependencies in this round:")
-            for removed_dependency in sorted(
-                removed, key=lambda req: key_from_req(req.req)
-            ):
+            for removed_dependency in sorted(removed, key=key_from_ireq):
                 log.debug("  removing {}".format(removed_dependency))
 
         # Store the last round's results in the their_constraints
diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py
index 1f5bbeb..39aaf3d 100755
--- a/piptools/scripts/compile.py
+++ b/piptools/scripts/compile.py
@@ -22,7 +22,6 @@ from ..utils import (
     get_trusted_hosts,
     is_pinned_requirement,
     key_from_ireq,
-    key_from_req,
 )
 from ..writer import OutputWriter
 
@@ -279,7 +278,7 @@ def cli(
     # Parse all constraints coming from --upgrade-package/-P
     upgrade_reqs_gen = (install_req_from_line(pkg) for pkg in upgrade_packages)
     upgrade_install_reqs = {
-        key_from_req(install_req.req): install_req for install_req in upgrade_reqs_gen
+        key_from_ireq(install_req): install_req for install_req in upgrade_reqs_gen
     }
 
     existing_pins_to_upgrade = set()
@@ -303,7 +302,7 @@ def cli(
         # constraints, and separately gather pins to be upgraded
         existing_pins = {}
         for ireq in filter(is_pinned_requirement, ireqs):
-            key = key_from_req(ireq.req)
+            key = key_from_ireq(ireq)
             if key in upgrade_install_reqs:
                 existing_pins_to_upgrade.add(key)
             else:
diff --git a/piptools/utils.py b/piptools/utils.py
index 1d807d9..463027b 100644
--- a/piptools/utils.py
+++ b/piptools/utils.py
@@ -137,7 +137,7 @@ def as_tuple(ireq):
     if not is_pinned_requirement(ireq):
         raise TypeError("Expected a pinned InstallRequirement, got {}".format(ireq))
 
-    name = key_from_req(ireq.req)
+    name = key_from_ireq(ireq)
     version = next(iter(ireq.specifier._specs))._spec[1]
     extras = tuple(sorted(ireq.extras))
     return name, version, extras
@@ -186,7 +186,7 @@ def lookup_table(values, key=None, keyval=None, unique=False, use_lists=False):
     ...     'q': ['qux', 'quux']
     ... }
 
-    The values of the resulting lookup table will be values, not sets.
+    The values of the resulting lookup table will be lists, not sets.
 
     For extra power, you can even change the values while building up the LUT.
     To do so, use the `keyval` function instead of the `key` arg:


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 54fb147..709c221 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -25,6 +25,7 @@ from piptools.resolver import Resolver
 from piptools.utils import (
     as_tuple,
     is_url_requirement,
+    key_from_ireq,
     key_from_req,
     make_install_requirement,
 )
@@ -51,21 +52,18 @@ class FakeRepository(BaseRepository):
 
         versions = list(
             ireq.specifier.filter(
-                self.index[key_from_req(ireq.req)], prereleases=prereleases
+                self.index[key_from_ireq(ireq)], prereleases=prereleases
             )
         )
         if not versions:
             tried_versions = [
                 InstallationCandidate(ireq.name, version, "https://fake.url.foo")
-                for version in self.index[key_from_req(ireq.req)]
+                for version in self.index[key_from_ireq(ireq)]
             ]
             raise NoCandidateFound(ireq, tried_versions, ["https://fake.url.foo"])
         best_version = max(versions, key=Version)
         return make_install_requirement(
-            key_from_req(ireq.req),
-            best_version,
-            ireq.extras,
-            constraint=ireq.constraint,
+            key_from_ireq(ireq), best_version, ireq.extras, constraint=ireq.constraint
         )
 
     def get_dependencies(self, ireq):
diff --git a/tests/test_cache.py b/tests/test_cache.py
index 6092424..eef516c 100644
--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -17,9 +17,9 @@ def _read_cache_file_helper(to_write):
     :param str to_write: the content to write to the file
     :yield: the path to the temporary file
     """
+    # Create the file and write to it
+    cache_file = NamedTemporaryFile(mode="w", delete=False)
     try:
-        # Create the file and write to it
-        cache_file = NamedTemporaryFile(mode="w", delete=False)
         cache_file.write(to_write)
         cache_file.close()
 
diff --git a/tests/test_repository_pypi.py b/tests/test_repository_pypi.py
index 14ba104..45c4aba 100644
--- a/tests/test_repository_pypi.py
+++ b/tests/test_repository_pypi.py
@@ -1,8 +1,11 @@
+import os
+
 import mock
 import pytest
 
 from piptools._compat import PackageFinder
 from piptools._compat.pip_compat import PIP_VERSION, Link, Session, path_to_url
+from piptools.repositories import PyPIRepository
 from piptools.repositories.pypi import open_local_or_remote_file
 
 
@@ -176,3 +179,29 @@ def test_wheel_cache_cleanup_called(
     ireq = from_line("six==1.10.0")
     pypi_repository.get_dependencies(ireq)
     WheelCache.return_value.cleanup.assert_called_once_with()
+
+
+def test_relative_path_cache_dir_is_normalized(from_line):
+    relative_cache_dir = "pypi-repo-cache"
+    pypi_repository = PyPIRepository([], cache_dir=relative_cache_dir)
+
+    assert os.path.isabs(pypi_repository._cache_dir)
+    assert pypi_repository._cache_dir.endswith(relative_cache_dir)
+
+
+def test_relative_path_pip_cache_dir_is_normalized(from_line, tmpdir):
+    relative_cache_dir = "pip-cache"
+    pypi_repository = PyPIRepository(
+        ["--cache-dir", relative_cache_dir], cache_dir=str(tmpdir / "pypi-repo-cache")
+    )
+
+    assert os.path.isabs(pypi_repository.options.cache_dir)
+    assert pypi_repository.options.cache_dir.endswith(relative_cache_dir)
+
+
+def test_pip_cache_dir_is_empty(from_line, tmpdir):
+    pypi_repository = PyPIRepository(
+        ["--no-cache-dir"], cache_dir=str(tmpdir / "pypi-repo-cache")
+    )
+
+    assert not pypi_repository.options.cache_dir