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

An export goal for exporting data to be read by IDEs. #13415

Merged
merged 8 commits into from
Nov 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
82 changes: 82 additions & 0 deletions src/python/pants/backend/python/goals/export.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

import os
from dataclasses import dataclass

from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex import VenvPex, VenvPexProcess
from pants.backend.python.util_rules.pex_environment import PexEnvironment
from pants.backend.python.util_rules.pex_from_targets import PexFromTargetsRequest
from pants.core.goals.export import ExportableData, ExportableDataRequest, ExportError, Symlink
from pants.engine.internals.selectors import Get
from pants.engine.process import ProcessResult
from pants.engine.rules import collect_rules, rule
from pants.engine.unions import UnionRule


@dataclass(frozen=True)
class ExportedVenvRequest(ExportableDataRequest):
pass


@rule
async def export_venv(
request: ExportedVenvRequest, python_setup: PythonSetup, pex_env: PexEnvironment
) -> ExportableData:
Comment on lines +26 to +29
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Rather than creating a virtualenv in the dest, this is symlinking from PEX's private cache of venvs into a destination... that seems maybe a bit dangerous, as someone who activated this venv and then fiddled with it would cause reproducibility issues by mutating PEX's cache.

Should this actually create a brand new venv instead, using pex-tool's facilities for that?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a valid concern. We're walking a perf line here,

Note that the fiddling would have to be editing by hand since these venvs don't get Pip installed into them (unlike PEX_TOOLS=1 ./my.pex venv here venvs, which do by default.).

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Mm... that does alleviate the concern quite a bit. Not a blocker then probably.

# Pick a single interpreter for the venv.
interpreter_constraints = InterpreterConstraints.create_from_targets(
request.targets, python_setup
)
min_interpreter = interpreter_constraints.snap_to_minimum(python_setup.interpreter_universe)
if not min_interpreter:
raise ExportError(
"There is no single Python interpreter compatible with all the "
"targets for which export was requested. Please restrict the target set "
"to one that shares a compatible interpreter."
benjyw marked this conversation as resolved.
Show resolved Hide resolved
)

venv_pex = await Get(
VenvPex,
PexFromTargetsRequest,
PexFromTargetsRequest.for_requirements(
(tgt.address for tgt in request.targets),
internal_only=True,
hardcoded_interpreter_constraints=min_interpreter,
),
)

complete_pex_env = pex_env.in_workspace()
venv_abspath = os.path.join(complete_pex_env.pex_root, venv_pex.venv_rel_dir)

# Run the venv_pex to ensure that the underlying venv is created if necessary.
benjyw marked this conversation as resolved.
Show resolved Hide resolved
# We also use this to get the full python version (including patch #), so we
# can use it in the symlink name (not critical, but nice to have).
res = await Get(
ProcessResult,
VenvPexProcess(
venv_pex=venv_pex,
description="Create virtualenv",
argv=["-c", "import sys; print('.'.join(str(x) for x in sys.version_info[0:3]))"],
input_digest=venv_pex.digest,
# TODO: Is there always a python_configured?
benjyw marked this conversation as resolved.
Show resolved Hide resolved
extra_env=complete_pex_env.environment_dict(python_configured=True),
),
)
py_version = res.stdout.strip().decode()

return ExportableData(
f"virtualenv for {min_interpreter}",
os.path.join("python", "virtualenv"),
symlinks=[Symlink(venv_abspath, py_version)],
)


def rules():
return [
*collect_rules(),
UnionRule(ExportableDataRequest, ExportedVenvRequest),
]
2 changes: 2 additions & 0 deletions src/python/pants/backend/python/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from pants.backend.python.dependency_inference import rules as dependency_inference_rules
from pants.backend.python.goals import (
coverage_py,
export,
lockfile,
package_pex_binary,
pytest_runner,
Expand Down Expand Up @@ -62,6 +63,7 @@ def build_file_aliases():
def rules():
return (
*coverage_py.rules(),
*export.rules(),
*lockfile.rules(),
*tailor.rules(),
*ancestor_files.rules(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,18 @@ def interpreter_constraints(self) -> InterpreterConstraintsField:
_FS = TypeVar("_FS", bound=FieldSetWithInterpreterConstraints)


# The current maxes are 2.7.18 and 3.6.13.
_EXPECTED_LAST_PATCH_VERSION = 18
# The current maxes are 2.7.18 and 3.6.15. We go much higher, for safety.
_PATCH_VERSION_UPPER_BOUND = 30


# Normally we would subclass `DeduplicatedCollection`, but we want a custom constructor.
class InterpreterConstraints(FrozenOrderedSet[Requirement], EngineAwareParameter):
@classmethod
def for_fixed_python_version(
cls, cpython_version_str: str, interpreter_type: str = "CPython"
) -> InterpreterConstraints:
return cls([f"{interpreter_type}=={cpython_version_str}"])

def __init__(self, constraints: Iterable[str | Requirement] = ()) -> None:
# #12578 `parse_constraint` will sort the requirement's component constraints into a stable form.
# We need to sort the component constraints for each requirement _before_ sorting the entire list
Expand Down Expand Up @@ -180,7 +186,7 @@ def generate_pex_arg_list(self) -> list[str]:
return args

def _valid_patch_versions(self, major: int, minor: int) -> Iterator[int]:
for p in range(0, _EXPECTED_LAST_PATCH_VERSION + 1):
for p in range(0, _PATCH_VERSION_UPPER_BOUND + 1):
for req in self:
if req.specifier.contains(f"{major}.{minor}.{p}"): # type: ignore[attr-defined]
yield p
Expand All @@ -202,17 +208,29 @@ def minimum_python_version(self, interpreter_universe: Iterable[str]) -> str | N
The constraints may also be compatible with later versions; this is the lowest version that
still works.
"""
snapped = self.snap_to_minimum(interpreter_universe)
if snapped:
# We know by construction that snapped has a single requirement, and that has a single
# (op, version) spec whose version is "major.minor.*", so we just strip off the ".*" .
return next(iter(snapped)).specs[0][1][:-2]
return None

def snap_to_minimum(self, interpreter_universe: Iterable[str]) -> InterpreterConstraints | None:
"""Snap to the lowest Python major.minor version that works with these constraints."""
for major, minor in sorted(_major_minor_to_int(s) for s in interpreter_universe):
if self._includes_version(major, minor):
return f"{major}.{minor}"
for p in range(0, _PATCH_VERSION_UPPER_BOUND + 1):
for req in self:
if req.specifier.contains(f"{major}.{minor}.{p}"): # type: ignore[attr-defined]
snapped = Requirement.parse(f"{req.project_name}=={major}.{minor}.*")
return InterpreterConstraints([snapped])
return None

def _requires_python3_version_or_newer(
self, *, allowed_versions: Iterable[str], prior_version: str
) -> bool:
if not self:
return False
patch_versions = list(reversed(range(0, _EXPECTED_LAST_PATCH_VERSION)))
patch_versions = list(reversed(range(0, _PATCH_VERSION_UPPER_BOUND)))
# We only look at the prior Python release. For example, consider Python 3.8+
# looking at 3.7. If using something like `>=3.5`, Py37 will be included.
# `==3.6.*,!=3.7.*,==3.8.*` is unlikely, and even that will work correctly as
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import InterpreterConstraintsField
from pants.backend.python.util_rules.interpreter_constraints import (
_EXPECTED_LAST_PATCH_VERSION,
_PATCH_VERSION_UPPER_BOUND,
InterpreterConstraints,
)
from pants.build_graph.address import Address
Expand Down Expand Up @@ -192,6 +192,29 @@ def test_interpreter_constraints_minimum_python_version(
assert ics.minimum_python_version(sorted(universe)) == expected


@pytest.mark.parametrize(
"constraints,expected",
[
(["CPython>=2.7"], "CPython==2.7.*"),
(["CPython>=3.7"], "CPython==3.7.*"),
(["CPython>=3.8.6"], "CPython==3.8.*"),
(["CPython==3.5.*", "CPython>=3.6"], "CPython==3.5.*"),
(["CPython==3.7.*", "PyPy==3.6.*"], "PyPy==3.6.*"),
(["CPython"], "CPython==2.7.*"),
(["CPython==3.7.*,<3.6"], None),
([], None),
],
)
def test_snap_to_minimum(constraints, expected) -> None:
universe = ["2.7", "3.5", "3.6", "3.7", "3.8", "3.9", "3.10"]
ics = InterpreterConstraints(constraints)
snapped = ics.snap_to_minimum(universe)
if expected is None:
assert snapped is None
else:
assert snapped == InterpreterConstraints([expected])


@pytest.mark.parametrize(
"constraints",
[
Expand Down Expand Up @@ -304,7 +327,7 @@ def test_to_poetry_constraint(constraints: list[str], expected: str) -> None:
assert InterpreterConstraints(constraints).to_poetry_constraint() == expected


_ALL_PATCHES = list(range(_EXPECTED_LAST_PATCH_VERSION + 1))
_ALL_PATCHES = list(range(_PATCH_VERSION_UPPER_BOUND + 1))


def patches(
Expand All @@ -324,7 +347,7 @@ def patches(
["==2.7.1", ">=3.6.15"],
(
[(2, 7, 1)]
+ patches(3, 6, range(15, _EXPECTED_LAST_PATCH_VERSION + 1))
+ patches(3, 6, range(15, _PATCH_VERSION_UPPER_BOUND + 1))
+ patches(3, 7, _ALL_PATCHES)
+ patches(3, 8, _ALL_PATCHES)
+ patches(3, 9, _ALL_PATCHES)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ class VenvPexRequest:
bin_names: tuple[str, ...] = ()

def __init__(self, pex_request: PexRequest, bin_names: Iterable[str] = ()) -> None:
"""A request for a PEX that runs in a venv and optionally exposes select vanv `bin` scripts.
"""A request for a PEX that runs in a venv and optionally exposes select venv `bin` scripts.

:param pex_request: The details of the desired PEX.
:param bin_names: The names of venv `bin` scripts to expose for execution.
Expand Down
115 changes: 115 additions & 0 deletions src/python/pants/core/goals/export.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

import os
from dataclasses import dataclass
from typing import Iterable, cast

from pants.core.util_rules.distdir import DistDir
from pants.engine.console import Console
from pants.engine.fs import EMPTY_DIGEST, AddPrefix, Digest, MergeDigests, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.internals.selectors import Get, MultiGet
from pants.engine.rules import collect_rules, goal_rule
from pants.engine.target import Targets
from pants.engine.unions import UnionMembership, union
from pants.util.dirutil import absolute_symlink
from pants.util.meta import frozen_after_init


class ExportError(Exception):
pass


@union
@dataclass(frozen=True)
class ExportableDataRequest:
"""A union for exportable data provided by a backend.

Subclass and install a member of this type to export data.
"""

targets: Targets


@dataclass(frozen=True)
class Symlink:
"""A symlink from link_rel_path pointing to source_abs_path.

link_rel_path is relative to the enclosing ExportableData's reldir, and will be absolutized when
a location for that dir is chosen.
"""

source_abs_path: str
link_rel_path: str


@frozen_after_init
@dataclass(unsafe_hash=True)
class ExportableData:
description: str
# Materialize digests and create symlinks under this reldir.
reldir: str
# Materialize this digest.
digest: Digest
# Create these symlinks. Symlinks are created after the digest is materialized,
# so may reference files/dirs in the digest.
symlinks: tuple[Symlink, ...]

def __init__(
self,
description: str,
reldir: str,
*,
digest: Digest = EMPTY_DIGEST,
symlinks: Iterable[Symlink] = tuple(),
):
self.description = description
self.reldir = reldir
self.digest = digest
self.symlinks = tuple(symlinks)


class ExportSubsystem(GoalSubsystem):
name = "export"
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the story with export-codegen? Should that still exist? I wonder if this should be something like export-ide?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

We can probably get rid of export-codegen once export exports it. I don't want to call it export-ide because it may be useful for other purposes. And that is a clunky name. This can be an "export various things" goal, with IDEs being a good use-case, but perhaps not the only one.

Choose a reason for hiding this comment

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

I'm interested to see the evolution of export. Seems like it will have one-or-more subcommands for various (sub-?)subsystems.... sub-goals?.

I'm interested in possibly adding functionality to the export goal for exporting a best-guess module_mapping.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

How would this be consumed? That functionality would be awesome, but export may not be the best place for it.

Choose a reason for hiding this comment

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

By humans I suppose. You're probably right, let me switch hats: ... and I've forgotten my other use-case. Ignore me for now 😄

Copy link
Member

Choose a reason for hiding this comment

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

I tend to agree, that export could be a generic goal for getting anything out of the pants world to disk.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

Well, export is more about exporting internal information, for consumption by other tools, such as IDEs. We'd export codegen for that reason only - in normal use codegen is a build byproduct that is not visible to the end user. But since the user may want to manually inspect it, and may want to view it in their IDE, we'd export it.

Documentation, though, is a build product, like a PEX file or a distribution or a docker image or whatever. It is an actual formal output of the system that you can request. So I don't think export is the right place for it, just as we don't use export to get compiled code, or any other build product.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Agree with @benjyw on this one... I expect that docs meet the bar for a dedicated goal. Likewise, I think that export being "for anything you want to get out of pants" is probably too broad: focusing on the IDE usecase is valid (i.e., making this page as short as possible: https://www.pantsbuild.org/docs/setting-up-an-ide).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah OK I agree. But now you have me wondering if we should package documentation 😂

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Good question!

help = "Export Pants data for use in other tools, such as IDEs."


class Export(Goal):
subsystem_cls = ExportSubsystem


@goal_rule
async def export(
console: Console,
targets: Targets,
export_subsystem: ExportSubsystem,
workspace: Workspace,
union_membership: UnionMembership,
dist_dir: DistDir,
) -> Export:
request_types = cast(
"Iterable[type[ExportableDataRequest]]", union_membership.get(ExportableDataRequest)
)
requests = tuple(request_type(targets) for request_type in request_types)
exportables = await MultiGet(
Get(ExportableData, ExportableDataRequest, request) for request in requests
)
output_dir = os.path.join(str(dist_dir.relpath), "export")
merged_digest = await Get(Digest, MergeDigests(exp.digest for exp in exportables))
dist_digest = await Get(Digest, AddPrefix(merged_digest, output_dir))
workspace.write_digest(dist_digest)
for exp in exportables:
for symlink in exp.symlinks:
link_abspath = os.path.abspath(
os.path.join(output_dir, exp.reldir, symlink.link_rel_path)
)
absolute_symlink(symlink.source_abs_path, link_abspath)
console.print_stdout(f"Wrote {exp.description} to {os.path.join(output_dir, exp.reldir)}")
return Export(exit_code=0)


def rules():
return collect_rules()
2 changes: 2 additions & 0 deletions src/python/pants/core/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from pants.core.goals import (
check,
export,
fmt,
lint,
package,
Expand Down Expand Up @@ -48,6 +49,7 @@ def rules():
return [
# goals
*check.rules(),
*export.rules(),
*fmt.rules(),
*lint.rules(),
*update_build_files.rules(),
Expand Down