From 0984bab787158d6bb174e54876ab45f084dee960 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sat, 5 May 2018 01:02:27 +0800 Subject: [PATCH] Do not expand environ in cache on package install Fix #2100. --- .gitignore | 4 ++-- pipenv/core.py | 2 +- pipenv/project.py | 21 +++++++++-------- pipenv/utils.py | 8 +++++++ tests/integration/test_install_basic.py | 30 +++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index d35b0966ee..bc92a75a2d 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,8 @@ test_project /.vscode/ .idea +/tests/pytest-pypi/pytest_pypi.egg-info + /.pytest_cache/ /.env @@ -21,8 +23,6 @@ test_project /test/ -/test/ - /results.tap /report.tap diff --git a/pipenv/core.py b/pipenv/core.py index 7ca6587272..09bdcaa7d8 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1452,7 +1452,7 @@ def pip_install( sources = [{'url': index}] if extra_indexes: if isinstance(extra_indexes, six.string_types): - extra_indexes = [extra_indexes,] + extra_indexes = [extra_indexes] for idx in extra_indexes: try: extra_src = project.find_source(idx).get('url') diff --git a/pipenv/project.py b/pipenv/project.py index 0990afa171..2294d43f73 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -34,6 +34,7 @@ is_valid_url, normalize_drive, python_version, + safe_expandvars, ) from .environments import ( PIPENV_MAX_DEPTH, @@ -647,17 +648,15 @@ def write_lockfile(self, content): @property def pipfile_sources(self): - if 'source' in self.parsed_pipfile: - sources = [] - for i, s in enumerate(self.parsed_pipfile['source']): - for k in s.keys(): - if k == 'verify_ssl': - continue - val = os.path.expandvars(self.parsed_pipfile['source'][i][k]) - s[k] = val - sources.append(s) - return sources - return [DEFAULT_SOURCE] + if 'source' not in self.parsed_pipfile: + return [DEFAULT_SOURCE] + # We need to make copies of the source info so we don't + # accidentally modify the cache. See #2100 where values are + # written after the os.path.expandvars() call. + return [ + {k: safe_expandvars(v) for k, v in source.items()} + for source in self.parsed_pipfile['source'] + ] @property def sources(self): diff --git a/pipenv/utils.py b/pipenv/utils.py index 46add117c8..e5a1cb8713 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -1340,3 +1340,11 @@ def atomic_open_for_write(target, binary=False, newline=None, encoding=None): except OSError: pass os.rename(f.name, target) # No os.replace() on Python 2. + + +def safe_expandvars(value): + """Call os.path.expandvars if value is a string, otherwise do nothing. + """ + if isinstance(value, six.string_types): + return os.path.expandvars(value) + return value diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index e46cc4e070..f995ef91ba 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -1,3 +1,7 @@ +import contextlib +import os + +from pipenv.utils import temp_environ from pipenv.vendor import delegator import pytest @@ -234,3 +238,29 @@ def test_clean_on_empty_venv(PipenvInstance, pypi): with PipenvInstance(pypi=pypi) as p: c = p.pipenv('clean') assert c.return_code == 0 + + +@pytest.mark.install +def test_install_does_not_extrapolate_environ(PipenvInstance, pypi): + with temp_environ(), PipenvInstance(pypi=pypi, chdir=True) as p: + os.environ['PYPI_URL'] = pypi.url + + with open(p.pipfile_path, 'w') as f: + f.write(""" +[[source]] +url = '${PYPI_URL}/simple' +verify_ssl = true +name = 'mockpi' + """) + + # Ensure simple install does not extrapolate. + c = p.pipenv('install') + assert c.return_code == 0 + assert p.pipfile['source'][0]['url'] == '${PYPI_URL}/simple' + assert p.lockfile['_meta']['sources'][0]['url'] == '${PYPI_URL}/simple' + + # Ensure package install does not extrapolate. + c = p.pipenv('install six') + assert c.return_code == 0 + assert p.pipfile['source'][0]['url'] == '${PYPI_URL}/simple' + assert p.lockfile['_meta']['sources'][0]['url'] == '${PYPI_URL}/simple'