diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index f247c30ee..cf3d63e09 100644 --- a/piptools/repositories/pypi.py +++ b/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: diff --git a/tests/conftest.py b/tests/conftest.py index 2febbc605..7cbb6814c 100644 --- a/tests/conftest.py +++ b/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 diff --git a/tests/test_repository_pypi.py b/tests/test_repository_pypi.py index a239aa1c9..e072bdd7c 100644 --- a/tests/test_repository_pypi.py +++ b/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"