fix(locking): use portalocker for cross-platform advisory locks#15
Draft
abpatramsft wants to merge 1 commit intoevo-hq:mainfrom
Draft
fix(locking): use portalocker for cross-platform advisory locks#15abpatramsft wants to merge 1 commit intoevo-hq:mainfrom
abpatramsft wants to merge 1 commit intoevo-hq:mainfrom
Conversation
Replace the fcntl-only implementation with portalocker, which dispatches to fcntl.flock on POSIX and LockFileEx on Windows. Preserves the existing exclusive/non-blocking/bounded-polling semantics and the LockTimeoutError contract, so all callers keep working unchanged. Closes the import-time crash on Windows where evo --version (and every downstream command) failed with ModuleNotFoundError: No module named 'fcntl'. Adds tests/unit/test_locking.py covering acquire/release, timeout under contention, and cross-thread reacquire. Passes on Windows 11 / Python 3.10. Refs evo-hq#14 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fcntl.flockinplugins/evo/src/evo/locking.pyforportalocker, which dispatches tofcntl.flockon POSIX andLockFileExon Windows — fixing theModuleNotFoundError: No module named 'fcntl'that blocksevo --versionand every downstream command on Windows (Not working on Windows #14).portalocker>=2.8.0toplugins/evo/pyproject.tomland regeneratesplugins/evo/uv.lock.tests/unit/test_locking.pycovering acquire/release, timeout under contention, and cross-thread reacquire.Why portalocker over conditional
fcntl/msvcrtportalockeris the de-facto standard cross-platform file-locking library (pip itself depends on it) and lets the lock call site stay a single code path. The alternative — asys.platform == "win32"branch importingmsvcrt.locking— saves one dependency at the cost of doubling the code and forcing any future contributor to reason about two locking APIs. Happy to switch to the conditional-import approach if you'd prefer to keep the dep list minimal.Semantics preserved
The exclusive, non-blocking, bounded-polling behavior is identical to the pre-patch implementation:
portalocker.LOCK_EX | portalocker.LOCK_NBmirrorsfcntl.LOCK_EX | fcntl.LOCK_NBportalocker.LockExceptionreplacesBlockingIOErrorin the retry loopLockTimeoutError(the public contract) is unchanged — callers catching it keep workingTest plan
python3 tests/unit/test_locking.py— 3/3 pass on Windows 11 / Python 3.10python3 tests/unit/test_core.py— 7/7 still pass (no regression)evo --versionruns cleanly on Windows (previously crashed on import)from evo import dashboard+ static-asset smoke check fromci.ymlstill passesfcntl.flock, so behavior should be byte-identical, but worth a reviewer checkNot in this PR
.github/workflows/ci.ymlcurrently pinsubuntu-latestand uses bash-specific shell (unzip,/tmp/smoke). Addingwindows-latestis doable but is a larger diff touching the smoke-test step — happy to send it as a follow-up PR once this lands, since the locking fix is the blocker.uv.lockregeneration. Pulled in viauv lockafter adding the dep; includes transitivepywin32on Windows platform markers. No version changes to existing deps.Refs #14