diff --git a/news/6242.bugfix.rst b/news/6242.bugfix.rst new file mode 100644 index 0000000000..ef09c8c53d --- /dev/null +++ b/news/6242.bugfix.rst @@ -0,0 +1 @@ +Fix issue where installing a vcs dependency using pipenv CLI yielded the wrong Pipfile entry such that it could not lock. diff --git a/pipenv/project.py b/pipenv/project.py index dab4915e35..ec692fa6d0 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -15,6 +15,7 @@ from urllib.parse import unquote, urljoin from pipenv.utils.constants import VCS_LIST +from pipenv.utils.dependencies import extract_vcs_url from pipenv.vendor.tomlkit.items import SingleKey, Table try: @@ -1210,7 +1211,9 @@ def generate_package_pipfile_entry(self, package, pip_line, category=None): vcs_parts = vcs_part.rsplit("@", 1) if len(vcs_parts) > 1: entry["ref"] = vcs_parts[1].split("#", 1)[0].strip() - entry[vcs] = vcs_parts[0].strip() + vcs_url = vcs_parts[0].strip() + vcs_url = extract_vcs_url(vcs_url) + entry[vcs] = vcs_url # Check and extract subdirectory fragment if package.link.subdirectory_fragment: diff --git a/pipenv/utils/dependencies.py b/pipenv/utils/dependencies.py index 09f61f3c22..aa110e9b1a 100644 --- a/pipenv/utils/dependencies.py +++ b/pipenv/utils/dependencies.py @@ -11,7 +11,7 @@ from pathlib import Path from tempfile import NamedTemporaryFile, TemporaryDirectory from typing import Any, AnyStr, Dict, List, Mapping, Optional, Sequence, Union -from urllib.parse import urlparse, urlsplit, urlunsplit +from urllib.parse import urlparse, urlsplit, urlunparse, urlunsplit from pipenv.patched.pip._internal.models.link import Link from pipenv.patched.pip._internal.network.download import Downloader @@ -199,6 +199,45 @@ def unearth_hashes_for_dep(project, dep): return [] +def extract_vcs_url(vcs_url): + # Remove leading/trailing whitespace + vcs_url = vcs_url.strip() + + # Check if it's a file URI + parsed = urlparse(vcs_url) + if parsed.scheme == "file": + # For file URIs, we want to keep the entire URL intact + return vcs_url + + # Remove the package name and '@' if present at the start + if "@" in vcs_url and not vcs_url.startswith(tuple(f"{vcs}+" for vcs in VCS_LIST)): + vcs_url = vcs_url.split("@", 1)[1] + + # Remove the VCS prefix (e.g., 'git+') + for prefix in VCS_LIST: + vcs_prefix = f"{prefix}+" + if vcs_url.startswith(vcs_prefix): + vcs_url = vcs_url[len(vcs_prefix) :] + break + + # Parse the URL + parsed = urlparse(vcs_url) + + # Reconstruct the URL, preserving authentication details + clean_url = urlunparse( + ( + parsed.scheme, + parsed.netloc, + parsed.path, + "", # params + "", # query + "", # fragment + ) + ) + + return clean_url + + def clean_resolved_dep(project, dep, is_top_level=False, current_entry=None): from pipenv.patched.pip._vendor.packaging.requirements import ( Requirement as PipRequirement, @@ -237,15 +276,17 @@ def clean_resolved_dep(project, dep, is_top_level=False, current_entry=None): is_vcs_or_file = False for vcs_type in VCS_LIST: if vcs_type in dep: - if "[" in dep[vcs_type] and "]" in dep[vcs_type]: - extras_section = dep[vcs_type].split("[").pop().replace("]", "") + vcs_url = dep[vcs_type] + if "[" in vcs_url and "]" in vcs_url: + extras_section = vcs_url.split("[").pop().replace("]", "") lockfile["extras"] = sorted( [extra.strip() for extra in extras_section.split(",")] ) - if has_name_with_extras(dep[vcs_type]): - lockfile[vcs_type] = dep[vcs_type].split("@ ", 1)[1] - else: - lockfile[vcs_type] = dep[vcs_type] + + # Extract the clean VCS URL + clean_vcs_url = extract_vcs_url(vcs_url) + + lockfile[vcs_type] = clean_vcs_url lockfile["ref"] = dep.get("ref") if "subdirectory" in dep: lockfile["subdirectory"] = dep["subdirectory"] diff --git a/tests/integration/test_import_requirements.py b/tests/integration/test_import_requirements.py index b2a496a5b7..0f87bac316 100644 --- a/tests/integration/test_import_requirements.py +++ b/tests/integration/test_import_requirements.py @@ -32,10 +32,7 @@ def test_auth_with_pw_redacted( requirements_file.close() import_requirements(project, r=requirements_file.name) os.unlink(requirements_file.name) - assert p.pipfile["packages"]["myproject"] == { - "git": "git+https://${AUTH_USER}:****@github.com/user/myproject.git", - "ref": "main", - } + assert p.pipfile["packages"]["myproject"] == {'git': 'https://${AUTH_USER}:****@github.com/user/myproject.git', 'ref': 'main'} @pytest.mark.cli @@ -60,10 +57,8 @@ def test_auth_with_username_redacted( requirements_file.close() import_requirements(project, r=requirements_file.name) os.unlink(requirements_file.name) - assert p.pipfile["packages"]["myproject"] == { - "git": "git+https://****@github.com/user/myproject.git", - "ref": "main", - } + assert p.pipfile["packages"]["myproject"] == {'git': 'https://****@github.com/user/myproject.git', 'ref': 'main'} + @pytest.mark.cli @@ -88,11 +83,7 @@ def test_auth_with_pw_are_variables_passed_to_pipfile( requirements_file.close() import_requirements(project, r=requirements_file.name) os.unlink(requirements_file.name) - assert p.pipfile["packages"]["myproject"] == { - "git": "git+https://${AUTH_USER}:${AUTH_PW}@github.com/user/myproject.git", - "ref": "main", - } - + assert p.pipfile["packages"]["myproject"] == {'git': 'https://${AUTH_USER}:${AUTH_PW}@github.com/user/myproject.git', 'ref': 'main'} @pytest.mark.cli @pytest.mark.deploy @@ -116,7 +107,4 @@ def test_auth_with_only_username_variable_passed_to_pipfile( requirements_file.close() import_requirements(project, r=requirements_file.name) os.unlink(requirements_file.name) - assert p.pipfile["packages"]["myproject"] == { - "git": "git+https://${AUTH_USER}@github.com/user/myproject.git", - "ref": "main", - } + assert p.pipfile["packages"]["myproject"] == {'git': 'https://${AUTH_USER}@github.com/user/myproject.git', 'ref': 'main'} diff --git a/tests/integration/test_install_uri.py b/tests/integration/test_install_uri.py index 9794c2fc76..745138dd63 100644 --- a/tests/integration/test_install_uri.py +++ b/tests/integration/test_install_uri.py @@ -27,7 +27,7 @@ def test_basic_vcs_install_with_env_var(pipenv_instance_pypi): assert all(package in p.pipfile["packages"] for package in ["six", "gitdb2"]) assert "git" in p.pipfile["packages"]["six"] assert p.lockfile["default"]["six"] == { - "git": "git+https://${GIT_HOST}/benjaminp/six.git", + "git": "https://${GIT_HOST}/benjaminp/six.git", "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", "ref": "15e31431af97e5e64b80af0a3f598d382bcdd49a", } @@ -103,7 +103,7 @@ def test_install_git_tag(pipenv_instance_private_pypi): assert "git" in p.lockfile["default"]["six"] assert ( p.lockfile["default"]["six"]["git"] - == "git+https://github.com/benjaminp/six.git" + == "https://github.com/benjaminp/six.git" ) assert "ref" in p.lockfile["default"]["six"] diff --git a/tests/integration/test_install_vcs.py b/tests/integration/test_install_vcs.py new file mode 100644 index 0000000000..d32e079d90 --- /dev/null +++ b/tests/integration/test_install_vcs.py @@ -0,0 +1,10 @@ +import pytest + + +@pytest.mark.basic +@pytest.mark.install +def test_install_github_vcs(pipenv_instance_pypi): + with pipenv_instance_pypi() as p: + c = p.pipenv("install git+https://github.com/reagento/adaptix.git@2.16") + assert not c.returncode + assert "dataclass-factory" in p.pipfile["packages"]