-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Support installing requirements from inline script metadata (PEP 723) #13052
Open
SnoopJ
wants to merge
15
commits into
pypa:main
Choose a base branch
from
SnoopJ:feature/gh12891-inline-metadata
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
d66153f
Add --script to install command
SnoopJ 0035c41
Add failing test for --script
SnoopJ e62ce80
Default --script to None
SnoopJ 610bc12
Add minimum implementation of parsing requirements from inline metadata
SnoopJ 2e61298
Issue an error if --script is given multiple times
SnoopJ bd0df37
Add scripts() to download, wheel subcommands
SnoopJ e6231e9
Test that --script can only be given once
SnoopJ 1fd7c08
Remove TODO (I think the answer is 'no')
SnoopJ bfa57d4
Add failing test for incompatible requires-python
SnoopJ 6e0fd8d
Correct type annotation of PEP 723 helper
SnoopJ 0b2b7f4
Remove PEP 723 requirements helper in favor of direct access
SnoopJ 55d3049
Check requires-python specified in script metadata
SnoopJ 418bc01
Appease the linters
SnoopJ 924859c
Write return annotation correctly
SnoopJ cab2786
Add NEWS fragment
SnoopJ File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Support installing dependencies declared with inline script metadata | ||
(PEP 723). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,13 +10,21 @@ | |
from optparse import Values | ||
from typing import Any, List, Optional, Tuple | ||
|
||
from pip._vendor.packaging.requirements import Requirement | ||
|
||
from pip._internal.cache import WheelCache | ||
from pip._internal.cli import cmdoptions | ||
from pip._internal.cli.cmdoptions import make_target_python | ||
from pip._internal.cli.index_command import IndexGroupCommand | ||
from pip._internal.cli.index_command import SessionCommandMixin as SessionCommandMixin | ||
from pip._internal.exceptions import CommandError, PreviousBuildDirError | ||
from pip._internal.exceptions import ( | ||
CommandError, | ||
PreviousBuildDirError, | ||
UnsupportedPythonVersion, | ||
) | ||
from pip._internal.index.collector import LinkCollector | ||
from pip._internal.index.package_finder import PackageFinder | ||
from pip._internal.metadata.pep723 import pep723_metadata | ||
from pip._internal.models.selection_prefs import SelectionPreferences | ||
from pip._internal.models.target_python import TargetPython | ||
from pip._internal.network.session import PipSession | ||
|
@@ -31,6 +39,7 @@ | |
from pip._internal.req.req_file import parse_requirements | ||
from pip._internal.req.req_install import InstallRequirement | ||
from pip._internal.resolution.base import BaseResolver | ||
from pip._internal.utils.packaging import check_requires_python | ||
from pip._internal.utils.temp_dir import ( | ||
TempDirectory, | ||
TempDirectoryTypeRegistry, | ||
|
@@ -268,11 +277,37 @@ def get_requirements( | |
) | ||
requirements.append(req_to_add) | ||
|
||
if options.scripts: | ||
if len(options.scripts) > 1: | ||
raise CommandError("--script can only be given once") | ||
|
||
script = options.scripts[0] | ||
script_metadata = pep723_metadata(script) | ||
|
||
script_requires_python = script_metadata.get("requires-python", "") | ||
|
||
if script_requires_python and not options.ignore_requires_python: | ||
target_python = make_target_python(options) | ||
|
||
if not check_requires_python( | ||
requires_python=script_requires_python, | ||
version_info=target_python.py_version_info, | ||
): | ||
raise UnsupportedPythonVersion( | ||
f"Script {script!r} requires a different Python: " | ||
f"{target_python.py_version} not in {script_requires_python!r}" | ||
) | ||
|
||
for req in script_metadata.get("dependencies", []): | ||
requirements.append( | ||
InstallRequirement(Requirement(req), comes_from=None) | ||
) | ||
Comment on lines
+280
to
+304
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels ungraceful to put all of this logic here but I'm not really sure how I would plumb it into a separate function. Suggestions welcome |
||
|
||
# If any requirement has hash options, enable hash checking. | ||
if any(req.has_hash_options for req in requirements): | ||
options.require_hashes = True | ||
|
||
if not (args or options.editables or options.requirements): | ||
if not (args or options.editables or options.requirements or options.scripts): | ||
opts = {"name": self.name} | ||
if options.find_links: | ||
raise CommandError( | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import re | ||
from typing import Any, Dict | ||
|
||
from pip._vendor import tomli as tomllib | ||
|
||
REGEX = r"(?m)^# /// (?P<type>[a-zA-Z0-9-]+)$\s(?P<content>(^#(| .*)$\s)+)^# ///$" | ||
|
||
|
||
def pep723_metadata(scriptfile: str) -> Dict[str, Any]: | ||
with open(scriptfile) as f: | ||
script = f.read() | ||
|
||
name = "script" | ||
matches = list( | ||
filter(lambda m: m.group("type") == name, re.finditer(REGEX, script)) | ||
) | ||
if len(matches) > 1: | ||
raise ValueError(f"Multiple {name} blocks found") | ||
elif len(matches) == 1: | ||
content = "".join( | ||
line[2:] if line.startswith("# ") else line[1:] | ||
for line in matches[0].group("content").splitlines(keepends=True) | ||
) | ||
return tomllib.loads(content) | ||
else: | ||
raise ValueError(f"File does not contain 'script' metadata: {scriptfile!r}") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
import sys | ||
import textwrap | ||
|
||
import pytest | ||
|
||
from tests.lib import PipTestEnvironment | ||
|
||
|
||
# TODO:2024-10-05:snoopj:need a test for requires-python support, too. | ||
# Implement in terms of sys.version_info ? | ||
@pytest.mark.network | ||
def test_script_file(script: PipTestEnvironment) -> None: | ||
""" | ||
Test installing from a script with inline metadata (PEP 723). | ||
""" | ||
|
||
other_lib_name, other_lib_version = "peppercorn", "0.6" | ||
script_path = script.scratch_path.joinpath("script.py") | ||
script_path.write_text( | ||
textwrap.dedent( | ||
f"""\ | ||
# /// script | ||
# dependencies = [ | ||
# "INITools==0.2", | ||
# "{other_lib_name}<={other_lib_version}", | ||
# ] | ||
# /// | ||
|
||
print("Hello world from a dummy program") | ||
""" | ||
) | ||
) | ||
result = script.pip("install", "--script", script_path) | ||
|
||
# NOTE:2024-10-05:snoopj:assertions same as in test_requirements_file | ||
result.did_create(script.site_packages / "INITools-0.2.dist-info") | ||
result.did_create(script.site_packages / "initools") | ||
assert result.files_created[script.site_packages / other_lib_name].dir | ||
fn = f"{other_lib_name}-{other_lib_version}.dist-info" | ||
assert result.files_created[script.site_packages / fn].dir | ||
|
||
|
||
def test_multiple_scripts(script: PipTestEnvironment) -> None: | ||
""" | ||
Test that --script can only be given once in an install command. | ||
""" | ||
result = script.pip( | ||
"install", | ||
"--script", | ||
"does_not_exist.py", | ||
"--script", | ||
"also_does_not_exist.py", | ||
allow_stderr_error=True, | ||
expect_error=True, | ||
) | ||
|
||
assert "ERROR: --script can only be given once" in result.stderr, ( | ||
"multiple script did not fail as expected -- " + result.stderr | ||
) | ||
|
||
|
||
@pytest.mark.network | ||
def test_script_file_python_version(script: PipTestEnvironment) -> None: | ||
""" | ||
Test installing from a script with an incompatible `requires-python` | ||
""" | ||
|
||
other_lib_name, other_lib_version = "peppercorn", "0.6" | ||
script_path = script.scratch_path.joinpath("script.py") | ||
target_python_ver = f"{sys.version_info.major}.{sys.version_info.minor + 1}" | ||
script_path.write_text( | ||
textwrap.dedent( | ||
f"""\ | ||
# /// script | ||
# requires-python = ">={target_python_ver}" | ||
# dependencies = [ | ||
# "INITools==0.2", | ||
# "{other_lib_name}<={other_lib_version}", | ||
# ] | ||
# /// | ||
|
||
print("Hello world from a dummy program") | ||
""" | ||
) | ||
) | ||
|
||
result = script.pip( | ||
"install", "--script", script_path, expect_stderr=True, expect_error=True | ||
) | ||
assert ( | ||
f"ERROR: Script '{script_path}' requires a different Python" in result.stderr | ||
), ( | ||
"Script with incompatible requires-python did not fail as expected -- " | ||
+ result.stderr | ||
) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ruff
is upset that I wrote a loop with.append()
here (PERF401
) but I don't know if a comprehension would really be justified here? I can maybe see the case forrequirements.extend(...)
but it still seems like it'd be harder to read to me. Looking for feedback.Edit: here's a diff of the
extend()
version that passes the new tests: