-
-
Notifications
You must be signed in to change notification settings - Fork 631
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_distribution editable installs in exports #18639
Merged
Merged
Changes from 25 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
d1cfd68
naive PEP660
cognifloyd d36b839
refactor package_python_dist so most of the logic can be reused for e…
cognifloyd 49465d9
begin writing PEP 660 backend that wraps PEP 517 backend to get dist-…
cognifloyd 692afac
finish pep660 backend/wrapper
cognifloyd eb941f4
move tags calculation out of wrapper script
cognifloyd d15ecb4
reorder rules so that they get registered in an order pants can handle
cognifloyd b18f000
generate metadata required to build a valid wheel
cognifloyd cfd7213
add tests for local_dists_pep660
cognifloyd 7a52ce0
simplify LocalDistsPEP660Pex by dropping pointless remaining_sources
cognifloyd 6909550
simplify LocalDistsPEP660PexRequest by dropping pointless sources
cognifloyd d86a9a7
wire up export to install editable wheels in mutable resolves for use…
cognifloyd cec0583
get the resolve for python_distributions which do not have a resolve …
cognifloyd 750c562
revert some export changes to simplify editable dists implementation
cognifloyd f2b8b58
refactor editable wheel passing and installation
cognifloyd 3557fac
add test_sort_all_python_distributions_by_resolve
cognifloyd 6357b85
add [export].py-editables-in-resolves option
cognifloyd b65a079
typos
cognifloyd 0aa9172
fix option access
cognifloyd d5b3562
drop unnecessary option in test
cognifloyd 2001e4f
PEP660: Standardize wheel contents for older PEP517 backends
cognifloyd b660c57
clean up editable wheel install to not re-install deps
cognifloyd 3f57983
Merge branch 'main' into editables-in-exports
cognifloyd 9c1df00
adjust for moved rules
cognifloyd 017dc7b
address review feedback
cognifloyd d230ea5
reformat
cognifloyd 0d95146
refactor PEP 660 wrapper into a resource script
cognifloyd fb4fbcd
update INSTALLER file afeter installing PEP-660 editable wheels
cognifloyd 26d0091
rename option to [export].py_editable_in_resolve
cognifloyd c50724e
reformat and misc fixes
cognifloyd e4f0748
fix arg name
cognifloyd 9090edf
add deps
cognifloyd 15fbc1e
backend wrapper bugs
cognifloyd 8250103
make explicit BUILD dep more precise
cognifloyd 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 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 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 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 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
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.
This option name reads weird to me, but I've not yet been able to come up with something better..
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.
Yeah, I have also failed to come up with anything.
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.
I went through a lot of different names - I tried putting it on a
PythonSetup(Subsystem)
but that felt weird because this is specifically about export. So, then I moved it here and went with thepy_
prefix similar topy_resolve_format
. I'm still not entirely happy with it. But, this is the least bad I've come up with so far.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.
How about
py_editable_by_resolve
?I know it's a list; so plural. But it is used on a "one by one" basis, so this reads better to me. It answers: "Are python distributions to be installed in
editable
mode in this resolve?" for a given resolve. The "editable" thing is not plural in that question, nor is the resolve, but the source for the answer is used for multiple instances of that question.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.
Each dist in the resolve is installed as editable, which is why I was thinking of using
editables
-- each resolve has more than one editable wheel installed.As I struggle with naming this, one thing I'm worried about is: You can do an editable install of ~anything. If you request an editable install of a git repo, for example, pip will clone it and then you'll be able to see where it is cloned in
pip list
output. So, I want to make it clear that this is only editable for first party code.That said, Hmm.
by
makes this sound like a map. I could however drop the plural and dopy_editable_in_resolve
. Is that any better?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.
To me, yes. As I see the
editable
as an adjective describing how something(s) are installed it looks weird to pluralize it. Maybe I'm overanalyzing this however.. language is.. rather subjective, perhaps.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.
OK. I just pushed a change with
py_editable_in_resolve
.