Merge pull request #7929 from McSinyx/tmp-file

Use better temporary files mechanism

Author: pfmoore

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

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(

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:

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,