From 209c74f6902e32c89977555f59adc883e80a36ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Sun, 29 Mar 2020 17:19:34 +0700 Subject: [PATCH] Use better temporary files mechanism --- news/7699.bugfix | 2 + src/pip/_internal/operations/install/wheel.py | 54 +++++++++---------- src/pip/_internal/utils/filesystem.py | 16 ++++-- tests/unit/test_wheel.py | 2 +- 4 files changed, 40 insertions(+), 34 deletions(-) create mode 100644 news/7699.bugfix diff --git a/news/7699.bugfix b/news/7699.bugfix new file mode 100644 index 00000000000..51dbef88fda --- /dev/null +++ b/news/7699.bugfix @@ -0,0 +1,2 @@ +Use better mechanism for handling temporary files, when recording metadata +about installed files (RECORD) and the installer (INSTALLER). diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py index 329d90c67e6..e66d12b4bf0 100644 --- a/src/pip/_internal/operations/install/wheel.py +++ b/src/pip/_internal/operations/install/wheel.py @@ -27,6 +27,7 @@ from pip._internal.exceptions import InstallationError from pip._internal.locations import get_major_minor_version +from pip._internal.utils.filesystem import adjacent_tmp_file, replace from pip._internal.utils.misc import captured_stdout, ensure_dir, hash_file from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -36,7 +37,7 @@ if MYPY_CHECK_RUNNING: from email.message import Message from typing import ( - Dict, List, Optional, Sequence, Tuple, IO, Text, Any, + Dict, List, Optional, Sequence, Tuple, Any, Iterable, Callable, Set, ) @@ -64,15 +65,15 @@ def rehash(path, blocksize=1 << 20): return (digest, str(length)) # type: ignore -def open_for_csv(name, mode): - # type: (str, Text) -> IO[Any] - if sys.version_info[0] < 3: - nl = {} # type: Dict[str, Any] - bin = 'b' +def csv_io_kwargs(mode): + # type: (str) -> Dict[str, Any] + """Return keyword arguments to properly open a CSV file + in the given mode. + """ + if sys.version_info.major < 3: + return {'mode': '{}b'.format(mode)} else: - nl = {'newline': ''} # type: Dict[str, Any] - bin = '' - return open(name, mode + bin, **nl) + return {'mode': mode, 'newline': ''} def fix_script(path): @@ -563,28 +564,25 @@ def is_entrypoint_wrapper(name): logger.warning(msg) # Record pip as the installer - installer = os.path.join(dest_info_dir, 'INSTALLER') - temp_installer = os.path.join(dest_info_dir, 'INSTALLER.pip') - with open(temp_installer, 'wb') as installer_file: + installer_path = os.path.join(dest_info_dir, 'INSTALLER') + with adjacent_tmp_file(installer_path) as installer_file: installer_file.write(b'pip\n') - shutil.move(temp_installer, installer) - generated.append(installer) + replace(installer_file.name, installer_path) + generated.append(installer_path) # Record details of all files installed - record = os.path.join(dest_info_dir, 'RECORD') - temp_record = os.path.join(dest_info_dir, 'RECORD.pip') - with open_for_csv(record, 'r') as record_in: - with open_for_csv(temp_record, 'w+') as record_out: - reader = csv.reader(record_in) - outrows = get_csv_rows_for_installed( - reader, installed=installed, changed=changed, - generated=generated, lib_dir=lib_dir, - ) - writer = csv.writer(record_out) - # Sort to simplify testing. - for row in sorted_outrows(outrows): - writer.writerow(row) - shutil.move(temp_record, record) + record_path = os.path.join(dest_info_dir, 'RECORD') + with open(record_path, **csv_io_kwargs('r')) as record_file: + rows = get_csv_rows_for_installed( + csv.reader(record_file), + installed=installed, + changed=changed, + generated=generated, + lib_dir=lib_dir) + with adjacent_tmp_file(record_path, **csv_io_kwargs('w')) as record_file: + writer = csv.writer(record_file) + writer.writerows(sorted_outrows(rows)) # sort to simplify testing + replace(record_file.name, record_path) def install_wheel( diff --git a/src/pip/_internal/utils/filesystem.py b/src/pip/_internal/utils/filesystem.py index 161c450aafd..36578fb6244 100644 --- a/src/pip/_internal/utils/filesystem.py +++ b/src/pip/_internal/utils/filesystem.py @@ -17,7 +17,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING, cast if MYPY_CHECK_RUNNING: - from typing import BinaryIO, Iterator + from typing import Any, BinaryIO, Iterator class NamedTemporaryFileResult(BinaryIO): @property @@ -85,16 +85,22 @@ def is_socket(path): @contextmanager -def adjacent_tmp_file(path): - # type: (str) -> Iterator[NamedTemporaryFileResult] - """Given a path to a file, open a temp file next to it securely and ensure - it is written to disk after the context reaches its end. +def adjacent_tmp_file(path, **kwargs): + # type: (str, **Any) -> Iterator[NamedTemporaryFileResult] + """Return a file-like object pointing to a tmp file next to path. + + The file is created securely and is ensured to be written to disk + after the context reaches its end. + + kwargs will be passed to tempfile.NamedTemporaryFile to control + the way the temporary file will be opened. """ with NamedTemporaryFile( delete=False, dir=os.path.dirname(path), prefix=os.path.basename(path), suffix='.tmp', + **kwargs ) as f: result = cast('NamedTemporaryFileResult', f) try: diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 05300b96438..9328f6fb8bc 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -141,7 +141,7 @@ def call_get_csv_rows_for_installed(tmpdir, text): generated = [] lib_dir = '/lib/dir' - with wheel.open_for_csv(path, 'r') as f: + with open(path, **wheel.csv_io_kwargs('r')) as f: reader = csv.reader(f) outrows = wheel.get_csv_rows_for_installed( reader, installed=installed, changed=changed,