Skip to content

Commit

Permalink
apacheGH-41783: [C++] Make git-dependent definitions internal (apache…
Browse files Browse the repository at this point in the history
…#41781)

### Rationale for this change

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()`.

### Are these changes tested?

By existing builds and tests.

### Are there any user-facing changes?

Use cases depending on these preprocessor variables, which is unlikely, may break. They can be fixed by calling `arrow::GetBuildInfo()` instead.

* GitHub Issue: apache#41783

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
  • Loading branch information
pitrou committed May 22, 2024
1 parent 9bd0ddb commit 9185d7d
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ repos:
?^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$|
)
- repo: https://github.com/sphinx-contrib/sphinx-lint
rev: v0.9.1
Expand Down
6 changes: 0 additions & 6 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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")
6 changes: 6 additions & 0 deletions cpp/src/arrow/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cpp/src/arrow/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <cstdint>

#include "arrow/util/config.h"
#include "arrow/util/config_internal.h"
#include "arrow/util/cpu_info.h"
#include "arrow/vendored/datetime.h"

Expand Down
3 changes: 0 additions & 3 deletions cpp/src/arrow/util/config.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions cpp/src/arrow/util/config_internal.h.cmake
Original file line number Diff line number Diff line change
@@ -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@"
2 changes: 1 addition & 1 deletion dev/archery/archery/utils/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9185d7d

Please sign in to comment.