Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork939
Commitb12fd4a
committed
Let Windows subprocess open or rename onto temp file
On Windows, a file created by NamedTemporaryFile cannot be reopenedwhile already open, under most circumstances. This applies toattempts to open it both from the original process and from otherprocesses. This differs from the behavior on other operatingsystems, and it works this way to help ensure the file can beautomatically deleted (since having a file open on Windows usuallyprevents the file from being deleted, unlike on other OSes).However, this can cause problems when NamedTemporaryFile is used toproduce a safe filename for accessing with an external process, asIndexFile.from_tree does. On Windows, git subprocess can't open andwrite to the file. This breaks IndexFile.from_tree on Windows. Thisis the cause of#1630 -- IndexFile.reset uses IndexFile.from_tree,and the Repo.index property returns an IndexFile instance, soRepo.index.reset fails -- and some other breakages.While passing delete=False to NamedTemporaryFile (and deletingseparately) often overcomes that, it won't fix things here, becauseIndexFile.from_tree uses "git read-tree --index-output=<file>".That needs more than the ability to open the file for write. Itneeds to be able to create or overwrite the given path by renamingan existing file to it. The description of "--index-output=<file>"in git-read-tree(1) notes: The file must allow to be rename(2)ed into from a temporary file that is created next to the usual index file; typically this means it needs to be on the same filesystem as the index file itself, and you need write permission to the directories the index file and index output file are located in.On Windows, more is required: there must be no currently open filewith that path, because on Windows, open files typically cannot bereplaced, just as, on Windows, they typically cannot be deleted(nor renamed). This is to say that passing delete=False toNamedTemporaryFile isn't enough to solve this because it merelycauses NamedTemporaryFile to behave like the lower-level mkstempfunction, leaving the responsibility of deletion to the caller butstill *opening* the file -- and while the file is open, the gitsubprocess can't replace it. Switching to mkstemp, with no furtherchange, would likewise not solve this.This commit is an initial fix for the problem, but not the bestfix. On Windows, it closes the temporary file created byNamedTemporaryFile -- so the file can be opened by name, includingby a git subprocess, and also overwritten, include by a file move.As currently implemented, this is not ideal, as it creates a racecondition, because closing the file actually deletes it. During thetime the file does not exist, the filename may end up being reusedby another call to NamedTemporaryFile, mkstemp, or similar facility(in any process, written in any language) before the git subprocesscan recreate it. Passing delete=False would avoid this, but thendeletion would have to be handled separately, and at that point itis not obvious that NamedTemporaryFile is the best facility to use.This fix, though not yet adequate, confirms the impact on#1630.The following 8 tests go from failing to passing due to this changeon a local Windows development machine (as listed in the summary atthe end of pytest output):- test/test_docs.py:210 Tutorials.test_references_and_objects- test/test_fun.py:37 TestFun.test_aggressive_tree_merge- test/test_index.py:781 TestIndex.test_compare_write_tree- test/test_index.py:294 TestIndex.test_index_file_diffing- test/test_index.py:182 TestIndex.test_index_file_from_tree- test/test_index.py:232 TestIndex.test_index_merge_tree- test/test_index.py:428 TestIndex.test_index_mutation- test/test_refs.py:218 TestRefs.test_head_resetOn CI, one test still fails, but later and for an unrelated reason:- test/test_index.py:428 TestIndex.test_index_mutationThat `open(fake_symlink_path, "rt")` call raises FileNotFoundError.1 parente359718 commitb12fd4a
1 file changed
+2
-0
lines changedLines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
362 | 362 | | |
363 | 363 | | |
364 | 364 | | |
| 365 | + | |
| 366 | + | |
365 | 367 | | |
366 | 368 | | |
367 | 369 | | |
| |||
0 commit comments
Comments
(0)