Improve Windows build support #760
Conversation
Added vcpkg.json as package manifest for dependencies. Changed the build options in CMakeLists.txt by specifying the output directories to make sure DLLs are copied next to executables for Windows debugging in an IDE (e.g. Visual Studio or CLion) Disabled the build of NATS streaming since protobuf-c dependency doesn't seem to be working properly, and it's deprecated anyway. Further, updated .gitignore to exclude Visual Studio related files and the 'out' directory which VS seems to use by default for build output.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #760 +/- ##
==========================================
+ Coverage 68.68% 68.72% +0.03%
==========================================
Files 39 39
Lines 15186 15186
Branches 3139 3139
==========================================
+ Hits 10431 10437 +6
+ Misses 1702 1695 -7
- Partials 3053 3054 +1 ☔ View full report in Codecov by Sentry. |
| #set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}) | ||
| # Set output directories for libraries and executables. | ||
| # This is important for Windows builds to have the DLLs in the same directory as the executables. | ||
| set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) |
There was a problem hiding this comment.
@kozlovic Do you think this may break any existing user build scripts? As I understand, this should not affect make install results.
There was a problem hiding this comment.
fyi just compared make install results for this PR and main on a couple of docker container diffs and they were identical:
A /usr/local/include/nats.h
A /usr/local/include/nats/adapters/libevent.h
A /usr/local/include/nats/adapters/libuv.h
A /usr/local/include/nats/nats.h
A /usr/local/include/nats/status.h
A /usr/local/include/nats/version.h
A /usr/local/lib/cmake/cnats/cnats-config-release.cmake
A /usr/local/lib/cmake/cnats/cnats-config.cmake
A /usr/local/lib/libnats.so
A /usr/local/lib/libnats.so.3.9
A /usr/local/lib/libnats.so.3.9.0
A /usr/local/lib/libnats_static.a
A /usr/local/lib/pkgconfig/libnats.pc
There was a problem hiding this comment.
If the purpose of this PR is to have build for Windows on GH, then I think it is good, although the build logs there report a bunch of warnings (that I don't get when I compile on my Windows VM). As far of testing, this PR generates the test list but does not run the tests. Is that expected? Based on answer, I will LGTM or not. Thanks!
| - name: Test | ||
| run: | | ||
| cd build | ||
| ./bin/Debug/testsuite |
There was a problem hiding this comment.
This does not run the test suite, it just builds the list of tests. Was it intended?
There was a problem hiding this comment.
yes, that was my intention to have a GH Windows build and improved debugging experience using VS and CLion on Windows. I propose we address testing in a follow up PR.
Added vcpkg.json as package manifest for dependencies. Changed the build options in CMakeLists.txt by specifying the output directories to make sure DLLs are copied next to executables for Windows debugging in an IDE (e.g. Visual Studio or CLion) Disabled the build of NATS streaming for windows builds since protobuf-c dependency doesn't seem to be working properly, and it's deprecated anyway. Further, updated .gitignore to exclude Visual Studio related files and the 'out' directory which VS seems to use by default for build output.