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

Generate PEP 440 direct reference whenever possible #1455

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 34 additions & 8 deletions piptools/utils.py
Expand Up @@ -14,6 +14,7 @@
Tuple,
TypeVar,
Union,
cast,
)

import click
Expand Down Expand Up @@ -115,14 +116,7 @@ def format_requirement(
if ireq.editable:
line = f"-e {ireq.link.url}"
elif is_url_requirement(ireq):
if ireq.name:
line = (
ireq.link.url
if ireq.link.egg_fragment
else f"{ireq.name.lower()} @ {ireq.link.url}"
)
else:
line = ireq.link.url
line = _build_direct_reference_best_efforts(ireq)
else:
line = str(ireq.req).lower()

Expand All @@ -136,6 +130,38 @@ def format_requirement(
return line


def _build_direct_reference_best_efforts(ireq: InstallRequirement) -> str:
"""
Returns a string of a direct reference URI, whenever possible.
See https://www.python.org/dev/peps/pep-0508/
"""
# If the requirement has no name then we cannot build a direct reference.
if not ireq.name:
return cast(str, ireq.link.url)

# Look for a relative file path, the direct reference currently does not work with it.
if ireq.link.is_file and not ireq.link.path.startswith("/"):
return cast(str, ireq.link.url)

# If we get here then we have a requirement that supports direct reference.
# We need to remove the egg if it exists and keep the rest of the fragments.
direct_reference = f"{ireq.name.lower()} @ {ireq.link.url_without_fragment}"
fragments = []

# Check if there is any fragment to add to the URI.
if ireq.link.subdirectory_fragment:
fragments.append(f"subdirectory={ireq.link.subdirectory_fragment}")

if ireq.link.has_hash:
fragments.append(f"{ireq.link.hash_name}={ireq.link.hash}")

# Then add the fragments into the URI, if any.
if fragments:
direct_reference += f"#{'&'.join(fragments)}"

return direct_reference


def format_specifier(ireq: InstallRequirement) -> str:
"""
Generic formatter for pretty printing the specifier part of
Expand Down
17 changes: 10 additions & 7 deletions tests/test_cli_compile.py
Expand Up @@ -537,8 +537,8 @@ def test_locally_available_editable_package_is_not_archived_in_cache_dir(
"pytest-django @ git+git://github.com/pytest-dev/pytest-django"
"@21492afc88a19d4ca01cd0ac392a5325b14f95c7"
"#egg=pytest-django",
"git+git://github.com/pytest-dev/pytest-django"
"@21492afc88a19d4ca01cd0ac392a5325b14f95c7#egg=pytest-django",
"pytest-django @ git+git://github.com/pytest-dev/pytest-django"
"@21492afc88a19d4ca01cd0ac392a5325b14f95c7",
id="VCS with direct reference and egg",
),
),
Expand Down Expand Up @@ -607,9 +607,12 @@ def test_url_package(runner, line, dependency, generate_hashes):
pytest.param(
path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_subdir"))
+ "#subdirectory=subdir&egg=small_fake_a",
path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_subdir"))
+ "#subdirectory=subdir&egg=small_fake_a",
None,
"small-fake-a @ "
+ path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_subdir"))
+ "#subdirectory=subdir",
"small-fake-a @ "
+ path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_subdir"))
+ "#subdirectory=subdir",
id="Local project with subdirectory",
),
),
Expand Down Expand Up @@ -843,8 +846,8 @@ def test_generate_hashes_with_url(runner):
)
out = runner.invoke(cli, ["--no-annotate", "--generate-hashes"])
expected = (
"https://github.com/jazzband/pip-tools/archive/"
"7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3.zip#egg=pip-tools \\\n"
"pip-tools @ https://github.com/jazzband/pip-tools/archive/"
"7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3.zip \\\n"
" --hash=sha256:d24de92e18ad5bf291f25cfcdcf"
"0171be6fa70d01d0bef9eeda356b8549715e7\n"
)
Expand Down
52 changes: 38 additions & 14 deletions tests/test_utils.py
Expand Up @@ -52,48 +52,72 @@ def test_format_requirement(from_line):
),
pytest.param(
"example @ https://example.com/example.zip#egg=example",
"https://example.com/example.zip#egg=example",
id="url with egg in fragment",
"example @ https://example.com/example.zip",
id="direct reference with egg in fragment",
),
pytest.param(
"example @ https://example.com/example.zip#subdirectory=test&egg=example",
"https://example.com/example.zip#subdirectory=test&egg=example",
id="url with subdirectory and egg in fragment",
"example @ https://example.com/example.zip#subdirectory=test",
id="direct reference with subdirectory and egg in fragment",
),
pytest.param(
"example @ https://example.com/example.zip#subdirectory=test"
"&egg=example&sha1=594b7dd32bec37d8bf70a6ffa8866d30e93f3c42",
"example @ https://example.com/example.zip#subdirectory=test"
"&sha1=594b7dd32bec37d8bf70a6ffa8866d30e93f3c42",
id="direct reference with subdirectory, hash and egg in fragment",
),
pytest.param(
"example @ https://example.com/example.zip?egg=test#subdirectory=project_a",
"example @ https://example.com/example.zip?egg=test#subdirectory=project_a",
id="url with egg in query",
"example @ https://example.com/example.zip?egg=test",
"example @ https://example.com/example.zip?egg=test",
id="direct reference with egg in query",
),
pytest.param(
"file:./vendor/package.zip",
"file:./vendor/package.zip",
id="relative path",
id="file scheme relative path",
),
pytest.param(
"file:vendor/package.zip",
"file:vendor/package.zip",
id="relative path",
id="file scheme relative path",
),
pytest.param(
"file:vendor/package.zip#egg=example",
"file:vendor/package.zip#egg=example",
id="relative path with egg",
id="file scheme relative path with egg",
),
pytest.param(
"file:./vendor/package.zip#egg=example",
"file:./vendor/package.zip#egg=example",
id="file scheme relative path with egg",
),
pytest.param(
"file:///vendor/package.zip",
"file:///vendor/package.zip",
id="full path without direct reference",
id="file scheme absolute path without direct reference",
),
pytest.param(
"file:///vendor/package.zip#egg=test",
"test @ file:///vendor/package.zip",
id="file scheme absolute path with egg",
),
pytest.param(
"package @ file:///vendor/package.zip",
"package @ file:///vendor/package.zip",
id="full path with direct reference",
id="file scheme absolute path with direct reference",
),
pytest.param(
"package @ file:///vendor/package.zip#egg=example",
"file:///vendor/package.zip#egg=example",
id="full path with direct reference and egg",
"package @ file:///vendor/package.zip",
id="file scheme absolute path with direct reference and egg",
),
pytest.param(
"package @ file:///vendor/package.zip#egg=example&subdirectory=test"
"&sha1=594b7dd32bec37d8bf70a6ffa8866d30e93f3c42",
"package @ file:///vendor/package.zip#subdirectory=test"
"&sha1=594b7dd32bec37d8bf70a6ffa8866d30e93f3c42",
id="full path with direct reference, egg, subdirectory and hash",
),
),
)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_writer.py
Expand Up @@ -152,7 +152,7 @@ def test_iter_lines__hash_missing(capsys, writer, from_line):

expected_lines = (
MESSAGE_UNHASHED_PACKAGE,
"file:///example/#egg=example",
"example @ file:///example/",
"test==1.2 \\\n --hash=FAKEHASH",
)
assert tuple(lines) == expected_lines
Expand All @@ -177,8 +177,8 @@ def test_iter_lines__no_warn_if_only_unhashable_packages(writer, from_line):
lines = writer._iter_lines(ireqs, hashes=hashes)

expected_lines = (
"file:///unhashable-pkg1/#egg=unhashable-pkg1",
"file:///unhashable-pkg2/#egg=unhashable-pkg2",
"unhashable-pkg1 @ file:///unhashable-pkg1/",
"unhashable-pkg2 @ file:///unhashable-pkg2/",
)
assert tuple(lines) == expected_lines

Expand Down