Skip to content

Implement utf8cpp dependency check for EBML #354

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.

Sign up for GitHub

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

Open
SunBlack wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement utf8cpp dependency check for EBML #354

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions EBMLConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@

include(${CMAKE_CURRENT_LIST_DIR}/EBMLTargets.cmake)

include(CMakeFindDependencyMacro)
if(NOT @BUILD_SHARED_LIBS@)

@robUx4 robUx4 Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It was causing link issues when the utf8cpp library was found in the system.

I used if(@utf8cpp_FOUND@) and it works fine. I don't think we need to differentiate between static and shared builds because in both case the utf8cpp package needs to be there, it won't be linked in the static or dynamic library we build.

@SunBlack SunBlack Aug 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Feel free to take over with #357 as I just tested it with vcpkg, I don't know the difference to your setup (or don't want spent time on this now)

@SunBlack SunBlack Aug 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@BillyONeal In case you have an opinion to (@utf8cpp_FOUND@) vs NOT @BUILD_SHARED_LIBS@ feel free.

@robUx4 robUx4 Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you confirm this also works with vcpkg?

@BillyONeal BillyONeal Sep 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@BillyONeal In case you have an opinion to (@utf8cpp_FOUND@) vs NOT @BUILD_SHARED_LIBS@ feel free.

I don't have a strong opinion here. It is true that normally what you check in the config should exactly match what is checked in the CMakeLists.txt, but in this case when not utf8cpp_FOUND CMakeLists.txt responds by trying to do FetchContent so there really isn't a direct comparison here. I would argue that it probably should be unconditional rather than guarded by anything since that seems to be what CMakeLists.txt expects.

Can you confirm this also works with vcpkg?

We did merge microsoft/vcpkg#53661 which has this patch; I looked more at "build passed with the patch" and assumed that meant OK rather than following back to the CMakeLists.txt myself. The big thing that is problematic for vcpkg is the FetchContent but we ensure the find_package succeeds so we never see that path.

find_dependency(utf8cpp REQUIRED)
endif()

check_required_components(EBML)
Loading