Merge pull request #2141 from pypa/bugfix/2100-do-not-extrapolate-in-…

…lock

Do not expand environ in cache on package install

.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

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')

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):

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

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'