Patch auto-downloaded arrow for C++ for latest macos libtool#12749
Patch auto-downloaded arrow for C++ for latest macos libtool#12749nikolaseu wants to merge 1 commit into
Conversation
Wumpf
left a comment
Wumpf
left a comment
There was a problem hiding this comment.
Thanks for looking into this and upstreaming your solution. See no reason not to take that in, other than needing to document it a bit better so we remove the patch!
That said, the version we pinned on arrow here is getting quite old, we should soon look into picking up a newer one. Afaik they switched to a newer C++ version and I'd like us to be C++17 compatible for a bit longer, so options might be a bit limited. (either way that's for a different iteration ofc)
| # Apply patch after checkout but before configure | ||
| # TODO(apache/arrow#45985): Arrow can't support CMake 4.0 yet |
There was a problem hiding this comment.
docs need to move to the actual patch and should also document the new patch (why it's needed, what version would make it go away etc.)
|
Hi Andreas, thanks for taking a look. I will check and continue with this again later. I marked it as draft because I can't really get it working locally, it seems like the patch was not being applied, but it didn't fail either. I'm not sure what was going on. |
|
I suspect copying lines 94-146 from latest BuildUtils.cmake to replace lines 100-121 would be an effective fix but I don't have a machine to test that on. |
Sorry, I was not clear. The patch works when applied manually, but something is maybe wrong in the setup or my changes in There's no need to patch a lot more than this in the currently used version of arrow, even if latest arrow changed a lot more. |
nikolaseu
force-pushed
the
chore-arrow-macos-latest-libtool
branch
from
7a3dfff to
6245414
Compare
|
@Wumpf the issue was that the patches were not being applied, they were just skipped by git. The previous setup was not working, so I guess the mimalloc issue was not an issue anymore. |
Wumpf
changed the title
Wumpf
added
📺 re_viewer
| set(ARROW_PATCHES | ||
| # TODO(apache/arrow#45985): Arrow can't support CMake 4.0 yet | ||
| ${CMAKE_CURRENT_LIST_DIR}/patches/mimalloc_cmake4.patch | ||
| # TODO update to arrow 24 and remove this patch |
There was a problem hiding this comment.
our CI flags TODO notes without ticket or name references.
Also, do you know whether this is fixed in earlier arrow versions as well? I checked and it looks like arrow 23 starts requiring C++20, so anything prior to that would be easier to upgrade for us.
| # Since this is downloading a release tarball, we need to init a git repository in order to apply the patches. | ||
| PATCH_COMMAND git init && git apply --check ${ARROW_PATCHES} && git apply ${ARROW_PATCHES} || true |
There was a problem hiding this comment.
afaik that's not entirely true, but the problem is rather that we're more likely than not already in an existing git repo
Digging a bit: at the time of the introducing PR #11223 it still did a full git clone, but at the same time I can build with cmake4 just fine off main for me :/. probably still best to leave the patch for now, cc @ntjohnson1
3 participants
Related
Closes #12748
What
Latest macOS's libtool returns the version with a slightly different format so the regex used by arrow fails to recognize it, and aborts as if GNU's libtool is being used (which is not supported).
See apache/arrow#49370