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

Decrease start-up time of editable-installed entry points on newer versions of Python #2194

Merged
merged 3 commits into from Jun 15, 2020

Conversation

ofek
Copy link
Sponsor Contributor

@ofek ofek commented Jun 14, 2020

Summary of changes

When importlib.metadata is present, the generated scripts will use that rather than the significantly slower pkg_resources.

Closes #510 completely as editable installs are the final affected part (that is actionable) brought up in that issue and what I've been experiencing the last 2 weeks (it's indeed annoying)

The issue is locked, so here are some pings: @jaraco @scopatz @ninjaaron @untitaker @asottile @pganssle @gaborbernat @pfmoore @pradyunsg

Benchmark

Before and after, using hyperfine:

Capture

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details

setuptools/tests/test_easy_install.py Outdated Show resolved Hide resolved

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
for entry_point in distribution(%(spec)r).entry_points:
Copy link
Contributor

@asottile asottile Jun 14, 2020

Choose a reason for hiding this comment

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

should this use entry_points()['console_scripts'] perhaps instead?

Copy link
Sponsor Contributor Author

@ofek ofek Jun 14, 2020

Choose a reason for hiding this comment

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

Wouldn't that enumerate every script for every package?

Copy link
Contributor

@asottile asottile Jun 14, 2020

Choose a reason for hiding this comment

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

iirc distribution already enumerates every package but I might be misremembering

Copy link
Sponsor Contributor Author

@ofek ofek Jun 14, 2020

Choose a reason for hiding this comment

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

Copy link
Member

@jaraco jaraco Jun 15, 2020

Choose a reason for hiding this comment

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

I was surprised this interface isn't nicer :/. Yes, I believe this usage is correct. Probably importlib-metadata should provide some helpers to make this usage simpler, though that won't help for this implementation.

setuptools/command/easy_install.py Outdated Show resolved Hide resolved
load_entry_point(%(spec)r, %(group)r, %(name)r)()
)
""").lstrip()
if sys.version_info >= (3, 8):
Copy link
Member

@jaraco jaraco Jun 15, 2020

Choose a reason for hiding this comment

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

I'm trying to imagine a way this could rely also on importlib_metadata and thus eliminate the dependency on pkg_resources. I dislike that the code has to be forked to support the legacy behavior (and until Python 3.7 support is dropped). Perhaps that can't be solved now.

load_entry_point(%(spec)r, %(group)r, %(name)r)()
)
""").lstrip()
if sys.version_info >= (3, 8):
Copy link
Member

@jaraco jaraco Jun 15, 2020

Choose a reason for hiding this comment

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

I also wonder if it would make sense to unify these two scripts into one which tries importlib-metadata then falls back to pkg_resources. That approach would avoid the fork and would also avoid the suppressed linter errors.

load_entry_point('spec', 'console_scripts', 'name')()
)
""") # noqa: E501
if sys.version_info >= (3, 8):
Copy link
Member

@jaraco jaraco Jun 15, 2020

Choose a reason for hiding this comment

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

I'll plan to consolidate this fork later... and just make some assertions about the result that are common to both behaviors.

jaraco
jaraco approved these changes Jun 15, 2020
Copy link
Member

@jaraco jaraco left a comment

My comments are largely for edification. No action is necessary. Let's adopt this and iterate on it. Thanks for the contrib.

@jaraco jaraco merged commit 9befd91 into pypa:master Jun 15, 2020
24 of 26 checks passed
@ofek ofek deleted the fast branch Jun 15, 2020
@ofek
Copy link
Sponsor Contributor Author

ofek commented Jun 15, 2020

Thank you!

@ofek ofek mentioned this pull request Jun 15, 2020
2 tasks
@ofek
Copy link
Sponsor Contributor Author

ofek commented Jun 15, 2020

Needs small fix #2196

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Feb 10, 2021
pypa/setuptools#2194

git-svn-id: file:///srv/repos/svn-community/svn@852053 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Feb 10, 2021
pypa/setuptools#2194


git-svn-id: file:///srv/repos/svn-community/svn@852053 9fca08f4-af9d-4005-b8df-a31f2cc04f65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants