fix(userspace/libsinsp): Include cri.hpp in container.cpp #1895
Conversation
Include cri.hpp in container.cpp in order to avoid ``` /usr/bin/ld: /usr/lib/aarch64-linux-gnu/libsinsp.so: undefined reference to `libsinsp::cri::cri_interface<libsinsp::cri::cri_api_v1>::get_cri_runtime_type() const' collect2: error: ld returned 1 exit status ``` when building with shared libs on Linux. Signed-off-by: Gerald Combs <gerald@wireshark.org>
| @@ -20,6 +20,7 @@ limitations under the License. | |||
|
|
|||
| #if !defined(MINIMAL_BUILD) && !defined(__EMSCRIPTEN__) | |||
| #include <libsinsp/container_engine/cri.h> | |||
| #include <libsinsp/cri.hpp> | |||
There was a problem hiding this comment.
Mmmh i don't see get_cri_runtime_type referenced anywhere in container.cpp. Is this the right place to include cri.hpp?
There was a problem hiding this comment.
I think it's due to container.cpp indirectly including userspace/cri.h, which has:
using cri_interface_v1alpha2 = cri_interface<cri_api_v1alpha2>;
using cri_interface_v1 = cri_interface<cri_api_v1>;
When I try to build a Debian package here, libsinsp.so ends up with undefined get_cri_runtime_type symbols:
$ nm libsinsp/libsinsp.so | grep get_cri_runtime_type
00000000002e0810 T _ZNK8libsinsp16container_engine3cri20get_cri_runtime_typeEv
U _ZNK8libsinsp3cri13cri_interfaceINS0_10cri_api_v1EE20get_cri_runtime_typeEv
U _ZNK8libsinsp3cri13cri_interfaceINS0_16cri_api_v1alpha2EE20get_cri_runtime_typeEv
Maybe a better option would be to include libsinsp/cri.hpp instead of libsinsp/cri.h in userspace/container_engine/cri.h, or to combine userspace/cri.h and userspace.hpp?
It's also possible that the combination of CMake options in the Debian rules file is triggering this but I didn't see anything obvious.
There was a problem hiding this comment.
Weird :/ anyway, the fix is so small that i wouldn't care much :) let's ship it!
|
LGTM label has been added. Git tree hash: a332c1ed6615a5176f344de845843851df06afde
|
|
/milestone 0.18.0 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, FedeDP, geraldcombs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/milestone 0.17.3 |
Include cri.hpp in container.cpp in order to avoid
when building with shared libs on Linux.
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area libsinsp
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: