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

Public Headers #3

Closed
julianxhokaxhiu opened this issue Jan 6, 2020 · 10 comments
Closed

Public Headers #3

julianxhokaxhiu opened this issue Jan 6, 2020 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@julianxhokaxhiu
Copy link

Hi,

I've successfully managed to build the library under 32bit platform thanks to #2.

Now although I'm facing another problem. As I'd like to use this library as a pre-built library, I'm missing the include headers. Is there a possibility to have public headers installed via CMake when running the install target?

Would be fine to have them generated as well, one per target.

Thank you in advance,
Julian

@TheLavaBlock
Copy link
Member

Hi Julian, Good point.

Once C++20 modules are supported in CMake, liblava will be upgraded completely.
This means that header files are no longer needed.

Currently it has to be done manually or via script. Unfortunately.

@TheLavaBlock TheLavaBlock added the enhancement New feature or request label Jan 6, 2020
@julianxhokaxhiu
Copy link
Author

Hi @TheLavaBlock,

I am currently checking the library again for some progress, and I notice this issue is still open. As C++ 20 modules on CMake will not get supported until who known when, is there a way to workaround this issue? You mentioned then it can be done manually or via a script. I would be fine with both, but can you please give me a hint on what can be done?

I was checking this CMake function, but I'm not sure it's exactly what we need: https://cmake.org/cmake/help/v3.17/module/GenerateExportHeader.html

Any help on this topic is appreciated :)

Thank you in advance,
Julian

@pezcode
Copy link
Contributor

pezcode commented Mar 13, 2020

The canonic way to do it would be with the PUBLIC_HEADERS target property + specifying an install rule for each target: https://stackoverflow.com/a/40285224/862300

A really easy alternative is something like

install(DIRECTORY liblava/ DESTINATION include/liblava FILES_MATCHING PATTERN "*.hpp")

@TheLavaBlock
Copy link
Member

I like the easy alternative.

@TheLavaBlock
Copy link
Member

And yeah, C++20 modules with CMake will take a while.

@julianxhokaxhiu
Copy link
Author

julianxhokaxhiu commented Mar 18, 2020

But *.hpp files do contain code. So now I think probably another way of including liblava is just adding the whole /liblava dir into the include path, there is no need to pre-compile it.

Although at the same time, pre-compiling it would provide faster compilation for whichever library may link to liblava. So I'm not sure we can merge the pre-built lib files with public headers, which do contain code anyway inside. Or am I missing something here?

@pezcode
Copy link
Contributor

pezcode commented Mar 19, 2020

There's no code in .hpp files, it's just a regular C++ header. But yes, you'd still need an install directive for the libraries.

I can give this a try tomorrow (no promises 🤡)

@pezcode
Copy link
Contributor

pezcode commented Mar 19, 2020

@julianxhokaxhiu PR with preliminary install support: #18

Feel free to give it a try and report back

@TheLavaBlock
Copy link
Member

@julianxhokaxhiu can this be closed? Think #18 is doing a good job.

@julianxhokaxhiu
Copy link
Author

Absolutely! Thank you again

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

No branches or pull requests

3 participants