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

benjyw · 2021-10-29T03:59:01Z

This change provides:

A generic extensible mechanism for backends to add exportable data.
An implementation in the python backend that exports a venv with
all the target set's 3rdparty deps.

Follow-up changes can do things like export codegenned sources, or export
source roots into formats that PyCharm and VSCode can read.

[ci skip-rust]

[ci skip-build-wheels]

benjyw · 2021-10-29T04:00:45Z

So far this creates a symlink to an appropriate venv in .cache/pex_root in the distdir.

To see it in action: ./pants export :: (or whatever subset of targets you want to try this out on).

joshua-cannon-techlabs · 2021-10-29T13:24:18Z

I suspect this is a red herring (perhaps just from using the cutting edge pants), but it's worth sharing. I get this output when running it:

Exception message: 1 Exception encountered:

  ResolveError: No owning targets could be found for the file `path/to/conftest.py`.

Please check that there is a BUILD file in the parent directory path/to with a target whose `sources` field includes the file. See https://www.pantsbuild.org/v2.8/docs/targets for more information on target definitions.

You may want to run `./pants tailor` to autogenerate your BUILD files. See https://www.pantsbuild.org/v2.8/docs/create-initial-build-files.



Use --print-stacktrace for more error details and/or -ldebug for more logs. 
See https://www.pantsbuild.org/v2.8/docs/troubleshooting for common issues.
Consider reaching out for help: https://www.pantsbuild.org/v2.8/docs/getting-help

free(): double free detected in tcache 2
Aborted (core dumped)

The double-free is probably of note (even if unrelated).

I'm just going to delete the file for now to move on.

joshua-cannon-techlabs · 2021-10-29T13:36:01Z

Wow, it's everything I imagined 🤩

Two non-blocking feature requests (which could always be filed as issues and followed up on after this PR):

Allow me to specify where this gets generated (ideally this would be a subdir in the workspace root, as that's where devs/tools expect it to be)
Allow me to specify this be done automatically after requirements.pex is created or updated. Essentially, I don't want to have to "remember" to run this lest it point at an older version of my requirements.

joshua-cannon-techlabs · 2021-10-29T13:40:44Z

Of course for feature request 1 I could make a symlink myself to the export dir, so it's really just a "nice-to-have".

joshua-cannon-techlabs · 2021-10-29T14:03:58Z

FWIW I saw the double free error on a different 2.9.X branch as well.

benjyw · 2021-10-29T15:24:03Z

Thanks for the feedback! An option to set the output dir is definitely part of this, just haven't added it yet.

As for it running automatically, if you run

./pants --loop export ::

That invocation will run in a loop and update the symlink whenever anything relevant changes. Would that work?

benjyw · 2021-10-29T15:28:43Z

thanks for noting the double free error (you're running directly against HEAD in the development branch, so expect some rough edges...) We'll take a look at that one. And the conftest.py error is fixable, as the error message says, by adding an appropriate python_sources target (manually, or via ./pants tailor).

Also, is "path/to" an actual path you're testing on, or is that a screwed up error message? :)

joshua-cannon-techlabs · 2021-10-29T15:33:14Z

I still haven't checked out --loop, I'm skeptical it will be as adopted by all of our devs. So if I could have something be done implicitly it's less headache for me to support them
the conftest did have an python_tests in the dir. I assume python_library excludes that.
path/to was my redacted version of the path 😁

benjyw · 2021-10-29T15:38:51Z

Yeah, python_tests is for files containing tests. conftest.py expects to be wrapped by a python_sources (formerly python_library) target.

And for --loop to work you'll need to pull my latest update (which I force-pushed, sorry).

benjyw · 2021-10-29T15:40:21Z

Re doing this implicitly, do you mean when the user runs unrelated pants commands for example? What would trigger an update?

joshua-cannon-techlabs · 2021-10-29T15:48:58Z

Re doing this implicitly, do you mean when the user runs unrelated pants commands for example? What would trigger an update?

Basically any time the path the symlink points to would need to change (due to the hashes in the path), the symlink would also be updated to the new path.
(Opt-in of course)

benjyw · 2021-10-29T16:21:56Z

Basically any time the path the symlink points to would need to change (due to the hashes in the path), the symlink would also be updated to the new path. (Opt-in of course)

Make sense. Today --loop is how you'd opt in to that. In the medium term it could/should be the IDE plugin that does this for you?

joshua-cannon-techlabs · 2021-10-29T16:29:46Z

I would honestly settle for just what's in this PR, lol. Anything extra is just gravy.

joshua-cannon-techlabs · 2021-10-29T16:54:01Z

One side-effect of this current dir structure is the virtual environment thinks it's name is (ddab8011daaee380698ac2fb9701af18c90c03f6.19d82559650744319baba2979601e291) which leads to my terminal looking like this after activation 😄

joshuacannon@CEPHANDRIUS:~/work/techlabs$ source /home/joshuacannon/work/techlabs/.venv/bin/activate
(ddab8011daaee380698ac2fb9701af18c90c03f6.19d82559650744319baba2979601e291) joshuacannon@CEPHANDRIUS:~/work/techlabs$

jsirois · 2021-10-29T18:21:52Z

One side-effect of this current dir structure is the virtual environment thinks it's name is (ddab8011daaee380698ac2fb9701af18c90c03f6.19d82559650744319baba2979601e291)

Hrm. This is because Benjy is re-using auto-created venvs never intended to be directly used. You can also create a venv for direct use (sourcing activate, mutating with pip, etc) via pex-tools <the pex> venv <venv dir to create>. If that were used, then the basename of <venv dir to create> would be your (...) prompt contents. That, though, is slow - up to O(seconds) if the venv is very large. IOW, Benjy is using the right approach here, I think, for speed. It looks like Pex could pass --prompt to both virtualenv (covering Python 2.7) and to python -mvenv (covering Python 3.6+). The only hole is Python 3.5 which has a venv module that does not support this customization. I could either add support for and then expose that option in Pex or else just have Pex create auto-venvs with the prompt named after the base name of the PEX file by default.

benjyw · 2021-10-30T02:36:54Z

Yeah, I'm using those venvs for performance reasons.

Having Pex always set the prompt to something readable would be great. And if a user happens to be using Python 3.5, then apologies...

jsirois · 2021-10-30T04:14:56Z

Having Pex always set the prompt to something readable would be great. And if a user happens to be using Python 3.5, then apologies...

pex-tool/pex#1499

This change provides: 1) A generic extensible mechanism for backends to add exportable data. 2) An implementation in the python backend that exports a venv with all the target set's 3rdparty deps. [ci skip-rust] [ci skip-build-wheels]

benjyw · 2021-11-01T01:14:31Z

This is ready for initial review, thanks.

jsirois

Tests would be great, but this looks right to me.

src/python/pants/backend/python/goals/export.py

kaos

Works for me. :)

Possible future feature that I would like is to also be able to get any 1st party distribution in "editable" mode in the venv..

jsirois · 2021-11-01T11:00:15Z

Possible future feature that I would like is to also be able to get any 1st party distribution in "editable" mode in the venv..

IIUC editable mode installs are just symlinks to live sources. Here we use copies and - per PR discussion - ./pants --loop ... for liveness refreshing.

kaos · 2021-11-01T14:24:38Z

Here we use copies

For 3rd party code only. There's no 1st party code in this export (from what I could tell in my case, at least)

Of course, a normal install would work too, in tandem with loop.

joshua-cannon-techlabs · 2021-11-01T14:32:35Z

Re: First-party. That's something I didn't think of (but not required for my use-case). If you're using source roots (E.g. src/python/...) your editor isn't likely to pick up on this without config. So either: A) You teach your editor about the source root, or B) export also symlinks the source roots in the shim "venv"

benjyw · 2021-11-01T16:15:54Z

I was thinking of exporting IDE config that points to source roots, since the IDE (at least pycharm) will treat sources in the venv as third-party.

jsirois · 2021-11-01T16:22:40Z

I was thinking of exporting IDE config that points to source roots, since the IDE (at least pycharm) will treat sources in the venv as third-party.

That will work for simple cases with consistent global resolves which you already ~limit this too anyhow. For other cases we'll need link farms just containing the sources compatible with the given resolve to avoid red-squiggleys.

[ci skip-rust] [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

stuhood · 2021-11-01T19:53:39Z

src/python/pants/backend/python/goals/export.py

+@rule
+async def export_venv(
+    request: ExportedVenvRequest, python_setup: PythonSetup, pex_env: PexEnvironment
+) -> ExportableData:


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?

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.).

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

[ci skip-rust] [ci skip-build-wheels]

benjyw · 2021-11-02T05:51:11Z

Addressed the comments and added tests. PTAL.

In a follow up I can optionally change symlinking to copying, to give the user a tradeoff between safety and performance (with the default on the side of safety).

[ci skip-rust] [ci skip-build-wheels]

Eric-Arellano · 2021-11-02T17:52:50Z

src/python/pants/core/goals/export.py

+
+
+class ExportSubsystem(GoalSubsystem):
+    name = "export"


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

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.

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.

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

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

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

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.

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).

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

Good question!

[ci skip-rust] [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

benjyw marked this pull request as draft October 29, 2021 03:59

benjyw mentioned this pull request Oct 29, 2021

Add "generic" IDE support via virtualenv and codegen export #11152

Open

benjyw force-pushed the ide_export branch 2 times, most recently from 33e9596 to 19bf49a Compare November 1, 2021 00:43

benjyw force-pushed the ide_export branch from 19bf49a to 1b72946 Compare November 1, 2021 01:13

benjyw changed the title ~~WIP~~ An export goal for exporting data to be read by IDEs. Nov 1, 2021

benjyw requested review from stuhood and Eric-Arellano November 1, 2021 01:14

benjyw marked this pull request as ready for review November 1, 2021 01:14

jsirois approved these changes Nov 1, 2021

View reviewed changes

src/python/pants/backend/python/goals/export.py Outdated Show resolved Hide resolved

src/python/pants/backend/python/goals/export.py Outdated Show resolved Hide resolved

src/python/pants/backend/python/goals/export.py Outdated Show resolved Hide resolved

kaos approved these changes Nov 1, 2021

View reviewed changes

benjyw added 2 commits November 1, 2021 12:13

Merge branch 'main' into ide_export

2dd03f4

[ci skip-rust] [ci skip-build-wheels]

Address codfe review comments

44983f5

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

stuhood reviewed Nov 1, 2021

View reviewed changes

Add tests.

c80db6b

[ci skip-rust] [ci skip-build-wheels]

Merge branch 'main' into ide_export

7228135

[ci skip-rust] [ci skip-build-wheels]

benjyw mentioned this pull request Nov 2, 2021

Fixes to interpreter constraint computation. #13465

Merged

Merge branch 'main' into ide_export

b549f8b

[ci skip-rust] [ci skip-build-wheels]

Eric-Arellano reviewed Nov 2, 2021

View reviewed changes

benjyw added 2 commits November 2, 2021 15:53

Merge branch 'main' into ide_export

4b8ddf8

[ci skip-rust] [ci skip-build-wheels]

Handle empty constraints

0a2477d

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

benjyw requested review from stuhood and Eric-Arellano November 2, 2021 21:57

benjyw merged commit 31c59b2 into pantsbuild:main Nov 3, 2021

benjyw deleted the ide_export branch November 3, 2021 02:46

stuhood mentioned this pull request Jan 6, 2022

[internal] Add a release artifact size verification step. #13234

Merged

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

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

Conversation

benjyw commented Oct 29, 2021 • edited Loading

benjyw commented Oct 29, 2021

joshua-cannon-techlabs commented Oct 29, 2021 • edited Loading

joshua-cannon-techlabs commented Oct 29, 2021

joshua-cannon-techlabs commented Oct 29, 2021 • edited Loading

joshua-cannon-techlabs commented Oct 29, 2021

benjyw commented Oct 29, 2021

benjyw commented Oct 29, 2021

joshua-cannon-techlabs commented Oct 29, 2021

benjyw commented Oct 29, 2021

benjyw commented Oct 29, 2021

joshua-cannon-techlabs commented Oct 29, 2021

benjyw commented Oct 29, 2021

joshua-cannon-techlabs commented Oct 29, 2021 • edited Loading

joshua-cannon-techlabs commented Oct 29, 2021

jsirois commented Oct 29, 2021 • edited Loading

benjyw commented Oct 30, 2021

jsirois commented Oct 30, 2021

benjyw commented Nov 1, 2021

jsirois left a comment

Choose a reason for hiding this comment

kaos left a comment

Choose a reason for hiding this comment

jsirois commented Nov 1, 2021

kaos commented Nov 1, 2021 • edited Loading

joshua-cannon-techlabs commented Nov 1, 2021

benjyw commented Nov 1, 2021

jsirois commented Nov 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjyw commented Nov 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjyw commented Oct 29, 2021 •

edited

Loading

joshua-cannon-techlabs commented Oct 29, 2021 •

edited

Loading

joshua-cannon-techlabs commented Oct 29, 2021 •

edited

Loading

joshua-cannon-techlabs commented Oct 29, 2021 •

edited

Loading

jsirois commented Oct 29, 2021 •

edited

Loading

kaos commented Nov 1, 2021 •

edited

Loading

jsirois commented Nov 1, 2021 •

edited

Loading