Adopt change from apache/avro#3299

wgtmac · Jan 23, 2025 · 923bf15 · 923bf15
1 parent 9358d17
commit 923bf15
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 39 deletions.
diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake
@@ -147,8 +147,11 @@ function(resolve_avro_dependency)
 
   fetchcontent_declare(Avro
                        ${FC_DECLARE_COMMON_OPTIONS}
-                       GIT_REPOSITORY https://github.com/apache/avro.git
-                       GIT_TAG 1144cb7322bab4cd1c8bf330a9c504a0d4252b56
+                       # TODO: switch to upstream once the PR below is merged
+                       # https://github.com/apache/avro/pull/3299
+                       # Eventually, we should switch to Apache Avro 1.3.0.
+                       GIT_REPOSITORY https://github.com/wgtmac/avro.git
+                       GIT_TAG 0aa7adf87a9af6d472a3e9d5966c5e7f1d6baa7d
                        SOURCE_SUBDIR
                        lang/c++
                        FIND_PACKAGE_ARGS
@@ -159,33 +162,34 @@ function(resolve_avro_dependency)
   fetchcontent_makeavailable(Avro)
 
   if(avro_SOURCE_DIR)
-    if(NOT TARGET Avro::avro_static)
-      add_library(Avro::avro_static INTERFACE IMPORTED)
-      target_link_libraries(Avro::avro_static INTERFACE avrocpp_s)
-      set_target_properties(avrocpp_s PROPERTIES OUTPUT_NAME "iceberg_vendored_avro")
+    if(NOT TARGET Avro::avrocpp_static)
+      add_library(Avro::avrocpp_static INTERFACE IMPORTED)
+      target_link_libraries(Avro::avrocpp_static INTERFACE avrocpp_s)
+      target_include_directories(Avro::avrocpp_static
+                                 INTERFACE ${avro_BINARY_DIR} ${avro_SOURCE_DIR}/lang/c++)
     endif()
 
-    if(NOT TARGET Avro::avro_shared)
-      add_library(Avro::avro_shared INTERFACE IMPORTED)
-      target_link_libraries(Avro::avro_shared INTERFACE avrocpp)
-      set_target_properties(avrocpp PROPERTIES OUTPUT_NAME "iceberg_vendored_avro")
-    endif()
+    set(AVRO_VENDORED TRUE)
+    set_target_properties(avrocpp_s PROPERTIES OUTPUT_NAME "iceberg_vendored_avrocpp")
+    install(TARGETS avrocpp_s
+            EXPORT iceberg_targets
+            RUNTIME DESTINATION "${ICEBERG_INSTALL_BINDIR}"
+            ARCHIVE DESTINATION "${ICEBERG_INSTALL_LIBDIR}"
+            LIBRARY DESTINATION "${ICEBERG_INSTALL_LIBDIR}")
 
-    if(ICEBERG_BUILD_STATIC)
-      install(TARGETS avrocpp_s
-              EXPORT fmt-targets
-              RUNTIME DESTINATION "${ICEBERG_INSTALL_BINDIR}"
-              ARCHIVE DESTINATION "${ICEBERG_INSTALL_LIBDIR}"
-              LIBRARY DESTINATION "${ICEBERG_INSTALL_LIBDIR}")
-    endif()
-    if(ICEBERG_BUILD_SHARED)
-      install(TARGETS avrocpp
-              EXPORT fmt-targets
-              RUNTIME DESTINATION "${ICEBERG_INSTALL_BINDIR}"
-              ARCHIVE DESTINATION "${ICEBERG_INSTALL_LIBDIR}"
-              LIBRARY DESTINATION "${ICEBERG_INSTALL_LIBDIR}")
-    endif()
+    # TODO: add vendored ZLIB and Snappy support
+    list(APPEND ICEBERG_SYSTEM_DEPENDENCIES ZLIB Snappy)
+  else()
+    set(AVRO_VENDORED FALSE)
+    list(APPEND ICEBERG_SYSTEM_DEPENDENCIES Avro)
   endif()
+
+  set(ICEBERG_SYSTEM_DEPENDENCIES
+      ${ICEBERG_SYSTEM_DEPENDENCIES}
+      PARENT_SCOPE)
+  set(AVRO_VENDORED
+      ${AVRO_VENDORED}
+      PARENT_SCOPE)
 endfunction()
 
 if(ICEBERG_AVRO)

diff --git a/example/CMakeLists.txt b/example/CMakeLists.txt
@@ -26,5 +26,6 @@ find_package(Iceberg CONFIG REQUIRED)
 
 add_executable(demo_example demo_example.cc)
 
-target_link_libraries(demo_example PRIVATE Iceberg::iceberg_puffin_static
-                                           Iceberg::iceberg_arrow_static)
+target_link_libraries(demo_example
+                      PRIVATE Iceberg::iceberg_puffin_static
+                              Iceberg::iceberg_arrow_static Iceberg::iceberg_avro_static)
diff --git a/example/demo_example.cc b/example/demo_example.cc
@@ -20,12 +20,14 @@
 #include <iostream>
 
 #include "iceberg/arrow/demo_arrow.h"
+#include "iceberg/avro/demo_avro.h"
 #include "iceberg/demo_table.h"
 #include "iceberg/puffin/demo_puffin.h"
 
 int main() {
   std::cout << iceberg::DemoTable().print() << std::endl;
   std::cout << iceberg::puffin::DemoPuffin().print() << std::endl;
   std::cout << iceberg::arrow::DemoArrow().print() << std::endl;
+  std::cout << iceberg::avro::DemoAvro().print() << std::endl;
   return 0;
 }
diff --git a/src/iceberg/avro/CMakeLists.txt b/src/iceberg/avro/CMakeLists.txt
@@ -28,20 +28,39 @@ set(ICEBERG_AVRO_SHARED_BUILD_INTERFACE_LIBS)
 set(ICEBERG_AVRO_STATIC_INSTALL_INTERFACE_LIBS)
 set(ICEBERG_AVRO_SHARED_INSTALL_INTERFACE_LIBS)
 
-list(APPEND ICEBERG_AVRO_STATIC_BUILD_INTERFACE_LIBS
+list(APPEND
+     ICEBERG_AVRO_STATIC_BUILD_INTERFACE_LIBS
      "$<IF:$<TARGET_EXISTS:iceberg_static>,iceberg_static,iceberg_shared>"
-     Avro::avro_static)
-list(APPEND ICEBERG_AVRO_SHARED_BUILD_INTERFACE_LIBS
+     "$<IF:$<TARGET_EXISTS:Avro::avrocpp_static>,Avro::avrocpp_static,Avro::avrocpp_shared>"
+)
+list(APPEND
+     ICEBERG_AVRO_SHARED_BUILD_INTERFACE_LIBS
      "$<IF:$<TARGET_EXISTS:iceberg_shared>,iceberg_shared,iceberg_static>"
-     Avro::avro_shared)
+     "$<IF:$<TARGET_EXISTS:Avro::avrocpp_shared>,Avro::avrocpp_shared,Avro::avrocpp_static>"
+)
+
+if(ARROW_VENDORED)
+  list(APPEND ICEBERG_AVRO_STATIC_INSTALL_INTERFACE_LIBS Iceberg::avrocpp_s)
+  list(APPEND ICEBERG_AVRO_SHARED_INSTALL_INTERFACE_LIBS Iceberg::avrocpp_s)
+else()
+  list(APPEND
+       ICEBERG_AVRO_STATIC_INSTALL_INTERFACE_LIBS
+       "$<IF:$<TARGET_EXISTS:Avro::avrocpp_static>,Avro::avrocpp_static,Avro::avrocpp_shared>"
+  )
+  list(APPEND
+       ICEBERG_AVRO_SHARED_INSTALL_INTERFACE_LIBS
+       "$<IF:$<TARGET_EXISTS:Avro::avrocpp_shared>,Avro::avrocpp_shared,Avro::avrocpp_static>"
+  )
+endif()
+
 list(APPEND
      ICEBERG_AVRO_STATIC_INSTALL_INTERFACE_LIBS
      "$<IF:$<TARGET_EXISTS:Iceberg::iceberg_static>,Iceberg::iceberg_static,Iceberg::iceberg_shared>"
-     Iceberg::avro_static)
+)
 list(APPEND
      ICEBERG_AVRO_SHARED_INSTALL_INTERFACE_LIBS
      "$<IF:$<TARGET_EXISTS:Iceberg::iceberg_shared>,Iceberg::iceberg_shared,Iceberg::iceberg_static>"
-     Iceberg::avro_shared)
+)
 
 add_iceberg_lib(iceberg_avro
                 SOURCES

diff --git a/src/iceberg/avro/demo_avro.cc b/src/iceberg/avro/demo_avro.cc
@@ -25,7 +25,7 @@
 #include "avro/ValidSchema.hh"
 #include "iceberg/demo_table.h"
 
-namespace iceberg {
+namespace iceberg::avro {
 
 std::string DemoAvro::print() const {
   std::string input =
@@ -42,11 +42,11 @@ std::string DemoAvro::print() const {
 }\n\
 ";
 
-  avro::ValidSchema schema = avro::compileJsonSchemaFromString(input);
+  ::avro::ValidSchema schema = ::avro::compileJsonSchemaFromString(input);
   std::ostringstream actual;
   schema.toJson(actual);
 
   return actual.str();
 }
 
-}  // namespace iceberg
+}  // namespace iceberg::avro
diff --git a/src/iceberg/avro/demo_avro.h b/src/iceberg/avro/demo_avro.h
@@ -24,7 +24,7 @@
 #include "iceberg/avro.h"
 #include "iceberg/avro/iceberg_avro_export.h"
 
-namespace iceberg {
+namespace iceberg::avro {
 
 class ICEBERG_AVRO_EXPORT DemoAvro : public Avro {
  public:
@@ -33,4 +33,4 @@ class ICEBERG_AVRO_EXPORT DemoAvro : public Avro {
   std::string print() const override;
 };
 
-}  // namespace iceberg
+}  // namespace iceberg::avro
diff --git a/src/iceberg/demo_table.cc b/src/iceberg/demo_table.cc
@@ -19,6 +19,7 @@
 
 #include "iceberg/demo_table.h"
 
+#include "iceberg/avro.h"  // include to export symbols
 #include "iceberg/puffin.h"
 
 namespace iceberg {

diff --git a/test/core/avro_unittest.cc b/test/core/avro_unittest.cc
@@ -35,6 +35,6 @@ TEST(AVROTest, TestDemoAvro) {
 }\n\
 ";
 
-  auto avro = iceberg::DemoAvro();
+  auto avro = iceberg::avro::DemoAvro();
   EXPECT_EQ(avro.print(), expected);
 }