GH-50753: [C++] Co-locate static install interface libraries with dependencies - #50764
Closed
KHARSHAVARDHAN-eng wants to merge 1 commit into
Closed
Conversation
…th dependencies * Extend resolve_dependency() in ThirdpartyToolchain.cmake to accept STATIC_INSTALL_INTERFACE_LIBS. * Declare static installation interface targets at dependency resolution site. * Remove redundant manual ARROW_STATIC_INSTALL_INTERFACE_LIBS declarations in cpp/src/arrow/CMakeLists.txt and cpp/src/arrow/flight/CMakeLists.txt.
|
|
Collaborator
|
@KHARSHAVARDHAN-eng, please stop opening new PRs until you've addressed the feedback on your existing ones. Several maintainers have already spent time reviewing your previous PRs, but you haven't responded to that feedback. Continuously opening new PRs also prevents other contributors from working on these issues while they remain effectively unaddressed. |
Member
|
I close this for now because there are conflicts. Let's revisit this after you completed other PRs. |
Member
|
Thanks @kou and @Reranko05 |
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
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.
Rationale for this change
In Apache Arrow's CMake build system, third-party dependency resolution and static library interface target registration were decoupled across two separate locations:
resolve_dependency()incpp/cmake_modules/ThirdpartyToolchain.cmakeARROW_STATIC_INSTALL_INTERFACE_LIBSchecks incpp/src/arrow/CMakeLists.txtBecause these declarations were separate, developers adding or updating a dependency in
ThirdpartyToolchain.cmakefrequently forgot to updateARROW_STATIC_INSTALL_INTERFACE_LIBSincpp/src/arrow/CMakeLists.txt(e.g.simdjsonin GH-50716 / GH-50566). As a result, static builds (-DARROW_DEPENDENCY_SOURCE=SYSTEM) generated exported target metadata (ArrowTargets.cmake) missing required transitive interface link libraries.What changes are included in this PR?
resolve_dependency()macro incpp/cmake_modules/ThirdpartyToolchain.cmaketo accept an optionalSTATIC_INSTALL_INTERFACE_LIBSargument. When a dependency is resolved asSYSTEM, its static interface targets are automatically appended toARROW_STATIC_INSTALL_INTERFACE_LIBS(orARROW_FLIGHT_STATIC_INSTALL_INTERFACE_LIBS).ThirdpartyToolchain.cmakeforsimdjson,Snappy,Brotli,OpenSSL,glog,Protobuf,ZLIB,lz4,zstd,re2,BZip2,utf8proc,opentelemetry-cpp,google_cloud_cpp_storage,orc, andAWSSDK.ARROW_STATIC_INSTALL_INTERFACE_LIBSchecks fromcpp/src/arrow/CMakeLists.txtand updatedcpp/src/arrow/flight/CMakeLists.txt.Are these changes tested?
Verified via CMake configurations and generated export metadata:
SYSTEMdependencies and verified thatArrowTargets.cmakecontainssimdjson::simdjson,Snappy::snappy,ZLIB::ZLIB,LZ4::lz4,BZip2::BZip2, etc. inINTERFACE_LINK_LIBRARIESforArrow::arrow_static.-DARROW_DEPENDENCY_SOURCE=BUNDLED) and confirmed system targets do not leak into exported static interface libraries.Closes #50753