From 3ac8b89a83cfe90eb0a938ff01ea7ec712e5740e Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Wed, 22 Nov 2017 21:55:49 -0500 Subject: [PATCH 1/4] Do sequential installs on editable dependencies - Fixes #1111 - Uses existing infrastructure to add editable dependencies to the list of sequentially installed packages - Only alters functions used for this purpose - Essentially there is a race condition because multiple editors are trying to write to lib/site-packages/easy-install.pth for local editable packages, so they overwrite one another --- pipenv/cli.py | 26 ++++++++++++-------------- pipenv/utils.py | 13 ++++++++----- tests/test_utils.py | 10 +++++----- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/pipenv/cli.py b/pipenv/cli.py index 08d612d9..4d2e20cf 100644 --- a/pipenv/cli.py +++ b/pipenv/cli.py @@ -33,7 +33,7 @@ from click_didyoumean import DYMCommandCollection from .project import Project from .utils import ( convert_deps_from_pip, convert_deps_to_pip, is_required_version, - proper_case, pep423_name, split_vcs, resolve_deps, shellquote, is_vcs, + proper_case, pep423_name, split_vcs_editable, resolve_deps, shellquote, is_vcs, python_version, suggest_package, find_windows_executable, is_file, prepare_pip_source_args, temp_environ, is_valid_url, download_file, get_requirement, need_update_check, touch_update_stamp @@ -765,10 +765,10 @@ def do_install_dependencies( if skip_lock or only or not project.lockfile_exists: if not bare: click.echo(crayons.normal(u'Installing dependencies from Pipfile…', bold=True)) - lockfile = split_vcs(project._lockfile) + lockfile = split_vcs_editable(project._lockfile) else: with open(project.lockfile_location) as f: - lockfile = split_vcs(simplejson.load(f)) + lockfile = split_vcs_editable(simplejson.load(f)) if not bare: click.echo( @@ -784,26 +784,25 @@ def do_install_dependencies( no_deps = (not skip_lock) deps = {} - vcs_deps = {} + vcs_editable_deps = {} # Store dev only deps for a requirements output dev_deps = {} - dev_vcs_deps = {} + dev_vcs_editable = {} # Add development deps if --dev was passed. if dev: deps.update(lockfile['develop']) - vcs_deps.update(lockfile.get('develop-vcs', {})) + vcs_editable_deps.update(lockfile.get('develop-editable', {})) # Add only dev deps if requirements was passed if requirements: dev_deps.update(lockfile['develop']) - dev_vcs_deps.update(lockfile.get('develop-vcs', {})) - + dev_vcs_editable.update(lockfile.get('develop-editable', {})) # Install default dependencies, always. deps.update(lockfile['default'] if not only else {}) - vcs_deps.update(lockfile.get('default-vcs', {})) + vcs_editable_deps.update(lockfile.get('default-editable', {})) if ignore_hashes: # Remove hashes from generated requirements. @@ -815,8 +814,8 @@ def do_install_dependencies( deps_list = [(d, ignore_hashes, blocking) for d in convert_deps_to_pip(deps, project, r=False, include_index=True)] failed_deps_list = [] - if len(vcs_deps): - deps_list.extend((d, True, True) for d in convert_deps_to_pip(vcs_deps, project, r=False)) + if len(vcs_editable_deps): + deps_list.extend((d, True, True) for d in convert_deps_to_pip(vcs_editable_deps, project, r=False)) # --requirements was passed. if requirements: @@ -828,13 +827,12 @@ def do_install_dependencies( # Output only dev dependencies if dev: dev_deps_list = [(d, ignore_hashes, blocking) for d in convert_deps_to_pip(dev_deps, project, r=False, include_index=True)] - if len(dev_vcs_deps): - dev_deps_list.extend((d, True, True) for d in convert_deps_to_pip(dev_vcs_deps, project, r=False)) + if len(dev_vcs_editable): + dev_deps_list.extend((d, True, True) for d in convert_deps_to_pip(dev_vcs_editable, project, r=False)) click.echo('\n'.join(d[0] for d in dev_deps_list)) sys.exit(0) - procs = [] deps_list_bar = progress.bar(deps_list, label=INSTALL_LABEL if os.name != 'nt' else '') diff --git a/pipenv/utils.py b/pipenv/utils.py index e357ca18..a68a7b85 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -919,19 +919,22 @@ def proper_case(package_name): return good_name -def split_vcs(split_file): - """Split VCS dependencies out from file.""" +def split_vcs_editable(split_file): + """Split VCS and editable dependencies out from file.""" if 'packages' in split_file or 'dev-packages' in split_file: sections = ('packages', 'dev-packages') elif 'default' in split_file or 'develop' in split_file: sections = ('default', 'develop') - # For each vcs entry in a given section, move it to section-vcs. + # For each vcs or editable entry in a given section, move it to section-editable. for section in sections: entries = split_file.get(section, {}) - vcs_dict = dict((k, entries.pop(k)) for k in list(entries.keys()) if is_vcs(entries[k])) - split_file[section + '-vcs'] = vcs_dict + editable_dict = {} + for k in list(entries.keys()): + if is_vcs(entries[k]) or (hasattr(entries[k], 'keys') and entries[k].get('editable') is True): + editable_dict[k] = entries.pop(k) + split_file[section + '-editable'] = editable_dict return split_file diff --git a/tests/test_utils.py b/tests/test_utils.py index 303c45aa..c8122838 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -131,7 +131,7 @@ class TestUtils: def test_is_vcs(self, entry, expected): assert pipenv.utils.is_vcs(entry) is expected - def test_split_vcs(self): + def test_split_editable_vcs(self): pipfile_dict = { 'packages': { 'requests': {'git': 'https://github.com/kennethreitz/requests.git'}, @@ -143,13 +143,13 @@ class TestUtils: 'crayons': {'hg': 'https://hg.alsonotreal.com/crayons'} } } - split_dict = pipenv.utils.split_vcs(pipfile_dict) + split_dict = pipenv.utils.split_vcs_editable(pipfile_dict) assert list(split_dict['packages'].keys()) == ['Flask'] - assert split_dict['packages-vcs'] == {'requests': {'git': 'https://github.com/kennethreitz/requests.git'}} + assert split_dict['packages-editable'] == {'requests': {'git': 'https://github.com/kennethreitz/requests.git'}} assert list(split_dict['dev-packages'].keys()) == ['Django'] - assert 'click' in split_dict['dev-packages-vcs'] - assert 'crayons' in split_dict['dev-packages-vcs'] + assert 'click' in split_dict['dev-packages-editable'] + assert 'crayons' in split_dict['dev-packages-editable'] def test_python_version_from_bad_path(self): assert pipenv.utils.python_version("/fake/path") is None From 6e7dd14cd37f6fbdb7a88b479dcda0f0e161d420 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Wed, 29 Nov 2017 12:38:52 -0500 Subject: [PATCH 2/4] Refactor do_install lockfile splitting - Break out functions to split file and sections - Break out function to merge dependencies --- pipenv/cli.py | 53 +++++----------------- pipenv/utils.py | 104 ++++++++++++++++++++++++++++++++++++++------ tests/test_utils.py | 14 +++--- 3 files changed, 110 insertions(+), 61 deletions(-) diff --git a/pipenv/cli.py b/pipenv/cli.py index 4d2e20cf..e76cbb65 100644 --- a/pipenv/cli.py +++ b/pipenv/cli.py @@ -33,7 +33,7 @@ from click_didyoumean import DYMCommandCollection from .project import Project from .utils import ( convert_deps_from_pip, convert_deps_to_pip, is_required_version, - proper_case, pep423_name, split_vcs_editable, resolve_deps, shellquote, is_vcs, + proper_case, pep423_name, split_file, merge_deps, resolve_deps, shellquote, is_vcs, python_version, suggest_package, find_windows_executable, is_file, prepare_pip_source_args, temp_environ, is_valid_url, download_file, get_requirement, need_update_check, touch_update_stamp @@ -765,10 +765,10 @@ def do_install_dependencies( if skip_lock or only or not project.lockfile_exists: if not bare: click.echo(crayons.normal(u'Installing dependencies from Pipfile…', bold=True)) - lockfile = split_vcs_editable(project._lockfile) + lockfile = split_file(project._lockfile) else: with open(project.lockfile_location) as f: - lockfile = split_vcs_editable(simplejson.load(f)) + lockfile = split_file(simplejson.load(f)) if not bare: click.echo( @@ -783,41 +783,16 @@ def do_install_dependencies( # Allow pip to resolve dependencies when in skip-lock mode. no_deps = (not skip_lock) - deps = {} - vcs_editable_deps = {} - - # Store dev only deps for a requirements output - dev_deps = {} - dev_vcs_editable = {} - - # Add development deps if --dev was passed. - if dev: - deps.update(lockfile['develop']) - vcs_editable_deps.update(lockfile.get('develop-editable', {})) - - # Add only dev deps if requirements was passed - if requirements: - dev_deps.update(lockfile['develop']) - dev_vcs_editable.update(lockfile.get('develop-editable', {})) - - # Install default dependencies, always. - deps.update(lockfile['default'] if not only else {}) - vcs_editable_deps.update(lockfile.get('default-editable', {})) - - if ignore_hashes: - # Remove hashes from generated requirements. - for k, v in deps.items(): - if 'hash' in v: - del v['hash'] - - # Convert the deps to pip-compatible arguments. - deps_list = [(d, ignore_hashes, blocking) for d in convert_deps_to_pip(deps, project, r=False, include_index=True)] + deps_list, dev_deps_list = merge_deps( + lockfile, + project, + dev=dev, + requirements=requirements, + ignore_hashes=ignore_hashes, + blocking=blocking, + only=only + ) failed_deps_list = [] - - if len(vcs_editable_deps): - deps_list.extend((d, True, True) for d in convert_deps_to_pip(vcs_editable_deps, project, r=False)) - - # --requirements was passed. if requirements: # Output only default dependencies if not dev: @@ -826,10 +801,6 @@ def do_install_dependencies( # Output only dev dependencies if dev: - dev_deps_list = [(d, ignore_hashes, blocking) for d in convert_deps_to_pip(dev_deps, project, r=False, include_index=True)] - if len(dev_vcs_editable): - dev_deps_list.extend((d, True, True) for d in convert_deps_to_pip(dev_vcs_editable, project, r=False)) - click.echo('\n'.join(d[0] for d in dev_deps_list)) sys.exit(0) diff --git a/pipenv/utils.py b/pipenv/utils.py index a68a7b85..5ec00bdf 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -919,24 +919,100 @@ def proper_case(package_name): return good_name -def split_vcs_editable(split_file): - """Split VCS and editable dependencies out from file.""" +def split_section(input_file, section_suffix, test_function): + """ + Split a pipfile or a lockfile section out by section name and test function + + :param dict input_file: A dictionary containing either a pipfile or lockfile + :param str section_suffix: A string of the name of the section + :param func test_function: A test function to test against the value in the key/value pair + + >>> split_section(my_lockfile, 'vcs', is_vcs) + { + 'default': { + "six": { + "hashes": [ + "sha256:832dc0e10feb1aa2c68dcc57dbb658f1c7e65b9b61af69048abc87a2db00a0eb", + "sha256:70e8a77beed4562e7f14fe23a786b54f6296e34344c23bc42f07b15018ff98e9" + ], + "version": "==1.11.0" + } + }, + 'default-vcs': { + "e1839a8": { + "editable": true, + "path": "." + } + } + } + """ + pipfile_sections = ('packages', 'dev-packages') + lockfile_sections = ('default', 'develop') + if any(section in input_file for section in pipfile_sections): + sections = pipfile_sections + elif any(section in input_file for section in lockfile_sections): + sections = lockfile_sections + # return the original file if we can't find any pipfile or lockfile sections + else: + return input_file - if 'packages' in split_file or 'dev-packages' in split_file: - sections = ('packages', 'dev-packages') - elif 'default' in split_file or 'develop' in split_file: - sections = ('default', 'develop') - - # For each vcs or editable entry in a given section, move it to section-editable. for section in sections: - entries = split_file.get(section, {}) - editable_dict = {} + split_dict = {} + entries = input_file.get(section, {}) for k in list(entries.keys()): - if is_vcs(entries[k]) or (hasattr(entries[k], 'keys') and entries[k].get('editable') is True): - editable_dict[k] = entries.pop(k) - split_file[section + '-editable'] = editable_dict + if test_function(entries.get(k)): + split_dict[k] = entries.pop(k) + input_file['-'.join([section, section_suffix])] = split_dict + return input_file - return split_file + +def split_file(file_dict): + """Split VCS and editable dependencies out from file.""" + sections = { + 'vcs': is_vcs, + 'editable': lambda x: hasattr(x, 'keys') and x.get('editable') + } + for k, func in sections.items(): + file_dict = split_section(file_dict, k, func) + return file_dict + + +def merge_deps(file_dict, project, dev=False, requirements=False, ignore_hashes=False, blocking=False, only=False): + """ + Given a file_dict, merges dependencies and converts them to pip dependency lists. + :param dict file_dict: The result of calling :func:`pipenv.utils.split_file` + :param :class:`pipenv.project.Project` project: Pipenv project + :param bool dev=False: Flag indicating whether dev dependencies are to be installed + :param bool requirements=False: Flag indicating whether to use a requirements file + :param bool ignore_hashes=False: + :param bool blocking=False: + :param bool only=False: + :return: Pip-converted 3-tuples of [deps, requirements_deps] + """ + deps = [] + requirements_deps = [] + + for section in list(file_dict.keys()): + # Turn develop-vcs into ['develop', 'vcs'] + section_name, suffix = section.rsplit('-', 1) if '-' in section and not section == 'dev-packages' else (section, None) + if not file_dict[section] or section_name not in ('dev-packages', 'packages', 'default', 'develop'): + continue + is_dev = section_name in ('dev-packages', 'develop') + + if ignore_hashes: + for k, v in file_dict[section]: + if 'hash' in v: + del v['hash'] + + # Block and ignore hashes for all suffixed sections (vcs/editable) + no_hashes = True if suffix else ignore_hashes + block = True if suffix else blocking + include_index = True if not suffix else False + converted = convert_deps_to_pip(file_dict[section], project, r=False, include_index=include_index) + deps.extend((d, no_hashes, block) for d in converted) + if dev and is_dev and requirements: + requirements_deps.extend((d, no_hashes, block) for d in converted) + return deps, requirements_deps def recase_file(file_dict): diff --git a/tests/test_utils.py b/tests/test_utils.py index c8122838..ffb875ef 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -131,11 +131,12 @@ class TestUtils: def test_is_vcs(self, entry, expected): assert pipenv.utils.is_vcs(entry) is expected - def test_split_editable_vcs(self): + def test_split_file(self): pipfile_dict = { 'packages': { 'requests': {'git': 'https://github.com/kennethreitz/requests.git'}, - 'Flask': '*' + 'Flask': '*', + 'tablib': {'path': '.', 'editable': True} }, 'dev-packages': { 'Django': '==1.10', @@ -143,13 +144,14 @@ class TestUtils: 'crayons': {'hg': 'https://hg.alsonotreal.com/crayons'} } } - split_dict = pipenv.utils.split_vcs_editable(pipfile_dict) + split_dict = pipenv.utils.split_file(pipfile_dict) assert list(split_dict['packages'].keys()) == ['Flask'] - assert split_dict['packages-editable'] == {'requests': {'git': 'https://github.com/kennethreitz/requests.git'}} + assert split_dict['packages-vcs'] == {'requests': {'git': 'https://github.com/kennethreitz/requests.git'}} + assert split_dict['packages-editable'] == {'tablib': {'path': '.', 'editable': True}} assert list(split_dict['dev-packages'].keys()) == ['Django'] - assert 'click' in split_dict['dev-packages-editable'] - assert 'crayons' in split_dict['dev-packages-editable'] + assert 'click' in split_dict['dev-packages-vcs'] + assert 'crayons' in split_dict['dev-packages-vcs'] def test_python_version_from_bad_path(self): assert pipenv.utils.python_version("/fake/path") is None From b5ecb02154d259274d439bb9aac804241e0ab697 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Fri, 1 Dec 2017 23:03:28 -0500 Subject: [PATCH 3/4] Move some control flow logic --- pipenv/utils.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 5ec00bdf..0efbfc85 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -950,11 +950,12 @@ def split_section(input_file, section_suffix, test_function): lockfile_sections = ('default', 'develop') if any(section in input_file for section in pipfile_sections): sections = pipfile_sections - elif any(section in input_file for section in lockfile_sections): - sections = lockfile_sections - # return the original file if we can't find any pipfile or lockfile sections else: - return input_file + if any(section in input_file for section in lockfile_sections): + sections = lockfile_sections + else: + # return the original file if we can't find any pipfile or lockfile sections + return input_file for section in sections: split_dict = {} From c3e203caead2143694678f2f8676495f17fd486d Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 3 Dec 2017 19:22:18 -0500 Subject: [PATCH 4/4] Undo control flow changes and move comment --- pipenv/utils.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pipenv/utils.py b/pipenv/utils.py index 0efbfc85..c079523a 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -950,12 +950,11 @@ def split_section(input_file, section_suffix, test_function): lockfile_sections = ('default', 'develop') if any(section in input_file for section in pipfile_sections): sections = pipfile_sections + elif any(section in input_file for section in lockfile_sections): + sections = lockfile_sections else: - if any(section in input_file for section in lockfile_sections): - sections = lockfile_sections - else: - # return the original file if we can't find any pipfile or lockfile sections - return input_file + # return the original file if we can't find any pipfile or lockfile sections + return input_file for section in sections: split_dict = {}