refinements to make installing vcs from CLI work smoother (#6242)

* refinements to make installing vcs from CLI work smoother

* refineent to not strip the environment variables from the url

* Correction to handle file urls for vcs, and the remaining test case

* Add new test case for vcs install

* Add news fragment.

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.

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:

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"]

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'}

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"]
 

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/[email protected]")
+        assert not c.returncode
+        assert "dataclass-factory" in p.pipfile["packages"]