From 1f2709b8c48e5ab8ffb645d86e7a6fcb4a709cc6 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 22 May 2024 16:49:43 +0200 Subject: [PATCH] [C++] Make git-dependent definitions internal Exposing the ARROW_GIT_ID and ARROW_GIT_DESCRIPTION preprocessor variables in our public headers tends to make incremental builds less efficient, since those values change very often during development. Also, these values don't need to be preprocessor variables since they're just strings: you can't write useful `#if` directives with them. Instead, they can be inspected using `GetBuildInfo()`. --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 6 ------ cpp/src/arrow/CMakeLists.txt | 6 ++++++ cpp/src/arrow/config.cc | 1 + cpp/src/arrow/util/config.h.cmake | 3 --- cpp/src/arrow/util/config_internal.h.cmake | 22 +++++++++++++++++++++ dev/archery/archery/utils/lint.py | 2 +- 6 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 cpp/src/arrow/util/config_internal.h.cmake diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index c24442dcb8749..f102c7bb81683 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -5348,9 +5348,3 @@ if(ARROW_WITH_UCX) endif() message(STATUS "All bundled static libraries: ${ARROW_BUNDLED_STATIC_LIBS}") - -# Write out the package configurations. - -configure_file("src/arrow/util/config.h.cmake" "src/arrow/util/config.h" ESCAPE_QUOTES) -install(FILES "${ARROW_BINARY_DIR}/src/arrow/util/config.h" - DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/arrow/util") diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 57a0b383a677a..150a304975cad 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -351,6 +351,12 @@ macro(append_runtime_avx512_src SRCS SRC) endif() endmacro() +# Write out compile-time configuration constants +configure_file("util/config.h.cmake" "util/config.h" ESCAPE_QUOTES) +configure_file("util/config_internal.h.cmake" "util/config_internal.h" ESCAPE_QUOTES) +install(FILES "${CMAKE_CURRENT_BINARY_DIR}/util/config.h" + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/arrow/util") + set(ARROW_SRCS builder.cc buffer.cc diff --git a/cpp/src/arrow/config.cc b/cpp/src/arrow/config.cc index 9e32e5437325f..a0e3a079b3157 100644 --- a/cpp/src/arrow/config.cc +++ b/cpp/src/arrow/config.cc @@ -20,6 +20,7 @@ #include #include "arrow/util/config.h" +#include "arrow/util/config_internal.h" #include "arrow/util/cpu_info.h" #include "arrow/vendored/datetime.h" diff --git a/cpp/src/arrow/util/config.h.cmake b/cpp/src/arrow/util/config.h.cmake index 9fbd685084fd5..08c2ae173601b 100644 --- a/cpp/src/arrow/util/config.h.cmake +++ b/cpp/src/arrow/util/config.h.cmake @@ -31,9 +31,6 @@ #define ARROW_BUILD_TYPE "@UPPERCASE_BUILD_TYPE@" -#define ARROW_GIT_ID "@ARROW_GIT_ID@" -#define ARROW_GIT_DESCRIPTION "@ARROW_GIT_DESCRIPTION@" - #define ARROW_PACKAGE_KIND "@ARROW_PACKAGE_KIND@" #cmakedefine ARROW_COMPUTE diff --git a/cpp/src/arrow/util/config_internal.h.cmake b/cpp/src/arrow/util/config_internal.h.cmake new file mode 100644 index 0000000000000..e90f7ee12da4d --- /dev/null +++ b/cpp/src/arrow/util/config_internal.h.cmake @@ -0,0 +1,22 @@ +// 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. + +// These variables are not exposed as they can make compilation caching +// and increment builds less efficient. + +#define ARROW_GIT_ID "@ARROW_GIT_ID@" +#define ARROW_GIT_DESCRIPTION "@ARROW_GIT_DESCRIPTION@" diff --git a/dev/archery/archery/utils/lint.py b/dev/archery/archery/utils/lint.py index 92b7f79fc1017..c9d05fffd9168 100644 --- a/dev/archery/archery/utils/lint.py +++ b/dev/archery/archery/utils/lint.py @@ -163,7 +163,7 @@ def cmake_linter(src, fix=False): 'cpp/cmake_modules/FindNumPy.cmake', 'cpp/cmake_modules/FindPythonLibsNew.cmake', 'cpp/cmake_modules/UseCython.cmake', - 'cpp/src/arrow/util/config.h.cmake', + 'cpp/src/arrow/util/*.h.cmake', ] ) method = cmake_format.fix if fix else cmake_format.check