lukester1975 commented on issue #15139: URL: https://github.com/apache/arrow/issues/15139#issuecomment-1384149687
Apologies for the delayed response, fighting a bout of the you-know-what. > > I assume the intention is not to have to do that, though? > > It's expected. I thought that `Requires: libbrotlidec libbrotlienc zlib liblz4 libzstd` will be generated with `PKG_CONFIG_PATH=...`. Hmm, OK. Shame! Should the `pkg-config package for ... for static link isn't found`s become hard errors? It doesn't seem very nice for config / build / install to suceed without `PKG_CONFIG_PATH` but for the generated pc files to be missing dependencies. I suppose the instructions could do with an update to explain the `PKG_CONFIG_PATH` requirement. > > Ah, wait. could you show `Get-ChildItem -Recurse D:/dev/checkouts/external/arrow.git/cpp/vcpkg_installed/x64-windows-static-md/`? Sure, it's a bit of a beast of a listing given the includes, so [zipped](https://github.com/apache/arrow/files/10426460/dir-listing.zip). I wasn't sure from what state you wanted the listing, so it is after a vcpkg install & cmake configure (*without* the `PKG_CONFIG_PATH`). > > Could you try the following? > [snip] > > It will generate `Libs: -L${libdir} -larrow D:/dev/checkouts/external/arrow.git/cpp/vcpkg_installed/x64-windows-static-md/lib/snappy.lib D:/dev/checkouts/external/arrow.git/cpp/vcpkg_installed/x64-windows-static-md/debug/lib/bz2d.lib`. Does this work? Do we need to change `*.lib` to something? Patch as below (needed a tiny tweak) did indeed generate that `Libs` line. I think it's OK, though given the bzip2 pc files from vcpkg are OK it would seem cleaner to be to just `Requires` bzip2, but I guess this is coping with non-vcpkg bzip2 too and you'd rather not special case, or something? (I honestly can't remember when or how I tripped over lib files being linked as plain object files causing problems, so happy to ignore that until it happens again ... sorry for the noise.) Thanks! ``` diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index d0c8c600d..d1323b575 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2548,7 +2548,28 @@ endmacro() if(ARROW_WITH_BZ2) resolve_dependency(BZip2) if(${BZip2_SOURCE} STREQUAL "SYSTEM") - string(APPEND ARROW_PC_LIBS_PRIVATE " ${BZIP2_LIBRARIES}") + set(ARROW_BZIP2_CONFIGS "") + if(BZIP2_LIBRARY_RELEASE) + string(APPEND ARROW_PC_LIBS_PRIVATE " $<$<CONFIG:RELEASE>:${BZIP2_LIBRARY_RELEASE}>") + list(APPEND ARROW_BZIP2_CONFIGS "$<CONFIG:RELEASE>") + endif() + if(BZIP2_LIBRARY_DEBUG) + string(APPEND ARROW_PC_LIBS_PRIVATE " $<$<CONFIG:DEBUG>:${BZIP2_LIBRARY_DEBUG}>") + list(APPEND ARROW_BZIP2_CONFIGS "$<CONFIG:DEBUG>") + endif() + string(APPEND ARROW_PC_LIBS_PRIVATE " $<$<NOT:$<OR:") + if(CMAKE_VERSION VERSION_LESS "3.12") + string(REPLACE ";" "," ARROW_BZIP2_CONFIGS_CSV "${ARROW_BZIP2_CONFIGS}") + else() + list(JOIN ARROW_BZIP2_CONFIGS "," ARROW_BZIP2_CONFIGS_CSV) + endif() + string(APPEND ARROW_PC_LIBS_PRIVATE "${ARROW_BZIP2_CONFIGS_CSV}>>:") + if(BZIP2_LIBRARY) + string(APPEND ARROW_PC_LIBS_PRIVATE "${BZIP2_LIBRARY}") + else() + string(APPEND ARROW_PC_LIBS_PRIVATE "${BZIP2_LIBRARIES}") + endif() + string(APPEND ARROW_PC_LIBS_PRIVATE ">") endif() if(NOT TARGET BZip2::BZip2) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
