From f29aedc65c14da43ca619cd2c41f7e5d47876b70 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Thu, 22 Mar 2018 20:46:13 +0800 Subject: [PATCH 1/9] fix reference losing for _pipfile --- pipenv/patched/contoml/file/file.py | 12 ++++++++++++ pipenv/project.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pipenv/patched/contoml/file/file.py b/pipenv/patched/contoml/file/file.py index 99ce1483..27001fc3 100755 --- a/pipenv/patched/contoml/file/file.py +++ b/pipenv/patched/contoml/file/file.py @@ -271,6 +271,18 @@ class TOMLFile: def elements(self): return self._elements + _marker = object() + + def pop(self, item, default=_marker): + """Pops an item and return the value""" + if item in self: + rv = self[item] + del self[item] + return rv + if default is self._marker: + raise KeyError(item) + return default + def __str__(self): is_empty = (not self['']) and (not tuple(k for k in self.keys() if k)) diff --git a/pipenv/project.py b/pipenv/project.py index 001652fe..0e6cb6ba 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -377,7 +377,7 @@ class Project(object): # mutation time! self.clear_pipfile_cache() for section in ('packages', 'dev-packages'): - p_section = dict(pfile.get(section, {})) + p_section = pfile.get(section, {}) for key in list(p_section.keys()): # Normalize key name to PEP 423. norm_key = pep423_name(key) From cd28874469f6454b12d56975cbca1c3bc4f3439a Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Thu, 22 Mar 2018 20:46:35 +0800 Subject: [PATCH 2/9] add tests fix the broken `items()` Fix test cases Keep contoml untouched --- pipenv/patched/contoml/file/file.py | 14 +---------- tests/test_pipenv.py | 37 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/pipenv/patched/contoml/file/file.py b/pipenv/patched/contoml/file/file.py index 27001fc3..16017b99 100755 --- a/pipenv/patched/contoml/file/file.py +++ b/pipenv/patched/contoml/file/file.py @@ -231,7 +231,7 @@ class TOMLFile: if has_anonymous_entry(): return items else: - return list(items) + [('', self[''])] + return items + [('', self[''])] @property def primitive(self): @@ -271,18 +271,6 @@ class TOMLFile: def elements(self): return self._elements - _marker = object() - - def pop(self, item, default=_marker): - """Pops an item and return the value""" - if item in self: - rv = self[item] - del self[item] - return rv - if default is self._marker: - raise KeyError(item) - return default - def __str__(self): is_empty = (not self['']) and (not tuple(k for k in self.keys() if k)) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index 4d6556b3..00ab99cd 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -590,6 +590,43 @@ tpfd = "*" c = p.pipenv('run python -c "import requests; import idna; import certifi; import records; import tpfd; import parse;"') assert c.return_code == 0 + @pytest.mark.install + @pytest.mark.run + def test_pep423_name_install(self, pypi): + with PipenvInstance(pypi=pypi) as p: + with open(p.pipfile_path, 'w') as f: + contents = """ +[packages] +python_dateutil = "*" +""" + f.write(contents) + + c = p.pipenv('install') + assert c.return_code == 0 + + c = p.pipenv('install python-dateutil') + assert c.return_code == 0 + assert 'python-dateutil' in p.pipfile['packages'] + assert 'python_dateutil' not in p.pipfile['packages'] + + @pytest.mark.uninstall + @pytest.mark.run + def test_pep423_name_uninstall(self, pypi): + with PipenvInstance(pypi=pypi) as p: + with open(p.pipfile_path, 'w') as f: + contents = """ +[packages] +python_dateutil = "*" +""" + f.write(contents) + + c = p.pipenv('install') + assert c.return_code == 0 + + c = p.pipenv('uninstall python-dateutil') + assert 'python_dateutil' not in p.pipfile['packages'] + assert 'python-dateutil' not in p.lockfile['default'] + @pytest.mark.install @pytest.mark.resolver @pytest.mark.backup_resolver From e2e2ac1ee10443ff7e4d9068063e9e77eb3ea413 Mon Sep 17 00:00:00 2001 From: frostming Date: Wed, 28 Mar 2018 11:20:20 +0800 Subject: [PATCH 3/9] Fix an error that was never hit --- pipenv/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipenv/core.py b/pipenv/core.py index 5fa05917..def2212f 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -290,7 +290,6 @@ def ensure_pipfile(validate=True, skip_requirements=False): if validate and project.virtualenv_exists and not PIPENV_SKIP_VALIDATION: # Ensure that Pipfile is using proper casing. p = project.parsed_pipfile - p.clear_pipfile_cache() changed = ensure_proper_casing(pfile=p) # Write changes out to disk. if changed: @@ -299,6 +298,7 @@ def ensure_pipfile(validate=True, skip_requirements=False): err=True, ) project.write_toml(p) + project.clear_pipfile_cache() def find_python_from_py(python): From fc17c757de160d625da2bc481776f5b28001d6e0 Mon Sep 17 00:00:00 2001 From: frostming Date: Wed, 28 Mar 2018 12:32:27 +0800 Subject: [PATCH 4/9] test preserve comments --- tests/test_pipenv.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index 00ab99cd..a077efab 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -596,8 +596,9 @@ tpfd = "*" with PipenvInstance(pypi=pypi) as p: with open(p.pipfile_path, 'w') as f: contents = """ +# Pre comment [packages] -python_dateutil = "*" +python_dateutil = "*" # Inline comment """ f.write(contents) @@ -608,6 +609,9 @@ python_dateutil = "*" assert c.return_code == 0 assert 'python-dateutil' in p.pipfile['packages'] assert 'python_dateutil' not in p.pipfile['packages'] + contents = open(p.pipfile_path).read() + assert '# Pre comment' in contents + assert '# Inline comment' in contents @pytest.mark.uninstall @pytest.mark.run From 4b290407d31b6059b4f682d2f317c977abf7f90a Mon Sep 17 00:00:00 2001 From: frostming Date: Wed, 28 Mar 2018 16:24:08 +0800 Subject: [PATCH 5/9] only change things on way in --- pipenv/core.py | 39 +-------------------- pipenv/project.py | 82 ++++++++++++++++++++++++++++++-------------- tests/test_pipenv.py | 22 ++++++++---- 3 files changed, 73 insertions(+), 70 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index def2212f..fb045b53 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -290,7 +290,7 @@ def ensure_pipfile(validate=True, skip_requirements=False): if validate and project.virtualenv_exists and not PIPENV_SKIP_VALIDATION: # Ensure that Pipfile is using proper casing. p = project.parsed_pipfile - changed = ensure_proper_casing(pfile=p) + changed = project.ensure_proper_casing() # Write changes out to disk. if changed: click.echo( @@ -298,7 +298,6 @@ def ensure_pipfile(validate=True, skip_requirements=False): err=True, ) project.write_toml(p) - project.clear_pipfile_cache() def find_python_from_py(python): @@ -635,42 +634,6 @@ def ensure_project( ensure_pipfile(validate=validate, skip_requirements=skip_requirements) -def ensure_proper_casing(pfile): - """Ensures proper casing of Pipfile packages, writes changes to disk.""" - casing_changed = proper_case_section(pfile.get('packages', {})) - casing_changed |= proper_case_section(pfile.get('dev-packages', {})) - return casing_changed - - -def proper_case_section(section): - """Verify proper casing is retrieved, when available, for each - dependency in the section. - """ - # Casing for section. - changed_values = False - unknown_names = [ - k for k in section.keys() if k not in set(project.proper_names) - ] - # Replace each package with proper casing. - for dep in unknown_names: - try: - # Get new casing for package name. - new_casing = proper_case(dep) - except IOError: - # Unable to normalize package name. - continue - - if new_casing != dep: - changed_values = True - project.register_proper_name(new_casing) - # Replace old value with new value. - old_value = section[dep] - section[new_casing] = old_value - del section[dep] - # Return whether or not values have been changed. - return changed_values - - def shorten_path(location, bold=False): """Returns a visually shorter representation of a given system path.""" original = location diff --git a/pipenv/project.py b/pipenv/project.py index 0e6cb6ba..d17900b9 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -3,7 +3,6 @@ import codecs import json import os import re -import six import sys import shlex import base64 @@ -21,7 +20,7 @@ from .utils import ( mkdir_p, convert_deps_from_pip, pep423_name, - recase_file, + proper_case, find_requirements, is_editable, is_file, @@ -370,20 +369,6 @@ class Project(object): except Exception: return toml.loads(contents) - @property - def _pipfile(self): - """Pipfile divided by PyPI and external dependencies.""" - pfile = self.parsed_pipfile - # mutation time! - self.clear_pipfile_cache() - for section in ('packages', 'dev-packages'): - p_section = pfile.get(section, {}) - for key in list(p_section.keys()): - # Normalize key name to PEP 423. - norm_key = pep423_name(key) - p_section[norm_key] = p_section.pop(key) - return pfile - @property def settings(self): """A dictionary of the settings added to the Pipfile.""" @@ -416,7 +401,6 @@ class Project(object): p['pipenv'] = settings # Write the changes to disk. self.write_toml(p) - self.clear_pipfile_cache() @property def _lockfile(self): @@ -572,6 +556,8 @@ class Project(object): formatted_data = cleanup_toml(formatted_data) with open(path, 'w') as f: f.write(formatted_data) + # pipfile is mutated! + self.clear_pipfile_cache() @property def sources(self): @@ -607,17 +593,19 @@ class Project(object): def remove_package_from_pipfile(self, package_name, dev=False): # Read and append Pipfile. - p = self._pipfile + p = self.parsed_pipfile package_name = pep423_name(package_name) key = 'dev-packages' if dev else 'packages' - if key in p and package_name in p[key]: - del p[key][package_name] - # Write Pipfile. - self.write_toml(recase_file(p)) + if key in p: + for name in dict(p[key]): + if pep423_name(name) == package_name: + del p[key][name] + # Write Pipfile. + return self.write_toml(p) def add_package_to_pipfile(self, package_name, dev=False): # Read and append Pipfile. - p = self._pipfile + p = self.parsed_pipfile # Don't re-capitalize file URLs or VCSs. converted = convert_deps_from_pip(package_name) converted = converted[[k for k in converted.keys()][0]] @@ -631,6 +619,14 @@ class Project(object): p[key] = {} package = convert_deps_from_pip(package_name) package_name = [k for k in package.keys()][0] + for name in p[key]: + # Normalize names to compare + if ( + pep423_name(name) == pep423_name(package_name) and + name != package_name + ): + # Replace the package name + del p[key][name] # Add the package to the group. p[key][package_name] = package[package_name] # Write Pipfile. @@ -639,7 +635,7 @@ class Project(object): def add_index_to_pipfile(self, index): """Adds a given index to the Pipfile.""" # Read and append Pipfile. - p = self._pipfile + p = self.parsed_pipfile source = {'url': index, 'verify_ssl': True} # Add the package to the group. if 'source' not in p: @@ -650,7 +646,8 @@ class Project(object): self.write_toml(p) def recase_pipfile(self): - self.write_toml(recase_file(self._pipfile)) + if self.ensure_proper_casing(): + self.write_toml(self.parsed_pipfile) def get_lockfile_hash(self): if not os.path.exists(self.lockfile_location): @@ -664,3 +661,38 @@ class Project(object): # Update the lockfile if it is out-of-date. p = pipfile.load(self.pipfile_location, inject_env=False) return p.hash + + def ensure_proper_casing(self): + """Ensures proper casing of Pipfile packages""" + pfile = self.parsed_pipfile + casing_changed = self.proper_case_section(pfile.get('packages', {})) + casing_changed |= self.proper_case_section(pfile.get('dev-packages', {})) + return casing_changed + + def proper_case_section(self, section): + """Verify proper casing is retrieved, when available, for each + dependency in the section. + """ + # Casing for section. + changed_values = False + unknown_names = [ + k for k in section.keys() if k not in set(self.proper_names) + ] + # Replace each package with proper casing. + for dep in unknown_names: + try: + # Get new casing for package name. + new_casing = proper_case(dep) + except IOError: + # Unable to normalize package name. + continue + + if new_casing != dep: + changed_values = True + self.register_proper_name(new_casing) + # Replace old value with new value. + old_value = section[dep] + section[new_casing] = old_value + del section[dep] + # Return whether or not values have been changed. + return changed_values diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index a077efab..b03875f7 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -598,17 +598,20 @@ tpfd = "*" contents = """ # Pre comment [packages] -python_dateutil = "*" # Inline comment +python_DateUtil = "*" # Inline comment """ f.write(contents) c = p.pipenv('install') assert c.return_code == 0 - c = p.pipenv('install python-dateutil') + c = p.pipenv('install requests') + assert c.return_code == 0 + assert 'python_DateUtil' in p.pipfile['packages'] + c = p.pipenv('install python_dateutil') assert c.return_code == 0 assert 'python-dateutil' in p.pipfile['packages'] - assert 'python_dateutil' not in p.pipfile['packages'] + assert 'python_DateUtil' not in p.pipfile['packages'] contents = open(p.pipfile_path).read() assert '# Pre comment' in contents assert '# Inline comment' in contents @@ -619,17 +622,22 @@ python_dateutil = "*" # Inline comment with PipenvInstance(pypi=pypi) as p: with open(p.pipfile_path, 'w') as f: contents = """ +# Pre comment [packages] -python_dateutil = "*" +Requests = "*" +python_DateUtil = "*" # Inline comment """ f.write(contents) c = p.pipenv('install') assert c.return_code == 0 - c = p.pipenv('uninstall python-dateutil') - assert 'python_dateutil' not in p.pipfile['packages'] - assert 'python-dateutil' not in p.lockfile['default'] + c = p.pipenv('uninstall python_dateutil') + assert 'Requests' in p.pipfile['packages'] + assert 'python_DateUtil' not in p.pipfile['packages'] + contents = open(p.pipfile_path).read() + assert '# Pre comment' in contents + assert '# Inline comment' in contents @pytest.mark.install @pytest.mark.resolver From 29e9b237ae05a64e8cc476e2243339ed9dc3a5dc Mon Sep 17 00:00:00 2001 From: frostming Date: Wed, 28 Mar 2018 16:47:46 +0800 Subject: [PATCH 6/9] fix iteration --- pipenv/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipenv/project.py b/pipenv/project.py index d17900b9..361cd6a6 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -619,7 +619,7 @@ class Project(object): p[key] = {} package = convert_deps_from_pip(package_name) package_name = [k for k in package.keys()][0] - for name in p[key]: + for name in dict(p[key]): # Normalize names to compare if ( pep423_name(name) == pep423_name(package_name) and From 3c0770fb7a6ec2c47935cdc5681a1a034dbd1a7d Mon Sep 17 00:00:00 2001 From: frostming Date: Wed, 28 Mar 2018 17:43:53 +0800 Subject: [PATCH 7/9] Fix tests failures --- pipenv/core.py | 9 ++++----- pipenv/project.py | 34 ++++++++++++++++++---------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index fb045b53..1f43a232 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -2036,11 +2036,10 @@ def do_uninstall( c = delegator.run(cmd) click.echo(crayons.blue(c.out)) if pipfile_remove: - norm_name = pep423_name(package_name) - in_dev_packages = ( - norm_name in project._pipfile.get('dev-packages', {}) - ) - in_packages = (norm_name in project._pipfile.get('packages', {})) + in_packages = project.get_package_name_in_pipfile( + package_name, dev=False) + in_dev_packages = project.get_package_name_in_pipfile( + package_name, dev=True) if not in_dev_packages and not in_packages: click.echo( 'No package {0} to remove from Pipfile.'.format( diff --git a/pipenv/project.py b/pipenv/project.py index 361cd6a6..595f0b93 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -591,17 +591,23 @@ class Project(object): except OSError: pass + def get_package_name_in_pipfile(self, package_name, dev=False): + """Get the equivalent package name in pipfile""" + key = 'dev-packages' if dev else 'packages' + section = self.parsed_pipfile.get(key, {}) + for name in section.keys(): + if pep423_name(name) == pep423_name(package_name): + return name + return None + def remove_package_from_pipfile(self, package_name, dev=False): # Read and append Pipfile. - p = self.parsed_pipfile - package_name = pep423_name(package_name) + name = self.get_package_name_in_pipfile(package_name, dev) key = 'dev-packages' if dev else 'packages' - if key in p: - for name in dict(p[key]): - if pep423_name(name) == package_name: - del p[key][name] - # Write Pipfile. - return self.write_toml(p) + p = self.parsed_pipfile + if name: + del p[key][name] + self.write_toml(p) def add_package_to_pipfile(self, package_name, dev=False): # Read and append Pipfile. @@ -619,14 +625,10 @@ class Project(object): p[key] = {} package = convert_deps_from_pip(package_name) package_name = [k for k in package.keys()][0] - for name in dict(p[key]): - # Normalize names to compare - if ( - pep423_name(name) == pep423_name(package_name) and - name != package_name - ): - # Replace the package name - del p[key][name] + name = self.get_package_name_in_pipfile(package_name, dev) + if name and name != package_name: + # Replace the packge name + del p[key][name] # Add the package to the group. p[key][package_name] = package[package_name] # Write Pipfile. From d82b1faf937fdcda4cc424fdd5ebf000bb2e1a63 Mon Sep 17 00:00:00 2001 From: frostming Date: Mon, 9 Apr 2018 08:55:41 +0800 Subject: [PATCH 8/9] revert the change --- pipenv/patched/contoml/file/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipenv/patched/contoml/file/file.py b/pipenv/patched/contoml/file/file.py index 16017b99..99ce1483 100755 --- a/pipenv/patched/contoml/file/file.py +++ b/pipenv/patched/contoml/file/file.py @@ -231,7 +231,7 @@ class TOMLFile: if has_anonymous_entry(): return items else: - return items + [('', self[''])] + return list(items) + [('', self[''])] @property def primitive(self): From b9bd4835ee1a130263a4ae99828386a9e159ce5f Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Tue, 10 Apr 2018 20:39:51 +0800 Subject: [PATCH 9/9] Improve according to comments --- pipenv/project.py | 11 ++++++----- tests/test_pipenv.py | 15 +++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/pipenv/project.py b/pipenv/project.py index 595f0b93..6ce151a1 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -595,8 +595,9 @@ class Project(object): """Get the equivalent package name in pipfile""" key = 'dev-packages' if dev else 'packages' section = self.parsed_pipfile.get(key, {}) + package_name = pep423_name(package_name) for name in section.keys(): - if pep423_name(name) == pep423_name(package_name): + if pep423_name(name) == package_name: return name return None @@ -626,11 +627,11 @@ class Project(object): package = convert_deps_from_pip(package_name) package_name = [k for k in package.keys()][0] name = self.get_package_name_in_pipfile(package_name, dev) - if name and name != package_name: - # Replace the packge name - del p[key][name] + if name and converted == '*': + # Skip for wildcard version + return # Add the package to the group. - p[key][package_name] = package[package_name] + p[key][name or package_name] = package[package_name] # Write Pipfile. self.write_toml(p) diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index b03875f7..51a12d2f 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -592,13 +592,13 @@ tpfd = "*" @pytest.mark.install @pytest.mark.run - def test_pep423_name_install(self, pypi): + def test_normalize_name_install(self, pypi): with PipenvInstance(pypi=pypi) as p: with open(p.pipfile_path, 'w') as f: contents = """ # Pre comment [packages] -python_DateUtil = "*" # Inline comment +Requests = "==2.14.0" # Inline comment """ f.write(contents) @@ -607,18 +607,21 @@ python_DateUtil = "*" # Inline comment c = p.pipenv('install requests') assert c.return_code == 0 - assert 'python_DateUtil' in p.pipfile['packages'] - c = p.pipenv('install python_dateutil') + assert 'requests' not in p.pipfile['packages'] + assert p.pipfile['packages']['Requests'] == '==2.14.0' + c = p.pipenv('install requests==2.18.4') + assert c.return_code == 0 + assert p.pipfile['packages']['Requests'] == '==2.18.4' + c = p.pipenv('install python_DateUtil') assert c.return_code == 0 assert 'python-dateutil' in p.pipfile['packages'] - assert 'python_DateUtil' not in p.pipfile['packages'] contents = open(p.pipfile_path).read() assert '# Pre comment' in contents assert '# Inline comment' in contents @pytest.mark.uninstall @pytest.mark.run - def test_pep423_name_uninstall(self, pypi): + def test_normalize_name_uninstall(self, pypi): with PipenvInstance(pypi=pypi) as p: with open(p.pipfile_path, 'w') as f: contents = """