fix: restore Windows CLI and preserve controller locks - #129
Conversation
repo2rlenv is completely non-functional on Windows right now, in the current v0.9.0 PyPI release: `repo2rlenv --version` raises `ModuleNotFoundError: No module named 'fcntl'` before argument parsing even completes. Verified from a fresh `pip install repo2rlenv` on a clean venv, from a from-scratch git clone + uv sync, and from upstream/main directly — not specific to any one install path. Root cause: cli.py's dispatcher imports every subsystem's argparse registration unconditionally, before routing to any subcommand, so this runs on every invocation including --version and --help. Four files reachable from that chain do `import fcntl` at module level — fcntl is POSIX-only and has no stdlib equivalent on Windows at all (msvcrt.locking byte-ranges a file rather than advisory-locking a whole handle, so it isn't a drop-in swap): quality/loop/runner.py, tasksmith/runner.py, tasksmith/batch.py, pipelines/recipes/history/worker.py Each uses it identically, for the same thing: an advisory, mostly non-blocking flock() on a ".lock" file so two concurrent controllers/loops don't run against the same output directory. Guard both the import and the flock() call by `sys.platform`, preserving each site's exact existing behavior (return type, exception handling) on POSIX unchanged. Locking becomes a documented no-op on Windows rather than a crash — a real gap, but not a regression, since locking was never functional there before this either. Added tests/test_posix_only_imports.py: an AST-based static check (same shape as test_subprocess_encoding.py) that fails if any file under src/repo2rlenv imports fcntl/pwd/grp/termios/tty/pty/posix/ resource/crypt/nis/spwd unconditionally at module level. Verified it flags all 4 original offenders when the fix is reverted, and passes clean with it applied — CI runs Linux-only, where none of these modules are missing, so nothing else would have caught a regression here. Verified on Windows 11 / Python 3.12: `repo2rlenv --version` and `--help` both now exit 0 and build the full subcommand list. Verified on Linux (WSL Ubuntu, uv run --all-extras pytest -q — CI's exact command): 1866 passed, 0 failed, both before and after this patch — no regressions. Refs huggingface#128
|
|
thanks for catching this and putting up the fix. i added real Windows locks so parallel controllers cannot write the same run, plus Windows wheel checks in CI and the release gate. local focused tests are passing; waiting on the native Windows jobs before merging. broader controller portability will be tracked separately. |
adithya-s-k
changed the title
adithya-s-k
left a comment
adithya-s-k
left a comment
There was a problem hiding this comment.
verified the final changes: locks remain exclusive on both platforms, the shared checkout still waits correctly, and all CI checks pass including Windows 3.12–3.14. thanks for the report and initial fix. the wider portability work is tracked in #130.
|
Nice — Pulled 640fbc into a clean worktree and ran it on a separate Windows 11 machine, independent of CI:
All green independently of the CI run. Looks ready from my side. |
|
merged and released in 0.9.1. Windows CI and the fresh PyPI install checks passed. thanks for catching this and getting the fix started! |
* fix(ci): harden workflow files flagged on #129 * Complete workflow defaults and release credential hardening --------- Co-authored-by: hf-security-analysis[bot] <265538906+hf-security-analysis[bot]@users.noreply.github.com> Co-authored-by: adithya-s-k <adithyaskolavi@gmail.com>
2 participants
Summary
Windows installations of 0.9.0 crash on every CLI command, including
--versionand--help, because parser registration reaches four modules that import POSIX-onlyfcntl.Use a shared standard-library lock with lazy platform imports: retain
flockon POSIX and lock byte zero throughmsvcrton Windows. Controller locks remain nonblocking; the history checkout lock still waits for its owner. Closing the handle releases either lock, and the lock path is retained.Add fresh-wheel Windows CI for Python 3.12–3.14, including CLI discovery, native UTF-8 task emission/static validation and real subprocess lock contention. Publication depends on the same Windows gate. Document the current host support scope. Thanks to @KNambiarDJsc for the report, initial fix and import regression test.
Test plan
Out of scope
Full native Windows Tasksmith, research-recipe and quality-controller execution is tracked in #130. Use Linux, macOS or WSL for those controllers; choosing a remote sandbox does not remove host artifact/process requirements. No generated datasets need rewriting for this CLI/locking fix.
Closes #128