paleolimbot commented on code in PR #406:
URL: https://github.com/apache/arrow-nanoarrow/pull/406#discussion_r1531340192
##########
CMakeLists.txt:
##########
@@ -154,13 +154,48 @@ else()
endif()
endif()
- install(TARGETS nanoarrow DESTINATION lib)
+ install(TARGETS nanoarrow
+ DESTINATION lib
+ EXPORT nanoarrow-exports)
+ install(TARGETS coverage_config
+ DESTINATION lib
+ EXPORT nanoarrow-exports)
install(DIRECTORY src/
DESTINATION include
FILES_MATCHING
- PATTERN "*.h")
+ PATTERN "*.h*")
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/generated/nanoarrow_config.h
DESTINATION include/nanoarrow)
+
+ # Set up build tree as a CMake package.
+ include(CMakePackageConfigHelpers)
+ write_basic_package_version_file(
+ "${PROJECT_BINARY_DIR}/nanoarrow-config-version.cmake"
+ VERSION ${nanoarrow_VERSION}
+ # Not sure if nanoarrow plans to promise any compatibility.
+ COMPATIBILITY AnyNewerVersion)
+
configure_package_config_file("${CMAKE_CURRENT_LIST_DIR}/cmake/config.cmake.in"
+ "${PROJECT_BINARY_DIR}/nanoarrow-config.cmake"
+ INSTALL_DESTINATION "${PROJECT_BINARY_DIR}")
+ export(EXPORT nanoarrow-exports FILE
"${PROJECT_BINARY_DIR}/nanoarrow-targets.cmake")
+
+ # Set up install tree as a CMake package.
+ include(GNUInstallDirs)
+ set(install_location "${CMAKE_INSTALL_LIBDIR}/cmake/nanoarrow")
+ set(build_location "${CMAKE_BINARY_DIR}/${install_location}")
+ write_basic_package_version_file(
+ "${build_location}/nanoarrow-config-version.cmake"
+ VERSION ${nanoarrow_VERSION}
+ # Not sure if nanoarrow plans to promise any compatibility.
Review Comment:
No plans at the moment (hence the care about namespacing and vendoring),
although I think in a year or so the API will stabilize and we'll consider
1.0.0.
##########
examples/build_tests/src/library.c:
##########
@@ -0,0 +1,18 @@
+// 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
+//
+// http://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.
+
+#include "nanoarrow/nanoarrow.h"
Review Comment:
Should these example files have a `main()` function that prints out
`ArrowNanoarrowVersion()`? I'm not sure what the danger is of missing symbols
is here but it seems like it might be nice to check that at least one makes it
through.
##########
CMakeLists.txt:
##########
@@ -154,13 +154,48 @@ else()
endif()
endif()
- install(TARGETS nanoarrow DESTINATION lib)
+ install(TARGETS nanoarrow
+ DESTINATION lib
+ EXPORT nanoarrow-exports)
+ install(TARGETS coverage_config
+ DESTINATION lib
+ EXPORT nanoarrow-exports)
Review Comment:
Does this need to be exported? It's come up before that the name
`coverage_config` conflicts with another target...is this a good time to prefix
it (`nanoarrow_coverage_config`)?
##########
examples/build_tests/CMakeLists.txt:
##########
@@ -0,0 +1,50 @@
+# 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
+#
+# http://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.
+
+message(STATUS "Building using CMake version: ${CMAKE_VERSION}")
+# Setting minimum to 3.12 for CMP0074
+cmake_minimum_required(VERSION 3.25)
+
+project(NanoArrowExampleCMakeMinimal LANGUAGES C CXX)
+
+option(FIND_NANOARROW "Find an existing nanoarrow" ON)
+
+# When adding nanoarrow's CMake directory to a CMake project that contains a
library
+# intended for use by others, set NANOARROW_NAMESPACE to rename symbols in the
+# nanoarrow library such that they do not collide with other libraries that
may also
+# link to their own copy of nanoarrow. You may wish to include the namespace
only
+# on release builds, since the namespace implementation obscures inline help
+# available in many text editors.
+set(NANOARROW_NAMESPACE "ExampleCmakeMinimal")
+
+if(FIND_NANOARROW)
+ find_package(nanoarrow 0.5.0 REQUIRED)
Review Comment:
Will the version bit have to be updated with every release? Can it be
dropped?
##########
examples/build_tests/build.sh:
##########
Review Comment:
Should running this file be added to `.github/workflows/examples.yaml`?
##########
cmake/config.cmake.in:
##########
@@ -0,0 +1,28 @@
+# 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
+#
+# http://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.
+
+
+@PACKAGE_INIT@
+
+cmake_minimum_required(VERSION @CMAKE_MINIMUM_REQUIRED_VERSION@)
Review Comment:
I wonder if this needs to be configured to a lower value...when I run
`build.sh` I get:
```
-- Building using CMake version: 3.24.1
CMake Error at CMakeLists.txt:20 (cmake_minimum_required):
CMake 3.25 or higher is required. You are running version 3.24.1
```
3.25 seems like a pretty steep version constraint?
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
