Skip to content
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

AVRO-3088: [C++] Export CMake package config file #3299

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
121 changes: 74 additions & 47 deletions lang/c++/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ string(REPLACE "." ";" AVRO_VERSION ${AVRO_VERSION})
list(GET AVRO_VERSION 0 AVRO_VERSION_MAJOR)
list(GET AVRO_VERSION 1 AVRO_VERSION_MINOR)
list(GET AVRO_VERSION 2 AVRO_VERSION_PATCH)
set(AVRO_VERSION "${AVRO_VERSION_MAJOR}.${AVRO_VERSION_MINOR}.${AVRO_VERSION_PATCH}")

project (Avro-cpp)
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_SOURCE_DIR})
Expand All @@ -69,17 +70,16 @@ endif()

if (CMAKE_COMPILER_IS_GNUCXX)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wuseless-cast -Wconversion -pedantic -Werror")
if (AVRO_ADD_PROTECTOR_FLAGS)
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fstack-protector-all -D_GLIBCXX_DEBUG")
# Unset _GLIBCXX_DEBUG for avrogencpp.cc because using Boost Program Options
# leads to linking errors when compiling with _GLIBCXX_DEBUG as described on
# https://stackoverflow.com/questions/19729036/
set_source_files_properties(impl/avrogencpp.cc PROPERTIES COMPILE_FLAGS "-U_GLIBCXX_DEBUG")
endif ()
endif ()

if (AVRO_BUILD_TESTS OR AVRO_USE_BOOST)
find_package (Boost 1.38 REQUIRED COMPONENTS system)
# Boost 1.70 and above provide a BoostConfig.cmake package configuration file.
find_package (Boost 1.70 REQUIRED CONFIG COMPONENTS system)
if (TARGET Boost::system)
message("Found Boost version: ${Boost_VERSION}")
else ()
message(FATAL_ERROR "Boost::system not found")
endif ()
Copy link
Contributor

Choose a reason for hiding this comment

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

You mark this dependency as required. I think this check if target exists is not needed. CMake will fail if it does not find Boost and all requested components.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just want to make sure that the target with Boost:: namespace exists. Same for Snappy::snappy and ZLIB::ZLIB

Copy link
Contributor

Choose a reason for hiding this comment

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

Snappy and ZLIB are not marked as required.

Copy link
Member Author

Choose a reason for hiding this comment

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

Boost is not required. It is required only when AVRO_BUILD_TESTS or AVRO_USE_BOOST is enabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what @mitjap was trying to say is that this if statement is superfluous as find_package is called with a REQUIRED keyword. It will fail by itself when Boost::system is not found, no need for doing it explicitly

Copy link
Member Author

Choose a reason for hiding this comment

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

This is slightly different. CMIW, sometimes Boost::system target is not available even if Boost_FOUND is true since older Boost distributions may not add Boost:: namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say in that case maybe reflect this in an error message? At this point Boost::system is definitely available. But it would not be available as a target.

Not sure however what would be the best error message in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have changed the message. Please check again.

endif ()

include(FetchContent)
Expand All @@ -92,30 +92,22 @@ FetchContent_Declare(
)
FetchContent_MakeAvailable(fmt)

find_package(Snappy)
if (SNAPPY_FOUND)
set(SNAPPY_PKG libsnappy)
find_package(Snappy CONFIG)
if (TARGET Snappy::snappy)
add_definitions(-DSNAPPY_CODEC_AVAILABLE)
message("Enabled snappy codec")
else (SNAPPY_FOUND)
set(SNAPPY_PKG "")
set(SNAPPY_LIBRARIES "")
set(SNAPPY_INCLUDE_DIR "")
message("Disabled snappy codec. libsnappy not found.")
endif (SNAPPY_FOUND)
message("Enabled snappy codec, version: ${Snappy_VERSION}")
else ()
message("Disabled snappy codec. Snappy::snappy not found.")
endif ()

find_package(ZLIB REQUIRED)
if (ZLIB_FOUND)
message("Enabled zlib codec")
else (ZLIB_FOUND)
message(FATAL_ERROR "ZLIB is not found")
endif (ZLIB_FOUND)

add_definitions (${Boost_LIB_DIAGNOSTIC_DEFINITIONS})

add_definitions (-DAVRO_VERSION="${AVRO_VERSION_MAJOR}.${AVRO_VERSION_MINOR}.${AVRO_VERSION_PATCH}")
if (TARGET ZLIB::ZLIB)
message("Enabled zlib codec, version: ${ZLIB_VERSION}")
else ()
message(FATAL_ERROR "ZLIB::ZLIB not found but required.")
endif ()

include_directories (include/avro ${CMAKE_CURRENT_BINARY_DIR} ${Boost_INCLUDE_DIRS})
include_directories (include/avro ${CMAKE_CURRENT_BINARY_DIR})

set (AVRO_SOURCE_FILES
impl/Compiler.cc impl/Node.cc impl/LogicalType.cc
Expand All @@ -136,25 +128,32 @@ set (AVRO_SOURCE_FILES
)

add_library (avrocpp SHARED ${AVRO_SOURCE_FILES})

set_property (TARGET avrocpp
APPEND PROPERTY COMPILE_DEFINITIONS AVRO_DYN_LINK)

add_library (avrocpp_s STATIC ${AVRO_SOURCE_FILES})
target_include_directories(avrocpp_s PRIVATE ${SNAPPY_INCLUDE_DIR} ${ZLIB_INCLUDE_DIR})
target_link_libraries(avrocpp_s ${Boost_LIBRARIES} ${SNAPPY_LIBRARIES} ${ZLIB_LIBRARIES} fmt::fmt-header-only)

set_property (TARGET avrocpp avrocpp_s
APPEND PROPERTY COMPILE_DEFINITIONS AVRO_SOURCE)
target_compile_definitions(avrocpp PRIVATE AVRO_SOURCE AVRO_DYN_LINK)
target_compile_definitions(avrocpp_s PRIVATE AVRO_SOURCE)

set_target_properties (avrocpp PROPERTIES
VERSION ${AVRO_VERSION_MAJOR}.${AVRO_VERSION_MINOR}.${AVRO_VERSION_PATCH})
set_target_properties (avrocpp PROPERTIES VERSION ${AVRO_VERSION})
set_target_properties (avrocpp_s PROPERTIES VERSION ${AVRO_VERSION})

set_target_properties (avrocpp_s PROPERTIES
VERSION ${AVRO_VERSION_MAJOR}.${AVRO_VERSION_MINOR}.${AVRO_VERSION_PATCH})

target_link_libraries (avrocpp ${Boost_LIBRARIES} ${SNAPPY_LIBRARIES} ${ZLIB_LIBRARIES} fmt::fmt-header-only)
target_include_directories(avrocpp PRIVATE ${SNAPPY_INCLUDE_DIR} ${ZLIB_INCLUDE_DIR})
target_link_libraries(avrocpp PUBLIC
$<BUILD_INTERFACE:fmt::fmt-header-only>
$<BUILD_INTERFACE:ZLIB::ZLIB>
$<BUILD_INTERFACE:$<TARGET_NAME_IF_EXISTS:Snappy::snappy>>
$<BUILD_INTERFACE:$<TARGET_NAME_IF_EXISTS:Boost::system>>
$<INSTALL_INTERFACE:ZLIB::ZLIB>
$<INSTALL_INTERFACE:$<TARGET_NAME_IF_EXISTS:Snappy::snappy>>
$<INSTALL_INTERFACE:$<TARGET_NAME_IF_EXISTS:Boost::system>>
)
target_link_libraries(avrocpp_s PUBLIC
$<BUILD_INTERFACE:fmt::fmt-header-only>
$<BUILD_INTERFACE:ZLIB::ZLIB>
$<BUILD_INTERFACE:$<TARGET_NAME_IF_EXISTS:Snappy::snappy>>
$<BUILD_INTERFACE:$<TARGET_NAME_IF_EXISTS:Boost::system>>
$<INSTALL_INTERFACE:ZLIB::ZLIB>
$<INSTALL_INTERFACE:$<TARGET_NAME_IF_EXISTS:Snappy::snappy>>
$<INSTALL_INTERFACE:$<TARGET_NAME_IF_EXISTS:Boost::system>>
)

target_include_directories(avrocpp PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
Expand Down Expand Up @@ -202,15 +201,16 @@ if (AVRO_BUILD_EXECUTABLES)
gen (union_redundant_types redundant_types)

add_executable (avrogencpp impl/avrogencpp.cc)
target_link_libraries (avrogencpp avrocpp_s ${Boost_LIBRARIES})
target_link_libraries (avrogencpp avrocpp_s)
target_compile_definitions(avrogencpp PRIVATE AVRO_VERSION="${AVRO_VERSION}")
endif ()

if (AVRO_BUILD_TESTS)
enable_testing()

macro (unittest name)
add_executable (${name} test/${name}.cc)
target_link_libraries (${name} avrocpp_s ${Boost_LIBRARIES} ${SNAPPY_LIBRARIES} ${ZLIB_LIBRARIES})
target_link_libraries (${name} avrocpp_s Boost::system ZLIB::ZLIB $<$<TARGET_EXISTS:Snappy::snappy>:Snappy::snappy>)
add_test (NAME ${name} WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
COMMAND ${CMAKE_CURRENT_BINARY_DIR}/${name})
endmacro (unittest)
Expand Down Expand Up @@ -248,12 +248,14 @@ set (CPACK_PACKAGE_FILE_NAME "avrocpp-${AVRO_VERSION_MAJOR}")
include (CPack)

install (TARGETS avrocpp avrocpp_s
EXPORT avrocpp_targets
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib
RUNTIME DESTINATION lib)
RUNTIME DESTINATION lib
INCLUDES DESTINATION include)

if (AVRO_BUILD_EXECUTABLES)
install (TARGETS avrogencpp RUNTIME DESTINATION bin)
install (TARGETS avrogencpp EXPORT avrocpp_targets RUNTIME DESTINATION bin)
endif ()

install (DIRECTORY include/avro DESTINATION include
Expand All @@ -264,3 +266,28 @@ if (NOT CMAKE_BUILD_TYPE)
"Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel."
FORCE)
endif (NOT CMAKE_BUILD_TYPE)

include(CMakePackageConfigHelpers)

write_basic_package_version_file(
"${CMAKE_CURRENT_BINARY_DIR}/AvroConfigVersion.cmake"
VERSION ${AVRO_VERSION}
COMPATIBILITY SameMajorVersion)

configure_package_config_file(
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/AvroConfig.cmake.in"
"${CMAKE_CURRENT_BINARY_DIR}/AvroConfig.cmake"
INSTALL_DESTINATION lib/cmake/Avro
Copy link
Contributor

Choose a reason for hiding this comment

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

I see an inconsistency here. The headers are installed in include/avro while cmake files are in lib/cmake/Avro. Maybe it should be lib/cmake/avro instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a case-sensitive package name and it affects the behavior of calling find_package by downstream.

  • lib/cmake/avro -> find_package(avro)
  • lib/cmake/Avro -> find_package(Avro)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually not sure which one is more preferable here. I'll probably want to hear from the committers to this repo regarding the naming.

Copy link
Member

Choose a reason for hiding this comment

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

In my (poor) experience the convention is to use the one with the capital letter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. It is adopted by many libraries like Boost, Snappy and Arrow.

)

install(EXPORT avrocpp_targets
NAMESPACE Avro::
DESTINATION lib/cmake/Avro
FILE "AvroTargets.cmake"
)

install(FILES
"${CMAKE_CURRENT_BINARY_DIR}/AvroConfig.cmake"
"${CMAKE_CURRENT_BINARY_DIR}/AvroConfigVersion.cmake"
DESTINATION lib/cmake/Avro
)
61 changes: 61 additions & 0 deletions lang/c++/cmake/AvroConfig.cmake.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
# This config sets the following variables in your project::
#
# Avro_FOUND - true if Avro found on the system
# Avro_VERSION - version of the found Avro
#
# This config sets the following targets in your project::
#
# Avro::avrocpp_shared
# Avro::avrocpp_static

@PACKAGE_INIT@

include(CMakeFindDependencyMacro)

if(DEFINED CMAKE_MODULE_PATH)
set(AVRO_CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
else()
unset(AVRO_CMAKE_MODULE_PATH_OLD)
endif()
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")

find_dependency(ZLIB REQUIRED)
find_dependency(fmt REQUIRED)
if(@Snappy_FOUND@)
find_dependency(Snappy REQUIRED)
endif()
if(@Boost_FOUND@)
find_dependency(Boost 1.70 REQUIRED COMPONENTS system)
endif()

if(DEFINED AVRO_CMAKE_MODULE_PATH_OLD)
set(CMAKE_MODULE_PATH ${AVRO_CMAKE_MODULE_PATH_OLD})
unset(AVRO_CMAKE_MODULE_PATH_OLD)
else()
unset(CMAKE_MODULE_PATH)
endif()

include("${CMAKE_CURRENT_LIST_DIR}/AvroTargets.cmake")

add_library(Avro::avrocpp_static ALIAS Avro::avrocpp_s)
add_library(Avro::avrocpp_shared ALIAS Avro::avrocpp)

check_required_components(Avro)
Loading