fix: cross-platform config locking (Windows support)#51
Merged
Conversation
The profiles refactor (v0.2.0) added a top-level `import fcntl` to config.py. fcntl is Unix-only, so on Windows the import failed with ModuleNotFoundError before any command could run (config.py is imported transitively by every command). Import fcntl/msvcrt conditionally and route _config_lock() through small _lock_file/_unlock_file helpers: fcntl.flock on POSIX, msvcrt.locking on Windows, no-op degrade if neither is present. Guard the chmod(0o700) in _ensure_private_dir behind os.name == 'posix' since Windows chmod only toggles the read-only bit. The os.open mode bits in the atomic writers are already ignored on Windows, so they need no change. Fixes #49
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
=======================================
Coverage 84.28% 84.28%
=======================================
Files 15 15
Lines 2500 2520 +20
Branches 367 372 +5
=======================================
+ Hits 2107 2124 +17
- Misses 262 264 +2
- Partials 131 132 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Fixes #49.
Problem
Since the v0.2.0 profiles refactor,
config.pydoesimport fcntlat module top level.fcntlis Unix-only, so on Windows the import raisesModuleNotFoundErrorbefore any command can run.config.pyis imported transitively by everything (main.py->api.py->config.py), so the entire CLI is unusable on Windows from v0.2.0 through v0.4.1. Last working release on Windows was v0.1.1.Thanks to @leotulipan for the detailed report and root-cause analysis.
Fix (stdlib only, no new dependency)
fcntlandmsvcrtconditionally; either may beNone._config_lock()through_lock_file/_unlock_filehelpers:fcntl.flockon POSIX,msvcrt.lockingon Windows, graceful no-op if neither is available (acceptable for a single-user CLI).chmod(0o700)in_ensure_private_dirbehindos.name == "posix"(on Windowschmodonly toggles the read-only bit, so the owner-only guarantee is POSIX-only by nature).os.open(..., 0o600/0o644)mode bits in the atomic writers are already silently ignored on Windows, so they need no change.Tests
Added
TestCrossPlatformLock:fcntlpatched toNonethe lock routes tomsvcrt.locking(LK_LOCK / LK_UNLCK, 1 byte),set_active_profilemutation succeeds on the simulated-Windows path.Full suite: 300 passed, ruff clean. (CI on a Windows runner would be the ideal follow-up to lock this in for good.)