Skip to content
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

Add explicit dependency on absl for xrt_swemu abd xrt_hwemu targets #8002

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

unsatcore
Copy link

Problem solved by the commit

Fixes linkage of xrt_swemu and xrt_hwemu targets from runtime_src/core/pcie/emulation/

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

Mentioned components won't link on NixOS 23.11 (gcc 13.2.0, cmake 3.28.3):

...
/nix/store/...-binutils-2.41/bin/ld: /nix/store/...-protobuf-24.4/include/google/protobuf/repeated_ptr_field.h:704:(.text._ZN6google8protobuf8internal20RepeatedPtrFieldBase13ClearNonEmptyINS0_16RepeatedPtrFieldI45xclPerfMonReadTrace_Streaming_response_eventsE11TypeHandlerEEEvv[_ZN6google8protobuf8internal20RepeatedPtrFieldBase13ClearNonEmptyINS0_16RepeatedPtrFieldI45xclPerfMonReadTrace_Streaming_response_eventsE11TypeHandlerEEEvv]+0xc1): undefined reference to `absl::lts_20240116::log_internal::LogMessageFatal::~LogMessageFatal()
...

claiming undefined references to abseil-cpp library.

Corresponding CMakeFiles/xrt_[s|h]wemu.dir/link.txt files won't mention absl either.

How problem was solved, alternative solutions (if any) and why they were rejected

Explicit dependency on absl::log and absl::check was introduced into relevant CMakeLists.txt files

I'm aware NixOS is not supported, still not sure how this even works on Ubuntu / RHEL.
Hopefully being a bit more clear re dependencies won't hurt.

Risks (if any) associated the changes in the commit

The commit was not tested on any of the officially supported distros

What has been tested and how, request additional testing if necessary

The build was only tested locally.
It is required to make sure the build passes on all the supported systems.

Documentation impact (if any)

None

Add explicit dependency on absl::log and absl::check
Add explicit dependency on absl::log and absl::check
@gbuildx
Copy link
Collaborator

gbuildx commented Mar 11, 2024

unsatcore - is not a collaborator
Can XRT admins please validate PR

@gbuildx
Copy link
Collaborator

gbuildx commented Mar 11, 2024

Can one of the admins verify this patch?

Copy link
Collaborator

@stsoe stsoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is CMake supposed to find absl from? This change doesn't look like it will work.
Please make this change work on variants of Linux supported by XRT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants