[cmake] explicit feature=OFF should have hierarchical priority over cached or explicit builtin_feature=ON v2

[ferdymercury]

This Pull request:

Changes or fixes:

This is v2 of #18413

Fixes https://its.cern.ch/jira/browse/ROOT-10743

Related:

https://its.cern.ch/jira/browse/ROOT-9385

Checklist:

• tested changes locally
• updated the docs (if necessary)

[ferdymercury]

@pcanal I have tried to follow your suggestion.

My question now is, how do I prevent enabling fftw3=ON if I passed explicitly -Dfftw3=OFF ?

Right now the sequence

cmake -Dbuiltin_fftw3=ON ../root_src/
cmake -Dfftw3=OFF ../root_src/

still leaves fftw3 ON.

Do I need to do something like https://stackoverflow.com/a/54302498/7471760 or rather https://stackoverflow.com/a/61986176/7471760?

Sth like:

if (NOT fftw3 and NOT DEFINED CACHE{builtin_fftw3})
   set(fftw3 CACHE BOOL ON FORCE)
endif()

[pcanal]

Try without the FORCE ...

[github-actions]

Test Results

    18 files      18 suites   3d 23h 20m 26s ⏱️
 2 731 tests  2 730 ✅ 0 💤 1 ❌
47 771 runs  47 770 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 4a847a9.

♻️ This comment has been updated with latest results.

[ferdymercury]

Try without the FORCE ...

I tried and it does not solve the issue. Probably because the default option fftw3=OFF, so that has priority over the cache?

So without force -Dbuiltin_fftw3=ON does not turn fftw3=ON.

I tried also NOT DEFINED CACHE, to no avail.

It seems that this post is relevant:
https://stackoverflow.com/questions/43542381/set-a-cmake-variable-if-it-is-not-changed-by-the-user

But then we are back to the debate... is this extra complexity worth or can we stick with v1 #18413?

[hageboeck]

@ferdymercury, with yours and our recent work on the builtins, can we close this one?

[ferdymercury]

, can we close this one?

Hmmm not really.

This still fails:

cmake -Dbuiltin_fftw3=ON ../root_src/
cmake -Dfftw3=OFF ../root_src/
-- Enabled support for: asimage asimage_tiff builtin_clang builtin_cling builtin_civetweb builtin_fftw3 builtin_llvm builtin_openui5 builtin_vdt builtin_xxhash check_connection clad curl dataframe fftw3 gdml geom http imt opengl pyroot roofit root7 runtime_cxxmodules shared spectrum ssl thisroot_scripts tmva tmva-cpu tmva-cudnn tpython use_gsl_cblas vdt webgui x11 xml xrootd

The solution is easy, if the ROOT team agrees to this change of behavior:

-Dbuiltin_feature=ON is ignored if -Dfeature=OFF or if -Dfeature=UNSET (a warning will be issued)

Otherwise a more complex solution is needed. Considering that builtins are supposed to be the 'last exception' and that one should use system packages, I vote for the more simple solution. Less CMake lines of codes to maintain, not worth the complexity for a corner case scenario, that is anyway buggy in the current state.