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
31cab20079c1f2919f569119272923dea2e679c8 (#941).diff --git a/pkg/private/install.py.tpl b/pkg/private/install.py.tpl
index c6d5f8e..1ced5e6 100644
--- a/pkg/private/install.py.tpl
+++ b/pkg/private/install.py.tpl
@@ -80,19 +80,17 @@
def _do_file_copy(self, src, dest):
logging.debug("COPY %s <- %s", dest, src)
- # Copy to a temporary file and then move it to the destination.
+ # Copy to a temporary directory and then move it to the destination.
# This ensures code-signed executables on certain platforms
# behave correctly.
# See: https://developer.apple.com/documentation/security/updating-mac-software
+ # Use `TemporaryDirectory` instead of `NamedTemporaryFile` to avoid Windows file locking issues.
# Use `dir` to ensure the temporary file is created on the same file system as the destination,
# to avoid cross-filesystem replace which is an error on some platforms.
- with tempfile.NamedTemporaryFile(delete=False, dir=os.path.dirname(dest)) as tmp_file:
- try:
- shutil.copyfile(src, tmp_file.name)
- os.replace(tmp_file.name, dest)
- except:
- pathlib.Path(tmp_file.name).unlink(missing_ok=True)
- raise
+ with tempfile.TemporaryDirectory(dir=os.path.dirname(dest)) as tmp_dir:
+ tmp_file = os.path.join(tmp_dir, os.path.basename(dest))
+ shutil.copyfile(src, tmp_file)
+ os.replace(tmp_file, dest)
def _do_mkdir(self, dirname, mode):
logging.debug("MKDIR %s %s", mode, dirname)