Handle missing libc during fallocate setup - #351
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #351 +/- ##
==========================================
- Coverage 87.83% 87.67% -0.17%
==========================================
Files 2 2
Lines 1143 1160 +17
Branches 191 142 -49
==========================================
+ Hits 1004 1017 +13
- Misses 99 103 +4
Partials 40 40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Updates Whisper’s optional fallocate initialization to avoid import-time crashes when ctypes.util.find_library('c') can’t locate libc (notably on Windows), and adds a regression test to ensure fallocate is disabled in that scenario.
Changes:
- Guard fallocate setup so
ctypes.CDLL(...)is not called when libc can’t be located, disabling fallocate support instead. - Add a regression test that reloads
whisperwith libc lookup mocked as missing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| whisper.py | Adds a libc lookup guard to disable fallocate when libc cannot be found. |
| test_whisper.py | Adds a reload-based regression test for the “missing libc” fallocate-disable behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if libc_name is None: | ||
| CAN_FALLOCATE = False | ||
| else: | ||
| libc = ctypes.CDLL(libc_name) | ||
| c_off64_t = ctypes.c_int64 | ||
| c_off_t = ctypes.c_int |
There was a problem hiding this comment.
Handled in 94680e4 by catching OSError from ctypes.CDLL(...) and disabling fallocate support while leaving fallocate as None.
| def test_fallocate_disabled_when_libc_missing(self): | ||
| try: | ||
| with patch('ctypes.util.find_library', return_value=None): | ||
| reload(whisper) | ||
| self.assertFalse(whisper.CAN_FALLOCATE) | ||
| self.assertIsNone(whisper.fallocate) | ||
| finally: | ||
| reload(whisper) |
There was a problem hiding this comment.
Handled in 94680e4 by guarding the test with a ctypes import check and skipping it when ctypes is unavailable.
Summary
ctypes.CDLL(None)whenctypes.util.find_library('c')cannot locate libcwhispercan still be imported and used without sparse preallocationWhy
On Windows,
ctypesis available butctypes.util.find_library('c')returnsNone. The module currently treatsctypesimport success as enough to enable fallocate setup, then callsctypes.CDLL(None), which raises during import before callers or tests can use any other whisper functionality.Verification
python -m unittest test_whisper.TestWhisper.test_fallocate_disabled_when_libc_missingpython -m unittest test_whisper.TestWhisper.test_validate_archive_list test_whisper.TestParseRetentionDefpython -m py_compile whisper.py test_whisper.pypython -m flake8 whisper.py test_whisper.pygit diff --checkI also ran
python test_whisper.pyon Windows. It now gets past the original import failure and runs tests, then hits existing Windows lock-path failures becausetest_lockenablesLOCKwherefcntlis unavailable.