Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refinements to make installing vcs from CLI work smoother #6242

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion pipenv/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

try:
import tomllib as toml
Expand Down Expand Up @@ -1198,7 +1199,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:
Expand Down
55 changes: 48 additions & 7 deletions pipenv/utils/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"]
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/test_import_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_auth_with_pw_redacted(mock_find_package_name_from_directory, pipenv_ins
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
Expand All @@ -43,7 +43,7 @@ def test_auth_with_username_redacted(mock_find_package_name_from_directory, pipe
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
Expand All @@ -61,7 +61,7 @@ def test_auth_with_pw_are_variables_passed_to_pipfile(mock_find_package_name_fro
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
Expand All @@ -78,4 +78,4 @@ def test_auth_with_only_username_variable_passed_to_pipfile(mock_find_package_na
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'}
4 changes: 2 additions & 2 deletions tests/integration/test_install_uri.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,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",
}
Expand Down Expand Up @@ -99,7 +99,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"]

Expand Down
10 changes: 10 additions & 0 deletions tests/integration/test_install_vcs.py
Original file line number Diff line number Diff line change
@@ -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"]
Loading