Avoid URL requirement name collisions (#1149)

Co-Authored-By: geokala <geokala@users.noreply.github.com>
Co-Authored-By: Albert Tugushev <albert@tugushev.ru>

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

@@ -271,11 +271,13 @@ def run_setup_file():
     Run a setup.py file from a given package dir.
     """
 
-    def _make_wheel(package_dir_path, *args):
+    def _run_setup_file(package_dir_path, *args):
         setup_file = str(package_dir_path / "setup.py")
-        return check_call((sys.executable, setup_file) + args)  # nosec
+        return check_call(
+            (sys.executable, setup_file) + args, cwd=str(package_dir_path)
+        )  # nosec
 
-    return _make_wheel
+    return _run_setup_file
 
 
 @pytest.fixture

tests/test_repository_pypi.py

@@ -347,3 +347,45 @@ 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 = str(tmpdir / pkg_name)
+
+        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://{dist_path}#egg=test_package_1".format(
+        dist_path=tmpdir / "test_package_1" / "master.zip"
+    )
+    ireq = from_line(name_collision_1)
+    deps = pypi_repository.get_dependencies(ireq)
+    assert len(deps) == 0
+
+    name_collision_2 = "file://{dist_path}#egg=test_package_2".format(
+        dist_path=tmpdir / "test_package_2" / "master.zip"
+    )
+    ireq = from_line(name_collision_2)
+    deps = pypi_repository.get_dependencies(ireq)
+    assert len(deps) == 1
+    assert deps.pop().name == "test-package-1"