Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINOR: [C++] Bump bundled mimalloc version #44941

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Dec 4, 2024

No description provided.

@pitrou
Copy link
Member Author

pitrou commented Dec 4, 2024

Ok, we'll first have to bump our minimum CMake version because mimalloc now requires CMake 3.18:

-- stderr output is:
CMake Error at CMakeLists.txt:1 (cmake_minimum_required):
  CMake 3.18 or higher is required.  You are running version 3.16.3

@kou
Copy link
Member

kou commented Dec 5, 2024

I don't object bumping our minimum CMake version but we can require CMake 3.18 or later only for mimalloc like we did for Azure SDK for C++:

if(CMAKE_VERSION VERSION_LESS 3.22)
# We can't disable installing Azure SDK for C++ by
# "set_property(DIRECTORY ${azure_sdk_SOURCE_DIR} PROPERTY
# EXCLUDE_FROM_ALL TRUE)" with CMake 3.16.
#
# At least CMake 3.22 on Ubuntu 22.04 works. So we use 3.22
# here. We may be able to use more earlier version here.
message(FATAL_ERROR "Building Azure SDK for C++ requires at least CMake 3.22. "
"(At least we can't use CMake 3.16)")
endif()

@pitrou
Copy link
Member Author

pitrou commented Dec 5, 2024

I don't object bumping our minimum CMake version but we can require CMake 3.18 or later only for mimalloc like we did for Azure SDK for C++:

We can do that, but mimalloc is our default allocator, so perhaps we want to bump the CMake version unconditionally. Is there a ML thread about this already? cc @assignUser

@assignUser
Copy link
Member

I think the cmake discussion mostly happened in zulip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants