Fine to me, I'd just make the CI to test the code, as long as we don't have strong reasons to believe this is slowing down too much the process.
Considering, for instance, the code for ITS3 (see: #3699): it adds just two libraries and I think the time spent is negligible, since there are no tests out of the usual ones for macros.

Fine to me, I'd just make the CI to test the code, as long as we don't have strong reasons to believe this is slowing down too much the process.
Considering, for instance, the code for ITS3 (see: #3699) it adds just two libraries and I think the time spent is negligible, since there are no tests out of the usual ones for macros.

That is why I made the ENABLE_UPGRADES option default to ON. Do I miss something?

That is why I made the ENABLE_UPGRADES option default to ON. Do I miss something?

Ah, actually I didn't see that. Perhaps that is not ideal, since all people will build with it without even knowing.
I agree on the CI, but it should be only the build/O2/o2 CI, not the others. Than ensures that we also try to build without the upgrades.

Fine to me, I'd just make the CI to test the code, as long as we don't have strong reasons to believe this is slowing down too much the process.
Considering, for instance, the code for ITS3 (see: #3699) it adds just two libraries and I think the time spent is negligible, since there are no tests out of the usual ones for macros.

That is why I made the ENABLE_UPGRADES option default to ON. Do I miss something?

Fine to me, I'd just make the CI to test the code, as long as we don't have strong reasons to believe this is slowing down too much the process.
Considering, for instance, the code for ITS3 (see: #3699) it adds just two libraries and I think the time spent is negligible, since there are no tests out of the usual ones for macros.

That is why I made the ENABLE_UPGRADES option default to ON. Do I miss something?

Sorry, I did. Looks good.

Ok, it seems @mconcas wants the default to be ON, while @davidrohr wants it to be OFF.

Either way, I think it would be good if the CI covers both cases. Where do we have to configure that?

It's ok for me. Consistent with what @mconcas has in his branch.
Concerning the CI: Somebody needs to modify o2.sh in alidist and put ENABLE_UPGRADES=ON depending on the presence of an external variable (which needs to be set in a different default or in a build configuration of ali-bot)

IMO the ENABLE_UPGRADES should be OFF by default. That might not matter much now that we are not yet in production (even though it will slow down an already not-really-fast build a bit or a lot depending on what goes into the Upgrades subdirectory...) , but soon my feeling is that we won't want to risk breaking a release or loosing time debugging one for detectors that are not actually part of data taking.

Ok, so let's not build the upgrades by default, it doesn't take much to enable them for those affected. Once this PR has converged, I can take of the changes in o2.sh (alidist), but I guess someone else would then have to do the change for ali-bot.

NB: In addition, typo fixed and messages added that upgrades are built.

Perhaps a dedicated builder should be added, when we will have more to test rather than just the compilation, depending also on CI available resources.

I would add more builders only if we have more CI resources.
You can use ctests normally, and the builder that will test the upgrade compilation will also do these ctests.

Do we have any outstanding issues here? If not, can we merge this, please, so that we can then also move on with #3699?