From 8d8f0f8261a3d175ca654b31d23bc5dcf9377d26 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 11 Jan 2018 22:14:16 -0500 Subject: [PATCH] Revert "Clean up temporary files created during installation" --- pipenv/cli.py | 54 +++++++++----------------------------------- pipenv/utils.py | 13 +++++------ tests/test_legacy.py | 6 ++--- tests/test_pipenv.py | 32 -------------------------- 4 files changed, 20 insertions(+), 85 deletions(-) diff --git a/pipenv/cli.py b/pipenv/cli.py index 81bf29ec..549b6e0e 100644 --- a/pipenv/cli.py +++ b/pipenv/cli.py @@ -734,7 +734,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() @@ -759,9 +759,6 @@ def do_install_dependencies( ) ) - for cleanup in cleanups: - os.remove(cleanup) - if requirements: bare = True @@ -817,7 +814,7 @@ def do_install_dependencies( index = index.split()[0] # Install the module. - c, cleanups = pip_install( + c = pip_install( dep, ignore_hashes=ignore_hash, allow_global=allow_global, @@ -830,7 +827,7 @@ def do_install_dependencies( 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) @@ -1318,18 +1315,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 @@ -1412,37 +1411,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( @@ -1906,7 +1874,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: diff --git a/pipenv/utils.py b/pipenv/utils.py index e09c87e2..0f748c3d 100644 --- a/pipenv/utils.py +++ b/pipenv/utils.py @@ -467,16 +467,15 @@ def actually_resolve_reps(deps, index_lookup, markers_lookup, project, sources, 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') diff --git a/tests/test_legacy.py b/tests/test_legacy.py index f70a86c6..a9c1df5e 100644 --- a/tests/test_legacy.py +++ b/tests/test_legacy.py @@ -25,7 +25,7 @@ class TestPipenv(): 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 @@ class TestPipenv(): 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 @@ class TestPipenv(): 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 diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index cabdf4cf..cb59c876 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -28,9 +28,6 @@ class PipenvInstance(): 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 @@ class PipenvInstance(): 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 @@ setup( 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 @@ idna = "==2.6.0" 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):