Skip to content

Commit b438c45

Browse files
committed
Refactor TemporaryFileSwap.__init__ for clarity
This changes the mkstemp call TemporaryFileSwap uses to atomically create (and thus reserve) the temporary file, so that it passes only a filename (i.e., basename) prefix as `prefix`, and passes all other path components (i.e., the directory) as `dir`. (If the original path has no directory, then `dir` is "" as before.) This makes the mkstemp call *slightly* more idiomatic. This also makes it clearer, as it is no longer using `prefix` for something that feels like it should be possible to pass as a Path object. Although mkstemp does not accept a Path as `prefix`, it does (as expected) accept one as `dir`. However, to keep the code simple, I'm passing str for both. The os.path.split function accepts both str and Path (since Python 3.6), and returns str objects, which are now used for the `dir` and `prefix` arguments to mkstemp. For unusual cases, this may technically not be a refactoring. For example, a file_path of "a/b//c" will be split into "a/b" and "c". If the automatically generated temporary file suffix is "xyz", then that results in a tmp_file_path of "a/b/cxyz" where "a/b//cxyz" would have been used before. The tmp_file_path attribute of a TemporaryFileSwap object is public (and used in filesystem calls). However, no guarantee has ever been given that the temporary file path have the original path as an exact string prefix. I believe the slightly weaker relationship I expressed in the recently introduced test_temporary_file_swap -- another file in the same directory, named with the original filename with more characters, consisting of equivalent path components in the same order -- has always been the intended one. Note that this slight possible variation does not apply to the file_path attribute. That attribute is always kept exactly as it was, both in its type and its value, and it always used unmodified in calls that access the filesystem.
1 parent 1ddf953 commit b438c45

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

Diff for: git/index/util.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ class TemporaryFileSwap:
4141

4242
def __init__(self, file_path: PathLike) -> None:
4343
self.file_path = file_path
44-
fd, self.tmp_file_path = tempfile.mkstemp(prefix=str(self.file_path), dir="")
44+
dirname, basename = osp.split(file_path)
45+
fd, self.tmp_file_path = tempfile.mkstemp(prefix=basename, dir=dirname)
4546
os.close(fd)
4647
with contextlib.suppress(OSError): # It may be that the source does not exist.
4748
os.replace(self.file_path, self.tmp_file_path)

0 commit comments

Comments
 (0)