Add Windows Build Compatibility #131
Conversation
|
Thanks for the contribution; will surely look into it!! |
pooranjoyb
added
touch-of-magic
|
@pooranjoyb The build completes fine now and installs all the header files to the MinGW directories like they wanted, and I also cleaned up the nlohmann_json dependency . 
Kindly review and let me know if any changes are needed,
Thank you
|
|
what i can see is that linux build is not pointing to the build dirs. Moreover after the conan installation; why does it fetches the dependencies in the root of the project? Please fix it. Also lemme know if I'm running a wrong command for installation. these were the commands used to install the sdk in unix : mkdir build && cd build
conan install .. --build=missing
cmake ..
make
sudo make installEvidence : 
|
|
Also please update the README.md with updated installation guidelines for both linux and windows. |
|
updated the build system to fully support Windows (MinGW64) and Linux environments in a clean workflow, ensuring all Conan and CMake generated files stay inside the build/ directory rather than the project root. updated the readme too for user guidance for the commands to be used. kindly review and let me know if any changes are needed @pooranjoyb . tested on ubuntu and windows. |
Okay will check and retest. |
|
|
||
| add_library(AppwriteSDK STATIC ${SRCS}) | ||
| # Set C++ standard | ||
| set(CMAKE_CXX_STANDARD 17) |
There was a problem hiding this comment.
| set(CMAKE_CXX_STANDARD 17) | |
| set(CMAKE_CXX_STANDARD 11) |
This should be 17 i guess, keeping the same as the current standard.
There was a problem hiding this comment.
i have set the standard set as 17 only is there an issue?
There was a problem hiding this comment.
@Ashish-Kumar-Dash I've reviewed the CMake changes, but I'm noticing some differenced between the screenshot in the PR description and the actual implementation.
Correct me if i'm wrong but from my understanding of the CMake file you've written, it appears the current configuration may not be compiling all the necessary source files into a proper static library. The logic seems to be designed for a header-only (INTERFACE) library approach when dummy.cpp is missing, rather than building the complete static library with all implementation files like Appwrite.cpp, Account.cpp, Query.cpp, etc.
CMakeLists.txt
Outdated
|
|
||
| # Create library - handle both dummy.cpp and header-only scenarios | ||
| set(SOURCES | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src/dummy.cpp |
There was a problem hiding this comment.
This only includes dummy.cpp but misses all the actual source files (Appwrite.cpp, Account.cpp, Query.cpp, etc.). Doesn't it need to include all implementation files like the existing CMake does?
CMakeLists.txt
Outdated
| install(DIRECTORY include/ DESTINATION /usr/local/include/AppwriteSDK) | ||
| install(FILES ${HEADERS} DESTINATION /usr/local/include/AppwriteSDK) | ||
| install(TARGETS AppwriteSDK ARCHIVE DESTINATION /usr/local/lib) | ||
| if(NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/src/dummy.cpp) |
There was a problem hiding this comment.
Isnt this conditional INTERFACE library approach incorrect for a static library. We need to explicitly specify STATIC library type and include all source files, not just dummy.cpp, the dummy file doesn't exist in the src folder
CMakeLists.txt
Outdated
| ) | ||
| set(LIBRARY_TYPE "INTERFACE") | ||
| else() | ||
| add_library(AppwriteSDK ${SOURCES}) |
There was a problem hiding this comment.
Missing explicit STATIC library type specification.
CMakeLists.txt
Outdated
| PRIVATE | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src | ||
| ) | ||
| set(LIBRARY_TYPE "STATIC") |
There was a problem hiding this comment.
You are declaring the LIBRARY_TYPE variable but in the code below its never being checked for its "STATIC" value
CMakeLists.txt
Outdated
| DESTINATION include/AppwriteSDK) | ||
|
|
||
| # Install library only if not header-only | ||
| if(NOT LIBRARY_TYPE STREQUAL "INTERFACE") |
There was a problem hiding this comment.
This INTERFACE library logic adds unnecessary complexity. Since we're building a static library, we should always install the target.
|
The CMake setup was updated to explicitly build a STATIC library including all source files from src/ and remove the previous INTERFACE logic. Added mandatory find_package(CURL REQUIRED) with robust linking to Conan-provided curl targets. The build now requires -DCMAKE_BUILD_TYPE for proper dependency resolution. After these changes, the project builds and installs cleanly with all dependencies correctly handled on Linux and Windows. Apologies for the late update as I was busy for my midsem exams. Kindly review @pooranjoyb @SharonIV0x86 . attached ss of the build |
The million dollar question is, does it work on windows? |
3 participants
This PR addresses issue #10
What & Why
This PR implements Windows build compatibility for the cpp-sdk-appwrite project, addressing Issue #10 which requested extending the existing Unix-based Conan integration to Windows systems.
Original Requirement:
How - Technical Implementation
Core Approach:
Key Technical Solutions:
build/Release/generators/)Files Created/Modified
CMakeLists.txtbuild_windows.batinstall_windows.batconanfile.txtFile Purposes Explained:
build_windows.bat: One-command Windows build automation - handles Conan dependency installation, CMake configuration, and compilationinstall_windows.bat: Fulfills the core requirement by installing headers toC:\MinGW\include\AppwriteSDK\and libraries toC:\MinGW\lib\CMakeLists.txt: Cross-platform compatibility with proper Conan toolchain integration, Windows-specific configurations, and robust dependency handlingTesting Results
Build Success Evidence:
Terminal Output
Verification:
libAppwriteSDK.ageneratedCompleted Requirements:
Impact
Windows developers can now:
./build_windows.bat./install_windows.batg++ -o app.exe code.cpp -lAppwriteSDKKindly review and let me know if any changes are needed, @pooranjoyb @sristy17 . I'm keen to hear feedback because this has been a big learning curve for me.
Thank You