fix: enable native Windows source builds#126
Conversation
setup.py's check_dependencies() shells out to `which`, so a working build already required Go, MinGW-w64 gcc, MSYS2 make, and gopy on PATH. Once those are present, two remaining bugs still block a native Windows source install: - gopy's own generated Makefile indents its Windows-only PyInit_ sed fix-up with two spaces instead of a tab, which GNU Make rejects as "missing separator" before it reaches a shell. Patch it in place right after `gopy gen` runs. The check is content-matched (not line-numbered) and gated on `$(OS) == Windows_NT`, so it's a `true` no-op on Linux/macOS, where gopy doesn't emit that line at all. - pyproject.toml's package-data glob only listed *.so/*.dll/*.dylib, so the compiled extension gopy produces as *.pyd on Windows was silently dropped from the wheel. Adding *.pyd is purely additive. Verified end-to-end on Windows: `make gopy_build` produces working .pyd artifacts, `pip install --no-build-isolation` builds a wheel that actually contains them, and the installed SDK successfully signs and submits a real testnet transaction. This does not change the Linux or macOS build/install path in any way — both branches above resolve to their prior behavior on those platforms.
Time Submission Status
Submit or update total time with: Add time on top of previous submission with: See available commands to help comply with our Guidelines. |
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe build now applies a Windows-specific correction to generated binding Makefiles, while Python packaging includes ChangesCross-platform bindings
Estimated code review effort: 2 (Simple) | ~10 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 |
Bug Report Checklist
@vinarmani, please use git blame and specify the link to the commit link that has introduced this bug. Send the following message in this PR: See available commands to help comply with our Guidelines. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
43-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude Windows binary artifacts in the clean-up step.
Currently, the
rm -fcommand only removes.sofiles before generating the new bindings. On Windows, this can leave behind stale.pydand.dllfiles from previous builds. Extending the cleanup command to cover Windows artifacts ensures a consistently clean build environment across all platforms.♻️ Proposed fix to clean Windows artifacts
gopy_build: - rm -f src/trufnetwork_sdk_c_bindings/*.so + rm -f src/trufnetwork_sdk_c_bindings/*.so src/trufnetwork_sdk_c_bindings/*.pyd src/trufnetwork_sdk_c_bindings/*.dll gopy gen -output=src/trufnetwork_sdk_c_bindings -vm=python3 -name=trufnetwork_sdk_c_bindings $(DYNAMIC_LINK_FLAG) ./bindings @$(WINGOPY_PYINIT_FIX)🤖 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 `@Makefile` around lines 43 - 46, Update the cleanup command in the gopy_build target to remove Windows .pyd and .dll artifacts in addition to existing .so files before running gopy gen. Keep the cleanup scoped to src/trufnetwork_sdk_c_bindings and preserve the current build sequence.
🤖 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.
Nitpick comments:
In `@Makefile`:
- Around line 43-46: Update the cleanup command in the gopy_build target to
remove Windows .pyd and .dll artifacts in addition to existing .so files before
running gopy gen. Keep the cleanup scoped to src/trufnetwork_sdk_c_bindings and
preserve the current build sequence.
The gopy_build cleanup only removed *.so before regenerating, so a Windows rebuild never cleared its own *.pyd output (or *.dll, already listed in pyproject.toml's package-data). A failed regen could silently leave a stale binary in place. Harmless no-op on Linux/macOS, which never produce these extensions.
MicBun
left a comment
MicBun
left a comment
There was a problem hiding this comment.
It looks good, I assume it has been tested with windows environment? Currently I am not able to directly test on windows
|
@MicBun yes, builds successfully and tests pass on windows and linux |
2 participants
setup.py's check_dependencies() shells out to
which, so a working build already required Go, MinGW-w64 gcc, MSYS2 make, and gopy on PATH. Once those are present, two remaining bugs still block a native Windows source install:gopy genruns. The check is content-matched (not line-numbered) and gated on$(OS) == Windows_NT, so it's atrueno-op on Linux/macOS, where gopy doesn't emit that line at all.Verified end-to-end on Windows:
make gopy_buildproduces working .pyd artifacts,pip install --no-build-isolationbuilds a wheel that actually contains them, and the installed SDK successfully signs and submits a real testnet transaction.This does not change the Linux or macOS build/install path in any way — both branches above resolve to their prior behavior on those platforms.
Summary by CodeRabbit
Bug Fixes
Compatibility