- Notifications
You must be signed in to change notification settings - Fork 3.2k
Script generation code for wheels #1251
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -13,10 +13,12 @@ | |
| import sys | ||
| from base64 import urlsafe_b64encode | ||
| | ||
| from pip.backwardcompat import ConfigParser | ||
| from pip.locations import distutils_scheme | ||
| from pip.log import logger | ||
| from pip import pep425tags | ||
| from pip.util import call_subprocess, normalize_path, make_path_relative | ||
| from pip._vendor.distlib.scripts import ScriptMaker | ||
| | ||
| wheel_ext = '.whl' | ||
| | ||
| | @@ -33,7 +35,7 @@ def rehash(path, algo='sha256', blocksize=1<<20): | |
| """Return (hash, length) for path using hashlib.new(algo)""" | ||
| h = hashlib.new(algo) | ||
| length = 0 | ||
| with open(path) as f: | ||
| with open(path, 'rb') as f: | ||
| block = f.read(blocksize) | ||
| while block: | ||
| length += len(block) | ||
| | @@ -103,6 +105,19 @@ def root_is_purelib(name, wheeldir): | |
| return True | ||
| return False | ||
| | ||
| def get_entrypoints(filename): | ||
| if not os.path.exists(filename): | ||
| return {}, {} | ||
| cp = ConfigParser.RawConfigParser() | ||
| cp.read(filename) | ||
| console = {} | ||
| gui = {} | ||
| if cp.has_section('console_scripts'): | ||
| console = dict(cp.items('console_scripts')) | ||
| if cp.has_section('gui_scripts'): | ||
| gui = dict(cp.items('gui_scripts')) | ||
| return console, gui | ||
| | ||
| def move_wheel_files(name, req, wheeldir, user=False, home=None, root=None): | ||
| """Install a wheel""" | ||
| | ||
| | @@ -118,6 +133,7 @@ def move_wheel_files(name, req, wheeldir, user=False, home=None, root=None): | |
| source = wheeldir.rstrip(os.path.sep) + os.path.sep | ||
| installed = {} | ||
| changed = set() | ||
| generated = [] | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, a little picky, but maybe a "generated_scripts", or a comment that these are the console scripts we're going to generate Member Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In theory, these could be any generated files, not just scripts. So I'd rather add a comment than change the name. | ||
| | ||
| def normpath(src, p): | ||
| return make_path_relative(src, p).replace(os.path.sep, '/') | ||
| | @@ -130,7 +146,7 @@ def record_installed(srcfile, destfile, modified=False): | |
| if modified: | ||
| changed.add(destfile) | ||
| | ||
| def clobber(source, dest, is_base, fixer=None): | ||
| def clobber(source, dest, is_base, fixer=None, filter=None): | ||
| if not os.path.exists(dest): # common for the 'include' path | ||
| os.makedirs(dest) | ||
| | ||
| | @@ -152,6 +168,9 @@ def clobber(source, dest, is_base, fixer=None): | |
| if not os.path.exists(destsubdir): | ||
| os.makedirs(destsubdir) | ||
| for f in files: | ||
| # Skip unwanted files | ||
| if filter and filter(f): | ||
| continue | ||
| srcfile = os.path.join(dir, f) | ||
| destfile = os.path.join(dest, basedir, f) | ||
| shutil.move(srcfile, destfile) | ||
| | @@ -164,15 +183,66 @@ def clobber(source, dest, is_base, fixer=None): | |
| | ||
| assert info_dir, "%s .dist-info directory not found" % req | ||
| | ||
| # Get the defined entry points | ||
| ep_file = os.path.join(info_dir[0], 'entry_points.txt') | ||
| console, gui = get_entrypoints(ep_file) | ||
| | ||
| def is_entrypoint_wrapper(name): | ||
| # EP, EP.exe and EP-script.py are scripts generated for | ||
| # entry point EP by setuptools | ||
| if name.lower().endswith('.exe'): | ||
| matchname = name[:-4] | ||
| elif name.lower().endswith('-script.py'): | ||
| matchname = name[:-10] | ||
| else: | ||
| matchname = name | ||
| # Ignore setuptools-generated scripts | ||
| return (matchname in console or matchname in gui) | ||
| | ||
| for datadir in data_dirs: | ||
| fixer = None | ||
| filter = None | ||
| for subdir in os.listdir(os.path.join(wheeldir, datadir)): | ||
| fixer = None | ||
| if subdir == 'scripts': | ||
| fixer = fix_script | ||
| filter = is_entrypoint_wrapper | ||
| source = os.path.join(wheeldir, datadir, subdir) | ||
| dest = scheme[subdir] | ||
| clobber(source, dest, False, fixer=fixer) | ||
| clobber(source, dest, False, fixer=fixer, filter=filter) | ||
| | ||
| maker = ScriptMaker(None, scheme['scripts']) | ||
| maker.variants = set(('', )) | ||
| | ||
| # Special case pip and setuptools to generate versioned wrappers | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the story on this again? why not just honor what we have in our setup.py? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because entry points are computed at wheel build time, so because of the line you pasted it would otherwise make what could be a universal wheel, Python version specific. With this change we override those values so that a Wheel made on Python 2.7 when isntalling in 3.4 does not install a pip2.7 binary that is actually installed with Python 3.4. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i.e. it seems like we'd either honor setup.py for all projects, or override for all projects. why pip and setuptools only? and if it's just pip and setuptools, why not just update our setup.py whatever the answer, can we add a comment about it. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is mimic'ing our setup.py at wheel build time, and it's a temporary measure until Metadata 2.0 has proper support for versionined entry points. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do need to mimic? what's in setup.py, will be replicated in entry_points.txt, and we just follow that. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
oh, ok. for python-version agnostic wheels. let's comment on that. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because if we don't mimic it then all of the pip wheels are Python version specific, so we'll need a separate Wheel for 2.6, 2.7, 3.1, 3.2, 3.3, and 3.4. If we mimic it here then A single universal Wheel will work for all versions of Python. Member Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that section you quote would generate pip2.7.exe even when the wheel is installed using Python 3.3. The only way to get the versioned entry points matching the interpreter we're installing with, is to generate the names at install time. Or to have a separate wheel for each version, which is what we're trying to avoid. Long term, the proper solution will be for the metadata to be able to say that we want versioned entry points. But that metadata doesn't exist yet, and rather than make up our own temporary solution, it was easier to just special-case pip and setuptools for now. Agreed that this is confusing, though. I will add a comment (hopefully not an essay!) Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I think I got this now. we have no convention to do this generally for all projects. | ||
| pip_script = console.pop('pip', None) | ||
| if pip_script: | ||
| spec = 'pip = ' + pip_script | ||
| generated.extend(maker.make(spec)) | ||
| spec = 'pip%s = %s' % (sys.version[:1], pip_script) | ||
| generated.extend(maker.make(spec)) | ||
| spec = 'pip%s = %s' % (sys.version[:3], pip_script) | ||
| generated.extend(maker.make(spec)) | ||
| # Delete any other versioned pip entry points | ||
| pip_ep = [k for k in console if re.match(r'pip(\d(\.\d)?)?$', k)] | ||
| for k in pip_ep: | ||
| del console[k] | ||
| easy_install_script = console.pop('easy_install', None) | ||
| if easy_install_script: | ||
| spec = 'easy_install = ' + easy_install_script | ||
| generated.extend(maker.make(spec)) | ||
| spec = 'easy_install-%s = %s' % (sys.version[:3], easy_install_script) | ||
| generated.extend(maker.make(spec)) | ||
| # Delete any other versioned easy_install entry points | ||
| easy_install_ep = [k for k in console | ||
| if re.match(r'easy_install(-\d\.\d)?$', k)] | ||
| for k in easy_install_ep: | ||
| del console[k] | ||
| | ||
| if len(console) > 0: | ||
| generated.extend(maker.make_multiple(['%s = %s' % kv for kv in console.items()])) | ||
| if len(gui) > 0: | ||
| generated.extend(maker.make_multiple(['%s = %s' % kv for kv in gui.items()], {'gui': True})) | ||
| | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe a comment above these 4 lines "Generate console and gui scripts" | ||
| record = os.path.join(info_dir[0], 'RECORD') | ||
| temp_record = os.path.join(info_dir[0], 'RECORD.pip') | ||
| | @@ -185,6 +255,9 @@ def clobber(source, dest, is_base, fixer=None): | |
| if row[0] in changed: | ||
| row[1], row[2] = rehash(row[0]) | ||
| writer.writerow(row) | ||
| for f in generated: | ||
| h, l = rehash(f) | ||
| writer.writerow((f, h, l)) | ||
| for f in installed: | ||
| writer.writerow((installed[f], '', '')) | ||
| shutil.move(temp_record, record) | ||
| | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import os | ||
| import pytest | ||
| from tests.lib.path import Path | ||
| | ||
| | @@ -102,3 +103,77 @@ def test_install_user_wheel(script, virtualenv, data): | |
| assert egg_info_folder in result.files_created, str(result) | ||
| script_file = script.user_bin / 'script.py' | ||
| assert script_file in result.files_created | ||
| | ||
| def test_install_from_wheel_gen_entrypoint(script, data): | ||
| """ | ||
| Test installing scripts (entry points are generated) | ||
| """ | ||
| result = script.pip('install', 'script.wheel1a==0.1', '--use-wheel', | ||
| '--no-index', '--find-links='+data.find_links, | ||
| expect_error=False) | ||
| if os.name == 'nt': | ||
| wrapper_file = script.bin / 't1.exe' | ||
| else: | ||
| wrapper_file = script.bin / 't1' | ||
| assert wrapper_file in result.files_created | ||
| | ||
| def test_install_from_wheel_with_legacy(script, data): | ||
| """ | ||
| Test installing scripts (legacy scripts are preserved) | ||
| """ | ||
| result = script.pip('install', 'script.wheel2a==0.1', '--use-wheel', | ||
| '--no-index', '--find-links='+data.find_links, | ||
| expect_error=False) | ||
| | ||
| legacy_file1 = script.bin / 'testscript1.bat' | ||
| legacy_file2 = script.bin / 'testscript2' | ||
| | ||
| assert legacy_file1 in result.files_created | ||
| assert legacy_file2 in result.files_created | ||
| | ||
| def test_install_from_wheel_no_setuptools_entrypoint(script, data): | ||
| """ | ||
| Test installing scripts (setuptools entrypoints are omitted) | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This description threw me until I looked at the test wheels Member Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll update the description and add a comment in the test code. It's a fiddly test because I'm trying to avoid making the test platform dependent even though the test wheels were generated on my Windows PC and so are platform-specific. | ||
| """ | ||
| result = script.pip('install', 'script.wheel1==0.1', '--use-wheel', | ||
| '--no-index', '--find-links='+data.find_links, | ||
| expect_error=False) | ||
| if os.name == 'nt': | ||
| wrapper_file = script.bin / 't1.exe' | ||
| else: | ||
| wrapper_file = script.bin / 't1' | ||
| wrapper_helper = script.bin / 't1-script.py' | ||
| | ||
| assert wrapper_file in result.files_created | ||
| assert wrapper_helper not in result.files_created | ||
| | ||
| | ||
| def test_skipping_setuptools_doesnt_skip_legacy(script, data): | ||
| """ | ||
| Test installing scripts (legacy scripts are preserved even when we skip setuptools wrappers) | ||
| """ | ||
| result = script.pip('install', 'script.wheel2==0.1', '--use-wheel', | ||
| '--no-index', '--find-links='+data.find_links, | ||
| expect_error=False) | ||
| | ||
| legacy_file1 = script.bin / 'testscript1.bat' | ||
| legacy_file2 = script.bin / 'testscript2' | ||
| wrapper_helper = script.bin / 't1-script.py' | ||
| | ||
| assert legacy_file1 in result.files_created | ||
| assert legacy_file2 in result.files_created | ||
| assert wrapper_helper not in result.files_created | ||
| | ||
| def test_install_from_wheel_gui_entrypoint(script, data): | ||
| """ | ||
| Test installing scripts (gui entry points are generated) | ||
| """ | ||
| result = script.pip('install', 'script.wheel3==0.1', '--use-wheel', | ||
| '--no-index', '--find-links='+data.find_links, | ||
| expect_error=False) | ||
| if os.name == 'nt': | ||
| wrapper_file = script.bin / 't1.exe' | ||
| else: | ||
| wrapper_file = script.bin / 't1' | ||
| assert wrapper_file in result.files_created | ||
| | ||
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 function has become really large with 4 embedded functions. time for a class IMO.
it's not you're problem Paul, but it's become a problem for the project.
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.
Agreed. I'd have refactored more but the embedded functions use various locals and it was going to be messy to do so. I'll try and do a refactoring soon while the way the code works is fresh in my mind - it would be nice if this function were more maintainable.