[CI] enable dev=on on debian12

[silverweed]

This Pull request:

adds the -Ddev=on flag on the debian CI machine. It's a "test run" to see if we can later enable it on more (potentially all) build machines.

Here's what the flag does:

• enables asserts
• (Linux|BSD) enables -Werror
• (Linux|BSD) does not relink if a dependent .so has changed (CMAKE_LINK_DEPENDS_NO_SHARED On)
• (Linux|BSD) splits debug info (-gsplit-dwarf)
• (Linux|BSD) uses lld if available
• defines USE_LESS_INCLUDES

[github-actions]

Test Results

    18 files      18 suites   4d 13h 33m 56s ⏱️
 2 719 tests  2 719 ✅ 0 💤 0 ❌
47 245 runs  47 245 ✅ 0 💤 0 ❌

Results for commit 99a6fa3.

♻️ This comment has been updated with latest results.

[pcanal]

[enable on] (potentially all)

We need to make sure that we still test the environment the users will actually use (i.e dev=OFF on each PR).

[dpiparo]

Maybe we can add a "special" build, like "modules off" or "march=native"?

[vepadulano]

I also would start with -Ddev=ON being a special build tested on just a few platforms and nightly. In any case currently it cannot work (at least) because of the many warnings coming from LLVM so we need to suppress those before continuing

[silverweed]

@pcanal if that proves feasible, wouldn't it be beneficial to enable some of the flags everywhere instead of having them opt-in? Namely:

• enabling -Werror everywhere except llvm-facing parts
• using lld when possible (of course we still want to keep building with the default linker to test it works - but maybe it should be the exception rather than the rule)
• always defining R__LESS_INCLUDES (or, better, removing all the code in the #else parts). By the way, do you know why this define exists? Is there some compiler/platform where compilation breaks if it's not defined?

[hahnjo]

enabling -Werror everywhere except llvm-facing parts

We don't want -Werror by default because there will always be weird platforms that we cannot test, and future compilers may enable other warnings but we don't want to prevent them from building an older release of ROOT. FWIW LLVM sources should explicitly opt out of -Werror in our setup. The only place this doesn't work are some ROOT files including Clang headers - I don't have a good idea what to do about that case...

[silverweed]

@hahnjo that's fair; but I suppose we still wanna enable it on all our main tested platforms, at least for the CI nodes.
I tried adding appending -Wno-error to the CMakeLists of some directories (like interpreter), but it didn't really work (I guess -Werror gets added after it so it suppresses the other flag) and even if it did it looks sketchy. Not sure what's the best way to approach this.

[hahnjo]

I suppose we still wanna enable it on all our main tested platforms, at least for the CI nodes.

I tend to agree.

I tried adding appending -Wno-error to the CMakeLists of some directories (like interpreter), but it didn't really work (I guess -Werror gets added after it so it suppresses the other flag) and even if it did it looks sketchy. Not sure what's the best way to approach this.

As mentioned before, I think nothing is needed for interpreter/ because we already disable all warnings for LLVM libraries. For core/clingutils and core/metacling, I think the flags must be set before ROOT_OBJECT_LIBRARY.

[pcanal]

I suppose we still wanna enable it on all our main tested platforms, at least for the CI nodes.

We still need to test (at the very least in the nightly) what the user will see. If we don't test it we will eventually break it (for example some new CMake that inadvertently relies on side-effect of that code path).

[pcanal]

always defining R__LESS_INCLUDES (or, better, removing all the code in the #else parts). By the way, do you know why this define exists? Is there some compiler/platform where compilation breaks if it's not defined?

Per the commit log:

    When -Ddev=ON specified, R__LESS_INCLUDES is defined
    It will be used to reduce includes which are exposed to the public.
    While such changes can have side-effects on user code,
    option is off by default.

i.e. the changes of which header files are included where is a code-wise backward incompatible change (that has only light to zero actual benefit to the user), so we want to thread carefully in introducing them.

[agheata]

The fix in TGeoManager.cxx is fine, thanks for spotting the typo

[silverweed]

@agheata it was a compile error with dev=on due to -Werror.
It was actually already fixed by #16142, but I temporarily put it on this PR so the CI could build correctly

[bellenot]

@silverweed FYI -Wno-error is not known by MSVC:

cl : command line  error D8021: invalid numeric argument '/Wno-error'

You should use something like:

if(NOT MSVC)
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error")`
endif()