build: fix pkg-config output and USE_QCBOR definition - #30
Open
Ali Erdinc Koroglu (aekoroglu) wants to merge 4 commits into
Open
Ali Erdinc Koroglu (aekoroglu) wants to merge 4 commits into
Ali Erdinc Koroglu (aekoroglu) wants to merge 4 commits into
Conversation
qcomtee.pc.in hardcodes libdir as ${prefix}/lib, but the library itself
is installed to ${CMAKE_INSTALL_LIBDIR}. On distributions that use a
multiarch or 64-bit suffixed library directory, such as Fedora and
openSUSE where CMAKE_INSTALL_LIBDIR resolves to lib64, the two disagree
and every consumer of qcomtee.pc is handed a -L path that does not
contain libqcomtee.so.
Expand CMAKE_INSTALL_FULL_LIBDIR and CMAKE_INSTALL_FULL_INCLUDEDIR
instead. GNUInstallDirs is already included from the top-level
CMakeLists.txt, so both variables are available at configure time.
Signed-off-by: Ali Erdinc Koroglu <ali.koroglu@oss.qualcomm.com>
add_compile_definitions() takes no target and no scope keyword, so
add_compile_definitions(${PROJECT_NAME} PRIVATE -DUSE_QCBOR)
defines three macros for the whole directory instead of one:
-Dlibqcomtee -DPRIVATE -DUSE_QCBOR
Use target_compile_definitions() on the qcomtee target, next to the
matching target_link_libraries() call.
Signed-off-by: Ali Erdinc Koroglu <ali.koroglu@oss.qualcomm.com>
…private Commit "Enable libcbor fallback when QCBOR is unavailable" (PR qualcomm#23) made the CBOR backend selectable at configure time, but pc_req_private stayed hardcoded to qcbor. A build that falls back to libcbor therefore installs a qcomtee.pc declaring a private dependency on a module that is not present, which breaks static linking and dependency generation for downstream consumers. Set pc_req_private from QCBOR_FOUND. The libcbor pkg-config module is named libcbor, not cbor. Signed-off-by: Ali Erdinc Koroglu <ali.koroglu@oss.qualcomm.com>
QCBOR has been optional since the libcbor fallback landed, so REQUIRED now states the opposite of the intent. It has no effect today only because cmake/FindQCBOR.cmake sets QCBOR_FOUND by hand and never calls find_package_handle_standard_args(), which is what would otherwise honor REQUIRED and abort. Should that module ever be converted to the standard helper, the fallback path would become unreachable Signed-off-by: Ali Erdinc Koroglu <ali.koroglu@oss.qualcomm.com>
Ali Erdinc Koroglu (aekoroglu)
changed the title
Contributor
|
Can you drop the [PATCH] prefix from your commit headers and also only use the name of the folder is prefix which you modifying, |
| message(WARNING "QCBOR not found, falling back to libcbor") | ||
| endif() | ||
|
|
||
| include_directories(${QCBOR_INCLUDE_DIRS}) |
Contributor
There was a problem hiding this comment.
Should this be moved under if (QCBOR_FOUND) condition as well?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #23. Four build fixes, two files, no functional change to the library.
1.
qcomtee.pcpoints at the wrong libdirqcomtee.pc.inhardcodeslibdir="${prefix}/lib", but the library installs to${CMAKE_INSTALL_LIBDIR}. Where that resolves tolib64, as on Fedora andopenSUSE, consumers get a
-Lpath that does not containlibqcomtee.so. Nowexpands
CMAKE_INSTALL_FULL_LIBDIRandCMAKE_INSTALL_FULL_INCLUDEDIR.2.
USE_QCBORis set with the wrong commandadd_compile_definitions()takes no target and no scope keyword, sodefines three macros for the whole directory instead of one:
Switched to
target_compile_definitions()on the qcomtee target.3.
Requires.privateignores the libcbor fallback#23 made the CBOR backend selectable, but
pc_req_privatestayed hardcoded toqcbor. A libcbor build installs a.pcdeclaring a private dependency on amodule that is not present, which breaks static linking and downstream
dependency generation. Now follows
QCBOR_FOUND. The pkg-config module forlibcbor is named
libcbor, notcbor.4.
find_package(QCBOR REQUIRED)QCBOR is optional since #23, so
REQUIREDstates the opposite of the intent. Itis inert only because
cmake/FindQCBOR.cmakesetsQCBOR_FOUNDby hand andnever calls
find_package_handle_standard_args(). Happy to drop this commit ifyou would rather keep
REQUIREDfor now.Testing
Configured both backends with
-DCMAKE_INSTALL_LIBDIR=lib64:Not addressed
Requires: @pc_req_public@expands empty sincepc_req_publicis never set.Harmless, left alone to keep this minimal.