Skip to content

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

Open
ferdymercury wants to merge 1 commit intoroot-project:masterfrom
ferdymercury:xxh
Open

[builtins] download instead of bundle xxhash and bump from 0.8.0 to 0.8.3#21806
ferdymercury wants to merge 1 commit intoroot-project:masterfrom
ferdymercury:xxh

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

No description provided.

@ferdymercury ferdymercury changed the title [skip-ci,builtins] download instead of bundle xxhash [builtins] download instead of bundle xxhash Apr 7, 2026
@dpiparo dpiparo self-assigned this Apr 7, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

Test Results

    16 files      16 suites   2d 3h 36m 54s ⏱️
 3 804 tests  3 804 ✅ 0 💤 0 ❌
54 764 runs  54 764 ✅ 0 💤 0 ❌

Results for commit 29d679c.

♻️ This comment has been updated with latest results.

add CMakeLists patch

rm bundled copy
@ferdymercury ferdymercury changed the title [builtins] download instead of bundle xxhash [builtins] download instead of bundle xxhash and bump from 0.8.0 to 0.8.3 Apr 7, 2026
ExternalProject_Add(
BUILTIN_XXHASH
PREFIX ${ROOT_XXHASH_PREFIX}
URL https://github.com/Cyan4973/xxHash/archive/refs/tags/v${ROOT_XXHASH_VERSION}.tar.gz
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

potentially use LCG instead?

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

set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS xxHash::xxHash)
Copy link
Copy Markdown
Collaborator Author

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)

@@ -1,35 +1,57 @@
# Copyright (C) 1995-2021, Rene Brun and Fons Rademakers.
# All rights reserved.
# Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. All rights reserved.
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.

Question, should we add sth like:

ROOT_FIND_REQUIRED_DEP(xxhash builtin_xxhash)

in cmake/modules/SearchInstalledSoftware.cmake ?

set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS xxHash::xxHash)
# For the licensing terms see $ROOTSYS/LICENSE. For the list of contributors see
# $ROOTSYS/README/CREDITS.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I guess we do not want this #21689 for xxhash, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants