-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Support installing requirements from inline script metadata (PEP 723) #13052
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
Open
SnoopJ
wants to merge
25
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.
+178
−1
Open
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
239f181
Add --script to install command
SnoopJ b2a8ff6
Add failing test for --script
SnoopJ 4a7eeb1
Default --script to None
SnoopJ 4aba9b8
Add minimum implementation of parsing requirements from inline metadata
SnoopJ 916e4c0
Issue an error if --script is given multiple times
SnoopJ 0697ec2
Add scripts() to download, wheel subcommands
SnoopJ 517c636
Test that --script can only be given once
SnoopJ 866113b
Remove TODO (I think the answer is 'no')
SnoopJ c88ffba
Add failing test for incompatible requires-python
SnoopJ 26868f6
Correct type annotation of PEP 723 helper
SnoopJ c7a0656
Remove PEP 723 requirements helper in favor of direct access
SnoopJ 2a2efb4
Check requires-python specified in script metadata
SnoopJ 9e52c31
Appease the linters
SnoopJ eb5cc10
Write return annotation correctly
SnoopJ 62702db
Add NEWS fragment
SnoopJ aa6c8a2
Change --script to --requirements-from-script
SnoopJ 5961c4e
Replace --script usage in dedicated PEP 723 tests, fix INITools namin…
SnoopJ 004f144
Add --requirements-from-scripts to lock command
SnoopJ 0958182
Add extra layer of backslash escaping to match test stderr on Windows
SnoopJ a49fcea
Remove obsolete TODO
SnoopJ 839494b
Change typing.Dict[] to dict[] to appease ruff-check
SnoopJ d534187
Adjust test requires-python to 'not this version'
SnoopJ 382f9b2
Simplify test of expected error message
SnoopJ 0bfe3b8
Merge branch 'main' into feature/gh12891-inline-metadata
SnoopJ 3cec1f5
Fix syntax error
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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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,106 @@ | ||||
| import sys | ||||
| import textwrap | ||||
|
|
||||
| import pytest | ||||
|
|
||||
| from tests.lib import PipTestEnvironment | ||||
|
|
||||
|
|
||||
| @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", "--requirements-from-script", script_path) | ||||
|
|
||||
| # NOTE:2024-10-05:snoopj:assertions same as in test_requirements_file | ||||
|
Contributor
Author
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. Not meant to retain this comment but I figured I'd call it out for review that this test is more or less a duplicate of its cousin in terms of side effects, in case that's not appropriate.
Suggested change
|
||||
| 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 --requirements-from-script can only be given once in an install command. | ||||
| """ | ||||
| result = script.pip( | ||||
| "install", | ||||
| "--requirements-from-script", | ||||
| "does_not_exist.py", | ||||
| "--requirements-from-script", | ||||
| "also_does_not_exist.py", | ||||
| allow_stderr_error=True, | ||||
| expect_error=True, | ||||
| ) | ||||
|
|
||||
| assert ( | ||||
| "ERROR: --requirements-from-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}" | ||||
SnoopJ marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||
| # dependencies = [ | ||||
| # "INITools==0.2", | ||||
| # "{other_lib_name}<={other_lib_version}", | ||||
| # ] | ||||
| # /// | ||||
| print("Hello world from a dummy program") | ||||
| """ | ||||
| ) | ||||
| ) | ||||
|
|
||||
| result = script.pip( | ||||
| "install", | ||||
| "--requirements-from-script", | ||||
| script_path, | ||||
| expect_stderr=True, | ||||
| expect_error=True, | ||||
| ) | ||||
|
|
||||
| if sys.platform == "win32": | ||||
| # Special case: result.stderr contains an extra layer of backslash | ||||
| # escaping, transform our path to match | ||||
| script_path_str = str(script_path).replace("\\", "\\\\") | ||||
| else: | ||||
| script_path_str = str(script_path) | ||||
SnoopJ marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||
|
|
||||
| assert ( | ||||
| f"ERROR: Script '{script_path_str}' 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.
When first submitting this PR, I originally commented on this code:
I'm not sure I feel as strongly about it now, though. Worth highlighting, though.