Skip to content

[llvm] allow building with builtin_llvm=OFF in Ubuntu26 #22513

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 4 commits into
base: master
Choose a base branch
from
+13 −4
Open

[llvm] allow building with builtin_llvm=OFF in Ubuntu26 #22513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
639f9c6
[llvm] allow building with builtin_llvm=OFF in Ubuntu26
ferdymercury Jun 7, 2026
f1602e3
[ci] test system-llvm in Ubuntu26
ferdymercury Jun 7, 2026
4f89749
[ci] special ubuntu26 llvm off build
ferdymercury Jun 7, 2026
c766d9e
[ci] rm option from normal build
ferdymercury Jun 7, 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
4 changes: 4 additions & 0 deletions .github/workflows/root-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') && !matrix.platform == 'mac15' && !matrix.platform == 'mac26'}}
GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }}
OVERRIDES: ${{ join( matrix.overrides, ' ') }}
run: |

Check failure on line 160 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files 

"github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
[ -d "${VIRTUAL_ENV_DIR}" ] && source ${VIRTUAL_ENV_DIR}/bin/activate
echo "Python is now $(which python3) $(python3 --version)"
src/.github/workflows/root-ci-config/build_root.py \
Expand Down Expand Up @@ -287,7 +287,7 @@
INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') }}
GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }}
shell: cmd
run: "C:\\setenv.bat ${{ matrix.target_arch }} &&

Check failure on line 290 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files 

"github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
python .github/workflows/root-ci-config/build_root.py
--buildtype ${{ matrix.config }}
--platform windows10
Expand Down Expand Up @@ -438,6 +438,10 @@
# is_special: true
# property: gpu
# extra-runs-on: gpu
- image: ubuntu2604
is_special: true
property: "system-llvm"
overrides: ["builtin_llvm=off"]
# openSUSE Leap
- image: opensuse16
platform_config: opensuse16-march_native
Expand All @@ -448,7 +452,7 @@
- self-hosted
- linux
- ${{ matrix.architecture == null && 'x64' || matrix.architecture }}
- ${{ matrix.extra-runs-on == null && 'cpu' || matrix.extra-runs-on }}

Check failure on line 455 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files 

property "extra-runs-on" is not defined in object type {architecture: string; image: string; is_special: bool; overrides: array<string>; platform_config: string; property: string; python_venv: string}

Check failure on line 455 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files 

property "extra-runs-on" is not defined in object type {architecture: string; image: string; is_special: bool; overrides: array<string>; platform_config: string; property: string; python_venv: string}

name: |
${{ matrix.image }} ${{ matrix.property }}
Expand Down Expand Up @@ -515,7 +519,7 @@
INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') }}
GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }}
OVERRIDES: ${{ join( matrix.overrides, ' ') }}
run: ".github/workflows/root-ci-config/build_root.py

Check failure on line 522 in .github/workflows/root-ci.yml

View workflow job for this annotation

GitHub Actions / lint-action-files 

"github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
--buildtype RelWithDebInfo
--platform ${{ matrix.image }}
--platform_config ${{ matrix.platform_config }}
Expand Down
13 changes: 9 additions & 4 deletions interpreter/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ if (NOT builtin_clang)
message(WARNING "Due to ROOT-specific patches you need a special version of clang. You cannot use vanilla clang.")
endif()

#--Set the LLVM version required for ROOT-----------------------------------------------------------
#--Set the LLVM version required for ROOT to ensure API compatibility-------------------------------
set(ROOT_LLVM_VERSION_REQUIRED_MAJOR 20)
set(ROOT_LLVM_VERSION_REQUIRED_MINOR 1)

#---Define the way we want to build and what of llvm/clang/cling------------------------------------
set(LLVM_ENABLE_RTTI ON CACHE BOOL "")
Expand Down Expand Up @@ -259,7 +260,10 @@ if(builtin_llvm)
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/llvm-project/llvm/cmake/modules")
#---Set into parent scope LLVM_VERSION_MAJOR --------------------------------------------------------
get_directory_property(_llvm_version_major DIRECTORY llvm-project/llvm DEFINITION LLVM_VERSION_MAJOR)
get_directory_property(_llvm_version_minor DIRECTORY llvm-project/llvm DEFINITION LLVM_VERSION_MINOR)
set(LLVM_VERSION_MAJOR "${_llvm_version_major}" PARENT_SCOPE)
set(LLVM_VERSION_MINOR "${_llvm_version_minor}" PARENT_SCOPE)

#---Forward LLVM_TARGET_TRIPLE for check-cling-------------------------------------------------------
get_directory_property(LLVM_TARGET_TRIPLE DIRECTORY llvm-project/llvm DEFINITION LLVM_TARGET_TRIPLE)
#---Get back the potentially updated LLVM_TARGETS_TO_BUILD (expanding all/host/Native)---------------
Expand All @@ -275,7 +279,7 @@ if(builtin_llvm)
set(LLVM_TABLEGEN_EXE "${LLVM_BINARY_DIR}/bin/llvm-tblgen")
endif()
else()
find_package(LLVM REQUIRED CONFIG)
find_package(LLVM ${ROOT_LLVM_VERSION_REQUIRED_MAJOR}.${ROOT_LLVM_VERSION_REQUIRED_MINOR} REQUIRED CONFIG)

@hahnjo hahnjo Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure about this change: You're saying that the previous line find_package(LLVM REQUIRED CONFIG) doesn't work at all? It is the officially documented way to find LLVM: https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project Then we are manually checking the version further below.

@ferdymercury ferdymercury Jun 8, 2026 •
edited
Loading

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.

Yes, that's what I am saying/experiencing on Ubuntu 26 root docker image, it does not work at all. If you check the LLVMConfigVersion.cmake file https://github.com/ferdymercury/root/blob/master/interpreter/llvm-project/llvm/cmake/modules/LLVMConfigVersion.cmake.in, it's checking for strict match of major and minor versions. So found version 20.1.8 does not match requested version unspecified.unspecified
It's weird, but they seem to enforce that strict match to be API compatible.
In any case, the solution of adding/specifying major minor when calling find is not too bad, so that we ensure it matches the current builtin llvm.

@ferdymercury ferdymercury Jun 8, 2026 •
edited
Loading

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.

Seems it's not a bug but a known "feature": https://stackoverflow.com/questions/66305592/how-to-match-major-version-only-in-cmake

@hahnjo hahnjo Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems to work for me when specifying the right variables: #22544

message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION} in ${LLVM_CMAKE_DIR}")

separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS})
Expand Down Expand Up @@ -321,10 +325,10 @@ else()
include(VersionFromVCS)

set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
if (${PACKAGE_VERSION} MATCHES "${ROOT_LLVM_VERSION_REQUIRED_MAJOR}\\.1(|\\.[0-9]+)")
if (${PACKAGE_VERSION} MATCHES "${ROOT_LLVM_VERSION_REQUIRED_MAJOR}\\.${ROOT_LLVM_VERSION_REQUIRED_MINOR}(|\\.[0-9]+)")
message(STATUS "Using LLVM external library - ${PACKAGE_VERSION}")
else()
message(FATAL_ERROR "LLVM version ${LLVM_PACKAGE_VERSION} different from ROOT supported, please try ${ROOT_LLVM_VERSION_REQUIRED_MAJOR}.1.x")
message(FATAL_ERROR "LLVM version ${LLVM_PACKAGE_VERSION} different from ROOT supported, please try ${ROOT_LLVM_VERSION_REQUIRED_MAJOR}.${ROOT_LLVM_VERSION_REQUIRED_MINOR}.x or switch builtin_llvm=ON")
endif()

if (NOT DEFINED LLVM_INCLUDE_TESTS)
Expand Down Expand Up @@ -389,6 +393,7 @@ else()
# To simplify code, just forward that requirement to the rest of ROOT, for example to
# construct the resource directory in core/clingutils.
set(LLVM_VERSION_MAJOR ${ROOT_LLVM_VERSION_REQUIRED_MAJOR} PARENT_SCOPE)
set(LLVM_VERSION_MINOR ${ROOT_LLVM_VERSION_REQUIRED_MINOR} PARENT_SCOPE)

# We are in the case of NOT builtin_llvm
if (builtin_clang)
Expand Down
Loading