GH-40248: [R] fallback to the correct libtool when we find a GNU one #40259

jonkeane · 2024-02-27T06:03:54Z

Rationale for this change

On the CRAN build machines the GNU libtool is on the path in front of the macOS libtool. Though these are named the same thing, they are actually very different and don't actually appear to be substitutes

I checked on a non-developer's machine to see if /usr/bin/libtool exists, and it did. So I believe we should be ok with this even if xcode / command line tools haven't been installed.

One note: it's possible that we could get the GNU libtool in link mode to work with the right incantation (something like libtool --mode=link --tag=CXX ${cmake_compiler} -o ... but when I tried this I kept getting symbol not found errors. Ultimately, I think any mac that we are on will have the apple-provided libtool, so decided to go the route of finding it.

What changes are included in this PR?

When we detect we are on a GNU libtool, we look to /usr/bin/libtool instead.

Are these changes tested?

Yes. See a broken config failing at #40259 (comment) and then the next one passes

Are there any user-facing changes?

We will remain on CRAN

This PR contains a "Critical Fix".

GitHub Issue: [R] Support gnu libtool? #40248

cpp/cmake_modules/BuildUtils.cmake

dev/tasks/r/github.macos-linux.local.yml

jonkeane · 2024-02-27T06:11:09Z

@github-actions crossbow submit test-r-install-local

github-actions · 2024-02-27T06:13:20Z

Revision: 77188a0

Submitted crossbow builds: ursacomputing/crossbow @ actions-c1bc1ae976

Task	Status
test-r-install-local

jonkeane · 2024-02-27T22:24:51Z

@github-actions crossbow submit test-r-install-local

This should fail because we find the GNU libtool in the R.Framework dir first

github-actions · 2024-02-27T22:27:12Z

Revision: 19461ca

Submitted crossbow builds: ursacomputing/crossbow @ actions-89df1d84e3

Task	Status
test-r-install-local

jonkeane · 2024-02-27T22:28:57Z

@github-actions crossbow submit test-r-install-local

And now this should work because we find the apple-provided one first

github-actions · 2024-02-27T22:31:15Z

Revision: a62b889

Submitted crossbow builds: ursacomputing/crossbow @ actions-2975d7310c

Task	Status
test-r-install-local

jonkeane · 2024-02-27T23:03:40Z

cpp/cmake_modules/BuildUtils.cmake

+    # if it's not found in obvious places, check on the standard path
+    if(LIBTOOL_MACOS-NOTFOUND)
+      set(LIBTOOL_MACOS "libtool")
+    endif()


I believe this could technically be find_program(LIBTOOL_MACOS libtool) and that would find libtool along the search path, but be a no-op if line 106 above it found libtool.

Does that look better? Or just an obfuscated version of this?

jonkeane · 2024-02-27T23:04:59Z

cpp/cmake_modules/BuildUtils.cmake

+    if(NOT "${LIBTOOL_V_OUTPUT}" MATCHES ".*cctools-([0-9.]+).*")
+      message(FATAL_ERROR "libtool found appears to be the incompatible GNU libtool")
+    endif()


We might consider raising this as a warning and proceeding anyway. If it is GNU libtool, it fails pretty clearly next. That would insulate us incase Apple's libtool has a slightly different version string or something (although with a spurious error). I'm split on if this is better or not.

jonkeane · 2024-02-27T23:35:30Z

This is ready for review. The CI failure (C++ / AMD64 macOS 12 C++) is the same on main so I believe unrelated to this change.

CC @kou

kou

+1

dev/tasks/r/github.macos-linux.local.yml

kou · 2024-02-27T23:38:36Z

cpp/cmake_modules/BuildUtils.cmake

+                 PATHS /usr/bin /Library/Developer/CommandLineTools/usr/bin
+                 NO_DEFAULT_PATH)
+
+    # if it's not found in obvious places, check on the standard path
+    if(LIBTOOL_MACOS-NOTFOUND)
+      set(LIBTOOL_MACOS "libtool")
+    endif()


How about just using default paths? $PATH is used after PATHS.

Suggested change

PATHS /usr/bin /Library/Developer/CommandLineTools/usr/bin

NO_DEFAULT_PATH)

# if it's not found in obvious places, check on the standard path

if(LIBTOOL_MACOS-NOTFOUND)

set(LIBTOOL_MACOS "libtool")

endif()

PATHS /usr/bin /Library/Developer/CommandLineTools/usr/bin)

https://cmake.org/cmake/help/latest/command/find_program.html

Search the paths specified by the HINTS option. These should be paths computed by system introspection, such as a hint provided by the location of another item already found. Hard-coded guesses should be specified with the PATHS option.

Search the standard system environment variables. This can be skipped if NO_SYSTEM_ENVIRONMENT_PATH is passed or by setting the CMAKE_FIND_USE_SYSTEM_ENVIRONMENT_PATH to FALSE.

The directories in PATH itself.

I can try this. I will admit I read that list in the find docs a few times and was not 100% certain it would follow the path I was hoping it would (use our provided paths first, then project paths, then system paths).

Ok, this works though I had to switch from PATHS to HINTS since HINTS is higher in the search. This is a bit funny given the docs:

Search the paths specified by the HINTS option. These should be paths computed by system introspection, such as a hint provided by the location of another item already found. Hard-coded guesses should be specified with the PATHS option.

But it does seem to work!

cpp/cmake_modules/BuildUtils.cmake

dev/tasks/r/github.macos-linux.local.yml

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

jonkeane · 2024-02-28T01:38:06Z

@github-actions crossbow submit test-r-install-local

github-actions · 2024-02-28T01:40:20Z

Revision: e1cac51

Submitted crossbow builds: ursacomputing/crossbow @ actions-3f3456c02e

Task	Status
test-r-install-local

jonkeane · 2024-02-28T03:09:16Z

@github-actions crossbow submit test-r-install-local

github-actions · 2024-02-28T03:11:39Z

Revision: ee8e590

Submitted crossbow builds: ursacomputing/crossbow @ actions-e25ea07ad4

Task	Status
test-r-install-local

kou · 2024-02-28T13:02:29Z

Could you run archery lint --cmake-format --fix before we merge this?

jonkeane · 2024-02-28T14:15:53Z

Could you run archery lint --cmake-format --fix before we merge this?

Yup, I should have done this before sending that last commit, sorry!

conbench-apache-arrow · 2024-02-29T08:06:56Z

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 2fbf22a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.

…40259) ### Rationale for this change On the CRAN build machines the GNU libtool is on the path in front of the macOS libtool. Though these are named the same thing, they are actually very different and don't actually appear to be substitutes I checked on a non-developer's machine to see if `/usr/bin/libtool` exists, and it did. So I believe we _should_ be ok with this even if xcode / command line tools haven't been installed. One note: it's possible that we could get the GNU libtool in link mode to work with the right incantation (something like `libtool --mode=link --tag=CXX ${cmake_compiler} -o ...` but when I tried this I kept getting symbol not found errors. Ultimately, I think any mac that we are on will have the apple-provided libtool, so decided to go the route of finding it. ### What changes are included in this PR? When we detect we are on a GNU libtool, we look to `/usr/bin/libtool` instead. ### Are these changes tested? Yes. See a broken config failing at #40259 (comment) and then the next one passes ### Are there any user-facing changes? We will remain on CRAN **This PR contains a "Critical Fix".** * GitHub Issue: #40248 Lead-authored-by: Jonathan Keane <jkeane@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Jonathan Keane <jkeane@gmail.com>

…U one (apache#40259) ### Rationale for this change On the CRAN build machines the GNU libtool is on the path in front of the macOS libtool. Though these are named the same thing, they are actually very different and don't actually appear to be substitutes I checked on a non-developer's machine to see if `/usr/bin/libtool` exists, and it did. So I believe we _should_ be ok with this even if xcode / command line tools haven't been installed. One note: it's possible that we could get the GNU libtool in link mode to work with the right incantation (something like `libtool --mode=link --tag=CXX ${cmake_compiler} -o ...` but when I tried this I kept getting symbol not found errors. Ultimately, I think any mac that we are on will have the apple-provided libtool, so decided to go the route of finding it. ### What changes are included in this PR? When we detect we are on a GNU libtool, we look to `/usr/bin/libtool` instead. ### Are these changes tested? Yes. See a broken config failing at apache#40259 (comment) and then the next one passes ### Are there any user-facing changes? We will remain on CRAN **This PR contains a "Critical Fix".** * GitHub Issue: apache#40248 Lead-authored-by: Jonathan Keane <jkeane@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Jonathan Keane <jkeane@gmail.com>

…40259) ### Rationale for this change On the CRAN build machines the GNU libtool is on the path in front of the macOS libtool. Though these are named the same thing, they are actually very different and don't actually appear to be substitutes I checked on a non-developer's machine to see if `/usr/bin/libtool` exists, and it did. So I believe we _should_ be ok with this even if xcode / command line tools haven't been installed. One note: it's possible that we could get the GNU libtool in link mode to work with the right incantation (something like `libtool --mode=link --tag=CXX ${cmake_compiler} -o ...` but when I tried this I kept getting symbol not found errors. Ultimately, I think any mac that we are on will have the apple-provided libtool, so decided to go the route of finding it. ### What changes are included in this PR? When we detect we are on a GNU libtool, we look to `/usr/bin/libtool` instead. ### Are these changes tested? Yes. See a broken config failing at #40259 (comment) and then the next one passes ### Are there any user-facing changes? We will remain on CRAN **This PR contains a "Critical Fix".** * GitHub Issue: #40248 Lead-authored-by: Jonathan Keane <jkeane@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Jonathan Keane <jkeane@gmail.com>

jonkeane added 2 commits February 26, 2024 23:59

brew install libtool

50a64a7

remove an errant echo

77188a0

jonkeane requested review from assignUser, kou and raulcd as code owners February 27, 2024 06:03

github-actions bot added Component: C++ awaiting committer review Awaiting committer review labels Feb 27, 2024

jonkeane commented Feb 27, 2024

View reviewed changes

cpp/cmake_modules/BuildUtils.cmake Outdated Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Feb 27, 2024

jonkeane commented Feb 27, 2024

View reviewed changes

dev/tasks/r/github.macos-linux.local.yml Outdated Show resolved Hide resolved

Clean up / use find_program

87fa3b7

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Feb 27, 2024

Push a known-bad setup to fail

19461ca

Add the search path

a62b889

jonkeane commented Feb 27, 2024

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Feb 27, 2024

jonkeane commented Feb 27, 2024

View reviewed changes

kou approved these changes Feb 27, 2024

View reviewed changes

github-actions bot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels Feb 27, 2024

jonkeane and others added 2 commits February 27, 2024 17:47

Update cpp/cmake_modules/BuildUtils.cmake

bbd9959

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Update dev/tasks/r/github.macos-linux.local.yml

150deb0

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

github-actions bot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Feb 27, 2024

kou mentioned this pull request Feb 28, 2024

[R] Support gnu libtool? #40248

Closed

Use only find_program + hints

e1cac51

github-actions bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Feb 28, 2024

Only muck with the path on macOS

ee8e590

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Feb 28, 2024

jonkeane added 2 commits February 28, 2024 07:56

lint

a98893a

Merge branch 'gnulibtool' of github.com:jonkeane/arrow into gnulibtool

97bd2aa

jonkeane merged commit 2fbf22a into apache:main Feb 28, 2024
29 of 31 checks passed

jonkeane removed the awaiting change review Awaiting change review label Feb 28, 2024

thisisnic mentioned this pull request Mar 1, 2024

[R] CRAN packaging checklist for version 15.0.0 #40310

Closed

38 tasks

thisisnic mentioned this pull request Mar 6, 2024

[R] CRAN packaging checklist for version 15.0.1 #40388

Open

38 tasks

jonkeane mentioned this pull request Apr 26, 2024

[R] CRAN packaging checklist for version 16.0.0 #41404

Closed

38 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-40248: [R] fallback to the correct libtool when we find a GNU one #40259

GH-40248: [R] fallback to the correct libtool when we find a GNU one #40259

jonkeane commented Feb 27, 2024 •

edited

Loading

jonkeane commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

jonkeane commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

jonkeane commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

jonkeane Feb 27, 2024

jonkeane Feb 27, 2024

jonkeane commented Feb 27, 2024

kou left a comment

kou Feb 27, 2024

jonkeane Feb 27, 2024

jonkeane Feb 28, 2024

jonkeane commented Feb 28, 2024

github-actions bot commented Feb 28, 2024

jonkeane commented Feb 28, 2024

github-actions bot commented Feb 28, 2024

kou commented Feb 28, 2024

jonkeane commented Feb 28, 2024

conbench-apache-arrow bot commented Feb 29, 2024

GH-40248: [R] fallback to the correct libtool when we find a GNU one #40259

GH-40248: [R] fallback to the correct libtool when we find a GNU one #40259

Conversation

jonkeane commented Feb 27, 2024 • edited Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

jonkeane commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

jonkeane commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

jonkeane commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

jonkeane Feb 27, 2024

Choose a reason for hiding this comment

jonkeane Feb 27, 2024

Choose a reason for hiding this comment

jonkeane commented Feb 27, 2024

kou left a comment

Choose a reason for hiding this comment

kou Feb 27, 2024

Choose a reason for hiding this comment

jonkeane Feb 27, 2024

Choose a reason for hiding this comment

jonkeane Feb 28, 2024

Choose a reason for hiding this comment

jonkeane commented Feb 28, 2024

github-actions bot commented Feb 28, 2024

jonkeane commented Feb 28, 2024

github-actions bot commented Feb 28, 2024

kou commented Feb 28, 2024

jonkeane commented Feb 28, 2024

conbench-apache-arrow bot commented Feb 29, 2024

jonkeane commented Feb 27, 2024 •

edited

Loading