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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Jun 15, 2020
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01121d0
Decrease start-up time of editable-installed entry points on newer ve…
ofek Jun 14, 2020
d6501f3
address
ofek Jun 14, 2020
f8cd1f2
address
ofek Jun 15, 2020
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
1 change: 1 addition & 0 deletions changelog.d/2194.change.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Editable-installed entry points now load significantly faster on Python versions 3.8+.
41 changes: 28 additions & 13 deletions setuptools/command/easy_install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2070,19 +2070,34 @@ class ScriptWriter:
gui apps.
"""

template = textwrap.dedent(r"""
# EASY-INSTALL-ENTRY-SCRIPT: %(spec)r,%(group)r,%(name)r
__requires__ = %(spec)r
import re
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
sys.exit(
load_entry_point(%(spec)r, %(group)r, %(name)r)()
)
""").lstrip()
if sys.version_info >= (3, 8):
Copy link
Member

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.

Copy link
Member

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.

template = textwrap.dedent(r"""
# EASY-INSTALL-ENTRY-SCRIPT: %(spec)r,%(group)r,%(name)r
__requires__ = %(spec)r
ofek marked this conversation as resolved.
Show resolved Hide resolved
import re
import sys
from importlib.metadata import distribution

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

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
Contributor Author

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

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
Contributor Author

Choose a reason for hiding this comment

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

I think that's distributions:

https://github.com/python/cpython/blob/811e040b6e0241339545c2f055db8259b408802f/Lib/importlib/metadata.py#L498-L512

Copy link
Member

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.

if entry_point.group == %(group)r and entry_point.name == %(name)r:
sys.exit(entry_point.load()())
""").lstrip() # noqa: E501
else:
template = textwrap.dedent(r"""
# EASY-INSTALL-ENTRY-SCRIPT: %(spec)r,%(group)r,%(name)r
__requires__ = %(spec)r
import re
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
sys.exit(
load_entry_point(%(spec)r, %(group)r, %(name)r)()
)
""").lstrip() # noqa: E501

command_spec_class = CommandSpec

Expand Down
42 changes: 29 additions & 13 deletions setuptools/tests/test_easy_install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,35 @@ def test_install_site_py(self, tmpdir):

def test_get_script_args(self):
header = ei.CommandSpec.best().from_environment().as_header()
expected = header + DALS(r"""
# EASY-INSTALL-ENTRY-SCRIPT: 'spec','console_scripts','name'
__requires__ = 'spec'
import re
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
sys.exit(
load_entry_point('spec', 'console_scripts', 'name')()
)
""") # noqa: E501
if sys.version_info >= (3, 8):
Copy link
Member

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.

expected = header + DALS(r"""
# EASY-INSTALL-ENTRY-SCRIPT: 'spec','console_scripts','name'
__requires__ = 'spec'
import re
import sys
from importlib.metadata import distribution

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
for entry_point in distribution('spec').entry_points:
if entry_point.group == 'console_scripts' and entry_point.name == 'name':
sys.exit(entry_point.load()())
""") # noqa: E501
else:
expected = header + DALS(r"""
# EASY-INSTALL-ENTRY-SCRIPT: 'spec','console_scripts','name'
__requires__ = 'spec'
import re
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
sys.exit(
load_entry_point('spec', 'console_scripts', 'name')()
)
""") # noqa: E501

dist = FakeDist()

args = next(ei.ScriptWriter.get_args(dist))
Expand Down