diff --git a/news/5934.bugfix.rst b/news/5934.bugfix.rst new file mode 100644 index 00000000..5cdd12b2 --- /dev/null +++ b/news/5934.bugfix.rst @@ -0,0 +1 @@ +Eveb better handling of vcs branch references that contain special characters. diff --git a/pipenv/utils/dependencies.py b/pipenv/utils/dependencies.py index 90475c76..937585f7 100644 --- a/pipenv/utils/dependencies.py +++ b/pipenv/utils/dependencies.py @@ -1025,6 +1025,20 @@ def _file_path_from_pipfile(path_obj, pipfile_entry): return req_str +def normalize_vcs_url(vcs_url): + """Return vcs_url and possible vcs_ref from a given vcs_url.""" + # We have to handle the fact that some vcs urls have a ref in them + # and some have a netloc with a username and password in them, and some have both + vcs_ref = "" + if "@" in vcs_url: + parsed_url = urlparse(vcs_url) + if "@" in parsed_url.path: + url_parts = vcs_url.rsplit("@", 1) + vcs_url = url_parts[0] + vcs_ref = url_parts[1] + return vcs_url, vcs_ref + + def install_req_from_pipfile(name, pipfile): """Creates an InstallRequirement from a name and a pipfile entry. Handles VCS, local & remote paths, and regular named requirements. @@ -1051,14 +1065,7 @@ def install_req_from_pipfile(name, pipfile): subdirectory = _pipfile.get("subdirectory", "") if subdirectory: subdirectory = f"#subdirectory={subdirectory}" - fallback_ref = "" - if ("ssh://" in vcs_url and vcs_url.count("@") >= 2) or ( - "ssh://" not in vcs_url and "@" in vcs_url - ): - vcs_url_parts = vcs_url.rsplit("@", 1) - if re.match(r"^[\w\.]+$", vcs_url_parts[1]): - vcs_url = vcs_url_parts[0] - fallback_ref = vcs_url_parts[1] + vcs_url, fallback_ref = normalize_vcs_url(vcs_url) req_str = f"{vcs_url}@{_pipfile.get('ref', fallback_ref)}{extras_str}" if not req_str.startswith(f"{vcs}+"): req_str = f"{vcs}+{req_str}" diff --git a/pipenv/utils/requirements.py b/pipenv/utils/requirements.py index 73994443..44b9c14f 100644 --- a/pipenv/utils/requirements.py +++ b/pipenv/utils/requirements.py @@ -1,7 +1,7 @@ import os import re -import urllib.parse from typing import Tuple +from urllib.parse import quote, unquote from pipenv.patched.pip._internal.network.session import PipSession from pipenv.patched.pip._internal.req import parse_requirements @@ -41,7 +41,7 @@ def redact_netloc(netloc: str) -> Tuple[str]: else: # If it is, leave it as is password = ":" + password - user = urllib.parse.quote(user) + user = quote(user) return (f"{user}{password}@{netloc}",) @@ -86,7 +86,7 @@ def import_requirements(project, r=None, dev=False, categories=None): if package.editable: package_string = f"-e {package.link}" else: - package_string = urllib.parse.unquote( + package_string = unquote( redact_auth_from_url(package.original_link.url) ) @@ -143,7 +143,7 @@ BAD_PACKAGES = ( def requirement_from_lockfile( package_name, package_info, include_hashes=True, include_markers=True ): - from pipenv.utils.dependencies import is_editable_path, is_star + from pipenv.utils.dependencies import is_editable_path, is_star, normalize_vcs_url # Handle string requirements if isinstance(package_info, str): @@ -166,36 +166,31 @@ def requirement_from_lockfile( # Handling vcs repositories for vcs in VCS_LIST: if vcs in package_info: - url = package_info[vcs] + vcs_url = package_info[vcs] ref = package_info.get("ref", "") - if ("ssh://" in url and url.count("@") >= 2) or ( - "ssh://" not in url and "@" in url - ): - url_parts = url.rsplit("@", 1) - # Check if the second part matches the criteria to be a ref (vcs URLs would likely have a /) - if re.match(r"^[\w\.]+$", url_parts[1]): - url = url_parts[0] - if not ref: - ref = url_parts[1] - + # We have to handle the fact that some vcs urls have a ref in them + # and some have a netloc with a username and password in them, and some have both + vcs_url, fallback_ref = normalize_vcs_url(vcs_url) + if not ref: + ref = fallback_ref extras = ( "[{}]".format(",".join(package_info.get("extras", []))) if "extras" in package_info else "" ) subdirectory = package_info.get("subdirectory", "") - include_vcs = "" if f"{vcs}+" in url else f"{vcs}+" - egg_fragment = "" if "#egg=" in url else f"#egg={package_name}" - ref_str = "" if not ref or f"@{ref}" in url else f"@{ref}" + include_vcs = "" if f"{vcs}+" in vcs_url else f"{vcs}+" + egg_fragment = "" if "#egg=" in vcs_url else f"#egg={package_name}" + ref_str = "" if not ref or f"@{ref}" in vcs_url else f"@{ref}" if ( - is_editable_path(url) - or "file://" in url + is_editable_path(vcs_url) + or "file://" in vcs_url or package_info.get("editable", False) ): - pip_line = f"-e {include_vcs}{url}{ref_str}{egg_fragment}{extras}" + pip_line = f"-e {include_vcs}{vcs_url}{ref_str}{egg_fragment}{extras}" pip_line += f"&subdirectory={subdirectory}" if subdirectory else "" else: - pip_line = f"{package_name}{extras}@ {include_vcs}{url}{ref_str}" + pip_line = f"{package_name}{extras}@ {include_vcs}{vcs_url}{ref_str}" pip_line += f"#subdirectory={subdirectory}" if subdirectory else "" return pip_line # Handling file-sourced packages diff --git a/tests/integration/test_lockfile.py b/tests/integration/test_lockfile.py new file mode 100644 index 00000000..2617fa88 --- /dev/null +++ b/tests/integration/test_lockfile.py @@ -0,0 +1,63 @@ +from collections import defaultdict +import json +import pytest + +from pipenv.project import Project +from pipenv.utils import requirements + + +@pytest.fixture +def pypi_lockfile(): + lockfile = defaultdict(dict) + lockfile["_meta"] = { + "sources": [ + { + "name": "pypi", + "url": "https://pypi.org/simple", + "verify_ssl": True, + } + ] + } + yield lockfile + + +def test_git_branch_contains_slashes( + pipenv_instance_pypi, pypi_lockfile +): + pypi_lockfile["default"]["google-api-python-client"] = { + "git": "https://github.com/thehesiod/google-api-python-client.git@thehesiod/batch-retries2", + "markers": "python_version >= '3.7'", + "ref": "03803c21fc13a345e978f32775b2f2fa23c8e706" + } + + with pipenv_instance_pypi() as p: + with open(p.lockfile_path, "w") as f: + json.dump(pypi_lockfile, f) + + project = Project() + lockfile = project.load_lockfile(expand_env_vars=False) + deps = lockfile["default"] + pip_installable_lines = requirements.requirements_from_lockfile( + deps, include_hashes=False, include_markers=True + ) + assert pip_installable_lines == ["google-api-python-client@ git+https://github.com/thehesiod/google-api-python-client.git@03803c21fc13a345e978f32775b2f2fa23c8e706"] + + +def test_git_branch_contains_subdirectory_fragment(pipenv_instance_pypi, pypi_lockfile): + pypi_lockfile["default"]["pep508_package"] = { + "git": "https://github.com/techalchemy/test-project.git@master", + "subdirectory": "parent_folder/pep508-package", + "ref": "03803c21fc13a345e978f32775b2f2fa23c8e706" + } + + with pipenv_instance_pypi() as p: + with open(p.lockfile_path, "w") as f: + json.dump(pypi_lockfile, f) + + project = Project() + lockfile = project.load_lockfile(expand_env_vars=False) + deps = lockfile["default"] + pip_installable_lines = requirements.requirements_from_lockfile( + deps, include_hashes=False, include_markers=True + ) + assert pip_installable_lines == ['pep508_package@ git+https://github.com/techalchemy/test-project.git@03803c21fc13a345e978f32775b2f2fa23c8e706#subdirectory=parent_folder/pep508-package']