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

use the utf-8 C++ library as an external (header-only) library #124

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

Conversation

robUx4
Copy link
Contributor

@robUx4 robUx4 commented Oct 15, 2022

It might be used as an external package someday. For it's a CMake interface since it's a header only piece of code.

It might be used as an external package someday. For it's a CMake interface
since it's a header only piece of code.
Copy link
Contributor

@mbunkus mbunkus left a comment

Choose a reason for hiding this comment

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

My initial idea of bundling this was that to avoid having a new external dependency. However, I've changed my mind since then, and I'd really prefer to convert this into a proper dependency & to remove the bundled copy. The usual Linux distros all carry it as a package anyway, and it would cut down on the amount of testing we'd have to do (e.g. only verify using the external one works, not also having to verify the bundled one can still be used).

The bundled copy wasn't modified anyway.

@neheb
Copy link
Contributor

neheb commented Oct 15, 2022

External library sounds better.

@robUx4
Copy link
Contributor Author

robUx4 commented Oct 15, 2022

What happens with an external dependency when I build with MSVC (rather VSCode) and I don't have it on my system ? Or on mac which normally don't have packages ?

@neheb
Copy link
Contributor

neheb commented Oct 15, 2022

add some kind of subproject (no idea how to do that with cmake) and link it statically I guess.

@robUx4
Copy link
Contributor Author

robUx4 commented Oct 15, 2022

There's nothing to link, it's just a bunch of headers.

Also, there's the CI that is a bare environment with no dependencies installed.

@neheb
Copy link
Contributor

neheb commented Oct 15, 2022

right. then I guess add it as a git submodule that gets cloned when host utf8-cpp is missing.

@mbunkus
Copy link
Contributor

mbunkus commented Oct 16, 2022

I don't know anything about MSVC packaging. Maybe you could check how VLC does it?

In the end I don't really care either way, to be honest. The one thing I'd like to see wrt. to utf8cpp changes is the ability to optionally build against a system-provided utf8cpp if found, fixing #54. I'm fine carrying our bundled copy. Dropping it is a slight preference at most.

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

Successfully merging this pull request may close these issues.

3 participants