-
Notifications
You must be signed in to change notification settings - Fork 3
Replace external curl/wget with libcurl integration #606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: ckrause <[email protected]>
Co-authored-by: ckrause <[email protected]>
Co-authored-by: ckrause <[email protected]>
|
@copilot Please update the GitHub workflows to install libcurl. |
Co-authored-by: ckrause <[email protected]>
Updated in commit 8e3e3ef. Added libcurl installation steps to all CI and release workflows:
The Windows Makefile was also updated to support vcpkg's library paths when |
Co-authored-by: ckrause <[email protected]>
|
@copilot The compilation on windows fails with the following error: 2025-11-23T18:52:21.7014914Z file.cpp |
Co-authored-by: ckrause <[email protected]>
Fixed in commit c7f63ca. The issue was a macro conflict - on Windows, Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot Extend |
…dows.mk Co-authored-by: ckrause <[email protected]>
Added in commit b7388db. Both Makefiles now display linked libraries after the build step:
|
|
@copilot Update the branch. Then extend the Github release workflow for the two windows builds. We currently attach the loda.exe files to the releases. We should replace this by zip files which contain the loda executable and the libcurl dll. This is only needed for windows. |
Co-authored-by: ckrause <[email protected]>
Updated in commit 5e50e87. The Windows release workflow now packages both x64 and ARM64 builds as zip files containing:
The release assets will be named |
|
@copilot You modified the linker flags for |
Co-authored-by: ckrause <[email protected]>
Fixed in commit b7d2efb. Restored the pthread linker flags ( The |
Integrate libcurl into loda-cpp ✅
This PR successfully integrates libcurl to replace the external curl/wget tools currently used for HTTP requests.
✅ All Tasks Completed:
📝 Summary of Changes:
1. WebClient Implementation (src/sys/web_client.cpp)
#undef ERRORto prevent macro conflict between curl/Windows headers and Log::Level enum2. Build System (All Makefiles)
-Wl,--whole-archive -lpthread -Wl,--no-whole-archive) to prevent segmentation faults as described in https://stackoverflow.com/questions/35116327/when-g-static-link-pthread-cause-segmentation-fault-whyVCPKG_ARCHvariablex64-windowsif not specifiedlddotool -Ldumpbin /dependents3. GitHub Workflows
libcurl4-openssl-devinstallation in CI and release workflowscurl:x64-windowscurl:arm64-windowswithVCPKG_ARCH=arm64-windowsloda.exe- the main executablelibcurl.dll- the required DLL for HTTP requests4. Documentation
🎯 Key Benefits:
🐛 Windows Build Fix:
The Windows compilation was failing due to a macro conflict. On Windows,
curl/curl.hor Windows system headers define anERRORmacro which conflicts with theERRORenum value inLog::Level. The fix adds#undef ERRORafter including curl headers but before including log.hpp, ensuring clean compilation on Windows while maintaining compatibility with other platforms.🔒 Security:
Proper resource cleanup (no memory leaks)
Error checking on all file operations
Standard libcurl security practices followed
Fixes Integrate libcurl #605
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.