-
Notifications
You must be signed in to change notification settings - Fork 165
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 a pkg-config file #128
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't your buildsystem consume the CMake configuration directly?
cmake/base64.pc.in
Outdated
libdir="${prefix}/lib" | ||
includedir="${prefix}/include" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't account for user configured CMAKE_INSTALL_INCLUDEDIR
, CMAKE_INSTALL_LIBDIR
(or CMAKE_INSTALL_BINDIR
on Windows) variables.
cmake/base64.pc.in
Outdated
Description: @CMAKE_PROJECT_DESCRIPTION@ | ||
Version: @PROJECT_VERSION@ | ||
Cflags: -I${includedir} | ||
Libs: -L${libdir} -l@PROJECT_NAME@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, this doesn't account for CMAKE_(STATIC|SHARED|IMPORT)_LIBRARY_(PRE|SUF)FIX(_C)?
. Please also note that the PROJECT_NAME
<=> target name equivalence is rather coincidental.
to my knowledge, it does not (it's gn). electron creates their buildscripts manually https://github.com/electron/electron/blob/95d094d75bddb99c83d2902fbc9a4335632a41cf/patches/node/build_add_gn_build_files.patch#L450 |
is this correct? |
@BurningEnlightenment Does this look good to you after the fixes? I don't really have enough knowledge about In cases like this I'm usually in favour of merging it so it gets traction. Then we can see how the cards shake out (i.e. who starts complaining). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alas, this isn't quite right. CMAKE_IMPORT_LIBRARY_PREFIX_C
is only relevant when building shared libraries on Windows (and is not automatically set if the user only supplies CMAKE_IMPORT_LIBRARY_PREFIX
). My previous comment meant that you need to respect CMAKE_STATIC_LIBRARY_PREFIX
, CMAKE_SHARED_LIBRARY_PREFIX
or CMAKE_IMPORT_LIBRARY_PREFIX
and their corresponding _C
suffix depending on the target platform and the target configuriation, i.e. whether we are building static or shared libraries. So the easiest and least brittle way to do that is by letting CMake do its thing and pluck the "effective" values out of the target properties. I recommend restricting this feature to CMake 3.19+ and use file(GENERATE
with target generator expressions. You'll need to either inline the pkg-config template or do a double configuration dance due to the fact that the file(GENERATE INPUT <input-file>
form only expands generator expressions but no CMake variables.
Generating correct pkg-config
files is rather complicated due to its oversimplified and badly abstracted structure… I'm not really convinced that this will lead to better results than the custom gn
buildfiles included in electron/chromium. Wasn't it recommended practice for gn
to provide custom buildfiles for 3rdparty dependencies?
not sure how would that work since the same pkg-config files are used whether you add
ime that's what gn projects typically do for themselves, as they typically compile everything statically, but when they sometimes don't, they typically go with pkg-config, e.g. https://github.com/flutter/engine/blob/1db92c0ac09b1010535cd0c60f3b661ef203b1cf/shell/platform/linux/config/BUILD.gn, https://source.chromium.org/chromium/chromium/src/+/main:build/linux/unbundle/brotli.gn |
If you configure the project both times (once for static and once for shared) in a way which generates identical pkg-configs that shouldn't be an issue. And otherwise you've just reached the point where the impedance mismatch between CMake and pkg-config worlds starts to break things. |
|
Correct, I see how the wording from my previous comment is a bit unclear with regards to this (double configuration dance).
What I meant to say: You will need to do a |
makes reuse as shared library easier. example generated file: