Repository navigation
[ci] Add Debian testing (with builtin_llvm=OFF) - #23372
ferdymercury wants to merge 6 commits into
Conversation
|
@alja @osschar maybe you know why debiantesting CI build fails with a heap error? https://github.com/root-project/root/actions/runs/34956369673/job/104339538592?pr=23372 |
ferdymercury
added
the
skip code analysis
Test Results 23 files 23 suites 4d 3h 11m 36s ⏱️ For more details on these failures, see this check. Results for commit 86ff218. ♻️ This comment has been updated with latest results. |
b4684d8 to
9af027a
Compare
ferdymercury
changed the title
9af027a to
aca5e2d
Compare
|
@guitargeek in TMVA-SOFIE I see:
|
4330d25 to
62b7f41
Compare
020cfd0 to
106385b
Compare
It's like Debian13, but tests the new libcivetweb-dev package, so builtin_civetweb can be OFF.
[ci] disable pyspark on Debiantesting
CMake Error at cmake/modules/FindPySpark.cmake:34 (message):
Currently, there are no Spark versions that support Java version greater
than 21. Found Java version 25.0.4.1.
Call Stack (most recent call first):
cmake/modules/SearchInstalledSoftware.cmake:1259 (find_package)
CMakeLists.txt:131 (include)
rror: /github/home/ROOT-CI/src/roottest/root/tree/selectorreader/runCollections.C:169:37: error: invalid operands to binary expression ('iterator' (aka '__gnu_cxx::__normal_iterator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > *, vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >') and 'iterator')
for (std::string const &treeName : trees)
^
/github/home/ROOT-CI/build/include/TString.h:56:9: note: candidate function not viable: no known conversion from 'iterator' (aka '__gnu_cxx::__normal_iterator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > *, vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >') to 'const TString' for 1st argument
Bool_t operator==(const TString &s1, const char *s2);
^ ~~~~~~~~~~~~~~~~~
do not use debug build type TODO: proper fix would be to skip adding this flag into Cling based on ifdef buildtype Unknown command line argument '-optimize-regalloc=0'. Try: 'cling (LLVM option parsing) --help'2026-09-30T15:00:08.7528489Z cling (LLVM option parsing): Did you mean '--vp-static-alloc=0'?
106385b to
3b452ce
Compare
|
@aaronj0 it seems that |
|
As discussed several times, across several PRs, I'm against adding |
Would it be ok to add to the CI Debian testing with Just like there is mac-beta, one could have debian-beta. |
|
I wouldn't mind, but we already have mac-beta and Fedora Rawhide, and we're testing Ubuntu versions before they are released. I'm not sure how much we gain by adding yet another rolling release (except longer CI run times) |
|
Personally, I would refrain from adding any additional configurations to the CI, because it's already quite heavy... But given that Ubuntu is based on Debian testing, maybe we can swap out the latest Ubuntu non-LTS release for Debian testing, at least on |
Just for fun, here the AI-collected overview on where these several PRs are: hahnjo on
|
| Date | Thread | What hahnjo said |
|---|---|---|
| 2026-06-08/09 | root-ci-images#131 "packages needed for building with builtin_llvm=OFF" | The main discussion, and the one they later point back to. First comment: "this is bound to break again with the next upgrade". Reply to guitargeek's question: "I don't want us to 'officially' support building against a system-default LLVM. I have expressed this opinion before. I don't want to chase bugs that arise due to this combination, and that may or may not be relevant to Conda. [...] And there will ~always be an 'upgrade incoming'"; argues a Conda nightly build is what is really needed. Review: "Let me make it explicit: I don't think this is a good idea." |
| 2026-08-18 | root#23068 "[interpreter] fix cmake configuration with system-llvm on ubu26" | "This has been discussed multiple times, my reasoning against adding a special build are documented here: root-ci-images#131" |
| 2026-11-05 | root#23372 "[ci] Add Debian testing (with builtin_llvm=OFF)" | The comment quoted above. |
Related PRs in the same chain of events, where hahnjo did not argue the CI point:
- root#22513 "[llvm] allow building
with builtin_llvm=OFF in Ubuntu26" is "the other PR" hahnjo refers to in
root-ci-images#131. dpiparo suggested a special build there and
noted
that it would cease to function once LLVM 22 is merged. hahnjo only left an
inline comment
on thefind_packagechange. - root#22584 "[ci] add special
system-llvm ubu26 build" is ferdymercury's CI PR from June 2026, depending on
root-ci-images#131. Closed without any comment from hahnjo. - root#22544 "[interpreter] Set
LLVM_DIRforbuiltin_llvm" is hahnjo's own fix that came out of the
root-ci-images#131 discussion (standardfind_packagevariables instead of
HINTS).
The underlying stance: not an officially supported configuration
- root#18387
(2025-04-14): "builtin_llvm=OFFandbuiltin_clang=OFFare not officially
supported configurations, they are maintained on a best-effort basis at the
moment. The user will have to compile Clang from our sources anyway to get our
patches. For that reason, we must not add downstream patches just for this." - root#18386
(2025-04-14, "[CMake] Fixes and improvements for buildng with
builtin_clang=OFF"): "the default, and only supported configuration for
production use as far as I'm concerned, isbuiltin_llvm=ON builtin_clang=ON."
Follow-up:
"'best effort' doesn't mean that I don't care, and it doesn't mean that the use
cases can be broken at will." - root#18446
(2025-04-19, revert of [CMake] Fixes and improvements for buildng withbuiltin_clang=OFF#18386) and
root#18640
(2025-05-07, 6.36 backport) continue the same argument, mostly about build-system
changes late in the release cycle. - root#11146
(2022-08-08), earliest instance found: "I don't think it's feasible to write
CMake detection logic for every single patch that we distribute on top of LLVM
and Clang." - root#15696
(2024-07-19, LLVM 18 upgrade): system Clang still requires downstream patches;
their number shrinks with every upgrade. - root#19265
(2025-07-03, "ROOT package in Debian/Ubuntu"): no patches for LLVM, only for
Clang, but a specific LLVM major version is required.
What hahnjo proposes instead
- root#23160
(2026-08-31): "I don't see the point of adding more downstream patches for this
case. I've said it before, the problem should be fixed 'properly' by making it
possible to link against a providedlibLLVM.so. We know how to do that:
[rootcling] Removellvm::cldependency by using ROOT'soptparse#20903" - root#20903 "[rootcling] Remove
llvm::cldependency by using ROOT'soptparse" (January 2026, long exchange
with vgvassilev): wants linking against the distribution'slibLLVM.soto be
possible if the user asks for it, whilebuiltin_llvm=ONstays the default and
unchanged. Start at
this comment;
the key statement is
here:
"the plan is not to make this the default build mode".
Status as of 2026-09-01:
first step only, and not moving forward. - root#17698
(2025-02-17, "Allow building ROOT using the Clang shared library"): the two
technical blockers (-Woption clash withlibLLVM.so,clang-cpp
dependencies). - root#12156
(2023-08-11, "CommandLine Error: Option 'W' registered more than once!"): the
bug that [rootcling] Removellvm::cldependency by using ROOT'soptparse#20903 is meant to fix; advice at the time was to stay with
builtin_clang=ON.
Search coverage
Searched hahnjo's issue comments, review bodies and inline review comments across
the root-project GitHub org for builtin_llvm, builtin_clang, and
"external/system LLVM". Offline and chat discussions are not covered; root#18386
and root#16814 both refer to such discussions.
There are new packages in debian unstable (civetweb, mathtext) that are not in debian stable where one can switch -Dbuiltin_civetweb=OFF etc when building ROOT. By testing those in advance, we can report any potential bugs. Once they land in stable, is too late to fix them, they only get security fixes, not "Config.cmake" fixes.
or have one of the non-LTS as -Dminimal=ON ?
Alternative idea: have a "os-testing" GitHub label. When it is added by the user (or randomly 1 out of 10), we then also run all beta-versions of the OS ? |
|
The problem is that there are too many constraints and factors when designing the CI matrix (compiler, Python, C++ standard, OS, Right now it's on the table of Danilo to propose a sustainable, coherent, and high-coverage CI matrix, but I hope we can converge on this together at the next internal ROOT retreat at CERN the end of November (@ferdymercury if you have time to come 🙂, join in). So I think we can't and shouldn't rush to a decision here in this PR thread. PS: I feel your pain though, @ferdymercury 😆 I was dealing for the last week with the fallout of 6.42 for Nix packaging, and NixOS in the CI would have saved me a lot of work 🙂 |
|
Sure, I totally agree that there is no need to rush. If it does not bother, I will keep the PR open, and will rerun it once every two months or so, to get an idea. I have now packaged CDT and BVH into Debian (pending upload), so next thing I will try is to switch builtin_cdt and builtin_bvh to OFF to see if all tests pass. Thanks for the invite, however I am already busy with a one week conference this November (IEEE NSS/MIC) :) |
This Pull request:
Changes or fixes:
Fedora CI has rawhide, So Debian CI has testing for analogy. But no need to merge this, it can stay as a draft PR for individual testing of new packages. New package to test will be libmathtext-dev.
It's like Debian13, but tests the new libcivetweb-dev package, so builtin_civetweb can be OFF. fyi @linev confirmed that new Debian civetweb will be compatible.
First results not too bad: only 13 tests failing with
builtin_llvm=OFFMerge first
Checklist: