Skip to content

Commit 6cdaba6

Browse files
authored
Fix Windows file locking issue in pkg_install (#983)
`NamedTemporaryFile` turns out to lock the file on Windows, preventing `os.replace` from moving it: ``` windows> bazel run --enable_runfiles //[redacted]:install -- --destdir=d:\est [...] INFO: Running command line: [redacted]/install.exe <args omitted> INFO: Installing to [redacted] Traceback (most recent call last): File "[redacted]\install_install_script.py", line 92, in _do_file_copy os.replace(tmp_file.name, dest) PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t' -> 'd:\est/filename.ext' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "[redacted]\install_install_script.py", line 307, in <module> sys.exit(main(sys.argv)) ^^^^^^^^^^^^^^ File "[redacted]\install_install_script.py", line 303, in main installer.do_the_thing() File "[redacted]\install_install_script.py", line 206, in do_the_thing self._install_file(entry) File "[redacted]\install_install_script.py", line 126, in _install_file self._do_file_copy(entry.src, entry.dest) File "[redacted]\install_install_script.py", line 94, in _do_file_copy pathlib.Path(tmp_file.name).unlink(missing_ok=True) File "[redacted]\rules_python++python+python_3_12_x86_64-pc-windows-msvc\Lib\pathlib.py", line 1342, in unlink os.unlink(self) PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t' Intuition: _Any problem in computer science can be solved with another level of indirection._ (https://bwlampson.site/Slides/TuringLecture.htm) The change therefore proposes to use `TemporaryDirectory` instead of `NamedTemporaryFile` as an indirection to avoid a file lock from being held, which allows the file to be freely written and moved on all platforms while maintaining the same atomic replace behavior for macOS code-signed binaries introduced in commit 31cab20 (#941).
1 parent 4eed546 commit 6cdaba6

File tree

1 file changed

+6
-8
lines changed

1 file changed

+6
-8
lines changed

pkg/private/install.py.tpl

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,17 @@ class NativeInstaller(object):
8080

8181
def _do_file_copy(self, src, dest):
8282
logging.debug("COPY %s <- %s", dest, src)
83-
# Copy to a temporary file and then move it to the destination.
83+
# Copy to a temporary directory and then move it to the destination.
8484
# This ensures code-signed executables on certain platforms
8585
# behave correctly.
8686
# See: https://developer.apple.com/documentation/security/updating-mac-software
87+
# Use `TemporaryDirectory` instead of `NamedTemporaryFile` to avoid Windows file locking issues.
8788
# Use `dir` to ensure the temporary file is created on the same file system as the destination,
8889
# to avoid cross-filesystem replace which is an error on some platforms.
89-
with tempfile.NamedTemporaryFile(delete=False, dir=os.path.dirname(dest)) as tmp_file:
90-
try:
91-
shutil.copyfile(src, tmp_file.name)
92-
os.replace(tmp_file.name, dest)
93-
except:
94-
pathlib.Path(tmp_file.name).unlink(missing_ok=True)
95-
raise
90+
with tempfile.TemporaryDirectory(dir=os.path.dirname(dest)) as tmp_dir:
91+
tmp_file = os.path.join(tmp_dir, os.path.basename(dest))
92+
shutil.copyfile(src, tmp_file)
93+
os.replace(tmp_file, dest)
9694

9795
def _do_mkdir(self, dirname, mode):
9896
logging.debug("MKDIR %s %s", mode, dirname)

0 commit comments

Comments
 (0)