Draft
Conversation
blt
approved these changes
Sep 4, 2025
GeorgeHahn
reviewed
Sep 4, 2025
Cargo.toml
Outdated
Comment on lines
+18
to
+20
| # Platform-specific metrics-exporter-prometheus dependencies | ||
| # Unix platforms (Linux, macOS) get both HTTP and UDS listeners | ||
| # Windows gets only HTTP listener (no Unix Domain Sockets) |
Contributor
There was a problem hiding this comment.
This note looks misplaced
GeorgeHahn
reviewed
Sep 4, 2025
Cargo.toml
Outdated
| [target.'cfg(unix)'.workspace.dependencies] | ||
| metrics-exporter-prometheus = { version = "0.15", default-features = false, features = [ | ||
| "http-listener", | ||
| "uds-listener", |
Contributor
There was a problem hiding this comment.
Do we need the uds listener in practice?
GeorgeHahn
reviewed
Sep 4, 2025
ci/windows/build.ps1
Outdated
|
|
||
| Write-Host "Building release binary for Windows..." | ||
| # Build with default features (platform-specific deps handled in Cargo.toml) | ||
| cargo build --locked --release --features default |
Contributor
There was a problem hiding this comment.
Can we drop the --features default bit here? Presumably that's the default behavior
GeorgeHahn
reviewed
Sep 4, 2025
GeorgeHahn
reviewed
Sep 4, 2025
GeorgeHahn
reviewed
Sep 4, 2025
GeorgeHahn
reviewed
Sep 4, 2025
GeorgeHahn
approved these changes
Sep 4, 2025
Contributor
GeorgeHahn
left a comment
GeorgeHahn
left a comment
There was a problem hiding this comment.
Overall this looks good. Left a couple of comments on nits. I suspect there are a few more dependencies and lading modules that will need flagging off before the build succeeds. (I'm mostly thinking about nix and the observer module.)
scottopell
force-pushed
the
windows-ci-support
branch
from
120393d to
f4f5aa2
Compare
scottopell
commented
Sep 8, 2025
This adds experimental Windows support to lading with opt-in CI builds triggered by the 'build-windows' label on PRs. The implementation includes comprehensive cross-platform compatibility fixes and Windows-specific adaptations throughout the codebase. ## Key Changes **CI Infrastructure:** - New Windows build workflow with PowerShell scripts (check, test, build) - Protobuf installation support for Windows runners - Opt-in builds via 'build-windows' label (experimental feature) **Cross-Platform Compatibility:** - Fixed metrics-exporter-prometheus dependency for Windows builds - Added conditional compilation for platform-specific code - Windows-compatible process monitoring using sysinfo crate - Platform-specific PID tracking and process tree management **Test Infrastructure:** - Converted integration tests from Unix sockets to TCP for Windows compatibility - Skip Unix-specific sheepdog integration tests on Windows - Updated ducks/sheepdog test configurations for cross-platform use **Build System:** - Updated workspace dependencies for Windows compatibility - Fixed unused code warnings specific to Windows builds - Improved error handling for platform-specific features ## Testing Status - ✅ Compilation and basic functionality verified on Windows - ✅ Core build, check, and test workflows functional -⚠️ UDP generators have known issues on Windows (documented) -⚠️ Some Unix-specific features disabled on Windows This establishes the foundation for Windows support while maintaining full compatibility with existing Unix/Linux deployments. The Windows support is marked experimental pending further testing and validation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
scottopell
force-pushed
the
windows-ci-support
branch
from
55a4c22 to
819518c
Compare
Updates all Windows PowerShell CI scripts to match their Linux bash counterparts exactly, ensuring consistent behavior across platforms: - check.ps1: Use --all-features (was --features default) - clippy.ps1: Use --all-features, remove --locked, --all-targets, and -D warnings - test.ps1: Use --all-features (was --features default) - build.ps1: Remove --locked flag (cleaner release build) This works correctly now that we have platform-specific conditional dependencies in Cargo.toml - --all-features only activates features available for the target platform. The scripts now have semantic parity with Linux CI behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
After this gets rebased I'm still willing to test, if you would tell me the commands to do so! |
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
4 participants
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
Test plan
This is experimental/alpha Windows support - kept separate from main CI pipeline.
🤖 Generated with Claude Code