Skip to content

Add depended on libraries and C++ runtime to link to in pkg-config file #119

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
robUx4 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add depended on libraries and C++ runtime to link to in pkg-config file #119

Show file tree
Hide file tree
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
22 changes: 22 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,28 @@ else()
endif()

if(NOT BUILD_FRAMEWORK)
foreach(LIB ${CMAKE_CXX_IMPLICIT_LINK_LIBRARIES})
if(IS_ABSOLUTE ${LIB} AND EXISTS ${LIB})
list(APPEND PKG_LIBS "${LIB}")
elseif(LIB MATCHES "-l:lib.*.a")
string(LENGTH ${LIB} LIBLEN)
math(EXPR LIBLEN "${LIBLEN}-8")
string(SUBSTRING ${LIB} 6 ${LIBLEN} DIRECT_LIB)
list(APPEND PKG_LIBS "-l${DIRECT_LIB}")
Comment on lines +187 to +191
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

-l:lib.a pattern handling is correct but lacks defensive check.*

The regex ensures at least one character exists between lib and .a, so DIRECT_LIB won't be empty in practice. However, adding an explicit guard would make the code more robust against unexpected input formats:

elseif(LIB MATCHES "-l:lib.*.a")
  string(LENGTH ${LIB} LIBLEN)
  math(EXPR LIBLEN "${LIBLEN}-8")
  string(SUBSTRING ${LIB} 6 ${LIBLEN} DIRECT_LIB)
  if(DIRECT_LIB)
    list(APPEND PKG_LIBS "-l${DIRECT_LIB}")
  else()
    message(WARNING "Unexpected library format: ${LIB}")
  endif()

This prevents silent failures if the input format changes in future CMake versions.

🤖 Prompt for AI Agents 
In CMakeLists.txt around lines 187-191, the handling for LIB matching
"-l:lib.*.a" extracts DIRECT_LIB but lacks a defensive check; add an explicit
conditional after computing DIRECT_LIB that verifies it is non-empty before
appending to PKG_LIBS and otherwise emit a warning message about the unexpected
library format so malformed inputs do not silently produce incorrect list
entries.

Copy link
Copy Markdown
Contributor Author

@robUx4 robUx4 Oct 23, 2025

Choose a reason for hiding this comment

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

If CMAKE_CXX_IMPLICIT_LINK_LIBRARIES is poisoned we would be screwed no matter what.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 23, 2025

Choose a reason for hiding this comment

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

@robUx4 Fair point—if CMAKE_CXX_IMPLICIT_LINK_LIBRARIES contains garbage, the issue would surface elsewhere in the build. The regex already filters for the expected pattern, so an extra check here would be redundant. I'll drop the suggestion.

elseif(LIB MATCHES "-l.*")
list(APPEND PKG_LIBS "${LIB}")
else()
list(APPEND PKG_LIBS "-l${LIB}")
endif()
endforeach()
if(PKG_LIBS)
# Blacklist for MinGW-w64
list(REMOVE_DUPLICATES PKG_LIBS)
list(REMOVE_ITEM PKG_LIBS
"-lmingw32" "-lgcc_s" "-lgcc" "-lmoldname" "-lmingwex" "-lmingwthrd"
"-lmsvcrt" "-lpthread" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32")
endif()
string(REPLACE ";" " " CHROMAPRINT_ADDITIONAL_LIBS "${PKG_LIBS}")
Comment on lines 183 to +205
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 21, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider initializing CHROMAPRINT_ADDITIONAL_LIBS explicitly.

If PKG_LIBS is empty (line 198 evaluates false), CHROMAPRINT_ADDITIONAL_LIBS is never set. CMake substitutes undefined variables as empty strings in configure_file, so the .pc file gets an empty Libs.private—functionally correct. However, explicit initialization makes the intent clearer:

if(NOT BUILD_FRAMEWORK)
  set(CHROMAPRINT_ADDITIONAL_LIBS "")
  foreach(LIB ${CMAKE_CXX_IMPLICIT_LINK_LIBRARIES})
    # ... existing logic ...

This documents that an empty value is expected when no implicit libs are found.

🤖 Prompt for AI Agents 
In CMakeLists.txt around lines 183 to 205, CHROMAPRINT_ADDITIONAL_LIBS is only
set when PKG_LIBS is non-empty, so it may remain undefined; explicitly
initialize CHROMAPRINT_ADDITIONAL_LIBS to an empty string before the foreach
loop (e.g., set(CHROMAPRINT_ADDITIONAL_LIBS "")) so the variable is always
defined and the intent of an empty value is clear when no implicit libs are
found.

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/libchromaprint.pc.cmake ${CMAKE_CURRENT_BINARY_DIR}/libchromaprint.pc @ONLY)
install(
FILES ${CMAKE_CURRENT_BINARY_DIR}/libchromaprint.pc
Expand Down
1 change: 1 addition & 0 deletions libchromaprint.pc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ Description: Audio fingerprint library
URL: http://acoustid.org/chromaprint
Version: @PROJECT_VERSION@
Libs: -L${libdir} -lchromaprint
Libs.private: @CHROMAPRINT_ADDITIONAL_LIBS@
Cflags: -I${includedir}