[cmake] make fail-on-missing=ON the only behavior, ignore user setting and deprecate build option - #23015
[cmake] make fail-on-missing=ON the only behavior, ignore user setting and deprecate build option#23015ferdymercury wants to merge 1 commit into
Conversation
|
Thanks for the initiative! As I see it, the suggested change splits into an uncontroversial and a controversial part. The uncontroversial part: removing the The controversial part: maybe some people relied on the automatic disabling of default features if the dependencies that backed them were not available. Or they relied on the fallback to the I think your suggestion would fly if you split it in two parts: first a PR with the uncontroversial part that should sail through, and then a PR for the remaining "controversial" changes. Would that make sense to you? |
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 6a0f346. ♻️ This comment has been updated with latest results. |
|
Thanks for the feedback !
On the other hand, this is already broken now:
So this PR is making that current behavior consistent through all the build options. Yes, I could split the PR in two if that helps, but the gain is not high since users can not rely any more on that historical behavior. I prefer annoying (I have to turn things ON/OFF by hand) but coherent behavior (every option behaves likewise) than heterogeneous one (why does one dependency autodisable and another one does not). What do you think? |
|
I was suggesting to split this PR not for the users, but for the review process. If the uncontroversial part is merged first, then the diff of the "controversial" PR that removes Personally I'm all for this change though! |
Make sense, will do, thanks |
…g and deprecate flag [nfc] mention change in release notes [cmake] print more helpful error messages for builtins-backed options as suggested by guitargeek
After some usage, I find this change to be indeed more pedantic (useful for system maintainer) but also annoyingly-so for the 'quickly build ROOT on this system for my private usage'. (I.e. now installing ROOT on a system that does not already have the required builtin has to go through a step of parsing (by hand or mechanically) the output of cmake to find the |
But why not installing the required dependencies on the first place? All those builtins are available via Apt or Homebrew? |
Would it be better if Root printed at the end of Cmake:
That way we avoid a lot of code and responsibility while making it still easy to copy paste the solution Closing in favor of #23092 |
This Pull request:
Changes or fixes:
fail-on-missing flag no longer makes any sense: most ROOT builtins already call ROOT_FIND_REQUIRED_DEP which purposely bypasses this flag, so users might be confused about this hybrid undocumented behavior. Now all builtins can be installed via APT or via homebrew, so it's less of an issue.
This allows a huge simplification of search-installed-software, and easens code maintenance and bug fixing of the build system for the developers. The only downside is that some users might need to type a bit more consciously what is ON and OFF. Which some might even welcome, others complain.
Checklist: