Fix Windows build: cfg-gate Unix-only imports and helpers - #77
Fix Windows build: cfg-gate Unix-only imports and helpers#77HellaDopeWC wants to merge 1 commit into
Conversation
The crate denies all warnings ([lints.rust] warnings = deny), and on Windows targets several items are compiled but only used by cfg(unix) code paths, so unused_imports/dead_code warnings become hard errors and cargo install fails. Gate the Unix-only items behind #[cfg(unix)]: - gate_lock.rs: OpenOptions, io::Read, PathBuf, anyhow::Result and the git module import; the two lockfile-name constants; LockKind::file_name; read_holder; lock_path. All are used only by the flock-based Unix serialization path — the cfg(not(unix)) fallbacks never touch them. - run/append.rs: CString/c_char, thread, Duration imports and the OPEN_EVENT_FILE_RETRIES constant, used only by the cfg(unix) event-file open/retry path. No behavior change on any platform. cargo check and clippy --all-targets stay clean on Linux; cargo install now succeeds on x86_64-pc-windows-msvc without RUSTFLAGS=--cap-lints=warn.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📝 WalkthroughWalkthroughThe changes restrict Unix-specific imports, constants, methods, and helper functions in lock management and event appending to Unix builds. ChangesUnix compilation gates
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/domain/run/append.rs`:
- Around line 1-8: Update retry_open_event_file_at and its call chain so
Unix-only symbols OPEN_EVENT_FILE_RETRIES, thread, and Duration are never
referenced by non-Unix builds. Gate open_event_file, retry_open_event_file_at,
and every caller consistently with #[cfg(unix)], or provide a non-Unix
implementation that avoids those symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8958a8a1-fe5c-448d-8361-eac004700a16
📒 Files selected for processing (2)
src/domain/gate_lock.rssrc/domain/run/append.rs
| #[cfg(unix)] | ||
| use std::ffi::{CString, c_char}; | ||
| use std::fs::{self, File}; | ||
| use std::io::{self, Read, Seek, SeekFrom, Write}; | ||
| use std::path::Path; | ||
| #[cfg(unix)] | ||
| use std::thread; | ||
| #[cfg(unix)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Do not gate symbols that remain referenced unconditionally.
retry_open_event_file_at is still compiled on every target, but it references OPEN_EVENT_FILE_RETRIES, thread, and Duration, all now restricted to Unix. This causes Windows compilation to fail with unresolved names. Either gate the entire retry helper and its callers with #[cfg(unix)], or keep the required constant/imports available on non-Unix targets.
Proposed fix direction
#[cfg(unix)]
const OPEN_EVENT_FILE_RETRIES: usize = 8;
+#[cfg(unix)]
fn retry_open_event_file_at(repo_root: &Path, relative_path: &Path) -> io::Result<File> {Ensure open_event_file and every caller are gated consistently, or provide a non-Unix implementation that does not use these Unix-only symbols.
Also applies to: 22-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/domain/run/append.rs` around lines 1 - 8, Update retry_open_event_file_at
and its call chain so Unix-only symbols OPEN_EVENT_FILE_RETRIES, thread, and
Duration are never referenced by non-Unix builds. Gate open_event_file,
retry_open_event_file_at, and every caller consistently with #[cfg(unix)], or
provide a non-Unix implementation that avoids those symbols.
|
Thank you for reporting this and for taking the time to put together a fix. Maestro is currently undergoing a substantial vNext architecture refoundation, but that work has not been published to the remote repository yet. As a result, the current Maestro also doesn’t fully support native Windows yet, and vNext may significantly change the areas touched by this patch. I’ve noted this issue, and once vNext is complete, I’ll fix it as part of bringing full native Windows support to Maestro. Thank you again for investigating this. Your findings and patch will be helpful when I work on Windows support after vNext. |
2 participants
The crate denies all warnings ([lints.rust] warnings = deny), and on Windows targets several items are compiled but only used by cfg(unix) code paths, so unused_imports/dead_code warnings become hard errors and cargo install fails.
Gate the Unix-only items behind #[cfg(unix)]:
No behavior change on any platform. cargo check and clippy --all-targets stay clean on Linux; cargo install now succeeds on x86_64-pc-windows-msvc without RUSTFLAGS=--cap-lints=warn.
Summary by CodeRabbit