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

Python: consider adding support for local projects (not local requirements) #16621

Open
Eric-Arellano opened this issue Aug 24, 2022 · 8 comments
Labels
backend: Python Python backend-related issues enhancement

Comments

@Eric-Arellano
Copy link
Contributor

We now robustly support local requirements, i.e. having .whl and sdist files on the filesystem. But we don't yet support local projects, like my/project, unlike Pex: #16584 (comment). This is a useful feature for debugging because it avoids needing to first package the local project.

@jsirois
Copy link
Contributor

jsirois commented Aug 24, 2022

I sure hope this is s/consider/do/! It was my strong impression local project locking was required of Pex by Pants. If not, I'd like to know. There is alot of code I'd be happy to kill in Pex that supports this, including a PEP-517 / PEP-518 sdist builder that was needed to get this working.

@jsirois
Copy link
Contributor

jsirois commented Aug 24, 2022

See here: pex-tool/pex#1696 (comment)

@Eric-Arellano
Copy link
Contributor Author

Oof, that was some serious misunderstanding on my part between local requirements vs projects. I believe the thing Pants users cared the most about is local requirements, although projects would be nice to have.

There is alot of code I'd be happy to kill in Pex that supports this, including a PEP-517 / PEP-518 sdist builder that was needed to get this working.

Is that code different than what was needed for VCS requirements to work properly?

@jsirois
Copy link
Contributor

jsirois commented Aug 24, 2022

Is that code different than what was needed for VCS requirements to work properly?

Yes.

@jsirois
Copy link
Contributor

jsirois commented Aug 24, 2022

Oof, that was some serious misunderstanding on my part between local requirements vs projects.

You must have been completely mystified by several PRs! You definitely should have spoken up.

@Eric-Arellano
Copy link
Contributor Author

You must have been completely mystified by several PRs! You definitely should have spoken up.

I understood what the local projects were when reviewing the PRs that actually implemented it. I did not realize that pex-tool/pex#1696 (comment) was the reason all that work happened. I apologize for the miscommunication.

There is alot of code I'd be happy to kill in Pex that supports this, including a PEP-517 / PEP-518 sdist builder that was needed to get this working.

I think this is worth considering. It doesn't seem that Pants "needs" local project support, whereas it definitely does want local requirement support. I'm doubtful that we could budget the time in the next 2-3 months for wiring this all up to Pants.

cc @benjyw @stuhood if you have thoughts

@cognifloyd
Copy link
Member

By "local project", do you mean "editable installs" (which is only one kind of local project listed at https://pip.pypa.io/en/stable/topics/local-project-installs/)? If you do mean "editable install", then:

When exporting a venv with ./pants export --resolve=... (using the default mutable venv): It would be nice if pants would add an editable install to the exported venv of all the relevant (present in the resolve) python_distributions.

A "local project" might also be interesting if you wanted a sandbox to use local project installs instead of PYTHONPATH manipulation. eg maybe for an adhoc_tool that needs all installed metadata (which goes beyond the code being available on the PYTHONPATH)?

@jsirois
Copy link
Contributor

jsirois commented Mar 14, 2023

By "local project", do you mean "editable installs"

Basically - yes. But pip / setuptools need not be in the picture. This simply means locking a local project directory. The only requirement is the local project directory contain a top-level pyproject.toml or setup.py. There are 0 other requirements. The lock is formed by creating an sdist using pyproject.toml / setup.py and then hashing that sdist. If any of the local project files that are relevant (included in the sdist) are altered, consuming the lock will fail and the lock will need to be re-generated to lock the new state of the local project.

cognifloyd added a commit that referenced this issue Apr 10, 2023
## About Editable Installs

Editable installs were traditionally done via pip with `pip install
--editable`. It is primarily useful during development when software
needs access to the entry points metadata.

When [PEP 517](https://peps.python.org/pep-0517/) was adopted, they
punted on how to allow for editable installs. [PEP
660](https://peps.python.org/pep-0660/) extended the PEP 517 backend API
to support building "editable" wheels. Therefore, there is now a
standard way to collect and install the metadata for "editable"
installs, using the "editable" wheel as an interchange between the
backend (which collects the metadata + builds the editable wheel) and
the frontend (which marshals the backend to perform a user-requested
"editable" install).

## Why would we need editable installs in pants?

I need editable installs in pants-exported virtualenvs so that dev tools
outside of pants have access to:
- The locked requirements
- The editable sources on the python path
- The entry points (and any other package metadata that can be loaded
from `dist-info`. Entry points is the biggest most impactful example)

I need to point all the external dev tooling at a virtualenv, and
technically I could export a pex that includes all of the
python-distributions pre-installed and use pex-tools to create a
virtualenv, but then I would have to recreate that venv for every dev
change wouldn't be a good dev experience.

One of those dev tools is `nosetest`. I considred using `run` to support
running that, but I am leary of adding all the complex BUILD machinery
to support running a tool that I'm trying to get rid of. Editable
installs is a more generic solution that serves my current needs, while
allowing for using it in other scenarios.

This PR comes in part from
#16621 (comment)

## Overview of this PR

### Scope & Future work

This PR focuses on adding editable installs to exported virtualenvs. Two
other issues ask for editable installs while running tests:
- #11386
- #15481

We can probably reuse a significant portion of this to generate editable
wheels for use in testing as well. Parts of this code will need to be
refactored to support that use case. But we also have to figure out the
UX for how users will define dependencies on a `python_distribution`
when they want an editable install instead of the built wheel to show up
in the sandbox. Anyway, addressing that is out of scope for this PR.

### New option `[export].py_editables_in_resolves` (a `StrListOption`)

This option allows user resolves to opt in to using the editable
installs. After
[consulting](https://pantsbuild.slack.com/archives/C0D7TNJHL/p1680810411706569?thread_ts=1680809713.310039&cid=C0D7TNJHL)
with @kaos, I decided to add an option for this instead of always trying
to generate/install the editable wheels.

> `python_distribution` does not have a `resolve` field. So figuring out
which resolve a `python_distribution` belongs to can be expensive:
calculating the owned deps of all distributions, and for each
distribution, look through those deps until one of them has a resolve
field, and use that for that dist’s resolve.
>
> Plus there’s the cost of building the PEP-660 wheels - if the
configured PEP-517 build backend doesn’t support the PEP-660 methods,
then it falls back to a method that is, sadly, optional in PEP-517. If
that method isn’t there, then it falls back to telling that backend to
build the whole wheel, and then it extracts just the dist-info directory
from it and discards the rest.
>
> So, installing these editable wheels isn’t free. It’ll slow down the
export, though I’m not sure by how much.

For StackStorm, I plan to set this in `pants.toml` for the default
resolve that has python_distributions.

Even without this option, I tried to bail out early if there were no
`python_distribution`s to install.

### Installing editable wheels for exports

I added this feature to the new export code path, which requires using
`export --resolve=`. The legacy codepath, which uses cli specs `export
<address specs>` did not change at all. I also ignored the `tool`
resolves which cannot have any relevant dists (and `tool` resolves are
deprecated anyway). Also, this is only for `mutable_virtualenv` exports,
as we need modify the virtualenv to install the editable wheels in the
venv after pex creates it from the lockfile.

When exporting a user resolve, we do a `Get(EditableLocalDists,
EditableLocalDistsRequest(resolve=resolve))`: _I'll skip over exactly
how this builds the wheels for now so this section can focus on how
installing works._


https://github.com/pantsbuild/pants/blob/f3a4620e81713f5022bf9a2dd1a4aa5ca100d1af/src/python/pants/backend/python/goals/export.py#L373-L379

As described in the commit message of
b5aa26a, I tried a variety of methods
for installing the editable wheels using pex. Ultimately, the best I
came up with is telling pex that the digest containing our editable
wheels are `sources` when building the `requirements.pex` used to
populate the venv, so that they would land in the virtualenv (even
though they land as plain wheel files.

Then we run `pex-tools` in a `PostProcessingCommand` to create and
populate the virtualenv, just as we did before this PR.

Once the virtualenv is created, we add 3 more `PostProcessingCommands`
to actually do the editable install. In this step, Pants is essentially
acting as the PEP-660 front end, though we use pip for some of the heavy
lifting. These commands:
1. move the editable wheels out of the virtualenv lib directory to the
temp dir that gets deleted at the end of the export
2. use pip to install all of the editable wheels (which contain a `.pth`
file that injects the source dir into `sys.path` and a `.dist-info`
directory with dist metadata such as entry points).
3. replace some of the pip-generated install metadata
(`*.dist-info/direct_url.json`) with our own so that we comply with
PEP-660 and mark the install as editable with a file url pointing the
the sources in the build_root (vs in a sandbox).

Now, anything that uses the exported venv should have access to the
standardized package metadata (in `.dist-info`) and the relevant source
roots should be automatically included in `sys.path`.

### Building PEP-660 editable wheels

The logic that actually builds the editable wheels is in
`pants.backend.python.util_rules.local_dists_pep660`. Building these
wheels requires the same chroot that pants uses to build regular wheels
and sdists. So, I refactored the rule in `util_rules.setup_py` so that I
could reuse the part that builds the `DistBuildRequest`.

These `local_dists_pep660` rules do approx this, starting with the rule
called in export:
- `Get(EditableLocalDists, EditableLocalDistsRequest(resolve=resolve))`
uses rule `build_editable_local_dists`
- injected arg: `ResolveSortedPythonDistributionTargets` comes from
rule: `sort_all_python_distributions_by_resolve`
- injected arg: `AllPythonDistributionTargets` comes from rule:
`find_all_python_distributions`
- `Get(LocalDistPEP660Wheels, PythonDistributionFieldSet.create(dist))`
for each dist in the resolve uses rule:
`isolate_local_dist_pep660_wheels`
- create `DistBuildRequest` using the `create_dist_build_request` method
I exposed in `util_rules.setup_py`
- `Get(PEP660BuildResult, DistBuildRequest)` uses rule:
`run_pep660_build`
            - generates the `.pth` file that goes in the editable wheel
            - runs a PEP 517 build backend wrapper script I wrote
- uses the PEP 517 build backend configured for the
`python_distribution` to generate the `.dist-info` directory
- generates the `WHEEL` and `RECORD` file to build a conformant wheel
file
- includes the `.pth` file previously generated (and placed in the
sandbox with the wrapper script)
- uses `zipfile` to build the wheel (using a vendored+modified function
from the `wheel` package).
                - prints a path to the generated wheel
- collects the generated editable wheel into a digest and collects
metadata about the digest similar to how the `local_dists` rules do.
- merges the editable wheel digests for all of the `python_distribution`
targets. This gets wrapped in `EditableLocalDists`

Much of the rule logic was based on (copied then modified):
`pants.backend.python.util_rules.dists` and
`pants.backend.python.util_rules.local_dists`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues enhancement
Projects
None yet
Development

No branches or pull requests

3 participants