Skip to content

Commit 6c27507

Browse files
committed
Add 'tempfile_compat' to handle windows tmp files
`pip` is not aware that it's reading from a tempfile. On Windows, this is significant, and we can't guarantee good behavior except by setting `delete=False` on the tempfile object. That done, we become responsible for the cleanup as well, which is just a final pair of close()+unlink() calls.
1 parent 9ac94db commit 6c27507

File tree

3 files changed

+44
-16
lines changed

3 files changed

+44
-16
lines changed
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
"""
2+
Compatibility helpers for :mod:`tempfile` usage across platforms
3+
and python versions.
4+
"""
5+
6+
from __future__ import annotations
7+
8+
import contextlib
9+
import os
10+
import tempfile
11+
import typing as _t
12+
from collections.abc import Iterator
13+
14+
15+
@contextlib.contextmanager
16+
def named_temp_file(mode: str = "wt") -> Iterator[_t.IO[str]]:
17+
"""
18+
A safe wrapper over NamedTemporaryFile for usage on Windows as well as
19+
POSIX systems.
20+
21+
The issue we have is that we cannot guarantee that ``pip`` will open temporary
22+
files on Windows with ``O_TEMPORARY`` when passed the path, resulting in surprising
23+
behavior.
24+
"""
25+
temp_file = tempfile.NamedTemporaryFile(mode=mode, delete=False)
26+
yield temp_file
27+
temp_file.close()
28+
os.unlink(temp_file.name)

piptools/scripts/compile.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import os
55
import shlex
66
import sys
7-
import tempfile
87
import typing as _t
98
from pathlib import Path
109

@@ -14,7 +13,7 @@
1413
from pip._internal.req import InstallRequirement
1514
from pip._internal.utils.misc import redact_auth_from_url
1615

17-
from .._compat import canonicalize_name, parse_requirements
16+
from .._compat import canonicalize_name, parse_requirements, tempfile_compat
1817
from .._internal import _pip_api
1918
from ..build import ProjectMetadata, build_project_metadata
2019
from ..cache import DependencyCache
@@ -355,7 +354,7 @@ def cli(
355354
# pip requires filenames and not files. Since we want to support
356355
# piping from stdin, we need to briefly save the input from stdin
357356
# to a temporary file and have pip read that.
358-
with tempfile.NamedTemporaryFile(mode="wt") as tmpfile:
357+
with tempfile_compat.named_temp_file() as tmpfile:
359358
tmpfile.write(sys.stdin.read())
360359
tmpfile.flush()
361360
reqs = list(
@@ -414,7 +413,7 @@ def cli(
414413
)
415414

416415
if upgrade_packages:
417-
with tempfile.NamedTemporaryFile(mode="wt") as constraints_file:
416+
with tempfile_compat.named_temp_file() as constraints_file:
418417
constraints_file.write("\n".join(upgrade_packages))
419418
constraints_file.flush()
420419
reqs = list(

tests/test_cli_compile.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import shutil
1010
import subprocess
1111
import sys
12-
import tempfile
1312
import typing as _t
1413
from textwrap import dedent
1514
from unittest import mock
@@ -22,6 +21,7 @@
2221
from pip._internal.utils.urls import path_to_url
2322
from pip._vendor.packaging.version import Version
2423

24+
from piptools._compat import tempfile_compat
2525
from piptools._internal import _pip_api
2626
from piptools.build import ProjectMetadata
2727
from piptools.scripts.compile import cli
@@ -1487,32 +1487,33 @@ def test_tmpfile_for_stdin_is_cleaned_up(pip_conf, runner):
14871487
tmpfile_name = None
14881488

14891489
# save the real constructor so that it can be used while `mock.patch()` is
1490-
# active -- also, it's a function, not a class, so we can't inherit from it
1491-
real_constructor = tempfile.NamedTemporaryFile
1490+
# active
1491+
real_constructor = tempfile_compat.named_temp_file
14921492

1493-
# a "spy" which can be mocked into place for `NamedTemporaryFile` to
1493+
# a "spy" which can be mocked into place for `named_temp_file` to
14941494
# replace the implementation with one which has side-effects and makes test
14951495
# assertions
14961496
#
14971497
# this spy ensures that the file is deleted on exit
1498-
# it also sets a nonlocal to indicate that it was initialized at all
14991498
class NamedTempfileSpy:
15001499
def __init__(self, *args, **kwargs):
1501-
self._tmpfile = real_constructor(*args, **kwargs)
1500+
self._ctx_manager = real_constructor(*args, **kwargs)
15021501

15031502
def __enter__(self):
1504-
ret = self._tmpfile.__enter__()
1503+
ret = self._ctx_manager.__enter__()
15051504
nonlocal tmpfile_name
1506-
tmpfile_name = self._tmpfile.name
1505+
tmpfile_name = ret.name
15071506
return ret
15081507

15091508
def __exit__(self, *args, **kwargs):
1510-
assert os.path.exists(self._tmpfile.name)
1511-
ret = self._tmpfile.__exit__(*args, **kwargs)
1512-
assert not os.path.exists(self._tmpfile.name)
1509+
assert os.path.exists(tmpfile_name)
1510+
ret = self._ctx_manager.__exit__(*args, **kwargs)
1511+
assert not os.path.exists(tmpfile_name)
15131512
return ret
15141513

1515-
with mock.patch("tempfile.NamedTemporaryFile", NamedTempfileSpy):
1514+
with mock.patch(
1515+
"piptools._compat.tempfile_compat.named_temp_file", NamedTempfileSpy
1516+
):
15161517
out = runner.invoke(
15171518
cli,
15181519
["-", "--output-file", "-", "--quiet", "--no-emit-options", "--no-header"],

0 commit comments

Comments
 (0)