Skip to content

Commit

Permalink
apacheGH-44384: [C++] Use CMAKE_LIBTOOL on macOS (apache#44385)
Browse files Browse the repository at this point in the history
When a builder sets `CMAKE_LIBTOOL`, use that as the program to bundle dependencies.  This matches the behavior of the Windows build.

Also make a nitpicky minor update to the error message when a non-Apple libtool is detected.

### Are these changes tested?

I did not add a test case for this build configuration. I confirmed the behavior when `CMAKE_LIBTOOL` is set, using this script on a Mac.  It creates an ad-hoc libtool wrapper and then passes it as `CMAKE_LIBTOOL`.

```
#!/bin/bash

mylibtool=$(mktemp -d)/libtool.sh
cat << 'EOF' > "$mylibtool"
#!/bin/bash
say -v Tom -- "voice-enhanced libtool:" "$@"
/usr/bin/libtool "$@"
EOF
chmod +x "$mylibtool"

cmake -GNinja -DARROW_DEPENDENCY_SOURCE=BUNDLED -DCMAKE_LIBTOOL="$mylibtool" -S cpp -B build/
cmake --build build/
```

I am not sure how to best add a test for this to the repository.

### Are there any user-facing changes?

This changes how bundled builds behave when an Apple build-user sets `CMAKE_LIBTOOL` in their CMake invocation. I think the change is a sensible one and wouldn't be surprising to users, but it may break existing builds in environments where /usr/bin/libtool is Apple's libtool, and `CMAKE_LIBTOOL` is set to a non-Apple one.
* GitHub Issue: apache#44384

Authored-by: Tom Jakubowski <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
tomjakubowski authored Oct 16, 2024
1 parent da5a295 commit 25a2440
Showing 1 changed file with 17 additions and 13 deletions.
30 changes: 17 additions & 13 deletions cpp/cmake_modules/BuildUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -97,23 +97,27 @@ function(arrow_create_merged_static_lib output_target)
endforeach()

if(APPLE)
# The apple-distributed libtool is what we want for bundling, but there is
# a GNU libtool that has a namecollision (and happens to be bundled with R, too).
# We are not compatible with GNU libtool, so we need to avoid it.

# check in the obvious places first to find Apple's libtool
# HINTS is used before system paths and before PATHS, so we use that
# even though hard coded paths should go in PATHS
# TODO: use a VALIDATOR when we require cmake >= 3.25
find_program(LIBTOOL_MACOS libtool HINTS /usr/bin
/Library/Developer/CommandLineTools/usr/bin)

# confirm that the libtool we found is not GNU libtool
if(CMAKE_LIBTOOL)
set(LIBTOOL_MACOS ${CMAKE_LIBTOOL})
else()
# The apple-distributed libtool is what we want for bundling, but there is
# a GNU libtool that has a namecollision (and happens to be bundled with R, too).
# We are not compatible with GNU libtool, so we need to avoid it.

# check in the obvious places first to find Apple's libtool
# HINTS is used before system paths and before PATHS, so we use that
# even though hard coded paths should go in PATHS
# TODO: use a VALIDATOR when we require cmake >= 3.25
find_program(LIBTOOL_MACOS libtool
HINTS /usr/bin /Library/Developer/CommandLineTools/usr/bin)
endif()

# confirm that the libtool we found is Apple's libtool
execute_process(COMMAND ${LIBTOOL_MACOS} -V
OUTPUT_VARIABLE LIBTOOL_V_OUTPUT
OUTPUT_STRIP_TRAILING_WHITESPACE)
if(NOT "${LIBTOOL_V_OUTPUT}" MATCHES ".*cctools-([0-9.]+).*")
message(FATAL_ERROR "libtool found appears to be the incompatible GNU libtool: ${LIBTOOL_MACOS}"
message(FATAL_ERROR "libtool found appears not to be Apple's libtool: ${LIBTOOL_MACOS}"
)
endif()

Expand Down

0 comments on commit 25a2440

Please sign in to comment.