Fixed Windows shared libraries build & added CI #608
Conversation
| token: ${{ github.token }} | ||
| github-binarycache: true | ||
| - name: cmake configure | ||
| run: cmake . -B build -DBUILD_TESTING=ON -DBUILD_EXAMPLES=ON -DBUILD_SHARED_LIBS=ON -DCMAKE_TOOLCHAIN_FILE=D:/a/ouster-sdk/ouster-sdk/vcpkg/scripts/buildsystems/vcpkg.cmake |
There was a problem hiding this comment.
Just an absolute path which I had to use to get this to work, see the step above.
There was a problem hiding this comment.
We could probably parameterize the BUILD_SHARED_LIBS rather than duplicating the build step.
There was a problem hiding this comment.
Agreed. I'm not too familiar with this CI, excuse my shabby solution.
|
@eivan the build has failed .. I think you are missing the |
If you look at your CI of your current main branch, it is failing too for the Windows builds, the exact same way: When you're attempting a build locally, you need to re-run CMake to generate 'ouster_client_export.h'. I think you might be missing that. |
I went ahead and had a look at those two failed tests. This diff fixes those two failed tests in your main branch and here too: diff --git a/ouster_osf/tests/writerv2_test.cpp b/ouster_osf/tests/writerv2_test.cpp
index 69764e8..f50a1aa 100644
--- a/ouster_osf/tests/writerv2_test.cpp
+++ b/ouster_osf/tests/writerv2_test.cpp
@@ -178,7 +178,7 @@ TEST_F(WriterV2Test, WriterV2MultiIndexedTest) {
writer.save(0, ls);
writer.save(1, ls2);
}
- test_multi_file(output_osf_filename, ls2, ls);
+ test_multi_file(output_osf_filename, ls, ls2);
}
TEST_F(WriterV2Test, WriterV2MultiVectorTest) {
@@ -194,7 +194,7 @@ TEST_F(WriterV2Test, WriterV2MultiVectorTest) {
Writer writer(output_osf_filename, {info, info2});
writer.save({ls, ls2});
}
- test_multi_file(output_osf_filename, ls2, ls);
+ test_multi_file(output_osf_filename, ls, ls2);
}
} // namespace
} // namespace osf
The order of those scans look like they are reversed. |
| set(ouster_client_export_filename | ||
| ouster_client/ouster/ouster_client_export.h) | ||
|
|
||
| generate_export_header(ouster_client |
There was a problem hiding this comment.
Generating ouster_client_export.h here, on CMAKE build, using a standard CMAKE approach.
Then, I'm using OUSTER_CLIENT_EXPORT in code to export the symbols, a macro is defined in the generated header, marking functions, classes, etc.
During a Windows build this basically translates to __declspec(__dllexport). With this macro one can fine-tune which symbols to export.
For ouster_osf, ouster_pcap and ouster_viz It seemed safe to just go with exporting all symbols, see this line as an example:
ouster-sdk/ouster_osf/CMakeLists.txt
Line 150 in 5df706c
Summary of Changes
On Windows, when BUILD_SHARED_LIBS are set ON in CMake, libraries (ouster_client, ouster_pcap, ouster_osf, ouster_viz) do not generate symbol files / .lib-s, only .dll-s are output. This impedes other libraries and executables to link with said libraries to address compiled objects and linking fails.
Validation