Merge pull request #1289 from pypa/revert-1233-tmpfile_cleanup

Revert "Clean up temporary files created during installation"

pipenv/cli.py

@@ -744,7 +744,7 @@ def do_install_dependencies(
     """"Executes the install functionality."""
 
     def cleanup_procs(procs, concurrent):
-        for c, cleanups in procs:
+        for c in procs:
 
             if concurrent:
                 c.block()
@@ -769,9 +769,6 @@ def cleanup_procs(procs, concurrent):
                     )
                 )
 
-            for cleanup in cleanups:
-                os.remove(cleanup)
-
     if requirements:
         bare = True
 
@@ -827,7 +824,7 @@ def cleanup_procs(procs, concurrent):
                 index = index.split()[0]
 
             # Install the module.
-            c, cleanups = pip_install(
+            c = pip_install(
                 dep,
                 ignore_hashes=ignore_hash,
                 allow_global=allow_global,
@@ -840,7 +837,7 @@ def cleanup_procs(procs, concurrent):
             c.dep = dep
             c.ignore_hash = ignore_hash
 
-            procs.append((c, cleanups))
+            procs.append(c)
 
         if len(procs) >= PIPENV_MAX_SUBPROCESS or len(procs) == len(deps_list):
             cleanup_procs(procs, concurrent)
@@ -1328,18 +1325,20 @@ def do_init(
         do_activate_virtualenv()
 
 
-def _pip_install(
+def pip_install(
     package_name=None, r=None, allow_global=False, ignore_hashes=False,
     no_deps=True, verbose=False, block=True, index=None, pre=False
 ):
-    """
-    Perform a pip install.
 
-    Use pip_install for temporary file cleanup.
-    """
     if verbose:
         click.echo(crayons.normal('Installing {0!r}'.format(package_name), bold=True), err=True)
 
+    # Create files for hash mode.
+    if (not ignore_hashes) and (r is None):
+        r = tempfile.mkstemp(prefix='pipenv-', suffix='-requirement.txt')[1]
+        with open(r, 'w') as f:
+            f.write(package_name)
+
     # Install dependencies when a package is a VCS dependency.
     try:
         req = get_requirement(package_name.split('--hash')[0].split('--trusted-host')[0]).vcs
@@ -1422,37 +1421,6 @@ def _pip_install(
     return c
 
 
-def pip_install(
-    package_name=None, r=None, allow_global=False, ignore_hashes=False,
-    no_deps=True, verbose=False, block=True, index=None, pre=False
-):
-    """Wraps _pip_install to clean up temporary files."""
-    r_is_tmpfile = False
-    # Create files for hash mode.
-    if (not ignore_hashes) and (r is None):
-        r_is_tmpfile = True
-        r = tempfile.mkstemp(prefix='pipenv-', suffix='-requirement.txt')[1]
-        with open(r, 'w') as f:
-            f.write(package_name)
-
-    cleanups = []
-    try:
-        c = _pip_install(
-            package_name, r, allow_global, ignore_hashes, no_deps, verbose, block,
-            index, pre,
-        )
-    finally:
-        if r_is_tmpfile:
-            if block:
-                # This is a blocking call, so we can perform cleanup ourselves
-                os.remove(r)
-            else:
-                # Otherwise, the caller has to handle it
-                cleanups = [r]
-
-    return c, cleanups
-
-
 def pip_download(package_name):
     for source in project.sources:
         cmd = '"{0}" download "{1}" -i {2} -d {3}'.format(
@@ -1916,7 +1884,7 @@ def install(
         # pip install:
         with spinner():
 
-            c, _ = pip_install(package_name, ignore_hashes=True, allow_global=system, no_deps=False, verbose=verbose, pre=pre)
+            c = pip_install(package_name, ignore_hashes=True, allow_global=system, no_deps=False, verbose=verbose, pre=pre)
 
             # Warn if --editable wasn't passed.
             try:

pipenv/utils.py

@@ -501,16 +501,15 @@ class PipCommand(pip.basecommand.Command):
     constraints = []
 
     for dep in deps:
+        t = tempfile.mkstemp(prefix='pipenv-', suffix='-requirement.txt')[1]
+        with open(t, 'w') as f:
+            f.write(dep)
+
         if dep.startswith('-e '):
             constraint = pip.req.InstallRequirement.from_editable(dep[len('-e '):])
         else:
-            t = tempfile.mkstemp(prefix='pipenv-', suffix='-requirement.txt')[1]
-            try:
-                with open(t, 'w') as f:
-                    f.write(dep)
-                constraint = [c for c in pip.req.parse_requirements(t, session=pip._vendor.requests)][0]
-            finally:
-                os.remove(t)
+            constraint = [c for c in pip.req.parse_requirements(t, session=pip._vendor.requests)][0]
+            # extra_constraints = []
 
         if ' -i ' in dep:
             index_lookup[constraint.name] = project.get_source(url=dep.split(' -i ')[1]).get('name')

tests/test_legacy.py

@@ -25,7 +25,7 @@ def test_pip_install_should_try_every_possible_source(self, mocked_delegator, mo
         second_cmd_return = Mock()
         second_cmd_return.return_code = 0
         mocked_delegator.side_effect = [first_cmd_return, second_cmd_return]
-        c, _ = pip_install('package')
+        c = pip_install('package')
         assert c.return_code == 0
 
     @patch('pipenv.project.Project.sources', new_callable=PropertyMock)
@@ -41,7 +41,7 @@ def test_pip_install_should_return_the_last_error_if_no_cmd_worked(self, mocked_
         second_cmd_return = Mock()
         second_cmd_return.return_code = 1
         mocked_delegator.side_effect = [first_cmd_return, second_cmd_return]
-        c, _ = pip_install('package')
+        c = pip_install('package')
         assert c.return_code == 1
         assert c == second_cmd_return
 
@@ -58,7 +58,7 @@ def test_pip_install_should_return_the_first_cmd_that_worked(self, mocked_delega
         second_cmd_return = Mock()
         second_cmd_return.return_code = 0
         mocked_delegator.side_effect = [first_cmd_return, second_cmd_return]
-        c, _ = pip_install('package')
+        c = pip_install('package')
         assert c.return_code == 0
         assert c == first_cmd_return
 

tests/test_pipenv.py

@@ -28,9 +28,6 @@ def __init__(self, pipfile=True, chdir=False):
         self.pipfile_path = None
         self.chdir = chdir
 
-        self.tmpdir = None
-        self._before_tmpdir = None
-
         if pipfile:
             p_path = os.sep.join([self.path, 'Pipfile'])
             with open(p_path, 'a'):
@@ -42,21 +39,12 @@ def __init__(self, pipfile=True, chdir=False):
     def __enter__(self):
         if self.chdir:
             os.chdir(self.path)
-        self._before_tmpdir = os.environ.pop('TMPDIR', None)
-        self.tmpdir = tempfile.mkdtemp(suffix='tmp', prefix='pipenv')
-        os.environ['TMPDIR'] = self.tmpdir
         return self
 
     def __exit__(self, *args):
         if self.chdir:
             os.chdir(self.original_dir)
 
-        if self._before_tmpdir is None:
-            del os.environ['TMPDIR']
-        else:
-            os.environ['TMPDIR'] = self._before_tmpdir
-
-        shutil.rmtree(self.tmpdir)
         shutil.rmtree(self.path)
 
     def pipenv(self, cmd, block=True):
@@ -435,7 +423,6 @@ def test_editable_vcs_install(self):
             assert 'idna' in p.lockfile['default']
             assert 'urllib3' in p.lockfile['default']
             assert 'certifi' in p.lockfile['default']
-            assert os.listdir(p.tmpdir) == []
 
     @pytest.mark.install
     @pytest.mark.pin
@@ -477,25 +464,6 @@ def test_editable_vcs_install_in_pipfile_with_dependency_resolution_doesnt_trace
             assert 'Traceback' not in c.err
 
 
-    @pytest.mark.run
-    @pytest.mark.install
-    def test_install_doesnt_leave_tmpfiles(self):
-        with temp_environ():
-            os.environ['PIPENV_MAX_SUBPROCESS'] = '2'
-
-            with PipenvInstance() as p:
-                with open(p.pipfile_path, 'w') as f:
-                    contents = """
-[packages]
-records = "*"
-                    """.strip()
-                    f.write(contents)
-
-                c = p.pipenv('install')
-                assert c.return_code == 0
-                assert os.listdir(p.tmpdir) == []
-
-
     @pytest.mark.run
     @pytest.mark.install
     def test_multiprocess_bug_and_install(self):