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

Parse pyproject.toml statically where possible #1964

Merged
merged 19 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
92 changes: 91 additions & 1 deletion piptools/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@
import pyproject_hooks
from pip._internal.req import InstallRequirement
from pip._internal.req.constructors import install_req_from_line, parse_req_from_line
from pip._vendor.packaging.markers import Marker
from pip._vendor.packaging.requirements import Requirement

if sys.version_info >= (3, 11):
import tomllib
else:
import tomli as tomllib
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved

PYPROJECT_TOML = "pyproject.toml"

Expand All @@ -34,23 +41,93 @@ def get_all(self, name: str, failobj: _T) -> list[Any] | _T:
...


@dataclass
class StaticProjectMetadata:
extras: tuple[str, ...]
requirements: tuple[InstallRequirement, ...]


@dataclass
class ProjectMetadata:
extras: tuple[str, ...]
requirements: tuple[InstallRequirement, ...]
build_requirements: tuple[InstallRequirement, ...]


def maybe_statically_parse_project_metadata(
src_file: pathlib.Path,
) -> StaticProjectMetadata | None:
"""
Return the metadata for a project, if it can be statically parsed from pyproject.toml.
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved

This function is typically significantly faster than invoking a build backend.
Returns None if the project metadata cannot be statically parsed.
"""
if src_file.name != PYPROJECT_TOML:
return None

try:
with open(src_file, "rb") as f:
pyproject_contents = tomllib.load(f)
except tomllib.TOMLDecodeError:
return None
chrysle marked this conversation as resolved.
Show resolved Hide resolved

# Not valid PEP 621 metadata
if (
"project" not in pyproject_contents
or "name" not in pyproject_contents["project"]
):
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
return None

# Dynamic dependencies require build invocation
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
dynamic = pyproject_contents["project"].get("dynamic", [])
chrysle marked this conversation as resolved.
Show resolved Hide resolved
if "dependencies" in dynamic or "optional-dependencies" in dynamic:
return None

package_name = pyproject_contents["project"]["name"]
comes_from = f"{package_name} ({src_file})"

extras = pyproject_contents["project"].get("optional-dependencies", {}).keys()
install_requirements = [
InstallRequirement(Requirement(req), comes_from)
for req in pyproject_contents["project"].get("dependencies", [])
]
for extra, reqs in (
pyproject_contents.get("project", {}).get("optional-dependencies", {}).items()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we support Poetry for static metadata parsing?

Copy link
Contributor Author

@hauntsaninja hauntsaninja Mar 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone is interested in that, it can be a follow-up PR. It's been tough enough getting this one in, so I want to just stick with the standards based thing in this one

):
for req in reqs:
requirement = Requirement(req)
if requirement.name == package_name:
# Similar to logic for handling self-referential requirements
# from _prepare_requirements
requirement.url = src_file.parent.as_uri()
# Note we don't need to modify `requirement` to include this extra
marker = Marker(f"extra == '{extra}'")
install_requirements.append(
InstallRequirement(requirement, comes_from, markers=marker)
)

return StaticProjectMetadata(
extras=tuple(extras),
requirements=tuple(install_requirements),
)


def build_project_metadata(
src_file: pathlib.Path,
build_targets: tuple[str, ...],
*,
attempt_static_parse: bool,
isolated: bool,
quiet: bool,
) -> ProjectMetadata:
) -> ProjectMetadata | StaticProjectMetadata:
"""
Return the metadata for a project.

First, optionally attempt to determine the metadata statically from the
pyproject.toml file. This will not work if build_targets are specified,
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
since we cannot determine build requirements statically.

Uses the ``prepare_metadata_for_build_wheel`` hook for the wheel metadata
if available, otherwise ``build_wheel``.

Expand All @@ -60,12 +137,25 @@ def build_project_metadata(
:param src_file: Project source file
:param build_targets: A tuple of build targets to get the dependencies
of (``sdist`` or ``wheel`` or ``editable``).
:param attempt_static_parse: Whether to attempt to statically parse the
project metadata from pyproject.toml.
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
Cannot be used with ``build_targets``.
:param isolated: Whether to run invoke the backend in the current
environment or to create an isolated one and invoke it
there.
:param quiet: Whether to suppress the output of subprocesses.
"""

if attempt_static_parse:
if build_targets:
Copy link
Contributor Author

@hauntsaninja hauntsaninja Jan 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you prefer to do this implicitly based on the value of build_targets, instead of having a separate param. I did it this way because it feels a little easier to test / harder to mess up

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you did it.

raise ValueError(
"Cannot execute the PEP 517 optional get_requires_for_build* "
"hooks statically"
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
)
project_metadata = maybe_statically_parse_project_metadata(src_file)
if project_metadata is not None:
return project_metadata

src_dir = src_file.parent
with _create_project_builder(src_dir, isolated=isolated, quiet=quiet) as builder:
metadata = _build_project_wheel_metadata(builder)
Expand Down
4 changes: 3 additions & 1 deletion piptools/scripts/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from pip._internal.utils.misc import redact_auth_from_url

from .._compat import parse_requirements
from ..build import build_project_metadata
from ..build import ProjectMetadata, build_project_metadata
from ..cache import DependencyCache
from ..exceptions import NoCandidateFound, PipToolsError
from ..logging import log
Expand Down Expand Up @@ -348,6 +348,7 @@ def cli(
metadata = build_project_metadata(
src_file=Path(src_file),
build_targets=build_deps_targets,
attempt_static_parse=not bool(build_deps_targets),
isolated=build_isolation,
quiet=log.verbosity <= 0,
)
Expand All @@ -364,6 +365,7 @@ def cli(
raise click.BadParameter(msg)
extras = metadata.extras
if build_deps_targets:
assert isinstance(metadata, ProjectMetadata)
constraints.extend(metadata.build_requirements)
else:
constraints.extend(
Expand Down
101 changes: 99 additions & 2 deletions tests/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@

import pytest

from piptools.build import build_project_metadata
from piptools.build import (
ProjectMetadata,
StaticProjectMetadata,
build_project_metadata,
maybe_statically_parse_project_metadata,
)
from tests.constants import PACKAGES_PATH


Expand All @@ -25,8 +30,9 @@ def test_build_project_metadata_resolved_correct_build_dependencies(
shutil.copytree(src_pkg_path, tmp_path, dirs_exist_ok=True)
src_file = tmp_path / "setup.py"
metadata = build_project_metadata(
src_file, ("editable",), isolated=True, quiet=False
src_file, ("editable",), attempt_static_parse=False, isolated=True, quiet=False
)
assert isinstance(metadata, ProjectMetadata)
build_requirements = sorted(r.name for r in metadata.build_requirements)
assert build_requirements == [
"fake_dynamic_build_dep_for_all",
Expand All @@ -35,3 +41,94 @@ def test_build_project_metadata_resolved_correct_build_dependencies(
"setuptools",
"wheel",
]


def test_build_project_metadata_static(tmp_path):
"""Test static parsing branch of build_project_metadata"""
src_pkg_path = pathlib.Path(PACKAGES_PATH) / "small_fake_with_pyproject"
shutil.copytree(src_pkg_path, tmp_path, dirs_exist_ok=True)
src_file = tmp_path / "pyproject.toml"
metadata = build_project_metadata(
src_file, (), attempt_static_parse=True, isolated=True, quiet=False
)
assert isinstance(metadata, StaticProjectMetadata)
requirements = [(r.name, r.extras, str(r.markers)) for r in metadata.requirements]
requirements.sort(key=lambda x: x[0])
assert requirements == [
("fake_direct_extra_runtime_dep", {"with_its_own_extra"}, 'extra == "x"'),
("fake_direct_runtime_dep", set(), "None"),
]
assert metadata.extras == ("x",)


def test_build_project_metadata_raises_error(tmp_path):
src_pkg_path = pathlib.Path(PACKAGES_PATH) / "small_fake_with_build_deps"
shutil.copytree(src_pkg_path, tmp_path, dirs_exist_ok=True)
src_file = tmp_path / "setup.py"
with pytest.raises(
ValueError, match="Cannot execute the PEP 517 optional.* hooks statically"
):
build_project_metadata(
src_file,
("editable",),
attempt_static_parse=True,
isolated=True,
quiet=False,
)


def test_static_parse(tmp_path):
src_file = tmp_path / "pyproject.toml"

valid = """
[project]
name = "foo"
version = "0.1.0"
dependencies = ["bar>=1"]
[project.optional-dependencies]
baz = ["qux[extra]"]
"""
src_file.write_text(valid)
metadata = maybe_statically_parse_project_metadata(src_file)
assert isinstance(metadata, StaticProjectMetadata)
assert [str(r.req) for r in metadata.requirements] == ["bar>=1", "qux[extra]"]
assert metadata.extras == ("baz",)

invalid_toml = """this is not valid toml"""
src_file.write_text(invalid_toml)
assert maybe_statically_parse_project_metadata(src_file) is None

no_pep621 = """
[build-system]
requires = ["setuptools"]
"""
src_file.write_text(no_pep621)
assert maybe_statically_parse_project_metadata(src_file) is None
chrysle marked this conversation as resolved.
Show resolved Hide resolved

invalid_pep621 = """
[project]
# no name
version = "0.1.0"
"""
src_file.write_text(invalid_pep621)
assert maybe_statically_parse_project_metadata(src_file) is None

dynamic_deps = """
[project]
name = "foo"
dynamic = ["dependencies"]
"""
src_file.write_text(dynamic_deps)
assert maybe_statically_parse_project_metadata(src_file) is None

dynamic_optional_deps = """
[project]
name = "foo"
dynamic = ["optional-dependencies"]
"""
src_file.write_text(dynamic_optional_deps)
assert maybe_statically_parse_project_metadata(src_file) is None

src_file = tmp_path / "setup.py"
src_file.write_text("print('hello')")
assert maybe_statically_parse_project_metadata(src_file) is None
61 changes: 58 additions & 3 deletions tests/test_cli_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3295,7 +3295,7 @@ def test_pass_pip_cache_to_pip_args(tmpdir, runner, current_resolver):


@backtracking_resolver_only
def test_compile_recursive_extras(runner, tmp_path, current_resolver):
def test_compile_recursive_extras_static(runner, tmp_path, current_resolver):
(tmp_path / "pyproject.toml").write_text(
dedent(
"""
Expand Down Expand Up @@ -3329,8 +3329,63 @@ def test_compile_recursive_extras(runner, tmp_path, current_resolver):
small-fake-a==0.2
small-fake-b==0.3
"""
assert out.exit_code == 0
assert expected == out.stdout
try:
assert out.exit_code == 0
assert expected == out.stdout
except Exception: # pragma: no cover
print(out.stdout)
print(out.stderr)
raise


@backtracking_resolver_only
def test_compile_recursive_extras_build_targets(runner, tmp_path, current_resolver):
(tmp_path / "pyproject.toml").write_text(
dedent(
"""
[project]
name = "foo"
version = "0.0.1"
dependencies = ["small-fake-a"]
[project.optional-dependencies]
footest = ["small-fake-b"]
dev = ["foo[footest]"]
"""
)
)
out = runner.invoke(
cli,
[
"--no-build-isolation",
"--no-header",
"--no-annotate",
"--no-emit-options",
"--extra",
"dev",
"--build-deps-for",
"wheel",
"--find-links",
os.fspath(MINIMAL_WHEELS_PATH),
os.fspath(tmp_path / "pyproject.toml"),
"--output-file",
"-",
],
)
expected = rf"""foo[footest] @ {tmp_path.as_uri()}
small-fake-a==0.2
small-fake-b==0.3
wheel==0.42.0

# The following packages are considered to be unsafe in a requirements file:
# setuptools
"""
try:
assert out.exit_code == 0
assert expected == out.stdout
except Exception: # pragma: no cover
print(out.stdout)
print(out.stderr)
raise


def test_config_option(pip_conf, runner, tmp_path, make_config_file):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name="small_fake_with_pyproject"
version=0.1
dependencies=[
"fake_direct_runtime_dep",
]
[project.optional-dependencies]
x = ["fake_direct_extra_runtime_dep[with_its_own_extra]"]