Avoid URL requirement name collisions (#1145)

piptools/repositories/pypi.py

@@ -215,7 +215,7 @@ def get_dependencies(self, ireq):
                 # using git-checkout-index, which gets rid of the .git dir.
                 download_dir = None
             else:
-                download_dir = self._download_dir
+                download_dir = self._get_download_path(ireq)
                 if not os.path.isdir(download_dir):
                     os.makedirs(download_dir)
             if not os.path.isdir(self._wheel_download_dir):
@@ -288,6 +288,20 @@ def _get_project(self, ireq):
             return data
         return None
 
+    def _get_download_path(self, ireq):
+        """
+        Determine the download dir location in a way which avoids name
+        collisions.
+        """
+        if ireq.link:
+            salt = hashlib.sha224(ireq.link.url_without_fragment.encode()).hexdigest()
+            # Nest directories to avoid running out of top level dirs on some FS
+            # (see pypi _get_cache_path_parts, which inspired this)
+            salt = [salt[:2], salt[2:4], salt[4:6], salt[6:]]
+            return os.path.join(self._download_dir, *salt)
+        else:
+            return self._download_dir
+
     def get_hashes(self, ireq):
         """
         Given an InstallRequirement, return a set of hashes that represent all
@@ -308,7 +322,7 @@ def get_hashes(self, ireq):
                 # Directly hash URL requirements.
                 # URL requirements may have been previously downloaded and cached
                 # locally by self.resolve_reqs()
-                cached_path = os.path.join(self._download_dir, link.filename)
+                cached_path = os.path.join(self._get_download_path(ireq), link.filename)
                 if os.path.exists(cached_path):
                     cached_link = Link(path_to_url(cached_path))
                 else:

tests/conftest.py

@@ -272,8 +272,16 @@ def run_setup_file():
     """
 
     def _make_wheel(package_dir_path, *args):
+        old_dir = os.getcwd()
+        # Without changing the dir, manual runs of pytest in the pip-tools
+        # repo will result in sdists with unexpected content
+        os.chdir(package_dir_path)
+
         setup_file = str(package_dir_path / "setup.py")
-        return check_call((sys.executable, setup_file) + args)  # nosec
+        result = check_call((sys.executable, setup_file) + args)  # nosec
+
+        os.chdir(old_dir)
+        return result
 
     return _make_wheel
 

tests/test_repository_pypi.py

@@ -347,3 +347,49 @@ def mock_get(*args, **kwargs):
 
     actual_data = pypi_repository._get_project(ireq)
     assert actual_data is None
+
+
+def test_name_collision(from_line, pypi_repository, make_package, make_sdist, tmpdir):
+    """
+    Test to ensure we don't fail if there are multiple URL-based requirements
+    ending with the same filename where later ones depend on earlier, e.g.
+    https://git.example.com/requirement1/master.zip#egg=req_package_1
+    https://git.example.com/requirement2/master.zip#egg=req_package_2
+    In this case, if req_package_2 depends on req_package_1 we don't want to
+    fail due to issues such as caching the requirement based on filename.
+    """
+    packages = {
+        "test_package_1": make_package("test_package_1", version="0.1"),
+        "test_package_2": make_package(
+            "test_package_2", version="0.1", install_requires=["test-package-1"]
+        ),
+    }
+
+    for pkg_name, pkg in packages.items():
+        pkg_path = os.path.join(tmpdir, pkg_name)
+        os.mkdir(pkg_path)
+
+        make_sdist(pkg, pkg_path, "--formats=zip")
+
+        os.rename(
+            os.path.join(pkg_path, "{}-{}.zip".format(pkg_name, "0.1")),
+            os.path.join(pkg_path, "master.zip"),
+        )
+
+    name_collision_1 = (
+        "file://"
+        + str(os.path.join(tmpdir, "test_package_1", "master.zip"))
+        + "#egg=test_package_1"
+    )
+    ireq = from_line(name_collision_1)
+    pypi_repository.get_dependencies(ireq)
+
+    name_collision_2 = (
+        "file://"
+        + str(os.path.join(tmpdir, "test_package_2", "master.zip"))
+        + "#egg=test_package_2"
+    )
+    ireq = from_line(name_collision_2)
+    reqs = pypi_repository.get_dependencies(ireq)
+    assert len(reqs) == 1
+    assert reqs.pop().req.name == "test-package-1"