Fix runtime cache folder error when building with pyinstaller #805
Fix runtime cache folder error when building with pyinstaller #805lucemia merged 1 commit intolivingbio:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #805 +/- ##
==========================================
+ Coverage 79.97% 79.98% +0.01%
==========================================
Files 51 51
Lines 3425 3432 +7
==========================================
+ Hits 2739 2745 +6
- Misses 686 687 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR changes how typed-ffmpeg locates and populates its bundled cache so frozen/PyInstaller builds can run without failing due to missing on-disk cache directories.
Changes:
- Move the runtime
cache_pathfrom a package-relative directory to a system temp directory. - Add import-time initialization that copies bundled cache files into the temp cache directory via
importlib.resources.
src/ffmpeg/common/cache.py
Outdated
| cache_path = Path(tempfile.gettempdir()) / "ffmpeg_cache" | ||
| cache_path.mkdir(exist_ok=True) | ||
|
|
There was a problem hiding this comment.
cache_path is a fixed, predictable directory under the system temp dir. On multi-user systems (and especially if the process ever runs with elevated privileges), using a shared temp path can be vulnerable to symlink/hardlink tricks and can also cause cross-user/process cache corruption. Consider using a per-user (and ideally per-version) subdirectory and validating that cache_path is a real directory (not a symlink) before writing/copying into it.
src/ffmpeg/common/cache.py
Outdated
| cache_path = Path(tempfile.gettempdir()) / "ffmpeg_cache" | ||
| cache_path.mkdir(exist_ok=True) |
There was a problem hiding this comment.
Changing the cache location to the system temp dir affects non-PyInstaller/dev workflows too: the code-gen scripts in src/scripts/ use ffmpeg.common.cache.save() to (re)generate the checked-in cache data, but with this change they will write to the temp dir instead of the repo/package cache directory. Consider making the cache base dir configurable (e.g., env var) and/or only falling back to temp when the packaged cache/ directory isn’t available (such as in a frozen/PyInstaller build).
src/ffmpeg/common/cache.py
Outdated
| def _initialize_builtin_cache() -> None: | ||
| """Copy built-in cache files to temp directory if they don't exist.""" | ||
| with importlib.resources.as_file( | ||
| importlib.resources.files("ffmpeg.common").joinpath("cache") | ||
| ) as builtin_cache_path: | ||
| if builtin_cache_path.exists(): | ||
| for item in builtin_cache_path.rglob("*"): | ||
| if item.is_file(): | ||
| dest_path = cache_path / item.relative_to(builtin_cache_path) | ||
| dest_path.parent.mkdir(parents=True, exist_ok=True) | ||
| if not dest_path.exists(): | ||
| _ = shutil.copy2(item, dest_path) | ||
|
|
||
|
|
||
| _initialize_builtin_cache() |
There was a problem hiding this comment.
_initialize_builtin_cache() runs at import time and performs an unconditional rglob over the bundled cache tree (which is large) on every import, even when everything is already copied. This adds avoidable startup overhead and import-time side effects. Consider making initialization lazy (run on first load/save) and/or using a marker file/version check so you can skip the directory traversal once initialization is complete.
src/ffmpeg/common/cache.py
Outdated
| _ = shutil.copy2(item, dest_path) | ||
|
|
||
|
|
||
| _initialize_builtin_cache() |
There was a problem hiding this comment.
No tests currently cover the new built-in cache initialization/copying path (the behavior added here and especially relevant for frozen/PyInstaller builds). Consider adding a unit test that imports the module in an isolated temp dir and asserts the expected bundled cache artifacts are available (e.g., load(list[FFMpegOption], "options") succeeds) without relying on preexisting state in the system temp directory.
|
Sorry for overcomplicating things, the solution is actually this simple |
rudcode
changed the title
2 participants
When building executable binary through pyinstaller, the resulting binary will fail because the cache directory could not be found. Use system temp directory instead to fix this using python builtin tempfile library.