From 025f66899b2c53fd0abf6dea9808eabdedee3a69 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 7 Jun 2018 01:07:28 -0400 Subject: [PATCH] Refactor and cleanup some locking code - This has been driving me mad. Signed-off-by: Dan Ryan --- pipenv/core.py | 186 ++++++++++++++++++++---------------------------- pipenv/utils.py | 68 ++++++++++-------- 2 files changed, 116 insertions(+), 138 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index 51d21d65..e4a94d31 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -49,6 +49,7 @@ from .utils import ( split_argument, extract_uri_from_vcs_dep, fs_str, + clean_resolved_dep, ) from ._compat import ( TemporaryDirectory, @@ -1009,8 +1010,6 @@ def do_lock( ): """Executes the freeze functionality.""" from .utils import get_vcs_deps - from notpip._vendor.distlib.markers import DEFAULT_CONTEXT as marker_context - allowed_marker_keys = ['markers'] + [k for k in marker_context.keys()] cached_lockfile = {} if not pre: pre = project.settings.get('allow_prereleases') @@ -1023,16 +1022,6 @@ def do_lock( ) sys.exit(1) cached_lockfile = project.lockfile_content - if write: - # Alert the user of progress. - click.echo( - u'{0} {1} {2}'.format( - crayons.normal('Locking'), - crayons.red('[dev-packages]'), - crayons.normal('dependencies…'), - ), - err=True, - ) # Create the lockfile. lockfile = project._lockfile # Cleanup lockfile. @@ -1049,104 +1038,85 @@ def do_lock( pip_freeze = delegator.run( '{0} freeze'.format(escape_grouped_arguments(which_pip(allow_global=system))) ).out - deps = convert_deps_to_pip( - dev_packages, project, r=False, include_index=True - ) - results = venv_resolve_deps( - deps, - which=which, - verbose=verbose, - project=project, - clear=clear, - pre=pre, - allow_global=system, - pypi_mirror=pypi_mirror, - ) - # Add develop dependencies to lockfile. - for dep in results: - # Add version information to lockfile. - lockfile['develop'].update( - {dep['name']: {'version': '=={0}'.format(dep['version'])}} - ) - # Add Hashes to lockfile - lockfile['develop'][dep['name']]['hashes'] = dep['hashes'] - # Add index metadata to lockfile. - if 'index' in dep: - lockfile['develop'][dep['name']]['index'] = dep['index'] - # Add PEP 508 specifier metadata to lockfile if dep isnt top level - # or top level dep doesn't itself have markers - if 'markers' in dep: - if dep['name'] in dev_packages and not any(key in dev_packages[dep['name']] for key in allowed_marker_keys): - continue - lockfile['develop'][dep['name']]['markers'] = dep['markers'] - # Add refs for VCS installs. - # TODO: be smarter about this. - vcs_dev_lines, vcs_dev_lockfiles = get_vcs_deps(project, pip_freeze, which=which, verbose=verbose, clear=clear, pre=pre, allow_global=system, dev=True, pypi_mirror=pypi_mirror) - for lf in vcs_dev_lockfiles: - try: - name = first(lf.keys()) - except AttributeError: - continue - if hasattr(lf[name], 'keys'): - lockfile['develop'].update(lf) - if write: - # Alert the user of progress. - click.echo( - u'{0} {1} {2}'.format( - crayons.normal('Locking'), - crayons.red('[packages]'), - crayons.normal('dependencies…'), - ), - err=True, - ) - # Resolve package dependencies, with pip-tools. - deps = convert_deps_to_pip( - project.packages, project, r=False, include_index=True - ) - results = venv_resolve_deps( - deps, - which=which, - verbose=verbose, - project=project, - clear=False, - pre=pre, - allow_global=system, - pypi_mirror=pypi_mirror, - ) - # Add default dependencies to lockfile. - for dep in results: - # Add version information to lockfile. - pipfile_version = project.packages[dep['name']] if dep['name'] in project.packages else None - if pipfile_version and hasattr(pipfile_version, 'keys') and any(k for k in ['file', 'path'] if k in pipfile_version): - lockfile['default'].update( - {dep['name']: dict(pipfile_version)} + sections = { + 'dev': { + 'packages': project.dev_packages, + 'vcs': project.vcs_dev_packages, + 'pipfile_key': 'dev_packages', + 'lockfile_key': 'develop', + 'log_string': 'dev-packages', + 'dev': True + }, + 'default': { + 'packages': project.packages, + 'vcs': project.vcs_packages, + 'pipfile_key': 'packages', + 'lockfile_key': 'default', + 'log_string': 'packages', + 'dev': False + } + } + for section_name, settings in sections.items(): + if write: + # Alert the user of progress. + click.echo( + u'{0} {1} {2}'.format( + crayons.normal('Locking'), + crayons.red('[{0}]'.format(settings['log_string'])), + crayons.normal('dependencies…'), + ), + err=True, ) - continue - lockfile['default'].update( - {dep['name']: {'version': '=={0}'.format(dep['version'])}} - ) - # Add Hashes to lockfile - lockfile['default'][dep['name']]['hashes'] = dep['hashes'] - # Add index metadata to lockfile. - if 'index' in dep: - lockfile['default'][dep['name']]['index'] = dep['index'] - # Add PEP 508 specifier metadata to lockfile if dep isn't top level - # or top level dep has no specifiers itself - if 'markers' in dep: - if dep['name'] in project.packages and not any(key in project.packages[dep['name']] for key in allowed_marker_keys): - continue - lockfile['default'][dep['name']]['markers'] = dep['markers'] - # Add refs for VCS installs. - # TODO: be smarter about this. - _vcs_deps, vcs_lockfiles = get_vcs_deps(project, pip_freeze, which=which, verbose=verbose, clear=clear, pre=pre, allow_global=system, dev=False, pypi_mirror=pypi_mirror) - for lf in vcs_lockfiles: - try: - name = first(lf.keys()) - except AttributeError: - continue - if hasattr(lf[name], 'keys'): - lockfile['default'].update(lf) + deps = convert_deps_to_pip( + settings['packages'], project, r=False, include_index=True + ) + results = venv_resolve_deps( + deps, + which=which, + verbose=verbose, + project=project, + clear=clear, + pre=pre, + allow_global=system, + pypi_mirror=pypi_mirror, + ) + # Add develop dependencies to lockfile. + for dep in results: + is_top_level = dep['name'] in settings['packages'] + pipfile_entry = settings['packages'][dep['name']] if is_top_level else None + dep_lockfile = clean_resolved_dep(dep, is_top_level=is_top_level, pipfile_entry=pipfile_entry) + lockfile[settings['lockfile_key']].update(dep_lockfile) + # Add refs for VCS installs. + # TODO: be smarter about this. + vcs_lines, vcs_lockfile = get_vcs_deps( + project, + pip_freeze, + which=which, + verbose=verbose, + clear=clear, + pre=pre, + allow_global=system, + dev=settings['dev'] + ) + vcs_results = venv_resolve_deps( + vcs_lines, + which=which, + verbose=verbose, + project=project, + clear=clear, + pre=pre, + allow_global=system, + pypi_mirror=pypi_mirror, + ) + for dep in vcs_results: + if not hasattr(dep, 'keys') or not hasattr(dep['name'], 'keys'): + continue + is_top_level = dep['name'] in vcs_lockfile + pipfile_entry = vcs_lockfile[dep['name']] if is_top_level else None + dep_lockfile = clean_resolved_dep(dep, is_top_level=is_top_level, pipfile_entry=pipfile_entry) + vcs_lockfile.update(dep_lockfile) + lockfile[settings['lockfile_key']].update(vcs_lockfile) # Support for --keep-outdated… if keep_outdated: for section_name, section in ( diff --git a/pipenv/utils.py b/pipenv/utils.py index 1e765b0a..34140c81 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -1164,7 +1164,7 @@ def get_vcs_deps( section = "vcs_dev_packages" if dev else "vcs_packages" lines = [] - lockfiles = [] + lockfile = {} try: packages = getattr(project, section) except AttributeError: @@ -1208,37 +1208,45 @@ def get_vcs_deps( ) if installed.is_vcs: installed.req.ref = locked_rev - lockfiles.append({pipfile_name: installed.pipfile_entry[1]}) - pipfile_srcdir = (src_dir / pipfile_name).as_posix() - lockfile_srcdir = (src_dir / installed.normalized_name).as_posix() + lockfile[pipfile_name] = installed.pipfile_entry[1] lines.append(line) - if os.path.exists(pipfile_srcdir): - lockfiles.extend( - venv_resolve_deps( - ["-e {0}".format(pipfile_srcdir)], - which=which, - verbose=verbose, - project=project, - clear=clear, - pre=pre, - allow_global=allow_global, - pypi_mirror=pypi_mirror, - ) - ) + return lines, lockfile + + +def clean_resolved_dep(dep, is_top_level=False, pipfile_entry=None): + from notpip._vendor.distlib.markers import DEFAULT_CONTEXT as marker_context + allowed_marker_keys = ['markers'] + [k for k in marker_context.keys()] + name = dep['name'] + # We use this to determine if there are any markers on top level packages + # So we can make sure those win out during resolution if the packages reoccur + dep_keys = [k for k in getattr(pipfile_entry, 'keys', list)()] if is_top_level else [] + lockfile = { + 'version': '=={0}'.format(dep['version']), + } + for key in ['hashes', 'index']: + if key in dep: + lockfile[key] = dep[key] + # In case we lock a uri or a file when the user supplied a path + if pipfile_entry and any(k in pipfile_entry for k in ['file', 'path']): + fs_key = next((k for k in ['path', 'file'] if k in pipfile_entry), None) + lockfile_key = next((k for k in ['uri', 'file', 'path'] if k in lockfile), None) + if fs_key != lockfile_key: + del lockfile[lockfile_key] + lockfile[fs_key] = pipfile_entry[fs_key] + + # If a package is **PRESENT** in the pipfile but has no markers, make sure we + # **NEVER** include markers in the lockfile + if 'markers' in dep: + # First, handle the case where there is no top level dependency in the pipfile + if not is_top_level: + lockfile['markers'] = dep['markers'] + # otherwise make sure we are prioritizing whatever the pipfile says about the markers + # If the pipfile says nothing, then we should put nothing in the lockfile else: - lockfiles.extend( - venv_resolve_deps( - ["-e {0}".format(lockfile_srcdir)], - which=which, - verbose=verbose, - project=project, - clear=clear, - pre=pre, - allow_global=allow_global, - pypi_mirror=pypi_mirror, - ) - ) - return lines, lockfiles + pipfile_marker = next((k for k in dep_keys if k in allowed_marker_keys), None) + if pipfile_marker: + lockfile['markers'] = "{0}{1}".format(pipfile_marker, pipfile_entry[pipfile_marker]) + return {name: lockfile} def fs_str(string):