Skip to content

[builtins] download instead of bundle xxhash and bump from 0.8.0 to 0.8.3 #21806

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.

Sign up for GitHub

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

Open
ferdymercury wants to merge 12 commits into
base: master
Choose a base branch
from
Open

[builtins] download instead of bundle xxhash and bump from 0.8.0 to 0.8.3 #21806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
29d679c
[builtins] download instead of bundle xxhash
ferdymercury Apr 7, 2026
c78e6e4
move to LCG
ferdymercury Apr 8, 2026
c9b2f51
Mimick other compression builtins
ferdymercury Apr 8, 2026
349cca0
Update CMakeLists.txt
ferdymercury Apr 8, 2026
72ae58d
Update xxhash_add_cmakelists.patch
ferdymercury Apr 8, 2026
099d828
Update xxhash_add_cmakelists.patch
ferdymercury Apr 8, 2026
1fc8eae
Update CMakeLists.txt
ferdymercury Apr 8, 2026
997955e
Update xxhash_add_cmakelists.patch
ferdymercury Apr 8, 2026
2a2f8f5
Update CMakeLists.txt
ferdymercury Apr 9, 2026
3ddb695
Update CMakeLists.txt
ferdymercury Apr 9, 2026
ba55d20
[ci] turn on builtin_xxhash for CI testing on Linux
ferdymercury Apr 10, 2026
f94d4a9
[ci] fix alphabetical sorting
ferdymercury Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/root-ci-config/buildconfig/alma10-clang_ninja.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ builtin_lz4=ON
builtin_lzma=ON
builtin_png=ON
builtin_vdt=ON
builtin_xxhash=ON
builtin_zlib=ON
builtin_zstd=ON
pythia8=ON
Expand Down
89 changes: 56 additions & 33 deletions builtins/xxhash/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,58 @@
# Copyright (C) 1995-2021, Rene Brun and Fons Rademakers.
# All rights reserved.
# Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. All rights reserved.
#
# For the licensing terms see $ROOTSYS/LICENSE.
# For the list of contributors see $ROOTSYS/README/CREDITS.

project(xxhash C)

unset(xxHash_FOUND CACHE)
unset(xxHash_FOUND PARENT_SCOPE)
set(xxHash_FOUND TRUE CACHE BOOL "" FORCE)

file(STRINGS xxhash.h XXHASH_H REGEX "^#define XXH_VERSION_[A-Z]+[ ]+[0-9]+$")
string(REGEX REPLACE ".+XXH_VERSION_MAJOR[ ]+([0-9]+).*$" "\\1" xxHash_VERSION_MAJOR "${XXHASH_H}")
string(REGEX REPLACE ".+XXH_VERSION_MINOR[ ]+([0-9]+).*$" "\\1" xxHash_VERSION_MINOR "${XXHASH_H}")
string(REGEX REPLACE ".+XXH_VERSION_RELEASE[ ]+([0-9]+).*$" "\\1" xxHash_VERSION_PATCH "${XXHASH_H}")
set(xxHash_VERSION_STRING "${xxHash_VERSION_MAJOR}.${xxHash_VERSION_MINOR}.${xxHash_VERSION_PATCH}")

set(xxHash_VERSION ${xxHash_VERSION_STRING} CACHE INTERNAL "")
set(xxHash_VERSION_STRING ${xxHash_VERSION_STRING} CACHE INTERNAL "")

set(xxHash_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR} CACHE INTERNAL "")
set(xxHash_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} CACHE INTERNAL "")
mark_as_advanced(xxHash_INCLUDE_DIR)

add_library(xxhash STATIC xxhash.h xxhash.c)
set_target_properties(xxhash PROPERTIES C_VISIBILITY_PRESET hidden POSITION_INDEPENDENT_CODE ON)
target_include_directories(xxhash INTERFACE $<BUILD_INTERFACE:${xxHash_INCLUDE_DIR}>)

add_library(xxHash::xxHash ALIAS xxhash)

set(xxHash_LIBRARY $<TARGET_FILE:xxhash> CACHE INTERNAL "")
set(xxHash_LIBRARIES xxHash::xxHash CACHE INTERNAL "")

# For the licensing terms see $ROOTSYS/LICENSE. For the list of contributors see
# $ROOTSYS/README/CREDITS.

# **PLEASE UPDATE ALSO THE FOLLOWING LINE WHEN UPDATING THE VERSION**
# 30 Dec 2024, https://github.com/Cyan4973/xxHash/releases/tag/v0.8.3
set(ROOT_XXHASH_VERSION 0.8.3)
set(ROOT_XXHASH_HASH "aae608dfe8213dfd05d909a57718ef82f30722c392344583d3f39050c7f29a80")
# The sources come w/o CMakeList.txt as of version 0.8.3. We add ours, it's very simple.
set(ROOT_XXHASH_PATCH_FILE ${CMAKE_CURRENT_SOURCE_DIR}/xxhash_add_cmakelists.patch)

set(ROOT_XXHASH_PREFIX ${CMAKE_BINARY_DIR}/builtins/XXHASH-prefix)

set(ROOT_XXHASH_LIBRARY ${ROOT_XXHASH_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}xxhash${CMAKE_STATIC_LIBRARY_SUFFIX}
)

if(WIN32 AND NOT CMAKE_GENERATOR MATCHES Ninja)
if(winrtdebug)
set(ROOT_XXHASH_BUILD_COMMAND_FLAGS "--config Debug")
else()
set(ROOT_XXHASH_BUILD_COMMAND_FLAGS "--config $<IF:$<CONFIG:Debug,RelWithDebInfo>,RelWithDebInfo,Release>")
endif()
endif()

ExternalProject_Add(
BUILTIN_XXHASH
PREFIX ${ROOT_XXHASH_PREFIX}
URL https://lcgpackages.web.cern.ch/tarFiles/sources/xxHash-${ROOT_XXHASH_VERSION}.tar.gz
URL_HASH SHA256=${ROOT_XXHASH_HASH}
PATCH_COMMAND git apply ${ROOT_XXHASH_PATCH_FILE}
CMAKE_ARGS -G ${CMAKE_GENERATOR}
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
-DCMAKE_POSITION_INDEPENDENT_CODE=ON
-DCMAKE_C_VISIBILITY_PRESET=hidden
BUILD_COMMAND ${CMAKE_COMMAND} --build . ${ROOT_XXHASH_BUILD_COMMAND_FLAGS}
INSTALL_COMMAND ${CMAKE_COMMAND} --build . ${ROOT_XXHASH_BUILD_COMMAND_FLAGS} --target install
LOG_CONFIGURE 1
LOG_BUILD 1
LOG_INSTALL 1
LOG_OUTPUT_ON_FAILURE 1
BUILD_BYPRODUCTS ${ROOT_XXHASH_LIBRARY}
TIMEOUT 600)

file(MAKE_DIRECTORY ${ROOT_XXHASH_PREFIX}/include)
add_library(xxHash::xxHash IMPORTED STATIC GLOBAL)
set_target_properties(xxHash::xxHash PROPERTIES
IMPORTED_LOCATION ${ROOT_XXHASH_LIBRARY}
INTERFACE_INCLUDE_DIRECTORIES ${ROOT_XXHASH_PREFIX}/include)
set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS xxHash::xxHash)
Copy link
Copy Markdown
Collaborator Author

@ferdymercury ferdymercury Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some BUILTINs have this line, but not others such as libgif, etc.

Should we remove everywhere, or add everywhere, or is hybrid intended?

builtins/nlohmann/CMakeLists.txt:set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS builtin_nlohmann_json_incl)
builtins/pcre/CMakeLists.txt:set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS PCRE)
builtins/zlib/CMakeLists.txt:set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS ZLIB::ZLIB)
builtins/xrootd/CMakeLists.txt:set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS BUILTIN_XROOTD)
builtins/openssl/CMakeLists.txt:set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS OPENSSL)
cmake/modules/SearchInstalledSoftware.cmake:    set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS VDT::VDT)
CMakeLists.txt:get_property(__allBuiltins GLOBAL PROPERTY ROOT_BUILTIN_TARGETS)

Copy link
Copy Markdown
Member

@dpiparo dpiparo Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we tried to preserve the targets, and also to trim away the code which was not necessary. What we tried to achieve is to have these 4 variables for all builtins:

XYZ_INCLUDE_DIRS
XYZ_LIBRARIES
XYZ_FOUND
XYZ_VERSION

Copy link
Copy Markdown
Collaborator Author

@ferdymercury ferdymercury Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but I meant specifically the line:
set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS xxHash::xxHash)

the equivalent line is in the zlib builtin but not in the other builtins, so I was not sure which way to go

Copy link
Copy Markdown
Collaborator Author

@ferdymercury ferdymercury Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used later for

get_property(__allBuiltins GLOBAL PROPERTY ROOT_BUILTIN_TARGETS)
add_custom_target(move_headers ALL DEPENDS ${__allHeaders} ${__allBuiltins} gitinfotxt)

so I guess all of them should have this, right now only a couple of them have this ?

or maybe it depends on static vs shared?

Copy link
Copy Markdown
Collaborator Author

@ferdymercury ferdymercury Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See for example commit 929f249


# Set the canonical output of find_package according to
# https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#standard-variable-names
set(xxHash_INCLUDE_DIRS ${ROOT_XXHASH_PREFIX}/include PARENT_SCOPE)
set(xxHash_LIBRARIES ${ROOT_XXHASH_LIBRARY} PARENT_SCOPE)
set(xxHash_FOUND TRUE PARENT_SCOPE)
set(xxHash_VERSION ${ROOT_XXHASH_VERSION} PARENT_SCOPE)
43 changes: 0 additions & 43 deletions builtins/xxhash/xxhash.c
View file
Open in desktop

This file was deleted.

Loading
Loading